#1830 https://github.com/mosip/commons/issues/1830 Move release-1.3.x changes to develop#257
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR updates module/project versions to 1.4.0‑SNAPSHOT, relocates/adjusts build plugin properties, adds/updates THIRD‑PARTY‑NOTICES and AGENTS.md, changes biometrics serializers/deserializers and JAXB marshalling, and applies small test/schema and README formatting and matcher tweaks. ChangesRelease 1.4.0-SNAPSHOT
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
- Updates all module versions to 1.3.1 - Adds ObjectFactory to kernel-biometrics-api entities - Updates CbeffValidator, ByteArrayToIntArraySerializer, IntArrayToByteArrayDeserializer - Adds THIRD-PARTY-NOTICES.txt to all modules - Updates CBEFF XSD schema and NOTICE files - Updates biosdk-provider and cbeffutil-api pom dependencies Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Use typed any(byte[].class) matchers to disambiguate between validateXML(byte[], byte[]) and validateXML(Schema, byte[]) overloads. Signed-off-by: kameshsr <kameshsr1338@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
biometrics-util/pom.xml (1)
156-162:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winInvalid Maven scope configuration for BOM dependency.
The
junit-bomdependency has<scope>import</scope>but is located in the<dependencies>section. Theimportscope is only valid within<dependencyManagement>for BOM (Bill of Materials) dependencies.Impact: This will cause Maven build failures or unexpected behavior, as Maven does not support
importscope in the regular dependencies section.Solution: Move this dependency to the
<dependencyManagement>section withscopeset toimport, or change the scope back totestif it must remain in dependencies.🔧 Recommended fix
Move to dependencyManagement (lines 57-67):
<dependencyManagement> <dependencies> <dependency> <groupId>io.mosip.kernel</groupId> <artifactId>kernel-bom</artifactId> <version>${kernel.bom.version}</version> <type>pom</type> <scope>import</scope> </dependency> + <dependency> + <groupId>org.junit</groupId> + <artifactId>junit-bom</artifactId> + <version>${org.junit.version}</version> + <type>pom</type> + <scope>import</scope> + </dependency> </dependencies> </dependencyManagement>And remove from dependencies section:
- <dependency> - <groupId>org.junit</groupId> - <artifactId>junit-bom</artifactId> - <version>${org.junit.version}</version> - <type>pom</type> - <scope>import</scope> - </dependency>🤖 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 `@biometrics-util/pom.xml` around lines 156 - 162, The junit BOM dependency (<groupId>org.junit</groupId>, <artifactId>junit-bom</artifactId>, <version>${org.junit.version}</version>) is using scope "import" but is placed in <dependencies>; move this BOM entry into the <dependencyManagement> section and keep <scope>import</scope> there, or alternatively if you intend it as a normal dependency change its scope to "test" and leave it in <dependencies>; update/remove the original entry accordingly so Maven no longer has an import-scoped dependency outside dependencyManagement.
🧹 Nitpick comments (1)
kernel-biometrics-api/src/main/java/io/mosip/kernel/biometrics/entities/ObjectFactory.java (1)
14-16: 💤 Low value
@Dataon a field-less factory adds no value.
ObjectFactoryhas no fields, so Lombok's@Dataonly generates trivialequals/hashCode/toString. JAXB-generated object factories conventionally carry no Lombok annotation; consider dropping@Datato keep this generated-style class minimal.🤖 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 `@kernel-biometrics-api/src/main/java/io/mosip/kernel/biometrics/entities/ObjectFactory.java` around lines 14 - 16, Remove the unnecessary Lombok `@Data` annotation from the ObjectFactory class since it has no fields and the generated equals/hashCode/toString are pointless; edit the class declaration in ObjectFactory (the class named ObjectFactory annotated with `@XmlRegistry`) to delete the `@Data` import and annotation so the class remains a minimal JAXB-style factory.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@kernel-biometrics-api/THIRD-PARTY-NOTICES.txt`:
- Around line 6-38: The THIRD-PARTY-NOTICES entry lists several MOSIP packages
(MOSIP Kernel BOM, MOSIP Kernel Core, MOSIP Kernel Logger Logback, MOSIP
Biometrics API, MOSIP Biometrics Util) with version "1.3.0-SNAPSHOT"; update all
those version strings to "1.3.1" so the attribution file matches the PR release
target, ensuring every occurrence of "1.3.0-SNAPSHOT" for these package entries
is replaced with "1.3.1".
- Around line 122-125: The H2 Database entry in THIRD-PARTY-NOTICES is missing
its dual-license designation; update the H2 Database block (the "Package: H2
Database" entry) so its License line reads the full dual-license string "EPL-1.0
OR MPL-2.0" (replace the current "EPL 1.0"), preserving existing formatting and
the Homepage and separator lines.
In `@kernel-biosdk-provider/THIRD-PARTY-NOTICES.txt`:
- Around line 6-38: Update the MOSIP component version strings in
THIRD-PARTY-NOTICES.txt so they match the release target (change occurrences of
"1.3.0-SNAPSHOT" to "1.3.1"); specifically update the version lines for entries
named "MOSIP Kernel BOM", "MOSIP Kernel Core", "MOSIP Kernel Logger Logback",
"MOSIP Biometrics API", and "MOSIP Biometrics Util" to reflect 1.3.1 to keep the
attribution file consistent with the PR.
- Around line 122-125: Update the H2 Database entry so the license field
reflects the dual-license correctly: locate the "H2 Database" package block and
replace the current "License: EPL 1.0" line with the full designation "License:
EPL-1.0 OR MPL-2.0" (preserving surrounding formatting) so the notice accurately
documents the dual license.
In `@kernel-cbeffutil-api/pom.xml`:
- Around line 87-91: Remove the redundant dependency declaration for groupId
"io.mosip.kernel" and artifactId "kernel-biometrics-api" (the block using
${kernel.biometrics.api.version}) so only the single original dependency
remains; locate the duplicate dependency block matching that
groupId/artifactId/version and delete it to avoid the duplicate Maven
declaration.
In `@kernel-cbeffutil-api/THIRD-PARTY-NOTICES.txt`:
- Around line 6-38: Update the MOSIP component versions in
THIRD-PARTY-NOTICES.txt so they match the release target by replacing all
occurrences of "1.3.0-SNAPSHOT" with "1.3.1" for each listed package (e.g.,
MOSIP Kernel BOM, MOSIP Kernel Core, MOSIP Kernel Logger Logback, MOSIP
Biometrics API, MOSIP Biometrics Util) ensuring the attribution entries reflect
the new 1.3.1 version.
In `@THIRD-PARTY-NOTICES.txt`:
- Around line 6-9: The THIRD-PARTY-NOTICES.txt lists the package version as
"1.3.0-SNAPSHOT" which contradicts the PR objective to bump to 1.3.1; update the
Version entry in THIRD-PARTY-NOTICES.txt from "1.3.0-SNAPSHOT" to "1.3.1" and
scan the file for any other occurrences of "1.3.0-SNAPSHOT" to replace so the
notice consistently reflects the target release version.
- Around line 152-156: Update the homepage URL for the package entry named
"imgscalr-lib" in THIRD-PARTY-NOTICES.txt to match Maven Central and other
modules; replace the current GitHub URL "https://github.com/rkalla/imgscalr"
with "https://github.com/thebuzzmedia/imgscalr" in the imgscalr-lib block so the
Package: imgscalr-lib / License: Apache License 2.0 section reflects the correct
repository.
---
Outside diff comments:
In `@biometrics-util/pom.xml`:
- Around line 156-162: The junit BOM dependency (<groupId>org.junit</groupId>,
<artifactId>junit-bom</artifactId>, <version>${org.junit.version}</version>) is
using scope "import" but is placed in <dependencies>; move this BOM entry into
the <dependencyManagement> section and keep <scope>import</scope> there, or
alternatively if you intend it as a normal dependency change its scope to "test"
and leave it in <dependencies>; update/remove the original entry accordingly so
Maven no longer has an import-scoped dependency outside dependencyManagement.
---
Nitpick comments:
In
`@kernel-biometrics-api/src/main/java/io/mosip/kernel/biometrics/entities/ObjectFactory.java`:
- Around line 14-16: Remove the unnecessary Lombok `@Data` annotation from the
ObjectFactory class since it has no fields and the generated
equals/hashCode/toString are pointless; edit the class declaration in
ObjectFactory (the class named ObjectFactory annotated with `@XmlRegistry`) to
delete the `@Data` import and annotation so the class remains a minimal JAXB-style
factory.
🪄 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: da1ce29f-44b9-46be-9b32-a9aecef666ff
📒 Files selected for processing (20)
THIRD-PARTY-NOTICES.txtbiometrics-util/pom.xmlkernel-biometrics-api/NOTICEkernel-biometrics-api/THIRD-PARTY-NOTICES.txtkernel-biometrics-api/pom.xmlkernel-biometrics-api/src/main/java/io/mosip/kernel/biometrics/commons/CbeffValidator.javakernel-biometrics-api/src/main/java/io/mosip/kernel/biometrics/entities/ByteArrayToIntArraySerializer.javakernel-biometrics-api/src/main/java/io/mosip/kernel/biometrics/entities/IntArrayToByteArrayDeserializer.javakernel-biometrics-api/src/main/java/io/mosip/kernel/biometrics/entities/ObjectFactory.javakernel-biometrics-api/src/test/io/mosip/kernel/biometrics/CbeffValidatorTest.javakernel-biometrics-api/src/test/resources/schema/updatedcbeff.xsdkernel-biosdk-provider/THIRD-PARTY-NOTICES.txtkernel-biosdk-provider/pom.xmlkernel-cbeffutil-api/NOTICEkernel-cbeffutil-api/README.mdkernel-cbeffutil-api/THIRD-PARTY-NOTICES.txtkernel-cbeffutil-api/pom.xmlkernel-cbeffutil-api/src/test/java/io/mosip/kernel/cbeffutil/test/CbeffImplTest.javapom.xmltest/pom.xml
CbeffImpl.validateXML(byte[]) passes a null xsd field to the two-arg overload, which then calls CbeffXSDValidator.validateXML(null, xmlBytes). Mockito's any(byte[].class) does not match null, causing the mock to not fire and returning false. Use nullable(byte[].class) to correctly match the null first argument. Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
biometrics-util/pom.xml (1)
156-162:⚠️ Potential issue | 🟠 Major | ⚡ Quick winJUnit BOM must be in
dependencyManagement, notdependencies.A BOM (Bill of Materials) with
<scope>import</scope>must be declared in the<dependencyManagement>section, not in<dependencies>. Placing it in<dependencies>will not provide version management for JUnit dependencies as intended.📦 Proposed fix: Move to dependencyManagement
Move this block to the
<dependencyManagement><dependencies>section (around line 57-67 wherekernel-bomis already declared):<dependencyManagement> <dependencies> <dependency> <groupId>io.mosip.kernel</groupId> <artifactId>kernel-bom</artifactId> <version>${kernel.bom.version}</version> <type>pom</type> <scope>import</scope> </dependency> + <dependency> + <groupId>org.junit</groupId> + <artifactId>junit-bom</artifactId> + <version>${org.junit.version}</version> + <type>pom</type> + <scope>import</scope> + </dependency> </dependencies> </dependencyManagement>Then remove the misplaced declaration from the
<dependencies>section:- <!-- JUnit 5 (Jupiter) BOM for dependency management --> - <dependency> - <groupId>org.junit</groupId> - <artifactId>junit-bom</artifactId> - <version>${org.junit.version}</version> - <type>pom</type> - <scope>import</scope> - </dependency> - <!-- Mockito for testing -->🤖 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 `@biometrics-util/pom.xml` around lines 156 - 162, The junit BOM declaration (artifactId junit-bom, groupId org.junit, version ${org.junit.version}, scope import) is incorrectly placed in the <dependencies> section; move this dependency block into the <dependencyManagement><dependencies> section alongside the existing kernel-bom entry so the import scope is effective for version management, and then remove the misplaced junit-bom from the <dependencies> section.
♻️ Duplicate comments (1)
kernel-cbeffutil-api/pom.xml (1)
87-91:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDuplicate
kernel-biometrics-apidependency declaration.This dependency is already declared at lines 67-71 with the same groupId, artifactId, and version. Maven will warn about the duplicate, and it should be removed.
🧹 Proposed fix: remove the redundant block
<dependency> <groupId>org.mockito</groupId> <artifactId>mockito-core</artifactId> <version>${org.mockito.version}</version> <scope>test</scope> </dependency> - <dependency> - <groupId>io.mosip.kernel</groupId> - <artifactId>kernel-biometrics-api</artifactId> - <version>${kernel.biometrics.api.version}</version> - </dependency> </dependencies>🤖 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 `@kernel-cbeffutil-api/pom.xml` around lines 87 - 91, Remove the duplicate Maven dependency declaration for groupId "io.mosip.kernel" and artifactId "kernel-biometrics-api" (the redundant block that repeats the existing dependency) from the pom.xml; keep only one declaration using the existing ${kernel.biometrics.api.version} and delete the second/redundant <dependency> element to eliminate the duplicate entry.
🧹 Nitpick comments (1)
AGENTS.md (1)
41-48: 💤 Low valueAdd language specifier to fenced code block.
The fenced code block showing the module dependency chain diagram should have a language identifier for better markdown linting compliance. For ASCII diagrams, use
textorplain.📝 Proposed fix
-``` +```text kernel-biometrics-api ← defines all SPIs, entities, constants, models ↑ kernel-cbeffutil-api ← implements CbeffUtil SPI; depends on kernel-biometrics-api🤖 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 `@AGENTS.md` around lines 41 - 48, The fenced code block in AGENTS.md containing the ASCII module dependency diagram lacks a language identifier; update that triple-backtick fence to include a language specifier (e.g., `text` or `plain`) so the block becomes ```text and preserves the diagram formatting and satisfies markdown linting—locate the ASCII diagram block (the lines starting with "kernel-biometrics-api ← defines all SPIs..." and the surrounding triple-backticks) and add the language token to the opening fence.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@biometrics-util/pom.xml`:
- Around line 156-162: The junit BOM declaration (artifactId junit-bom, groupId
org.junit, version ${org.junit.version}, scope import) is incorrectly placed in
the <dependencies> section; move this dependency block into the
<dependencyManagement><dependencies> section alongside the existing kernel-bom
entry so the import scope is effective for version management, and then remove
the misplaced junit-bom from the <dependencies> section.
---
Duplicate comments:
In `@kernel-cbeffutil-api/pom.xml`:
- Around line 87-91: Remove the duplicate Maven dependency declaration for
groupId "io.mosip.kernel" and artifactId "kernel-biometrics-api" (the redundant
block that repeats the existing dependency) from the pom.xml; keep only one
declaration using the existing ${kernel.biometrics.api.version} and delete the
second/redundant <dependency> element to eliminate the duplicate entry.
---
Nitpick comments:
In `@AGENTS.md`:
- Around line 41-48: The fenced code block in AGENTS.md containing the ASCII
module dependency diagram lacks a language identifier; update that
triple-backtick fence to include a language specifier (e.g., `text` or `plain`)
so the block becomes ```text and preserves the diagram formatting and satisfies
markdown linting—locate the ASCII diagram block (the lines starting with
"kernel-biometrics-api ← defines all SPIs..." and the surrounding
triple-backticks) and add the language token to the opening fence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07d8301d-fe9e-4f89-9396-a87b6d68cb2e
📒 Files selected for processing (8)
AGENTS.mdbiometrics-util/pom.xmlkernel-biometrics-api/pom.xmlkernel-biosdk-provider/pom.xmlkernel-cbeffutil-api/pom.xmlkernel-cbeffutil-api/src/test/java/io/mosip/kernel/cbeffutil/test/CbeffImplTest.javapom.xmltest/pom.xml
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with 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.
Inline comments:
In `@kernel-biosdk-provider/pom.xml`:
- Around line 50-51: The pom sets the property kernel.biometrics.api.version to
1.3.1 which pins the dependency io.mosip.kernel:kernel-biometrics-api to a
non-reactor release; update the property kernel.biometrics.api.version to
1.4.0-SNAPSHOT (or explicitly document/confirm in the POM that pinning to 1.3.1
is intentional) so the module resolves the local reactor module instead of the
released 1.3.1 artifact.
In `@kernel-cbeffutil-api/pom.xml`:
- Line 42: The POM property kernel.biometrics.api.version is pinned to 1.3.1 but
the sibling module kernel-biometrics-api in this reactor has been bumped to
1.4.0-SNAPSHOT; update the property kernel.biometrics.api.version to
1.4.0-SNAPSHOT so the dependency declared for kernel-biometrics-api resolves to
the reactor build (or explicitly confirm and document that pin to 1.3.1 is
intentional if you want the released artifact instead).
In `@pom.xml`:
- Around line 53-54: The build fails because kernel.bom.version and
kernel.core.version are set to 1.4.0-SNAPSHOT which isn’t published; update the
pom properties kernel.bom.version and kernel.core.version to a published release
(e.g., 1.3.x) or ensure the missing snapshot artifacts/BOM are published to your
snapshot repo; locate and change the property values for kernel.bom.version and
kernel.core.version in the pom.xml (or alternatively publish
io.mosip.kernel:kernel-bom:1.4.0-SNAPSHOT and corresponding kernel-core
artifacts to the snapshot repository) so the BOM import can be resolved.
In `@test/pom.xml`:
- Line 50: The test module's dependency property mosip.biometrics.util.version
is pinned to 1.3.1 while the module under test is bumped to 1.4.0-SNAPSHOT;
update the mosip.biometrics.util.version property in test/pom.xml to
1.4.0-SNAPSHOT so the test module exercises the locally built artifact (or
explicitly document/confirm if you intend to keep the pin to 1.3.1).
🪄 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: ac9ffc5d-1c4c-485b-96ea-abb7eb30ee30
📒 Files selected for processing (14)
THIRD-PARTY-NOTICESTHIRD-PARTY-NOTICES.txtbiometrics-util/pom.xmlkernel-biometrics-api/THIRD-PARTY-NOTICESkernel-biometrics-api/THIRD-PARTY-NOTICES.txtkernel-biometrics-api/pom.xmlkernel-biosdk-provider/THIRD-PARTY-NOTICESkernel-biosdk-provider/THIRD-PARTY-NOTICES.txtkernel-biosdk-provider/pom.xmlkernel-cbeffutil-api/THIRD-PARTY-NOTICESkernel-cbeffutil-api/THIRD-PARTY-NOTICES.txtkernel-cbeffutil-api/pom.xmlpom.xmltest/pom.xml
✅ Files skipped from review due to trivial changes (5)
- kernel-cbeffutil-api/THIRD-PARTY-NOTICES.txt
- THIRD-PARTY-NOTICES
- kernel-biosdk-provider/THIRD-PARTY-NOTICES.txt
- kernel-biometrics-api/THIRD-PARTY-NOTICES
- THIRD-PARTY-NOTICES.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- kernel-biometrics-api/THIRD-PARTY-NOTICES.txt
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Signed-off-by: kameshsr <kameshsr1338@gmail.com>
Summary by CodeRabbit