Updated the OTPListner: replaced synchronizedMap with ConcurrentHashMapp for better concurrency

[BhuvanaShreeBS]

Updated the OTPListner - replaced synchronizedMap with ConcurrentHashMapp for better concurrency

Summary by CodeRabbit

• Bug Fixes

  • Improved OTP notification retrieval to prevent potential null-reference errors.

• Refactor

  • Enhanced notification storage mechanism with improved reliability under concurrent operations.

[coderabbitai]

Walkthrough

OTPListener.java refactored to replace Map<Object, Object> backed by a synchronized HashMap with Map<String, String> backed by ConcurrentHashMap. The getNotification method now includes explicit null-checking for safer retrieval. One import added for ConcurrentHashMap.

Changes

Cohort / File(s)	Summary
OTP Listener Refactoring apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/OTPListener.java	Changed allNotificationMapS from Collections.synchronizedMap(HashMap) to ConcurrentHashMap with type narrowing from Map<Object, Object> to Map<String, String>. Added null-safe check in getNotification to return only non-null removal results.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Concurrent dreams in rabbit's sight,
HashMap's sync gives way to might,
ConcurrentHashMap takes the lead,
String-typed keys, the code we need!
Null-checks whisper, safe and bright,
Performance hops into the light! ✨

🚥 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: replacing synchronizedMap with ConcurrentHashMap in OTPListener for improved concurrency, which aligns with the file modifications.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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: 1

🤖 Fix all issues with AI agents

In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/OTPListener.java`:
- Around line 189-193: The code in OTPListener uses
allNotificationMapS.get(emailId) followed by
allNotificationMapS.remove(emailId), creating a TOCTOU race; replace the
two-step pattern with a single atomic call: call
allNotificationMapS.remove(emailId) directly, store its result in a local
variable, check that result for null and return it if non-null (remove the
redundant get and null-check on the first call) so the retrieval-and-delete is
atomic with respect to other threads.

🧹 Nitpick comments (1)

apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/OTPListener.java (1)

30-33: Inconsistency: emailNotificationMapS still uses synchronizedMap with raw Object types.

allNotificationMapS was improved to ConcurrentHashMap<String, String>, but emailNotificationMapS on lines 30–31 still uses Collections.synchronizedMap(new HashMap<Object, Object>()). Consider applying the same treatment for consistency and to avoid the unnecessary (String) cast on line 153/220.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

TOCTOU race: get followed by remove is not atomic.

Even with ConcurrentHashMap, a concurrent thread can remove the entry between your get check (line 189) and your remove call (line 190). The null-check on line 191 prevents returning null, which is good, but the initial get is now redundant — just call remove directly.

Proposed fix

-			if (allNotificationMapS.get(emailId) != null) {
-				String message = allNotificationMapS.remove(emailId);
-				if (message != null) {
-					return message;
-				}
+			String message = allNotificationMapS.remove(emailId);
+			if (message != null) {
+				return message;
 			}

🤖 Prompt for AI Agents

In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/OTPListener.java`
around lines 189 - 193, The code in OTPListener uses
allNotificationMapS.get(emailId) followed by
allNotificationMapS.remove(emailId), creating a TOCTOU race; replace the
two-step pattern with a single atomic call: call
allNotificationMapS.remove(emailId) directly, store its result in a local
variable, check that result for null and return it if non-null (remove the
redundant get and null-check on the first call) so the retrieval-and-delete is
atomic with respect to other threads.