MOSIP-33663: Added common ID validation(UIN,VID,PRID) and test data generation.#1882
MOSIP-33663: Added common ID validation(UIN,VID,PRID) and test data generation.#1882SradhaMohanty5899 wants to merge 6 commits intomosip:developfrom
Conversation
Signed-off-by: SradhaMohanty5899 <[email protected]>
WalkthroughAdds an ID generation and validation framework: new CheckSumUtil, IdValidator, and IdGenerator classes for UIN/VID/PRID generation and validation; AdminTestUtil integrates dynamic ID keyword replacements; ConfigManager gains actuator-backed int and list property access; GlobalConstants adds an actuator property section constant. Changes
Sequence DiagramsequenceDiagram
participant AdminTestUtil
participant IdGenerator
participant CheckSumUtil
participant IdValidator
AdminTestUtil->>IdGenerator: request generated ID (valid/invalid)
IdGenerator->>IdGenerator: randomDigits(baseLength)
IdGenerator->>CheckSumUtil: generateChecksumDigit(base)
CheckSumUtil->>CheckSumUtil: apply lookup tables (d, p, inv)
CheckSumUtil-->>IdGenerator: checksum digit
IdGenerator->>IdGenerator: assemble id = base + checksum
IdGenerator->>IdValidator: isValid{Type}(id)
IdValidator->>IdValidator: run pattern/sequence/length checks
IdValidator-->>IdGenerator: validation result
alt valid as requested
IdGenerator-->>AdminTestUtil: return id
else retry up to MAX_ATTEMPTS
IdGenerator->>IdGenerator: repeat generation
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.40.5)apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.javaThanks 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@apitest-commons/src/main/java/io/mosip/testrig/apirig/id/CheckSumUtil.java`:
- Around line 16-31: The public methods generateChecksumDigit and
validateChecksum currently pass invalid or null strings into
stringToReversedIntArray causing NumberFormatException/NullPointerException; add
input validation at the start of both methods to check for null/empty and ensure
the string contains only digits (e.g., regex or Character.isDigit loop) and if
invalid throw a clear IllegalArgumentException (with message identifying the
method and the bad input) instead of letting
NumberFormatException/NullPointerException propagate; this keeps
stringToReversedIntArray unchanged and centralizes validation in the public
APIs.
In `@apitest-commons/src/main/java/io/mosip/testrig/apirig/id/IdGenerator.java`:
- Around line 37-64: The generateValid and generateInvalid methods use unbounded
while(true) loops and need a max-attempts guard to avoid hangs; introduce a
MAX_ATTEMPTS constant and increment an attempt counter inside both
generateValid(int baseLength, Type type) and generateInvalid(int baseLength,
Type type), break/return when successful as now, and after exceeding
MAX_ATTEMPTS throw an IllegalStateException (or similar) with a clear message
referencing the method, baseLength and type so callers know why generation
failed; ensure both methods use the same guard and exception behavior for
consistency.
- Around line 3-33: IdGenerator currently uses hard-coded base lengths in
methods like generateValidUin(), generateValidVid(), generateValidPrid(),
generateInvalidUin(), generateInvalidVid(), and generateInvalidPrid(); change
these to read the configured lengths via ConfigManager (same keys used by
IdValidator, e.g., mosip.uin.length, mosip.vid.length, mosip.prid.length) at
class init or lazily and use those values when calling generateValid(...) and
generateInvalid(...); ensure you reference ConfigManager once (cache the values
in private static finals) and fall back to current defaults only if the config
lookup returns invalid.
In `@apitest-commons/src/main/java/io/mosip/testrig/apirig/id/IdValidator.java`:
- Around line 77-81: commonValidation currently calls id.matches("\\d+") which
will NPE on a null id; update commonValidation to first check for null or blank
inputs (e.g., id == null || id.trim().isEmpty()) and return false before
performing the regex or other validations so callers like isValidUin, isValidVid
and isValidPrid remain safe with null/blank inputs.
- Around line 11-54: IdValidator currently initializes many static config fields
(e.g., SEQUENCE_LIMIT, UIN_LENGTH, SEQ_ASC, CYCLIC_NUM) at class load using
ConfigManager, and its commonValidation() method has no null check for the id
parameter; if ConfigManager.init(...) hasn't run or properties are missing this
can cause NumberFormatException and commonValidation() can throw NPE. Fix by
making config access lazy and null-safe: replace eager static reads with
lazily-initialized getters (or validate ConfigManager.isInitialized() at start)
that return defaults or throw clear configuration exceptions if properties are
absent, and add a null check at the start of commonValidation(id) to return
false or throw IllegalArgumentException for null id; update references to
SEQUENCE_LIMIT/SEQ_ASC/etc. to use the new getters so class loading no longer
depends on ConfigManager.init().
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/ConfigManager.java`:
- Around line 305-315: The typed accessors don't guard against missing/blank
values from getproperty (which returns ""), so update getIntProperty,
getListProperty and getArrayProperty to first fetch the raw value via
getproperty(key), trim it and fail fast if it's null/empty by throwing an
IllegalArgumentException with a clear message including the key; for
getIntProperty pass the trimmed value to Integer.parseInt; for getListProperty
and getArrayProperty split on "," then trim each token and filter out empty
tokens before returning (return an empty list/empty array if no valid tokens
remain).
🧹 Nitpick comments (1)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java (1)
3917-3947: Avoid duplicate$BIOVALUE$handling.
There’s already a$BIOVALUE$replacement later in the same method, so this new block is redundant and can drift over time. Consider keeping a single handler.🧹 Suggested cleanup
- if (jsonString.contains("$BIOVALUE$")) { - jsonString = replaceKeywordWithValue(jsonString, "$BIOVALUE$", - BiometricDataProvider.getFromBiometricMap("BioValue")); - } -
apitest-commons/src/main/java/io/mosip/testrig/apirig/id/CheckSumUtil.java
Show resolved
Hide resolved
apitest-commons/src/main/java/io/mosip/testrig/apirig/id/IdGenerator.java
Outdated
Show resolved
Hide resolved
apitest-commons/src/main/java/io/mosip/testrig/apirig/id/IdGenerator.java
Outdated
Show resolved
Hide resolved
apitest-commons/src/main/java/io/mosip/testrig/apirig/id/IdValidator.java
Outdated
Show resolved
Hide resolved
apitest-commons/src/main/java/io/mosip/testrig/apirig/id/IdValidator.java
Outdated
Show resolved
Hide resolved
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/ConfigManager.java
Show resolved
Hide resolved
Signed-off-by: SradhaMohanty5899 <[email protected]>
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java
Outdated
Show resolved
Hide resolved
Signed-off-by: SradhaMohanty5899 <[email protected]>
Signed-off-by: SradhaMohanty5899 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java`:
- Around line 3917-3942: The placeholders $VALID_UIN_NOTIN_DB$,
$VALID_VID_NOTIN_DB$, and $VALID_PRID_NOTIN_DB` in AdminTestUtil are misleading
because IdGenerator.generateValid*() only validates format and does not ensure
DB uniqueness; update the code that handles these keywords (the jsonString
contains checks and calls to replaceKeywordWithValue /
IdGenerator.generateValidUin(), generateValidVid(), generateValidPrid()) to
guarantee non-existence by either: (A) query the DB to verify the candidate ID
is absent and retry generation until it is unique, (B) maintain a run-level
in-memory cache of generated IDs and regenerate on collision, or (C) change the
keyword semantics/name to remove the “NOTIN_DB” claim; implement one approach
consistently for UIN, VID and PRID generation and ensure replaceKeywordWithValue
uses the chosen uniqueness check before replacing the token.
Summary by CodeRabbit