Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdates client to Uploadcare REST API v0.7 (Accept header and Javadoc), extends File/FileData with content_info/metadata/appdata and nullable-fallback accessors, adds unit and live integration tests for v0.7, and revises CI workflow triggers, snapshot publishing, and secret injection. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as Java Client
participant Uploadcare as Uploadcare API
participant Storage as Cloud Storage
User->>Client: uploadFile(olympia.jpg)
Client->>Uploadcare: POST /upload/ (file)
Uploadcare->>Storage: store file
Storage-->>Uploadcare: stored
Uploadcare-->>Client: fileId + metadata
User->>Client: getFile(fileId)
Client->>Uploadcare: GET /files/{id} (Accept: application/vnd.uploadcare-v0.7+json)
Uploadcare-->>Client: file + content_info (mime, image, video)
Client->>Client: map response -> File, ContentInfo, AppData
Client-->>User: File object with v0.7 metadata
Note over Client: getImageInfo()/getVideoInfo() fallback: prefer primary field, else use contentInfo.image/video
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
carlspring
force-pushed
the
issue/68/add-support-for-rest-api-v07
branch
2 times, most recently
from
5605c13 to
8b65218
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/test/java/com/uploadcare/api/FileTest.java (1)
35-72: Please add assertions throughFile, not onlyFileData.These tests prove deserialization, but the behavior change is in
File#getImageInfo(),getVideoInfo(),getContentInfo(),getMetadata(), andgetAppData(). Since this class is already incom.uploadcare.api, a small test aroundnew File(null, fileData)would cover the actual API surface and fallback logic that changed.Also applies to: 75-124, 127-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/uploadcare/api/FileTest.java` around lines 35 - 72, Update the test to assert through the public File API rather than only on FileData: create a File instance with new File(null, fileData) and call File#getContentInfo(), getImageInfo(), getVideoInfo(), getMetadata(), and getAppData() and assert the same fields (mime.mime/type/subtype, image.format/width/height/colorMode) are exposed and mapped correctly (e.g., File.ColorMode.RGB). Replace or add assertions that reference fileData.contentInfo/... with equivalent assertions that use file.getContentInfo() and file.getImageInfo() to validate the API surface and fallback logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 49-53: The workflow exposes secrets to PR head code: the build job
injects UPLOADCARE_PUBLIC_KEY and UPLOADCARE_SECRET_KEY into the environment and
the publish-snapshot job uses GITHUB_TOKEN/packages:write while checking out and
running the PR head, allowing secrets exfiltration on forked PRs; to fix, gate
any steps that set these env vars or perform publishing (refer to the build job,
the env keys UPLOADCARE_PUBLIC_KEY/UPLOADCARE_SECRET_KEY, the publish-snapshot
job and GITHUB_TOKEN permission) behind a trusted-ref conditional such as if:
github.event_name == 'push' or if: github.ref == 'refs/heads/main', or
alternatively change the flow to use pull_request_target only for steps that
need secrets and fetch untrusted code via artifacts from a trusted runner (pass
build artifacts instead of running PR head with secrets); apply the conditional
to the steps that set env or run ./gradlew build --stacktrace and to any publish
steps with packages:write.
In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java`:
- Around line 77-87: The tests currently use response.getFirstHeader("Warning")
which misses cases with multiple Warning headers; update both
test_v06AcceptHeader_triggersDeprecationWarning() and
test_v07AcceptHeader_noDeprecationWarning() to call
response.getHeaders("Warning") and scan the returned Header[]: for the v06 test
assert that at least one header's getValue() contains "0.6" (replace the single
warningHeader/assertTrue logic), and for the v07 test assert that none of the
headers' getValue() contain "0.6" (replace the single
warningHeader/assertNull/assertTrue logic), keeping the existing failure
messages but iterating over the Header[] to avoid ordering-dependent false
failures/passes.
---
Nitpick comments:
In `@src/test/java/com/uploadcare/api/FileTest.java`:
- Around line 35-72: Update the test to assert through the public File API
rather than only on FileData: create a File instance with new File(null,
fileData) and call File#getContentInfo(), getImageInfo(), getVideoInfo(),
getMetadata(), and getAppData() and assert the same fields
(mime.mime/type/subtype, image.format/width/height/colorMode) are exposed and
mapped correctly (e.g., File.ColorMode.RGB). Replace or add assertions that
reference fileData.contentInfo/... with equivalent assertions that use
file.getContentInfo() and file.getImageInfo() to validate the API surface and
fallback logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6523c8d4-a723-452e-8475-3c5894eae185
📒 Files selected for processing (6)
.github/workflows/build.ymlsrc/main/java/com/uploadcare/api/File.javasrc/main/java/com/uploadcare/api/RequestHelper.javasrc/main/java/com/uploadcare/data/FileData.javasrc/test/java/com/uploadcare/api/FileTest.javasrc/test/java/com/uploadcare/api/RestApiVersionITTest.java
carlspring
force-pushed
the
issue/68/add-support-for-rest-api-v07
branch
from
8b65218 to
d3bbb10
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/uploadcare/api/FileTest.java (1)
143-170: Consider adding date format configuration or assertions for datetime fields.The JSON includes
datetime_createdanddatetime_updatedfields that map tojava.util.DateinAppData, but:
- The
ObjectMapperhas no date format configuration (e.g.,JavaTimeModuleor date format pattern)- The test doesn't assert on
datetimeCreated/datetimeUpdatedJackson's handling of ISO-8601 date strings for
java.util.Datecan be inconsistent. Either:
- Configure date handling on the mapper to ensure reliable parsing
- Remove the datetime fields from the test JSON if not under test
- Add assertions for the datetime fields to verify they're parsed correctly
Option 1: Remove datetime fields if not testing them
String json = "{" + "\"uuid\": \"22240276-2f06-41f8-9411-755c8ce926ed\"," + "\"appdata\": {" + " \"uc_clamav_virus_scan\": {" + " \"data\": {" + " \"infected\": false," + " \"infected_with\": null" + " }," - + " \"version\": \"0.104.2\"," - + " \"datetime_created\": \"2021-09-21T11:24:33\"," - + " \"datetime_updated\": \"2021-09-21T11:24:33\"" + + " \"version\": \"0.104.2\"" + " }" + "}" + "}";Option 2: Add assertions for datetime fields
assertEquals("0.104.2", appData.version); assertNotNull("appdata.data should not be null", appData.data); + assertNotNull("datetimeCreated should not be null", appData.datetimeCreated); + assertNotNull("datetimeUpdated should not be null", appData.datetimeUpdated); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/uploadcare/api/FileTest.java` around lines 143 - 170, The test includes ISO-8601 datetime strings but the test ObjectMapper has no date handling configuration and the test doesn't assert the parsed dates; update the test to configure the mapper for ISO-8601 Date parsing (e.g., register JavaTimeModule or setDateFormat(new StdDateFormat()) on the mapper used in this test) and then add assertions on the File.AppData datetime fields (e.g., check appData.datetimeCreated and appData.datetimeUpdated are non-null and equal to expected instants) so the datetime_created/datetime_updated values are parsed and verified; alternatively, if datetimes are out of scope, remove datetime_created/datetime_updated from the test JSON instead of changing mapper behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/java/com/uploadcare/api/FileTest.java`:
- Around line 143-170: The test includes ISO-8601 datetime strings but the test
ObjectMapper has no date handling configuration and the test doesn't assert the
parsed dates; update the test to configure the mapper for ISO-8601 Date parsing
(e.g., register JavaTimeModule or setDateFormat(new StdDateFormat()) on the
mapper used in this test) and then add assertions on the File.AppData datetime
fields (e.g., check appData.datetimeCreated and appData.datetimeUpdated are
non-null and equal to expected instants) so the
datetime_created/datetime_updated values are parsed and verified; alternatively,
if datetimes are out of scope, remove datetime_created/datetime_updated from the
test JSON instead of changing mapper behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 378a78a8-7c8a-4ac2-bc14-1cae06f1bc14
📒 Files selected for processing (6)
.github/workflows/build.ymlsrc/main/java/com/uploadcare/api/File.javasrc/main/java/com/uploadcare/api/RequestHelper.javasrc/main/java/com/uploadcare/data/FileData.javasrc/test/java/com/uploadcare/api/FileTest.javasrc/test/java/com/uploadcare/api/RestApiVersionITTest.java
✅ Files skipped from review due to trivial changes (2)
- src/main/java/com/uploadcare/data/FileData.java
- src/main/java/com/uploadcare/api/RequestHelper.java
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/build.yml
- src/main/java/com/uploadcare/api/File.java
- src/test/java/com/uploadcare/api/RestApiVersionITTest.java
carlspring
force-pushed
the
issue/68/add-support-for-rest-api-v07
branch
from
d3bbb10 to
55ad1e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/test/java/com/uploadcare/api/RestApiVersionITTest.java (1)
77-87:⚠️ Potential issue | 🟡 MinorScan all
Warningheaders, not only the first one.Current assertions are order-dependent (
getFirstHeader("Warning")) and can false-pass/false-fail when multipleWarningheaders are present.Suggested fix
- Header warningHeader = response.getFirstHeader("Warning"); - assertNotNull( + boolean hasV06Warning = Arrays.stream(response.getHeaders("Warning")) + .map(Header::getValue) + .anyMatch(value -> value.contains("0.6")); + assertTrue( "BUG REPRODUCTION: Expected a Warning response header when using the " + "deprecated v0.6 Accept header, but none was found.\n" + "All response headers received: " + Arrays.toString(response.getAllHeaders()), - warningHeader); - assertTrue( - "BUG REPRODUCTION: The Warning header should mention API version deprecation " - + "(e.g. 'not 0.6'), but was: " + warningHeader.getValue(), - warningHeader.getValue().contains("0.6")); + hasV06Warning);- Header warningHeader = response.getFirstHeader("Warning"); - // A Warning header is acceptable as long as it is NOT an API-version deprecation - // warning (e.g. demo-project notices are fine, but "not 0.6" / "API version" - // upgrade messages must not appear when the client already uses v0.7). - if (warningHeader != null) { - assertFalse("FIX VERIFICATION: No API-version deprecation Warning should be returned " + - "when using the v0.7 Accept header, but one was found: " + - warningHeader.getValue() + "\n" + - "All response headers: " + Arrays.toString(response.getAllHeaders()), - warningHeader.getValue().contains("0.6")); - } + boolean hasV06Warning = Arrays.stream(response.getHeaders("Warning")) + .map(Header::getValue) + .anyMatch(value -> value.contains("0.6")); + assertFalse("FIX VERIFICATION: No API-version deprecation Warning should be returned " + + "when using the v0.7 Accept header.\n" + + "All response headers: " + Arrays.toString(response.getAllHeaders()), + hasV06Warning);#!/bin/bash # Verify all Warning-header assertions in this IT class and ensure getHeaders("Warning") is used. rg -n -C2 'getFirstHeader\("Warning"\)|getHeaders\("Warning"\)' src/test/java/com/uploadcare/api/RestApiVersionITTest.javaAlso applies to: 114-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java` around lines 77 - 87, The test in RestApiVersionITTest currently uses response.getFirstHeader("Warning") (see the assertion block that references warningHeader) which is order-dependent; change it to response.getHeaders("Warning") and assert that the returned array is non-empty, then scan (loop or stream) the headers to check at least one header's getValue() contains "0.6" (update the assertion messages to include all headers via Arrays.toString(headers)); apply the same change to the other occurrence in this class (the block around the second assertion group currently using getFirstHeader).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java`:
- Around line 149-153: The test in RestApiVersionITTest opens an InputStream via
getClass().getClassLoader().getResourceAsStream("olympia.jpg") and never closes
it; wrap the resource acquisition and the call to
IOUtils.toByteArray(imageStream) in a try-with-resources so imageStream is
automatically closed (replace the standalone imageStream variable usage with a
try (InputStream imageStream =
getClass().getClassLoader().getResourceAsStream("olympia.jpg")) { byte[]
imageBytes = IOUtils.toByteArray(imageStream); ... } ensuring the assertNotNull
check remains meaningful inside or immediately after resource acquisition).
---
Duplicate comments:
In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java`:
- Around line 77-87: The test in RestApiVersionITTest currently uses
response.getFirstHeader("Warning") (see the assertion block that references
warningHeader) which is order-dependent; change it to
response.getHeaders("Warning") and assert that the returned array is non-empty,
then scan (loop or stream) the headers to check at least one header's getValue()
contains "0.6" (update the assertion messages to include all headers via
Arrays.toString(headers)); apply the same change to the other occurrence in this
class (the block around the second assertion group currently using
getFirstHeader).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f19536be-3309-4404-9d4a-74530edad6bf
📒 Files selected for processing (6)
.github/workflows/build.ymlsrc/main/java/com/uploadcare/api/File.javasrc/main/java/com/uploadcare/api/RequestHelper.javasrc/main/java/com/uploadcare/data/FileData.javasrc/test/java/com/uploadcare/api/FileTest.javasrc/test/java/com/uploadcare/api/RestApiVersionITTest.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/uploadcare/data/FileData.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/uploadcare/api/RequestHelper.java
- src/test/java/com/uploadcare/api/FileTest.java
- .github/workflows/build.yml
Comment on lines
+149
to
+153
| InputStream imageStream = getClass().getClassLoader().getResourceAsStream("olympia.jpg"); | ||
|
|
||
| assertNotNull("Test resource olympia.jpg must be on the classpath", imageStream); | ||
|
|
||
| byte[] imageBytes = IOUtils.toByteArray(imageStream); |
There was a problem hiding this comment.
Close the test resource stream with try-with-resources.
imageStream is not closed after IOUtils.toByteArray(...).
Suggested fix
- InputStream imageStream = getClass().getClassLoader().getResourceAsStream("olympia.jpg");
-
- assertNotNull("Test resource olympia.jpg must be on the classpath", imageStream);
-
- byte[] imageBytes = IOUtils.toByteArray(imageStream);
+ byte[] imageBytes;
+ try (InputStream imageStream = getClass().getClassLoader().getResourceAsStream("olympia.jpg")) {
+ assertNotNull("Test resource olympia.jpg must be on the classpath", imageStream);
+ imageBytes = IOUtils.toByteArray(imageStream);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| InputStream imageStream = getClass().getClassLoader().getResourceAsStream("olympia.jpg"); | |
| assertNotNull("Test resource olympia.jpg must be on the classpath", imageStream); | |
| byte[] imageBytes = IOUtils.toByteArray(imageStream); | |
| byte[] imageBytes; | |
| try (InputStream imageStream = getClass().getClassLoader().getResourceAsStream("olympia.jpg")) { | |
| assertNotNull("Test resource olympia.jpg must be on the classpath", imageStream); | |
| imageBytes = IOUtils.toByteArray(imageStream); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java` around lines 149
- 153, The test in RestApiVersionITTest opens an InputStream via
getClass().getClassLoader().getResourceAsStream("olympia.jpg") and never closes
it; wrap the resource acquisition and the call to
IOUtils.toByteArray(imageStream) in a try-with-resources so imageStream is
automatically closed (replace the standalone imageStream variable usage with a
try (InputStream imageStream =
getClass().getClassLoader().getResourceAsStream("olympia.jpg")) { byte[]
imageBytes = IOUtils.toByteArray(imageStream); ... } ensuring the assertNotNull
check remains meaningful inside or immediately after resource acquisition).
55ad1e5 to
b8b1620
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/test/java/com/uploadcare/api/RestApiVersionITTest.java (2)
77-87:⚠️ Potential issue | 🟡 MinorScan all
Warningheaders, not just the first one.Using
getFirstHeader("Warning")makes both tests order-dependent and can false-pass/false-fail when multiple warning headers are present.Suggested fix
- Header warningHeader = response.getFirstHeader("Warning"); - assertNotNull( + boolean hasV06Warning = Arrays.stream(response.getHeaders("Warning")) + .map(Header::getValue) + .anyMatch(value -> value.contains("0.6")); + assertTrue( "BUG REPRODUCTION: Expected a Warning response header when using the " + "deprecated v0.6 Accept header, but none was found.\n" + "All response headers received: " + Arrays.toString(response.getAllHeaders()), - warningHeader); - assertTrue( - "BUG REPRODUCTION: The Warning header should mention API version deprecation " - + "(e.g. 'not 0.6'), but was: " + warningHeader.getValue(), - warningHeader.getValue().contains("0.6")); + hasV06Warning); @@ - Header warningHeader = response.getFirstHeader("Warning"); - // A Warning header is acceptable as long as it is NOT an API-version deprecation - // warning (e.g. demo-project notices are fine, but "not 0.6" / "API version" - // upgrade messages must not appear when the client already uses v0.7). - if (warningHeader != null) { - assertFalse("FIX VERIFICATION: No API-version deprecation Warning should be returned " + - "when using the v0.7 Accept header, but one was found: " + - warningHeader.getValue() + "\n" + - "All response headers: " + Arrays.toString(response.getAllHeaders()), - warningHeader.getValue().contains("0.6")); - } + boolean hasV06Warning = Arrays.stream(response.getHeaders("Warning")) + .map(Header::getValue) + .anyMatch(value -> value.contains("0.6")); + assertFalse("FIX VERIFICATION: No API-version deprecation Warning should be returned " + + "when using the v0.7 Accept header.\n" + + "All response headers: " + Arrays.toString(response.getAllHeaders()), + hasV06Warning);#!/bin/bash # Verify warning-header access pattern in this test rg -n 'getFirstHeader\("Warning"\)|getHeaders\("Warning"\)' src/test/java/com/uploadcare/api/RestApiVersionITTest.javaAlso applies to: 114-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java` around lines 77 - 87, The test currently uses response.getFirstHeader("Warning") (variable warningHeader) which is order-dependent; change to use response.getHeaders("Warning") and scan the returned Header[] for presence and for at least one header value containing "0.6". Update both assertion blocks in RestApiVersionITTest (the presence check and the contains check) to iterate the headers array, assert it's non-empty, and assert that any header.getValue().contains("0.6") is true instead of relying on getFirstHeader.
149-153:⚠️ Potential issue | 🟡 MinorClose
imageStreamwith try-with-resources.
imageStreamis currently left open after reading bytes.Suggested fix
- InputStream imageStream = getClass().getClassLoader().getResourceAsStream("olympia.jpg"); - - assertNotNull("Test resource olympia.jpg must be on the classpath", imageStream); - - byte[] imageBytes = IOUtils.toByteArray(imageStream); + byte[] imageBytes; + try (InputStream imageStream = getClass().getClassLoader().getResourceAsStream("olympia.jpg")) { + assertNotNull("Test resource olympia.jpg must be on the classpath", imageStream); + imageBytes = IOUtils.toByteArray(imageStream); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java` around lines 149 - 153, In RestApiVersionITTest, the InputStream imageStream created for the "olympia.jpg" resource is not closed; wrap the getResourceAsStream call and the IOUtils.toByteArray(imageStream) usage in a try-with-resources block (using the imageStream variable) so the stream is automatically closed after reading, preserving the assertNotNull check before or inside the block as appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 4-7: Re-enable the pull_request_target workflow trigger or remove
the PR-only logic: restore the commented block that defines pull_request_target
(so the workflow triggers on pull_request_target events) and keep the existing
external-contributors environment guard, or alternatively remove/update the
publish-snapshot job's PR-specific checks (the condition github.event_name ==
'pull_request_target' in the publish-snapshot job and any references to
github.event.pull_request used in steps) so the job no longer expects
pull_request properties when running on push. Target the pull_request_target
trigger and the publish-snapshot job/steps to ensure PR snapshots can run
safely.
---
Duplicate comments:
In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java`:
- Around line 77-87: The test currently uses response.getFirstHeader("Warning")
(variable warningHeader) which is order-dependent; change to use
response.getHeaders("Warning") and scan the returned Header[] for presence and
for at least one header value containing "0.6". Update both assertion blocks in
RestApiVersionITTest (the presence check and the contains check) to iterate the
headers array, assert it's non-empty, and assert that any
header.getValue().contains("0.6") is true instead of relying on getFirstHeader.
- Around line 149-153: In RestApiVersionITTest, the InputStream imageStream
created for the "olympia.jpg" resource is not closed; wrap the
getResourceAsStream call and the IOUtils.toByteArray(imageStream) usage in a
try-with-resources block (using the imageStream variable) so the stream is
automatically closed after reading, preserving the assertNotNull check before or
inside the block as appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86fe486c-221f-46bb-98e5-d110860a7135
📒 Files selected for processing (6)
.github/workflows/build.ymlsrc/main/java/com/uploadcare/api/File.javasrc/main/java/com/uploadcare/api/RequestHelper.javasrc/main/java/com/uploadcare/data/FileData.javasrc/test/java/com/uploadcare/api/FileTest.javasrc/test/java/com/uploadcare/api/RestApiVersionITTest.java
✅ Files skipped from review due to trivial changes (3)
- src/main/java/com/uploadcare/data/FileData.java
- src/main/java/com/uploadcare/api/RequestHelper.java
- src/test/java/com/uploadcare/api/FileTest.java
Fixes for #68: * Applied the required code fixes. * Added unit + integration tests.
carlspring
force-pushed
the
issue/68/add-support-for-rest-api-v07
branch
from
b8b1620 to
0f58913
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/uploadcare/api/RestApiVersionITTest.java (1)
72-91: Consider simplifying with try-with-resources forhttpClient.The outer
httpClientcan be managed with try-with-resources, matching the pattern already used for the innerCloseableHttpResponse.♻️ Suggested simplification
- CloseableHttpClient httpClient = HttpClients.createDefault(); - try { + try (CloseableHttpClient httpClient = HttpClients.createDefault()) { HttpGet request = buildRequest(Urls.apiFiles(), "application/vnd.uploadcare-v0.6+json"); try (CloseableHttpResponse response = httpClient.execute(request)) { // ... assertions ... } - } finally { - httpClient.close(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java` around lines 72 - 91, The test currently creates CloseableHttpClient httpClient and closes it in a finally block; change it to use try-with-resources so the client is auto-closed and matches the existing try-with-resources for CloseableHttpResponse. Specifically, replace the manual creation of httpClient and the surrounding try/finally with a try (CloseableHttpClient httpClient = HttpClients.createDefault()) { ... } block in RestApiVersionITTest so buildRequest(...), the inner try (CloseableHttpResponse response = httpClient.execute(request)), and the assertions remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/java/com/uploadcare/api/RestApiVersionITTest.java`:
- Around line 72-91: The test currently creates CloseableHttpClient httpClient
and closes it in a finally block; change it to use try-with-resources so the
client is auto-closed and matches the existing try-with-resources for
CloseableHttpResponse. Specifically, replace the manual creation of httpClient
and the surrounding try/finally with a try (CloseableHttpClient httpClient =
HttpClients.createDefault()) { ... } block in RestApiVersionITTest so
buildRequest(...), the inner try (CloseableHttpResponse response =
httpClient.execute(request)), and the assertions remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 929e0c7b-7a28-496e-b9dc-fbe606afed1c
📒 Files selected for processing (6)
.github/workflows/build.ymlsrc/main/java/com/uploadcare/api/File.javasrc/main/java/com/uploadcare/api/RequestHelper.javasrc/main/java/com/uploadcare/data/FileData.javasrc/test/java/com/uploadcare/api/FileTest.javasrc/test/java/com/uploadcare/api/RestApiVersionITTest.java
✅ Files skipped from review due to trivial changes (3)
- src/main/java/com/uploadcare/data/FileData.java
- src/main/java/com/uploadcare/api/RequestHelper.java
- src/test/java/com/uploadcare/api/FileTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build.yml
alxgsv
approved these changes
Mar 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task Description
Fixes for #68:
Summary by CodeRabbit
New Features
Improvements
Tests
Chores