[INJICERT-1226] Add image compressor utility to data provider plugins#123
[INJICERT-1226] Add image compressor utility to data provider plugins#123Piyush7034 wants to merge 8 commits intoinji:developfrom
Conversation
Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>
Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>
Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>
Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>
Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>
WalkthroughAdds OpenCV dependencies and ImageCompressorUtil implementations (mock and postgres) to compress face images to ≤1 KB; data providers now compress face data and standardize error handling to "ERROR_FETCHING_IDENTITY_DATA". Project versions bumped to 0.5.1-SNAPSHOT and related tests updated/added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Plugin
participant DataSource
participant Compressor
Client->>Plugin: fetchData(individualId)
Plugin->>DataSource: queryRecordByIdentifier(individualId)
DataSource-->>Plugin: recordJSON (may include "face"/encodedPhoto)
alt face present
Plugin->>Compressor: compressImageData(faceData)
Compressor-->>Plugin: compressedFaceData (<=1KB) or error
Plugin->>Plugin: replace "face" with compressedFaceData
end
alt success
Plugin-->>Client: return JSON (with compressed face)
else error
Plugin-->>Client: throw DataProviderExchangeException("ERROR_FETCHING_IDENTITY_DATA", ...)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mock-certify-plugin/src/test/java/io/mosip/certify/mock/integration/service/MockCSVDataProviderPluginTest.java (1)
74-81: Test does not fail if no exception is thrown.The test catches
DataProviderExchangeExceptionbut doesn't callAssert.fail()if the exception is not thrown, meaning the test will pass silently even iffetchDatasucceeds unexpectedly.🔎 Proposed fix
@Test public void fetchJsonDataWithInValidIndividualId_thenFail() { try { mockCSVDataProviderPlugin.fetchData(Map.of("sub", "12345678", "client_id", "CLIENT_ID")); + Assert.fail("Expected DataProviderExchangeException"); } catch (DataProviderExchangeException e) { Assert.assertEquals("ERROR_FETCHING_IDENTITY_DATA", e.getMessage()); } }
🧹 Nitpick comments (12)
mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockIdaDataProviderPlugin.java (1)
69-69: Consider a more specific error message for missing individualId.While the unified error code is good for consistency, this path is reached when
individualIdis null. A more specific message like"Missing individualId in identity details"would provide clearer guidance for this specific failure case.🔎 Proposed refinement
- throw new DataProviderExchangeException("ERROR_FETCHING_IDENTITY_DATA", "Check the individualId or data source"); + throw new DataProviderExchangeException("ERROR_FETCHING_IDENTITY_DATA", "Missing individualId in identity details");postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtil.java (3)
20-23: Duplicate constant definition.
TARGET_SIZEis defined both as a class-level constant (line 20) and as a local variableTARGET_SIZE_BYTES(line 23) with the same value. Remove the redundant local variable and use the class constant.🔎 Proposed fix
public static byte[] validateAndCompressFaceImage(byte[] imageBytes, String extension) throws Exception { - final int TARGET_SIZE_BYTES = 1024; // 1 KB - Mat src = Imgcodecs.imdecode(new MatOfByte(imageBytes), Imgcodecs.IMREAD_UNCHANGED);And update line 68:
- if (result.length <= TARGET_SIZE_BYTES) { + if (result.length <= TARGET_SIZE) {
22-22: Avoid throwing genericException.The method signature declares
throws Exception, but the implementation only throwsDataProviderExchangeException. Narrowing the exception type improves API clarity and enables callers to handle exceptions more precisely.🔎 Proposed fix
-public static byte[] validateAndCompressFaceImage(byte[] imageBytes, String extension) throws Exception { +public static byte[] validateAndCompressFaceImage(byte[] imageBytes, String extension) throws DataProviderExchangeException {
83-83: Avoid throwing genericException.Same issue as
validateAndCompressFaceImage- narrow the exception type toDataProviderExchangeException.🔎 Proposed fix
-public static String compressImageData(String imageData) throws Exception { +public static String compressImageData(String imageData) throws DataProviderExchangeException {postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.java (1)
30-34: Remove unusedEnvironmentautowiring; clarifyImageCompressorUtilusage.
Environment envis autowired but never used in this class. Additionally,ImageCompressorUtilis autowired as an instance field but called statically on line 52. Either remove the autowiring or use instance methods for consistency.🔎 Proposed fix
- @Autowired - private Environment env; - - @Autowired - ImageCompressorUtil imageCompressorUtil; -mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockCSVDataProviderPlugin.java (3)
7-8: Remove commented-out code.Lines 7 and 38-39 contain commented-out imports and autowiring. These should be removed to keep the codebase clean.
🔎 Proposed fix
-//import io.mosip.certify.util.ImageCompressorUtil; import io.mosip.certify.util.ImageCompressorUtil;-// @Autowired -// private ImageCompressorUtil imageCompressorUtil;Also applies to: 38-39
26-26: Remove unused import.
java.util.Base64is imported but not used in this file.
88-93: ThejsonRes != nullcheck is redundant.
csvReader.getJsonObjectByIdentifier()already throwsDataProviderExchangeExceptionwhen the record is not found, sojsonReswill never be null at this point. Thehas("face")check is sufficient.🔎 Proposed fix
- if(jsonRes != null && jsonRes.has("face")) { + if(jsonRes.has("face")) {mock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.java (1)
24-33: Consider testing with more realistic image data.The dummy image created is a solid black image that compresses extremely well. Consider adding a test with random pixel data or a real-world image to better validate compression behavior under realistic conditions.
mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java (2)
20-23: Remove redundant constant.
TARGET_SIZE(line 20) andTARGET_SIZE_BYTES(line 23) are identical. Use a single constant for consistency.🔎 Proposed fix
private static final int TARGET_SIZE = 1024; public static byte[] validateAndCompressFaceImage(byte[] imageBytes, String extension) throws Exception { - final int TARGET_SIZE_BYTES = 1024; // 1 KB // ... - if (result.length <= TARGET_SIZE_BYTES) { + if (result.length <= TARGET_SIZE) {
54-57: Optimization: Move loop-invariant variables outside the loop.
isPng,outputExt, and the params template determination don't change between iterations. Consider hoisting them outside the loop to avoid redundant evaluations.postgres-dataprovider-plugin/src/test/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtilTest.java (1)
14-88: LGTM, but note code duplication across modules.This test class is nearly identical to
mock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.java. If both modules share the sameImageCompressorUtilimplementation, consider extracting the utility to a shared module to avoid maintaining duplicate code.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
mock-certify-plugin/pom.xmlmock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockCSVDataProviderPlugin.javamock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockIdaDataProviderPlugin.javamock-certify-plugin/src/main/java/io/mosip/certify/util/CSVReader.javamock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.javamock-certify-plugin/src/test/java/io/mosip/certify/mock/integration/service/MockCSVDataProviderPluginTest.javamock-certify-plugin/src/test/java/io/mosip/certify/mock/integration/service/MockIdaDataProviderPluginTest.javamock-certify-plugin/src/test/java/io/mosip/certify/util/CSVReaderTest.javamock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.javamosip-identity-certify-plugin/pom.xmlpostgres-dataprovider-plugin/pom.xmlpostgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.javapostgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtil.javapostgres-dataprovider-plugin/src/test/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtilTest.javasunbird-rc-certify-integration-impl/pom.xmlsunbird-rc-esignet-integration-impl/pom.xml
🧰 Additional context used
🧬 Code graph analysis (1)
postgres-dataprovider-plugin/src/test/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtilTest.java (1)
mock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.java (1)
ImageCompressorUtilTest(14-88)
🔇 Additional comments (17)
mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockIdaDataProviderPlugin.java (2)
61-63: LGTM!The explicit handling of
DataProviderExchangeExceptioncorrectly preserves and propagates the original exception while adding appropriate logging.
64-67: Improved error handling with unified error code.The updated exception handling provides a more descriptive error code and actionable message. The original exception is logged before wrapping, which aids debugging.
Note: All failures (network, compression, parsing) now map to the same error code. This is acceptable for a unified data provider interface, but ensure monitoring/alerting can distinguish root causes from the logged exceptions.
mosip-identity-certify-plugin/pom.xml (1)
11-11: LGTM!Version bump to 0.5.1-SNAPSHOT aligns with other modules in this PR.
sunbird-rc-esignet-integration-impl/pom.xml (1)
7-7: LGTM!Version bump to 0.5.1-SNAPSHOT is consistent with the other modules.
sunbird-rc-certify-integration-impl/pom.xml (1)
11-11: LGTM!Version bump to 0.5.1-SNAPSHOT is consistent with the other modules.
mock-certify-plugin/pom.xml (1)
260-281: LGTM!The maven-compiler-plugin configuration with Lombok annotation processor is correctly set up. The commented-out diagnostic block is acceptable for debugging purposes.
postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtil.java (2)
153-159: WebP images detected but not properly handled.WebP format is detected and returns
"image/webp", but on line 122, only PNG and JPEG extensions are derived. WebP images will be processed as JPEG (fallback), which may cause decoding issues since OpenCV'simdecodehas limited WebP support depending on the build.Consider either:
- Explicitly converting WebP to JPEG/PNG before compression
- Throwing an unsupported format exception for WebP
- Ensuring OpenCV is built with WebP support
46-76: LGTM on memory management and compression loop.The compression loop correctly releases
Matobjects after each iteration to prevent memory leaks. The iterative scaling approach with aggressive 0.7x reduction factor is a reasonable strategy for reaching the 1 KB target.mock-certify-plugin/src/test/java/io/mosip/certify/util/CSVReaderTest.java (1)
82-82: LGTM!Test expectation correctly updated to match the new error message format with the
ERROR_FETCHING_IDENTITY_DATA ->prefix, consistent with the updated error handling patterns across the PR.mock-certify-plugin/src/test/java/io/mosip/certify/mock/integration/service/MockIdaDataProviderPluginTest.java (2)
46-46: LGTM!Test now uses a valid data URL with a small base64-encoded PNG image, which is appropriate for testing the new image compression functionality.
78-78: LGTM!Error message expectation updated to match the new standardized error format across data provider plugins.
mock-certify-plugin/src/test/java/io/mosip/certify/mock/integration/service/MockCSVDataProviderPluginTest.java (1)
89-89: LGTM!Test expectations correctly updated to align with the new
DataProviderExchangeExceptionerror message format and the standardized error handling patterns across the codebase.Also applies to: 98-98, 106-106
mock-certify-plugin/src/main/java/io/mosip/certify/util/CSVReader.java (1)
54-62: LGTM!The error handling correctly uses the two-argument
DataProviderExchangeExceptionconstructor with an error code and descriptive message, aligning with the standardized error handling pattern across the PR.postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.java (1)
57-62: LGTM!Exception handling correctly re-throws
DataProviderExchangeExceptionand wraps other exceptions with an appropriate error code.mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockCSVDataProviderPlugin.java (1)
95-102: LGTM!Exception handling correctly propagates
DataProviderExchangeExceptionand wraps generic exceptions with standardized error codes and messages.mock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.java (1)
14-88: LGTM!Comprehensive test coverage for
ImageCompressorUtilincluding valid inputs (data URLs, plain base64), error conditions (invalid images, malformed URLs), and size constraints. The tests effectively validate the core functionality.mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java (1)
140-162: LGTM!The MIME type detection correctly identifies PNG and WebP magic bytes, with a sensible fallback to JPEG for unrecognized formats.
...lugin/src/main/java/io.mosip.certify.mock.integration/service/MockIdaDataProviderPlugin.java
Outdated
Show resolved
Hide resolved
...lugin/src/main/java/io.mosip.certify.mock.integration/service/MockIdaDataProviderPlugin.java
Outdated
Show resolved
Hide resolved
mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java
Show resolved
Hide resolved
...va/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.java
Show resolved
Hide resolved
Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java (1)
45-75: Loop may return an image exceeding 1 KB.This concern was previously raised: when
scale < 0.01, the loop exits at line 75 and returnsresult, which may still exceedTARGET_SIZE(1024 bytes). WhilecompressImageDataenforces the limit (lines 126-131),validateAndCompressFaceImagecan be called directly and would return an oversized image without validation.Consider throwing a
DataProviderExchangeExceptioninvalidateAndCompressFaceImagewhen the target size cannot be achieved after the loop completes.🔎 Proposed fix
// Aggressive reduction for next iteration scale *= 0.7; } - return result; // Returns the smallest possible version found + // If we couldn't compress to target size, throw an exception + if (result != null && result.length > TARGET_SIZE) { + throw new DataProviderExchangeException( + "COMPRESSION_FAILED", + "Unable to compress image to target size of " + TARGET_SIZE + " bytes" + ); + } + return result; } finally {
🧹 Nitpick comments (6)
mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockIdaDataProviderPlugin.java (1)
50-56: Previous review feedback addressed successfully.The redundant assignment and input validation issues from the previous review have been properly fixed. The code now:
- Validates that
encodedPhotoexists and is not null before processing- Checks that the image data string is not empty
- Only then compresses and stores the result
Optional: Consider validating compressed result.
As a defensive programming practice, you could validate that
compressedImageDatais not null or empty before putting it in the response:🔎 Optional defensive validation
if(res.containsKey("encodedPhoto") && res.get("encodedPhoto") != null) { String imageData = res.get("encodedPhoto").toString(); if(!imageData.isEmpty()) { String compressedImageData = ImageCompressorUtil.compressImageData(imageData); - jsonRes.put("face", compressedImageData); + if(compressedImageData != null && !compressedImageData.isEmpty()) { + jsonRes.put("face", compressedImageData); + } } }mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java (5)
16-18: Add error handling to static initialization block.If OpenCV native libraries fail to load, the static initializer will throw an
ExceptionInInitializerError, making the entire class unusable. Consider wrapping the loader call with try-catch and logging the error, or documenting the OpenCV dependency requirements clearly.🔎 Proposed fix
static { - Loader.load(opencv_java.class); + try { + Loader.load(opencv_java.class); + log.info("OpenCV native library loaded successfully"); + } catch (Exception e) { + log.error("Failed to load OpenCV native library. Image compression will not work.", e); + throw new ExceptionInInitializerError(e); + } }
22-22: Replace genericthrows Exceptionwith specific exception types.The method signature uses the overly broad
throws Exception, which makes it harder for callers to handle specific error cases. Consider declaring specific exceptions likeDataProviderExchangeExceptionorIOException.🔎 Proposed fix
-public static byte[] validateAndCompressFaceImage(byte[] imageBytes, String extension) throws Exception { +public static byte[] validateAndCompressFaceImage(byte[] imageBytes, String extension) throws DataProviderExchangeException {
36-38: Document magic numbers or extract as named constants.The compression algorithm uses several undocumented magic numbers:
80 * 80(line 37): target area assumption0.7(line 72): scale reduction factor40(line 56): initial JPEG quality16(line 46-47): minimum dimensionConsider extracting these as named constants with explanatory comments to improve maintainability.
🔎 Proposed refactor
+// Compression parameters +private static final int TARGET_AREA_PIXELS = 80 * 80; // Typical 1KB image dimensions +private static final double SCALE_REDUCTION_FACTOR = 0.7; // Aggressive 30% reduction per iteration +private static final int INITIAL_JPEG_QUALITY = 40; // Start with lower quality for faster convergence +private static final int MIN_DIMENSION = 16; // Minimum image dimension in pixels + public static byte[] validateAndCompressFaceImage(byte[] imageBytes, String extension) throws Exception { // ... - double area = src.width() * src.height(); - double targetArea = 80 * 80; - double scale = (area > targetArea) ? Math.sqrt(targetArea / area) : 1.0; + double area = src.width() * src.height(); + double scale = (area > TARGET_AREA_PIXELS) ? Math.sqrt(TARGET_AREA_PIXELS / area) : 1.0; // ... - int w = (int) Math.max(16, src.width() * scale); - int h = (int) Math.max(16, src.height() * scale); + int w = (int) Math.max(MIN_DIMENSION, src.width() * scale); + int h = (int) Math.max(MIN_DIMENSION, src.height() * scale); // ... MatOfInt params = isPng ? new MatOfInt(Imgcodecs.IMWRITE_PNG_COMPRESSION, 9) - : new MatOfInt(Imgcodecs.IMWRITE_JPEG_QUALITY, 40); + : new MatOfInt(Imgcodecs.IMWRITE_JPEG_QUALITY, INITIAL_JPEG_QUALITY); // ... - scale *= 0.7; + scale *= SCALE_REDUCTION_FACTOR;Also applies to: 46-47, 56-56, 72-72
52-53: Update misleading comment.The comment states "Force JPEG for small sizes even if input was PNG", but the code doesn't force JPEG—it respects the
isPngflag and encodes as PNG when appropriate. Either update the comment to match the actual behavior or revise the code to force JPEG for small dimensions if that's the intended logic.
139-161: Document WebP fallback behavior and consider explicit null handling.Two observations:
- Line 140: The method returns
"image/jpeg"fornullbytes. While unlikely given the calling context, consider throwing anIllegalArgumentExceptionfor null input to make the contract clearer.- Lines 152-157: WebP is detected and returns
"image/webp", but line 121 treats this as JPEG (.jpgextension). This fallback behavior should be documented in a comment for clarity.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockIdaDataProviderPlugin.javamock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.javapostgres-dataprovider-plugin/pom.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- postgres-dataprovider-plugin/pom.xml
🧰 Additional context used
🧬 Code graph analysis (1)
mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java (1)
postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtil.java (1)
Slf4j(13-163)
🔇 Additional comments (4)
mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MockIdaDataProviderPlugin.java (3)
59-61: Proper exception handling for DataProviderExchangeException.The specific catch block for
DataProviderExchangeExceptioncorrectly logs and rethrows the exception, ensuring that compression errors fromImageCompressorUtilare properly propagated while maintaining observability through logging.
64-64: Consistent error handling with improved error code.The updated error handling uses a descriptive error code
"ERROR_FETCHING_IDENTITY_DATA"consistently for both the exception case (line 64) and the null individualId case (line 67). This is an improvement over the previous error codes and aligns with the standardized error handling mentioned in the PR summary.Also applies to: 67-67
5-5: ImageCompressorUtil import is correct and necessary.The
ImageCompressorUtilclass exists atio.mosip.certify.utiland thecompressImageData(String)method is properly implemented to accept image data in both data URL and base64 formats. The method throwsDataProviderExchangeExceptionfor error cases (e.g., invalid images, malformed data URLs) and returns a compressed base64 string. The import statement is required for this functionality.mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java (1)
82-137: Verify consistent exception handling.The method correctly enforces the 1 KB limit at lines 126-131, which compensates for the issue in
validateAndCompressFaceImagewhere oversized images can be returned. However, ensure that all callers ofvalidateAndCompressFaceImageeither use this wrapper or implement similar size validation.
mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java
Show resolved
Hide resolved
Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.java
🔇 Additional comments (2)
postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.java (2)
50-56: LGTM! Null check properly addresses previous review feedback.The null and empty checks on line 52 correctly guard against potential
NullPointerException, addressing the concern raised in the previous review. The image compression logic is sound.
59-64: LGTM! Improved exception handling preserves compression error details.The addition of a specific catch block for
DataProviderExchangeException(lines 59-61) ensures that compression-related errors fromImageCompressorUtil(e.g., INVALID_DATA_URL, INVALID_BASE64, FACE_IMAGE_TOO_LARGE) are properly propagated with their original error codes, while other exceptions are standardized with a consistent error code.
...va/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Piyush7034 <piyushshukla2100@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.java (1)
14-14: Remove unused import.The
Environmentimport is not used anywhere in this file. Based on past review comments, the autowiredEnvironmentfield was already removed, but the import remains.🔎 Proposed fix
-import org.springframework.core.env.Environment;mock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.java (1)
78-89: Consider strengthening MIME type validation.The test only verifies that compression completes without error, but doesn't validate that MIME type detection worked correctly. Consider adding assertions to verify the detected MIME type or the output format (e.g., checking if PNG was preserved or converted to JPEG).
Example enhancement
@Test public void testMimeTypeDetection_PNG() throws Exception { byte[] pngBytes = createDummyImage(10, 10, "png"); String base64 = Base64.getEncoder().encodeToString(pngBytes); String result = ImageCompressorUtil.compressImageData(base64); assertNotNull(result); + // Decode and verify it's still a valid image + byte[] decoded = Base64.getDecoder().decode(result); + assertTrue("Result should be a valid image", decoded.length > 0); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.javamock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.javapostgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.javapostgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtil.javapostgres-dataprovider-plugin/src/test/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtilTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- postgres-dataprovider-plugin/src/test/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtilTest.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-31T09:50:29.181Z
Learnt from: Piyush7034
Repo: mosip/digital-credential-plugins PR: 123
File: mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java:1-162
Timestamp: 2025-12-31T09:50:29.181Z
Learning: In the mosip/digital-credential-plugins repository, each plugin (e.g., mock-certify-plugin, postgres-dataprovider-plugin) should remain independent and must not share common utility modules. Avoid introducing cross-plugin shared utilities. If a utility is needed, prefer duplicating it within the plugin rather than creating a shared module; otherwise assess whether the code really belongs to a separate shared library and refactor accordingly.
Applied to files:
mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.javamock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.java
🧬 Code graph analysis (2)
mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java (1)
postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtil.java (1)
Slf4j(13-161)
mock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.java (1)
postgres-dataprovider-plugin/src/test/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtilTest.java (1)
ImageCompressorUtilTest(14-96)
🔇 Additional comments (12)
postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.java (1)
41-47: LGTM!The null and empty check prevents potential
NullPointerExceptionissues. The image compression logic is correctly implemented with proper validation before processing.mock-certify-plugin/src/test/java/io/mosip/certify/util/ImageCompressorUtilTest.java (6)
17-22: LGTM!The helper method correctly creates dummy images for testing purposes using standard Java ImageIO APIs.
25-38: LGTM!This test effectively validates the core compression requirement (≤1 KB) for large images and verifies the expected output format.
41-55: LGTM!This test properly validates that data URL format is preserved through compression and that the size constraint is met.
58-67: LGTM!This test correctly validates that plain Base64 input produces plain Base64 output (not a data URL) while maintaining the size constraint.
69-76: LGTM!This test properly validates error handling for invalid image data.
91-95: LGTM!This test properly validates error handling for malformed data URLs.
postgres-dataprovider-plugin/src/main/java/io/mosip/certify/postgresdataprovider/integration/util/ImageCompressorUtil.java (4)
16-20: LGTM!The static initialization correctly loads OpenCV native libraries using Bytedeco's Loader mechanism, and the TARGET_SIZE constant is appropriately defined.
22-80: LGTM!The compression algorithm is well-implemented with proper resource management. The iterative approach with aggressive scale reduction (0.7 multiplier) is appropriate for achieving the 1 KB target. Memory cleanup is thorough with
release()calls in the loop and finally block. The method is correctly scoped asprivate, and the caller (compressImageData) enforces the size constraint.
82-136: LGTM!The public API is well-designed with comprehensive input validation and error handling. The method correctly:
- Handles both data URL and plain Base64 formats
- Validates Base64 encoding
- Falls back to MIME type detection when needed
- Enforces the 1 KB size constraint
- Preserves input format in output
138-160: LGTM!The MIME type detection correctly identifies PNG and WebP formats using their byte signatures, with a reasonable fallback to JPEG. The implementation properly handles edge cases (null, insufficient bytes) and uses appropriate bitwise operations for signature matching.
mock-certify-plugin/src/main/java/io/mosip/certify/util/ImageCompressorUtil.java (1)
1-162: LGTM!The implementation is identical to the postgres-dataprovider-plugin version, which is consistent with the architectural decision to keep plugins independent. As noted in the retrieved learnings, code duplication across plugins is acceptable to maintain plugin autonomy.
Based on learnings, plugins must remain independent and not share common utility modules.
...va/io/mosip/certify/postgresdataprovider/integration/service/PostgresDataProviderPlugin.java
Show resolved
Hide resolved
|
closing this as handled here #132 |
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.