Refactor OTPListener for Notification Handling & Update Module Check from equals to contains

[BhuvanaShreeBS]

Modified OTPListener to Handle Notifications (Remove Separate NotificationListener)
Updated Module Check from equals to contains

Summary by CodeRabbit

• Bug Fixes

  • Consolidated notification handling logic for improved reliability and reduced code duplication.
  • Enhanced module detection to support broader naming conventions in test execution.

• Refactor

  • Streamlined notification processing pipeline to centralize handling of OTP and mail notifications.
  • Improved notification storage and retrieval mechanisms for better performance.

[coderabbitai]

Walkthrough

The changes consolidate notification-listening functionality from a standalone AllNotificationListner class into OTPListener, introducing a unified notification map (allNotificationMapS) to track all incoming messages. Additionally, DSL module detection is broadened across multiple utility methods to use substring matching instead of exact string equality.

Changes

Cohort / File(s)	Summary
Notification Listener Consolidation apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/AllNotificationListner.java	Removed entire class (150 lines). WebSocket listener that connected to mock SMTP service and maintained notification map is deleted, with functionality migrated to OTPListener.
OTPListener Enhancement apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/OTPListener.java	Added public static allNotificationMapS map to collect all notifications separately from OTP-only tracking. Enhanced WebSocket onText handling with type-aware parsing (SMS vs MAIL), added getNotification(emailId) retrieval method with bounded retry logic, introduced private sleep() utility, and refined DSL module detection to use substring matching.
DSL Module Detection Refinement apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java	Modified three DSL module checks from equals (exact match) to contains (substring match), broadening conditions under which SMTP OTP handling and related paths are applied.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 A listener retired, its duty passed on,
The OTPListener now tracks notifications anon,
With maps that catch all messages in their net,
And DSL detection broader than ever yet—
The warren's tests now flow more complete! 🌙

🚥 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 captures the two main changes: refactoring OTPListener for notification handling and updating module checks from equals to contains.

_{✏️ 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

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.java

---

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

Caution

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

⚠️ Outside diff range comments (1)

apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java (1)

287-292: ⚠️ Potential issue | 🟡 Minor

Substring matching with contains() creates ambiguity; prefer explicit equality checks or delimited tokens.

The risk of substring collision is real: GlobalConstants.RESIDENTNEW = "residentNew" contains the substring "resident", so if currentModule is ever set to "residentNew", the check contains(GlobalConstants.RESIDENT) would incorrectly match. While "dsl" itself has no substring collision with other module names currently defined, the broader pattern of using contains() for module identification is fragile and should be replaced with equals() comparisons or, if composite modules must be supported, split on a consistent delimiter and check membership explicitly.

🤖 Fix all issues with AI agents

In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/OTPListener.java`:
- Around line 88-109: In OTPListener.onText, add defensive null/empty checks
before dereferencing nested fields on the parsed Root object (check root,
root.type, root.to, root.to.text, root.to.value != null &&
!root.to.value.isEmpty(), and root.to.value.get(0).address) and handle MAIL and
SMS branches accordingly to avoid NPE/IndexOutOfBoundsException; also update the
outer catch to log the raw incoming data (the 'data' string) and the exception
details via logger.error so dropped notifications are diagnosable. Ensure you
modify the branches that set otpMessage/allMessage/address and the catch block
where exceptions are currently swallowed.
- Around line 186-195: The code has a TOCTOU race because it calls
allNotificationMapS.get(emailId) and then allNotificationMapS.remove(emailId)
separately; replace the two-step check with a single atomic call to Object
message = allNotificationMapS.remove(emailId) inside the loop (referencing
allNotificationMapS and emailId in OTPListener), then test message for null and
log/return the value (or return "" if null) so callers never get an unexpected
null; keep the existing loop using otpCheckLoopCount and sleep() unchanged.

🧹 Nitpick comments (3)

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

89-90: ObjectMapper instantiated on every WebSocket message.

ObjectMapper is thread-safe and designed to be reused. Hoisting it to a private static final field avoids repeated construction overhead on every incoming message.

Proposed refactor

Add at the class level:

private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

Then in onText:

-               ObjectMapper om = new ObjectMapper();
-               Root root = om.readValue(data.toString(), Root.class);
+               Root root = OBJECT_MAPPER.readValue(data.toString(), Root.class);

---

111-113: Successive notifications for the same address silently overwrite previous entries.

allNotificationMapS.put(address, allMessage) replaces any unconsumed notification for the same address. If two notifications arrive before getNotification is called, the first is lost. The same applies to emailNotificationMapS. This is a pre-existing pattern, but worth noting since this new map broadens the scope.

If this becomes a problem, consider using a Map<String, Queue<String>> or a ConcurrentHashMap<String, BlockingQueue<String>> so messages queue up per address.

---

116-120: Double-parsing: OTP is parsed at ingestion (here) and again at retrieval in getOtp.

parseOtp(otpMessage) is called here to decide whether to store, and called again in getOtp() (line 158) when consuming. This is minor but could be simplified by always storing and letting the consumer decide, or by storing the parsed OTP directly.