✨ allow opening a LocalResponse from bytes - #352
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The updated BaseLocalResponse file/path constructors can now throw UncheckedIOException despite declaring throws IOException, and the new HMAC normalization should avoid locale-dependent case-mapping while also documenting stream-closing behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR expands LocalResponse to be constructible from raw byte[] payloads (in both v1 and v2), refactors local-response payload loading/normalization in BaseLocalResponse, and updates the test suite/resource helpers accordingly. It also updates HMAC signature verification to use a constant-time comparison primitive.
Changes:
- Add
LocalResponse(byte[])constructors for both v1 and v2 parsing. - Rework
BaseLocalResponsepayload ingestion to normalize JSON (remove line endings) across input types and update HMAC verification. - Rework tests to use a new
getV2ProductPath(...)helper and add broaderLocalResponseconstructor coverage.
File summaries
| File | Description |
|---|---|
| src/test/java/com/mindee/v2/product/SplitTest.java | Switches v2 product test fixtures to getV2ProductPath and simplifies fixture-relative paths. |
| src/test/java/com/mindee/v2/product/OcrTest.java | Updates OCR product tests to use getV2ProductPath and new fixture paths. |
| src/test/java/com/mindee/v2/product/ExtractionTest.java | Updates extraction product tests to use getV2ProductPath and new fixture paths. |
| src/test/java/com/mindee/v2/product/CropTest.java | Updates crop product tests to use getV2ProductPath and new fixture paths. |
| src/test/java/com/mindee/v2/product/CropIT.java | Updates integration test inputs to use getV2ProductPath. |
| src/test/java/com/mindee/v2/product/ClassificationTest.java | Updates classification product tests to use getV2ProductPath and new fixture paths. |
| src/test/java/com/mindee/v2/parsing/LocalResponseTest.java | Adds constructor coverage (String/byte[]/Path/stream), HMAC checks, and null/empty validation tests. |
| src/test/java/com/mindee/v2/MindeeClientTest.java | Renames test methods (removes _async suffix). |
| src/test/java/com/mindee/v2/MindeeClientIT.java | Updates v2 integration tests to use getV2ProductPath for fixtures and schema params. |
| src/test/java/com/mindee/v2/fileoperations/SplitTest.java | Updates file-operation tests to use getV2ProductPath and new fixture paths. |
| src/test/java/com/mindee/v2/fileoperations/CropTest.java | Updates file-operation tests to use getV2ProductPath and new fixture paths. |
| src/test/java/com/mindee/TestingUtilities.java | Introduces getV2ProductPath helper and reorders resource-path helpers. |
| src/main/java/com/mindee/v2/parsing/LocalResponse.java | Adds LocalResponse(byte[]) constructor (v2). |
| src/main/java/com/mindee/v1/parsing/LocalResponse.java | Adds LocalResponse(byte[]) constructor (v1). |
| src/main/java/com/mindee/parsing/BaseLocalResponse.java | Refactors payload ingestion, adds input validation, adds constant-time signature compare, and overrides toString(). |
Review details
Suppressed comments (1)
src/main/java/com/mindee/parsing/BaseLocalResponse.java:91
BaseLocalResponse(Path)declaresthrows IOException, butreadToCleanUtf8Bytes(...)wraps read failures inUncheckedIOException, so callers catchingIOExceptionmay miss real I/O errors. Consider unwrappingUncheckedIOExceptionhere and rethrowing itsIOExceptioncause to match the method signature.
public BaseLocalResponse(Path input) throws IOException {
if (input == null) {
throw new IllegalArgumentException("Input path cannot be null.");
}
this.file = readToCleanUtf8Bytes(Files.newBufferedReader(input, StandardCharsets.UTF_8));
- Files reviewed: 15/15 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
83f8fcc to
60f14c1
Compare
9a91d0c to
d267daa
Compare
d267daa to
756d1c1
Compare
756d1c1 to
09da6ba
Compare
09da6ba to
e2dd2d1
Compare
There was a problem hiding this comment.
🟢 Approval recommended
Only a minor Javadoc wording issue was found; the functional changes are straightforward and are backed by expanded constructor-path test coverage.
Review details
Suppressed comments (1)
src/main/java/com/mindee/parsing/BaseLocalResponse.java:32
- The Javadoc for the String constructor says "Assumes UTF-8 encoding", but a Java String has no encoding; what matters is that it is encoded to UTF-8 bytes internally (and line separators are normalized away via lines()+joining). This wording is misleading for API consumers.
/**
* Load from a {@link String}.
*
* @param input Assumes UTF-8 encoding.
*/
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
Description
Types of changes