Android Match SDK source code#8
Closed
SachinPremkumar wants to merge 21 commits into
Closed
Conversation
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
…into develop_test
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com> B
|
Warning Review limit reached
More reviews will be available in 56 minutes and 33 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (33)
WalkthroughThis pull request establishes a complete Android biometric matching SDK with build infrastructure, validation layer, format conversion pipeline, quality assessment, template extraction, modality-specific matching, and comprehensive test coverage for all operations. ChangesMatch SDK feature delivery
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (27)
.gitignore (2)
4-10: ⚡ Quick winRedundant ignore patterns under
/.idea.Lines 5-10 individually exclude
.idea/caches,.idea/libraries, etc., but line 4 already excludes the entire/.ideadirectory, making these sub-path entries redundant.♻️ Simplify by removing redundant patterns
*.iml .gradle /local.properties /.idea -/.idea/caches -/.idea/libraries -/.idea/modules.xml -/.idea/workspace.xml -/.idea/navEditor.xml -/.idea/assetWizardSettings.xml .DS_Store🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore around lines 4 - 10, The .gitignore contains a redundant explicit list of .idea sub-paths because the entry "/.idea" already ignores the whole directory; remove the explicit entries "/.idea/caches", "/.idea/libraries", "/.idea/modules.xml", "/.idea/workspace.xml", "/.idea/navEditor.xml", and "/.idea/assetWizardSettings.xml" so only the top-level "/.idea" pattern remains.
3-3: ⚡ Quick winDuplicate
local.propertiesentries.Both line 3 (
/local.properties) and line 16 (local.properties) exclude the same file. The leading slash variant (line 3) is sufficient.♻️ Remove duplicate entry
.externalNativeBuild .cxx -local.properties /app/buildAlso applies to: 16-16
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore at line 3, Remove the duplicate gitignore entry by keeping the leading-slash variant and deleting the plain "local.properties" line: in .gitignore remove the "local.properties" entry (the file currently appears twice as "/local.properties" and "local.properties"), leaving only "/local.properties" so the rule remains effective and non-redundant.matchsdk/src/main/java/io/mosip/mock/sdk/constant/SourceFormatCode.java (1)
22-38: 💤 Low valueConsider refactoring to eliminate duplicate iteration logic.
Both
fromCodeandvalidCodeiterate through enum values with similar logic. Consider havingfromCodecallvalidCodefirst, or extract the iteration logic into a private helper to reduce duplication and improve maintainability.♻️ Proposed refactor to reduce duplication
public static SourceFormatCode fromCode(String sourceCodeName) { + if (!validCode(sourceCodeName)) { + throw new ConversionException(ConverterErrorCode.INVALID_SOURCE_EXCEPTION.getErrorCode(), + ConverterErrorCode.INVALID_SOURCE_EXCEPTION.getErrorMessage()); + } for (SourceFormatCode sourceCode : SourceFormatCode.values()) { if (sourceCode.getCode().equalsIgnoreCase(sourceCodeName)) { return sourceCode; } } - throw new ConversionException(ConverterErrorCode.INVALID_SOURCE_EXCEPTION.getErrorCode(), ConverterErrorCode.INVALID_SOURCE_EXCEPTION.getErrorMessage()); + // Unreachable, but kept for safety + throw new ConversionException(ConverterErrorCode.INVALID_SOURCE_EXCEPTION.getErrorCode(), + ConverterErrorCode.INVALID_SOURCE_EXCEPTION.getErrorMessage()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/constant/SourceFormatCode.java` around lines 22 - 38, Refactor duplicated iteration in SourceFormatCode by extracting the common lookup into a single private helper (e.g., a method like private static Optional<SourceFormatCode> findByCode(String sourceCodeName)) that iterates SourceFormatCode.values() and returns the matching enum or empty; then reimplement fromCode to call this helper and throw ConversionException when empty, and reimplement validCode to call the helper and return presence boolean—ensure you reference SourceFormatCode.fromCode, SourceFormatCode.validCode, and the new private helper to consolidate the iteration logic.matchsdk/src/main/java/io/mosip/mock/sdk/constant/SdkConstant.java (1)
4-4: 💤 Low valueConsider removing or documenting the unused constant.
The constant
SDK_CHECK_ISO_TIMESTAMP_FORMATis defined but its usage inSDKService.java(Lines 221-233) is commented out with a TODO note indicating environment variables were removed. If this constant is intended for future use, document that intent; otherwise, remove it to reduce clutter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/constant/SdkConstant.java` at line 4, The constant SDK_CHECK_ISO_TIMESTAMP_FORMAT in SdkConstant is unused (its related logic in SDKService was commented out); either remove the constant to reduce dead code or add a clear JavaDoc comment on SDK_CHECK_ISO_TIMESTAMP_FORMAT indicating it’s intentionally reserved for future environment-driven timestamp checks and reference the commented-out logic in SDKService (TODO) so future maintainers know why it exists; also delete or update the TODO in SDKService referencing this constant to reflect the chosen action.gradle/wrapper/gradle-wrapper.properties (1)
4-4: Gradle 7.5 is compatible with the project’s AGP 7.2.0, but it’s not current—upgrade should be done together with AGP.
gradle/wrapper/gradle-wrapper.propertiesusesgradle-7.5-all.zip.- Root
build.gradleappliescom.android.application/com.android.library7.2.0; AGP 7.2.0 requires Gradle >= 7.3.3, so Gradle 7.5 should be compatible.- Latest stable Gradle is 9.5.1 (per Gradle releases), so 7.5 is behind; upgrading Gradle alone may cause tooling/AGP mismatch—plan an AGP upgrade with it if you’re targeting security/perf improvements.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gradle/wrapper/gradle-wrapper.properties` at line 4, The gradle-wrapper is pinned to distributionUrl=...gradle-7.5-all.zip which is outdated; update gradle-wrapper.properties to point to a newer Gradle distribution (e.g., latest stable) but only after coordinating an Android Gradle Plugin (AGP) upgrade in the root build.gradle where com.android.application/com.android.library is set to 7.2.0—ensure the target Gradle version is compatible with the AGP version you plan to use (or first bump AGP to a version that supports the newer Gradle), update distributionUrl accordingly, and run the wrapper task (or regenerate the wrapper) to refresh wrapper files.matchsdk/build.gradle (1)
5-17: 💤 Low valueConsider removing or activating commented Maven publishing block.
The Maven publishing configuration is commented out. If publishing is not needed, consider removing this block to reduce clutter. If it will be used later, consider uncommenting and configuring it properly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/build.gradle` around lines 5 - 17, The commented Maven publishing block (publishing -> publications -> release(MavenPublication) with groupId/artifactId/version and the afterEvaluate { from components.findByName('release') }) is leftover; either delete this entire commented block to reduce clutter or restore it by uncommenting and ensuring the publication is correctly configured (set correct groupId/artifactId/version, apply the 'maven-publish' plugin, and verify a 'release' component exists) so the release(MavenPublication) section can publish successfully.matchsdk/src/test/java/io/mosip/mock/sdk/SDKServiceTest.java (1)
421-432: ⚡ Quick winStrengthen assertions in branch-coverage tests.
These tests only assert non-null response, so they don’t fail if status/decision behavior regresses. Add explicit
statusCode(and relevant payload) assertions to make the coverage meaningful.Also applies to: 527-538
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/test/java/io/mosip/mock/sdk/SDKServiceTest.java` around lines 421 - 432, The test checkQuality_compoundFingerSubtype_coversSize2Branch currently only asserts the Response is non-null, which allows regressions; update this test (and the similar one around lines 527-538) to assert the Response<QualityCheck> statusCode and expected payload fields explicitly: call SampleSDK.checkQuality and then assert response.getStatusCode() equals the expected enum/value for the constructed input (e.g., SUCCESS or INVALID_INPUT depending on buildMinimalIsoBdb), and assert the QualityCheck payload (response.getPayload()) is not null and contains expected decision/status fields (e.g., decision/statusCode or quality score). Locate these assertions around SDKServiceTest.checkQuality_compoundFingerSubtype_coversSize2Branch and the other test and replace the Assert.assertNotNull(response) with explicit assertions against response.getStatusCode() and response.getPayload().matchsdk/src/main/java/io/mosip/mock/sdk/service/ConvertFormatService.java (5)
6-6: 💤 Low valueRemove unused import.
org.springframework.core.env.Environmentis imported but never used in this class.♻️ Proposed fix
import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.core.env.Environment; import java.util.HashMap;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/ConvertFormatService.java` at line 6, The import org.springframework.core.env.Environment is unused in the ConvertFormatService class; remove that import statement so the class no longer imports Environment (locate the import at the top of ConvertFormatService and delete the Environment import line) and run a quick build to ensure no other references to Environment remain.
32-32: ⚡ Quick winMake the logger field
final.The logger field should be declared
finalto ensure immutability and thread safety.♻️ Proposed fix
- Logger LOGGER = LoggerFactory.getLogger(ConvertFormatService.class); + private final Logger LOGGER = LoggerFactory.getLogger(ConvertFormatService.class);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/ConvertFormatService.java` at line 32, The LOGGER field in ConvertFormatService should be made immutable; change the declaration of the Logger named LOGGER (in class ConvertFormatService) to be declared final (e.g., private static final Logger LOGGER = LoggerFactory.getLogger(ConvertFormatService.class)) so the logger reference cannot be reassigned and is thread-safe.
26-26: ⚡ Quick winStored
modalitiesToConvertparameter is never used.The
modalitiesToConvertfield is stored in the constructor but never referenced ingetConvertFormatInfo(). The conversion logic filters bysourceFormat/bioTypepairing (line 69) but doesn't checkmodalitiesToConvert.Also applies to: 43-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/ConvertFormatService.java` at line 26, The field modalitiesToConvert is stored in the constructor but never used; update getConvertFormatInfo to also filter candidate ConvertFormatInfo entries against the stored modalitiesToConvert list (or treat null/empty as "allow all"). Specifically, inside getConvertFormatInfo (and any helper that builds/filters the stream of ConvertFormatInfo), add a predicate that checks ConvertFormatInfo.getBioType() (or the bioType variable used in the existing filter) is contained in the modalitiesToConvert list before returning a match; ensure the constructor-set modalitiesToConvert is referenced and handle null/empty safely so existing behavior remains unchanged when no modalities were provided.
186-196: ⚡ Quick winReplace hardcoded format strings with constants.
The
isValidBioTypeForSourceFormat()method uses hardcoded strings like"ISO19794_4_2011"that should reference constants fromSourceFormatCodefor maintainability.♻️ Proposed refactor using enum comparison
private boolean isValidBioTypeForSourceFormat(BiometricType bioType, String sourceFormat) { - boolean isValid = false; - switch (sourceFormat) { - case "ISO19794_4_2011": - if (bioType == BiometricType.FINGER) - isValid = true; - break; - case "ISO19794_5_2011": - if (bioType == BiometricType.FACE) - isValid = true; - break; - case "ISO19794_6_2011": - if (bioType == BiometricType.IRIS) - isValid = true; - break; + try { + SourceFormatCode code = SourceFormatCode.fromCode(sourceFormat); + switch (code) { + case ISO19794_4_2011: + return bioType == BiometricType.FINGER; + case ISO19794_5_2011: + return bioType == BiometricType.FACE; + case ISO19794_6_2011: + return bioType == BiometricType.IRIS; + default: + return false; + } + } catch (ConversionException e) { + return false; } - return isValid; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/ConvertFormatService.java` around lines 186 - 196, The switch in isValidBioTypeForSourceFormat() uses hardcoded format string literals (e.g., "ISO19794_4_2011", "ISO19794_5_2011", "ISO19794_6_2011"); replace those literals with the corresponding constants from the SourceFormatCode enum (e.g., SourceFormatCode.ISO19794_4_2011, SourceFormatCode.ISO19794_5_2011, SourceFormatCode.ISO19794_6_2011) and compare using the enum constants (or switch on SourceFormatCode.valueOf(sourceFormat) first) so the case branches still validate BiometricType.FINGER, BiometricType.FACE and BiometricType.IRIS respectively in isValidBioTypeForSourceFormat().
106-106: ⚡ Quick winFix invalid log message format.
Line 106 uses string concatenation (
"convertFormat -- error"+ex) instead of a placeholder. This should use proper SLF4J parameterized logging.♻️ Proposed fix
- LOGGER.error("convertFormat -- error"+ex); + LOGGER.error("convertFormat -- error: {}", ex.getMessage(), ex);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/ConvertFormatService.java` at line 106, In ConvertFormatService's convertFormat method replace the incorrect string concatenation in LOGGER.error("convertFormat -- error"+ex) with proper SLF4J parameterized logging or throwable overload; for example use LOGGER.error("convertFormat -- error {}", ex.getMessage(), ex) or LOGGER.error("convertFormat -- error", ex) so the exception and message are logged correctly (locate LOGGER and convertFormat to apply the change).Source: Linters/SAST tools
matchsdk/src/main/java/io/mosip/mock/sdk/service/MatchService.java (5)
253-260: 💤 Low valueRemove commented-out code.
Lines 253-260 contain commented-out alternative matching logic. If this code is no longer needed, remove it; otherwise, document why it's retained or extract it to a separate method for comparison.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/MatchService.java` around lines 253 - 260, Remove the commented-out alternative matching logic block in MatchService (the multi-line comment referencing trueMatchCount, matched, sampleSegments and decision.setMatch(Match.*)); if you need to preserve it for reference instead extract it into a clearly named helper method (e.g., computeAlternateMatchDecision) or add a brief comment explaining why it remains, but do not leave the unused commented code inline.
110-113: ⚡ Quick winAvoid catching
NullPointerException; validate inputs instead.Catching
NullPointerExceptionas a control-flow mechanism is an anti-pattern. The code should explicitly validatesampleBioSegmentMap.get(modality)andrecordBioSegmentMap.get(modality)before callingcompareModality()to prevent null-pointer scenarios.♻️ Proposed refactor with explicit null checks
for (BiometricType modality : sampleBioSegmentMap.keySet()) { try { + List<BIR> sampleSegments = sampleBioSegmentMap.get(modality); + List<BIR> gallerySegments = recordBioSegmentMap.get(modality); + if (sampleSegments == null || gallerySegments == null) { + decision.setMatch(Match.ERROR); + decision.getErrors().add("Modality " + modality.name() + " segments are null"); + decisions.put(modality, decision); + continue; + } - decision = compareModality(modality, sampleBioSegmentMap.get(modality), - recordBioSegmentMap.get(modality)); - } catch (NoSuchAlgorithmException | NullPointerException ex) { + decision = compareModality(modality, sampleSegments, gallerySegments); + } catch (NoSuchAlgorithmException ex) { ex.printStackTrace();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/MatchService.java` around lines 110 - 113, Replace the catch of NullPointerException in MatchService with explicit null checks: before calling compareModality(modality, ...), validate that sampleBioSegmentMap.get(modality) and recordBioSegmentMap.get(modality) are not null (and any other inputs used by compareModality); if either is null, set decision.setMatch(Match.ERROR) and add a descriptive error to decision.getErrors() (e.g., "Missing sample/record for modality X") and skip calling compareModality; keep handling NoSuchAlgorithmException separately and remove NullPointerException from the catch clause.
24-24: ⚡ Quick winMake the logger field
final.The logger field should be declared
finalto ensure immutability and thread safety.♻️ Proposed fix
- private final Logger LOGGER = LoggerFactory.getLogger(MatchService.class); + private final Logger LOGGER = LoggerFactory.getLogger(MatchService.class);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/MatchService.java` at line 24, The LOGGER field in MatchService is mutable—declare it as final to ensure immutability; update the field declaration for LOGGER (Logger LOGGER = LoggerFactory.getLogger(MatchService.class)) to include the final modifier (i.e., private final Logger LOGGER = ...) so the logger reference cannot be reassigned; locate the declaration in the MatchService class and make this change (optionally also consider making it static final if class-level shared logger semantics are desired).
43-43: ⚡ Quick winFix invalid log message format.
Similar to ExtractTemplateService, the log statement has no placeholders but passes an exception parameter.
♻️ Proposed fix
- LOGGER.error("match -- error", ex); + LOGGER.error("match -- error: {}", ex.getMessage(), ex);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/MatchService.java` at line 43, The log call in MatchService uses LOGGER.error("match -- error", ex) without a placeholder for the exception message; change it to include the exception message as a placeholder and pass the Throwable as the final argument (e.g., use LOGGER and MatchService to locate the statement and replace the message with something like "match -- error: {}" using ex.getMessage() as the placeholder argument while still passing ex as the last parameter).Source: Linters/SAST tools
399-399: ⚡ Quick winFix string concatenation in log message.
Line 399 has a missing space:
"SampleBIR Value check"+ sampleBIRshould be"SampleBIR Value check: " + sampleBIRfor proper formatting.♻️ Proposed fix
- LOGGER.info("SampleBIR Value check"+ sampleBIR.getBdbInfo().getSubtype()); + LOGGER.info("SampleBIR Value check: {}", sampleBIR.getBdbInfo().getSubtype());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/MatchService.java` at line 399, The log message built by LOGGER.info currently concatenates without spacing; update the call that references LOGGER and sampleBIR.getBdbInfo().getSubtype() to include a separator and space (e.g., "SampleBIR Value check: " + sampleBIR.getBdbInfo().getSubtype()) so the output is properly formatted; ensure you reference LOGGER and sampleBIR.getBdbInfo().getSubtype() when making the change.matchsdk/src/main/java/io/mosip/mock/sdk/service/ExtractTemplateService.java (4)
117-120: ⚡ Quick winInefficient random number generation.
Creating a new
Random()instance on every call togetRandomLevelType()is inefficient. For better performance, use a shared staticRandominstance orThreadLocalRandom.current()(Java 7+).⚡ Proposed fix using ThreadLocalRandom
public ProcessedLevelType getRandomLevelType() { - int rnd = new Random().nextInt(types.length); + int rnd = java.util.concurrent.ThreadLocalRandom.current().nextInt(types.length); return types[rnd]; }Or use a shared instance:
+ private static final Random RANDOM = new Random(); + public ProcessedLevelType getRandomLevelType() { - int rnd = new Random().nextInt(types.length); + int rnd = RANDOM.nextInt(types.length); return types[rnd]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/ExtractTemplateService.java` around lines 117 - 120, The getRandomLevelType() method creates a new Random() on every call which is inefficient; change it to use a shared PRNG such as ThreadLocalRandom.current() (or a private static final Random) to pick an index from the types array and return types[rnd]; update the method getRandomLevelType() to obtain rnd from ThreadLocalRandom.current().nextInt(types.length) (or from the shared Random) to avoid allocating a new Random each call.
67-98: ⚡ Quick winReduce duplication in error-response mapping.
Lines 67-98 contain six near-identical case blocks that each set status code, format a message, set response to null, and return. This repetition makes maintenance error-prone.
♻️ Proposed refactor to consolidate response building
} catch (SDKException ex) { LOGGER.error("extractTemplate -- error: {}", ex.getMessage(), ex); - switch (ResponseStatus.fromStatusCode(Integer.parseInt(ex.getErrorCode()))) { - case INVALID_INPUT: - response.setStatusCode(ResponseStatus.INVALID_INPUT.getStatusCode()); - response.setStatusMessage(String.format(ResponseStatus.INVALID_INPUT.getStatusMessage(), "sample")); - response.setResponse(null); - return response; - case MISSING_INPUT: - response.setStatusCode(ResponseStatus.MISSING_INPUT.getStatusCode()); - response.setStatusMessage(String.format(ResponseStatus.MISSING_INPUT.getStatusMessage(), "sample")); - response.setResponse(null); - return response; - case QUALITY_CHECK_FAILED: - response.setStatusCode(ResponseStatus.QUALITY_CHECK_FAILED.getStatusCode()); - response.setStatusMessage(String.format(ResponseStatus.QUALITY_CHECK_FAILED.getStatusMessage(), "")); - response.setResponse(null); - return response; - case BIOMETRIC_NOT_FOUND_IN_CBEFF: - response.setStatusCode(ResponseStatus.BIOMETRIC_NOT_FOUND_IN_CBEFF.getStatusCode()); - response.setStatusMessage( - String.format(ResponseStatus.BIOMETRIC_NOT_FOUND_IN_CBEFF.getStatusMessage(), "")); - response.setResponse(null); - return response; - case MATCHING_OF_BIOMETRIC_DATA_FAILED: - response.setStatusCode(ResponseStatus.MATCHING_OF_BIOMETRIC_DATA_FAILED.getStatusCode()); - response.setStatusMessage( - String.format(ResponseStatus.MATCHING_OF_BIOMETRIC_DATA_FAILED.getStatusMessage(), "")); - response.setResponse(null); - return response; - case POOR_DATA_QUALITY: - response.setStatusCode(ResponseStatus.POOR_DATA_QUALITY.getStatusCode()); - response.setStatusMessage(String.format(ResponseStatus.POOR_DATA_QUALITY.getStatusMessage(), "")); - response.setResponse(null); - return response; - default: - response.setStatusCode(ResponseStatus.UNKNOWN_ERROR.getStatusCode()); - response.setStatusMessage(String.format(ResponseStatus.UNKNOWN_ERROR.getStatusMessage(), "")); - response.setResponse(null); - return response; - } + ResponseStatus status = ResponseStatus.fromStatusCode(Integer.parseInt(ex.getErrorCode())); + String param = (status == ResponseStatus.INVALID_INPUT || status == ResponseStatus.MISSING_INPUT) ? "sample" : ""; + response.setStatusCode(status.getStatusCode()); + response.setStatusMessage(String.format(status.getStatusMessage(), param)); + response.setResponse(null); + return response; } catch (Exception ex) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/ExtractTemplateService.java` around lines 67 - 98, Several case branches in ExtractTemplateService repeat setting response fields; create a helper method (e.g., buildErrorResponse or setErrorResponse) that accepts a ResponseStatus and a formatting parameter, sets response.setStatusCode(...), response.setStatusMessage(String.format(..., param)), response.setResponse(null) and returns the response, then replace each case (INVALID_INPUT, MISSING_INPUT, QUALITY_CHECK_FAILED, BIOMETRIC_NOT_FOUND_IN_CBEFF, MATCHING_OF_BIOMETRIC_DATA_FAILED, POOR_DATA_QUALITY) to call that helper with the appropriate ResponseStatus and argument instead of duplicating the same four lines.
18-18: ⚡ Quick winMake the logger field
final.The logger field should be declared
finalto ensure immutability and thread safety.♻️ Proposed fix
- private Logger LOGGER = LoggerFactory.getLogger(ExtractTemplateService.class); + private final Logger LOGGER = LoggerFactory.getLogger(ExtractTemplateService.class);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/ExtractTemplateService.java` at line 18, Change the LOGGER field in ExtractTemplateService to be immutable by declaring it final; locate the Logger declaration "private Logger LOGGER = LoggerFactory.getLogger(ExtractTemplateService.class);" and update it to include the final modifier so the field becomes "private final Logger LOGGER = ..." to ensure thread-safety and immutability.
65-65: ⚡ Quick winFix invalid log message format.
The log statement
LOGGER.error("extractTemplate -- error", ex)uses a message string with no placeholders, but passesexas a parameter. SLF4J expects either placeholders ({}) in the message or the exception as the last parameter without placeholders. This causes the static analysis warning.♻️ Proposed fix
- LOGGER.error("extractTemplate -- error", ex); + LOGGER.error("extractTemplate -- error: {}", ex.getMessage(), ex);Or simply:
- LOGGER.error("extractTemplate -- error", ex); + LOGGER.error("extractTemplate -- error", ex);(The current form is actually correct for logging exceptions; the static analysis may be overly strict. However, adding a placeholder makes the intent clearer.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/ExtractTemplateService.java` at line 65, Update the LOGGER.error call in ExtractTemplateService (inside extractTemplate) to use a '{}' placeholder and pass the exception message plus the exception as the throwable so the call becomes: message with '{}' , ex.getMessage(), ex — this ensures SLF4J placeholders are used and the exception is still logged correctly.Source: Linters/SAST tools
matchsdk/src/main/java/io/mosip/mock/sdk/service/SegmentService.java (3)
12-12: 💤 Low valueRemove unused logger field.
The
LOGGERfield is declared but never used in this class.♻️ Proposed fix
- private Logger LOGGER = LoggerFactory.getLogger(SegmentService.class); - private BiometricRecord sample;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/SegmentService.java` at line 12, Remove the unused logger field LOGGER from the SegmentService class: delete the line declaring "private Logger LOGGER = LoggerFactory.getLogger(SegmentService.class);" and then remove the corresponding import(s) (e.g., org.slf4j.Logger and org.slf4j.LoggerFactory) if they become unused; ensure no other code in SegmentService references LOGGER before committing.
14-21: ⚡ Quick winStored constructor parameters are never used.
The
sampleandmodalitiesToSegmentfields are stored in the constructor but never referenced ingetSegmentInfo(). This suggests the implementation is incomplete or these fields are unnecessary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/SegmentService.java` around lines 14 - 21, The constructor stores sample and modalitiesToSegment but those fields are never used in getSegmentInfo(); either (A) update getSegmentInfo() to use the instance fields (sample and modalitiesToSegment) instead of any local vars or parameters so the stored values drive segmentation logic, or (B) if those values are meant to be passed directly into getSegmentInfo(), remove the fields and simplify the SegmentService constructor signature (and update all callers) to avoid storing unused state; modify the SegmentService constructor and getSegmentInfo() accordingly to keep implementation and API consistent.
29-29: ⚡ Quick winReplace hardcoded status code with constant.
The hardcoded
200should useResponseStatus.SUCCESS.getStatusCode()for consistency with other services.♻️ Proposed fix
- response.setStatusCode(200); + response.setStatusCode(ResponseStatus.SUCCESS.getStatusCode()); response.setResponse(record);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/SegmentService.java` at line 29, Replace the hardcoded status code 200 in the segment response by using the shared enum constant; change the call that currently sets the status via response.setStatusCode(200) to use ResponseStatus.SUCCESS.getStatusCode() instead so the SegmentService uses the same ResponseStatus constant as other services.matchsdk/src/test/java/io/mosip/mock/sdk/SampleSDKTest.java (1)
114-115: 💤 Low valueConsider using a logger instead of
printStackTrace().Multiple test methods use
e.printStackTrace()in catch blocks. While acceptable in tests, using a test logger (or Assert.fail with the exception message) would provide better integration with test reporting frameworks.♻️ Example refactor
} catch (ParserConfigurationException | IOException | SAXException e) { - e.printStackTrace(); + Assert.fail("XML parsing failed: " + e.getMessage()); }Also applies to: 131-132, 148-149, 185-186, 227-228, 245-246, 261-262, 280-281, 300-301, 321-322, 364-365
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/test/java/io/mosip/mock/sdk/SampleSDKTest.java` around lines 114 - 115, In SampleSDKTest replace all occurrences of e.printStackTrace() in the catch blocks with a proper test-friendly failure/logging approach: either call Assert.fail(e.getMessage()) (or Assert.fail("...", e) if available) inside the catch so the test framework records the failure, or obtain a logger instance (e.g., a private static final Logger in class SampleSDKTest) and call logger.error("Descriptive message", e) to log the exception; update each catch in SampleSDKTest where e.printStackTrace() appears (all listed instances) to use one of these alternatives consistently.matchsdk/src/main/java/io/mosip/mock/sdk/impl/SampleSDK.java (1)
8-9: 💤 Low valueRemove unused Spring imports.
Lines 8-9 import Spring
@AutowiredandEnvironmentbut these are never used in the class. The class uses@Singletonfromjavax.injectinstead.♻️ Proposed fix
import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.core.env.Environment; - import javax.inject.Singleton;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/impl/SampleSDK.java` around lines 8 - 9, The file imports unused Spring symbols `@Autowired` and Environment in SampleSDK; remove the unused import lines for org.springframework.beans.factory.annotation.Autowired and org.springframework.core.env.Environment to clean up imports and avoid warnings, keeping the class annotation `@Singleton` (javax.inject.Singleton) and any other existing imports intact.matchsdk/src/main/java/io/mosip/mock/sdk/service/impl/ConverterServiceImpl.java (1)
112-112: ⚖️ Poor tradeoffTODO: Implement targetParameters handling.
Comments at lines 112, 181, and 225 indicate that
targetParameters(width, height, DPI adjustments) are not yet implemented. If these parameters are part of the API contract, they should be handled or documented as unsupported.Also applies to: 181-181, 225-225
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@matchsdk/src/main/java/io/mosip/mock/sdk/service/impl/ConverterServiceImpl.java` at line 112, ConverterServiceImpl currently ignores targetParameters (width/height/dpi) where comments are present; update the methods in ConverterServiceImpl that produce outImage (the blocks marked with "change here outImage width, height, dpi based on targetParameters") to either (A) implement resizing and DPI adjustment using the existing image processing utilities so outImage respects targetParameters.width/height and targetParameters.dpi, or (B) if you cannot support these transforms, explicitly validate and reject requests by throwing a clear UnsupportedOperationException or returning a defined error response and document this behavior in the method javadoc and API contract; ensure you reference and use the targetParameters object passed into the conversion flow and update any logs or tests to reflect the implemented behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 437a38d1-f5c0-4f1f-ab68-2ed8d1730bd6
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (77)
.gitignore.idea/.gitignore.idea/compiler.xml.idea/deploymentTargetDropDown.xml.idea/gradle.xml.idea/migrations.xml.idea/misc.xml.idea/vcs.xmlapp/.gitignoreapp/build.gradleapp/proguard-rules.proapp/src/androidTest/java/io/mosip/mock/sdk/ExampleInstrumentedTest.javaapp/src/main/AndroidManifest.xmlapp/src/main/res/drawable/ic_launcher_background.xmlapp/src/main/res/drawable/ic_launcher_foreground.xmlapp/src/main/res/mipmap-anydpi/ic_launcher.xmlapp/src/main/res/mipmap-anydpi/ic_launcher_round.xmlapp/src/main/res/mipmap-hdpi/ic_launcher.webpapp/src/main/res/mipmap-hdpi/ic_launcher_round.webpapp/src/main/res/mipmap-mdpi/ic_launcher.webpapp/src/main/res/mipmap-mdpi/ic_launcher_round.webpapp/src/main/res/mipmap-xhdpi/ic_launcher.webpapp/src/main/res/mipmap-xhdpi/ic_launcher_round.webpapp/src/main/res/mipmap-xxhdpi/ic_launcher.webpapp/src/main/res/mipmap-xxhdpi/ic_launcher_round.webpapp/src/main/res/mipmap-xxxhdpi/ic_launcher.webpapp/src/main/res/mipmap-xxxhdpi/ic_launcher_round.webpapp/src/main/res/values-night/themes.xmlapp/src/main/res/values/colors.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/values/themes.xmlapp/src/main/res/xml/backup_rules.xmlapp/src/main/res/xml/data_extraction_rules.xmlapp/src/test/java/io/mosip/mock/sdk/ExampleUnitTest.javabuild.gradlegradle.propertiesgradle/wrapper/gradle-wrapper.propertiesmatchsdk/.gitignorematchsdk/build.gradlematchsdk/consumer-rules.promatchsdk/proguard-rules.promatchsdk/src/androidTest/java/io/mosip/mock/sdk/ExampleInstrumentedTest.javamatchsdk/src/main/AndroidManifest.xmlmatchsdk/src/main/java/io/mosip/mock/sdk/constant/ConverterErrorCode.javamatchsdk/src/main/java/io/mosip/mock/sdk/constant/ParameterCode.javamatchsdk/src/main/java/io/mosip/mock/sdk/constant/ResponseStatus.javamatchsdk/src/main/java/io/mosip/mock/sdk/constant/SdkConstant.javamatchsdk/src/main/java/io/mosip/mock/sdk/constant/SourceFormatCode.javamatchsdk/src/main/java/io/mosip/mock/sdk/constant/TargetFormatCode.javamatchsdk/src/main/java/io/mosip/mock/sdk/exception/ConversionException.javamatchsdk/src/main/java/io/mosip/mock/sdk/exception/SDKException.javamatchsdk/src/main/java/io/mosip/mock/sdk/impl/SampleSDK.javamatchsdk/src/main/java/io/mosip/mock/sdk/service/CheckQualityService.javamatchsdk/src/main/java/io/mosip/mock/sdk/service/ConvertFormatService.javamatchsdk/src/main/java/io/mosip/mock/sdk/service/ExtractTemplateService.javamatchsdk/src/main/java/io/mosip/mock/sdk/service/IConverterApi.javamatchsdk/src/main/java/io/mosip/mock/sdk/service/MatchService.javamatchsdk/src/main/java/io/mosip/mock/sdk/service/SDKInfoService.javamatchsdk/src/main/java/io/mosip/mock/sdk/service/SDKService.javamatchsdk/src/main/java/io/mosip/mock/sdk/service/SegmentService.javamatchsdk/src/main/java/io/mosip/mock/sdk/service/impl/ConverterServiceImpl.javamatchsdk/src/main/java/io/mosip/mock/sdk/util/Util.javamatchsdk/src/test/java/io/mosip/mock/sdk/MatchSDKTest.javamatchsdk/src/test/java/io/mosip/mock/sdk/SDKServiceTest.javamatchsdk/src/test/java/io/mosip/mock/sdk/SampleSDKTest.javamatchsdk/src/test/java/io/mosip/mock/sdk/ServiceCoverageTest.javamatchsdk/src/test/java/io/mosip/mock/sdk/UtilTest.javamatchsdk/src/test/resources/sample_files/test_sdk.xmlmatchsdk/src/test/resources/sample_files/test_sdk_face_no_match.xmlmatchsdk/src/test/resources/sample_files/test_sdk_finger.xmlmatchsdk/src/test/resources/sample_files/test_sdk_finger_no_match.xmlmatchsdk/src/test/resources/sample_files/test_sdk_iris_no_match.xmlmatchsdk/src/test/resources/sample_files/test_sdk_match.xmlmatchsdk/src/test/resources/sample_files/test_sdk_more_fingers.xmlmatchsdk/src/test/resources/sample_files/test_sdk_no_matching_gallery.xmlmatchsdk/src/test/resources/sample_files/test_sdk_no_matching_sample.xmlsettings.gradle
| return response; | ||
| } | ||
| } catch (ConversionException ex) { | ||
| LOGGER.error("convertFormat -- error"+ex); |
There was a problem hiding this comment.
LOGGER.error("convertFormat -- error", ex).
Author
There was a problem hiding this comment.
Updated the logging statement to LOGGER.error("convertFormat -- error", ex).
anushasunkada
approved these changes
Jun 15, 2026
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
Signed-off-by: Sachin S P <52343650+SachinPremkumar@users.noreply.github.com>
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
3 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.
Summary by CodeRabbit
New Features
Tests