Add User-Agent header to file upload requests#37
Conversation
Fivell
force-pushed
the
fix/upload-user-agent
branch
from
ba15f77 to
9972cdd
Compare
There was a problem hiding this comment.
Pull request overview
This PR ensures encrypted file uploads include the SDK User-Agent header, aligning multipart upload requests with the headers normally applied by the main OkHttpClient interceptor chain.
Changes:
- Add
User-Agent: didww-java-sdk/<version>toDidwwClient.uploadEncryptedFile()requests. - Extend WireMock verification in the existing upload test and add a new test asserting the exact
User-Agentvalue.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/main/java/com/didww/sdk/DidwwClient.java |
Adds User-Agent header to the multipart upload request. |
src/test/java/com/didww/sdk/EncryptedFileUploadTest.java |
Verifies User-Agent is sent for encrypted file uploads (regex + exact-value test). |
Comments suppressed due to low confidence (1)
src/main/java/com/didww/sdk/DidwwClient.java:232
uploadEncryptedFile()creates a brand-newOkHttpClientand only copies timeouts. This drops any advanced configuration applied to the main client (proxy, SSL settings, connection pool, DNS, user-provided interceptors viahttpClientBuilder, etc.), so uploads may behave differently from other requests. Consider building fromhttpClient.newBuilder()and removing only theApiKeyInterceptor(or replacing it with a headers-only interceptor) so uploads preserve the caller's OkHttp configuration while still avoiding JSON:API headers for multipart.
.header("User-Agent", SdkVersion.userAgent())
.build();
// Dedicated client without JSON:API interceptor, preserving timeouts from main client.
OkHttpClient uploadClient = new OkHttpClient.Builder()
.connectTimeout(httpClient.connectTimeoutMillis(), TimeUnit.MILLISECONDS)
.readTimeout(httpClient.readTimeoutMillis(), TimeUnit.MILLISECONDS)
.writeTimeout(httpClient.writeTimeoutMillis(), TimeUnit.MILLISECONDS)
.build();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Test | ||
| void testUploadEncryptedFileSendsUserAgent() { | ||
| wireMock.stubFor(post(urlPathEqualTo("/v3/encrypted_files")) | ||
| .willReturn(aResponse() | ||
| .withStatus(201) | ||
| .withHeader("Content-Type", "application/json") | ||
| .withBody(loadFixture("encrypted_files/create.json")))); | ||
|
|
||
| client.uploadEncryptedFile( | ||
| "example".getBytes(StandardCharsets.UTF_8), | ||
| "sample.pdf.enc", | ||
| "fingerprint-123", | ||
| "sample.pdf" | ||
| ); | ||
|
|
||
| wireMock.verify(postRequestedFor(urlPathEqualTo("/v3/encrypted_files")) | ||
| .withHeader("User-Agent", equalTo(SdkVersion.userAgent()))); | ||
| } |
There was a problem hiding this comment.
testUploadEncryptedFile() now asserts the User-Agent header, and testUploadEncryptedFileSendsUserAgent() repeats the same behavior with a separate stub + verify. This duplication increases test maintenance without adding new coverage. Consider asserting the exact header value (SdkVersion.userAgent()) in the existing test and removing the extra test (or differentiating the new test to cover a distinct scenario).
| wireMock.verify(postRequestedFor(urlPathEqualTo("/v3/encrypted_files")) | ||
| .withHeader("Api-Key", equalTo("test-api-key")) | ||
| .withHeader(ApiKeyInterceptor.API_VERSION_HEADER, equalTo(ApiKeyInterceptor.API_VERSION)) | ||
| .withHeader("User-Agent", matching("didww-java-sdk/.*")) |
There was a problem hiding this comment.
The User-Agent assertion in testUploadEncryptedFile() uses a very permissive regex (didww-java-sdk/.*), which can mask regressions in the exact header value. If the intent is to verify the SDK sends the canonical value, prefer equalTo(SdkVersion.userAgent()) (and then you can avoid needing a separate test for exactness).
| .withHeader("User-Agent", matching("didww-java-sdk/.*")) | |
| .withHeader("User-Agent", equalTo(SdkVersion.userAgent())) |
Replace permissive regex matching with equalTo(SdkVersion.userAgent()) in testUploadEncryptedFile and remove redundant testUploadEncryptedFileSendsUserAgent test method.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
uploadEncryptedFile()requestTest plan