[INJICERT-1277] Remove cache dependency from mdoc vc issuance

[Piyush7034]

Summary by CodeRabbit

• Refactor

  • Simplified mock certification plugin: document identifier now sourced directly from identity subject and extensive internal cryptographic/cache helper logic removed, reducing complexity.

• Tests

  • Streamlined tests to validate public-facing behavior, removed tests tied to deprecated internal mechanisms, and ensured unsupported credential formats remain handled as expected.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR removes internal identity/key helper methods and crypto/cache dependencies from the MDoc mock VC issuance plugin and obtains the documentNumber directly from identityDetails.get("sub"); tests were simplified and tests targeting removed internals were deleted.

Changes

Cohort / File(s)	Summary
Production Code Refactor mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MDocMockVCIssuancePlugin.java	Removed private helper methods and cryptographic/cache-related fields and constants (e.g., getIndividualId, decryptIndividualId, getUserInfoTransaction, getSecretKeyFromHSM, getKeyAlias, AES-related constants); now derives documentNumber from identityDetails.get("sub"); retained main VC issuance flow via mdocGenerator and issuerKeyAndCertificate.
Test Adjustments mock-certify-plugin/src/test/java/io/mosip/certify/mock/integration/service/MDocMockVCIssuancePluginTest.java	Removed mocks and tests for deleted internals (cache, keystore, HSM flows); updated setup to include "sub" in identityDetails, simplified mdocGenerator.generate behavior, and adjusted NotImplemented test to use VCFormats.LDP_VC.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• swatigoel

Poem

🐰 I nibbled through a tangled nest,

I pulled out threads and left the best.
"sub" now hops in place of keys,
Light paws dancing in the breeze.
🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing cache dependency from mdoc VC issuance, which aligns with the extensive removal of cache-related fields, autowired dependencies, and helper methods in the production code.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MDocMockVCIssuancePlugin.java (2)

77-83: Missing null validation for "sub" key.

If identityDetails does not contain the "sub" key or if the value is null, documentNumber will be null and flow through to mockDataForMsoMdoc() without any error. The broad catch (Exception e) only catches ClassCastException if the value is non-String, not missing/null values.

Consider validating the presence of "sub":

🛡️ Proposed fix to add validation

 String documentNumber;
 try {
+    if (!identityDetails.containsKey("sub") || identityDetails.get("sub") == null) {
+        log.error("Missing or null 'sub' in identityDetails");
+        throw new VCIExchangeException(ErrorConstants.VCI_EXCHANGE_FAILED);
+    }
     documentNumber = (String) identityDetails.get("sub");
 } catch (Exception e) {

---

34-64: Remove unused code from cache dependency removal refactoring.

This file contains significant dead code from methods that were deleted during cache dependency removal:

• Constants: AES_CIPHER_FAILED, NO_UNIQUE_ALIAS, USERINFO_CACHE, ACCESS_TOKEN_HASH, CERTIFY_SERVICE_APP_ID
• Autowired fields: cacheManager, keyStore, dbHelper
• @value fields: cacheSecretKeyRefId, aesECBTransformation, secureIndividualId, storeIndividualId
• Imports: OIDCTransaction, KeyStore, KeymanagerConstant, KeyAlias, KeymanagerDBHelper, CacheManager, Cipher, Key, LocalDateTime, ZoneOffset

Remove these declarations and imports to complete the refactoring.

mock-certify-plugin/src/test/java/io/mosip/certify/mock/integration/service/MDocMockVCIssuancePluginTest.java (1)

57-71: Test setup doesn't match production code after refactoring.

The test sets identityDetails.put("accessTokenHash", "tokenHash") at line 62, but the production code now retrieves identityDetails.get("sub"). Since "sub" is never set, documentNumber will be null during the test. The test passes only because anyMap() is used in the mock, hiding this mismatch.

Update the test to use the correct key:

🐛 Proposed fix

     Map<String, Object> identityDetails = new HashMap<>();
-    identityDetails.put("accessTokenHash", "tokenHash");
+    identityDetails.put("sub", "testDocumentNumber");

     when(mdocGenerator.generate(anyMap(), anyString(), anyString())).thenReturn("mockedMdoc");

Consider also using argument captors to verify that mockDataForMsoMdoc receives the expected document number:

ArgumentCaptor<Map<String, Object>> mapCaptor = ArgumentCaptor.forClass(Map.class);
verify(mdocGenerator).generate(mapCaptor.capture(), eq("holderId"), eq("empty"));
assertEquals("testDocumentNumber", mapCaptor.getValue().get("document_number"));

🧹 Nitpick comments (1)

mock-certify-plugin/src/test/java/io/mosip/certify/mock/integration/service/MDocMockVCIssuancePluginTest.java (1)

34-54: Test setup contains unused mocks and field configurations.

Several mocks and ReflectionTestUtils.setField calls correspond to production fields that are now unused:

• Unused mocks: cacheManager, keyStore, dbHelper, cache, key
• Unused field setups: cacheSecretKeyRefId, aesECBTransformation, storeIndividualId, secureIndividualId

After cleaning up the production code, remove these from the test as well to keep the test focused and maintainable.

[coderabbitai]

Actionable comments posted: 0

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/main/java/io.mosip.certify.mock.integration/service/MDocMockVCIssuancePlugin.java (1)

38-43: Add null validation for "sub" key from identityDetails.

The code retrieves "sub" from identityDetails and casts it to String, but identityDetails.get("sub") returns null if the key is absent—and casting null to String does not throw an exception. This results in a null documentNumber being passed to mockDataForMsoMdoc() and subsequently to mdocGenerator.generate().

The try-catch block only handles ClassCastException (when the value exists but isn't a String); it does not validate the presence of the "sub" key. Other similar plugins in this codebase (MockIdaDataProviderPlugin, MockCSVDataProviderPlugin) explicitly check for null after retrieval. Either validate that "sub" is present/non-null before use, or add an explicit null check after retrieval to match the defensive coding pattern used elsewhere.

🧹 Nitpick comments (2)

mock-certify-plugin/src/test/java/io/mosip/certify/mock/integration/service/MDocMockVCIssuancePluginTest.java (1)

59-64: Make the unsupported-format test independent of identityDetails validation.
If validation of "sub" is ever moved ahead of the format check, this test could fail for the wrong reason. Consider adding "sub" here (or asserting on the error code) to ensure the exception is strictly for unsupported formats.

💡 Optional test hardening

 Map<String, Object> identityDetails = new HashMap<>();
 identityDetails.put("accessTokenHash", "tokenHash");
+identityDetails.put("sub", "12345");

mock-certify-plugin/src/main/java/io.mosip.certify.mock.integration/service/MDocMockVCIssuancePlugin.java (1)

64-64: Consider using log.debug for mock data setup message.

Since this is a mock plugin, the "Setting up the data for mDoc" message at INFO level may add noise in production logs. DEBUG level would be more appropriate for internal setup messages.

♻️ Suggested change

 private Map<String, Object> mockDataForMsoMdoc(String documentNumber) {
     Map<String, Object> data = new HashMap<>();
-    log.info("Setting up the data for mDoc");
+    log.debug("Setting up the data for mDoc");
     data.put("family_name","Agatha");