PII issues fixes with backward compatibility for booking service [MOSIP-44379]

[GOKULRAJ136]

Summary by CodeRabbit

• Chores

  • Bumped kernel dependencies to version 1.3.0

• New Features

  • Added a PII backward-compatibility option to control how user identifiers are resolved for bookings
  • Added safe fallback and strict modes for identifier resolution behavior

• Tests

  • Added unit tests covering identifier resolution behaviors and the new backward-compatibility option

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c02e1978-61da-44ea-a145-972b00accf47

📥 Commits

Reviewing files that changed from the base of the PR and between 5d118e5 and 14a3e5b.

📒 Files selected for processing (2)

• pre-registration-booking-service/src/main/java/io/mosip/preregistration/booking/service/util/BookingServiceUtil.java
• pre-registration-booking-service/src/test/java/io/mosip/preregistration/booking/test/service/util/BookingServiceUtilTest.java

🚧 Files skipped from review as they are similar to previous changes (2)

• pre-registration-booking-service/src/main/java/io/mosip/preregistration/booking/service/util/BookingServiceUtil.java
• pre-registration-booking-service/src/test/java/io/mosip/preregistration/booking/test/service/util/BookingServiceUtilTest.java

---

Walkthrough

Adds canonical user-ID resolution to BookingServiceUtil with a configurable PII backward-compatibility flag, injects UserDetailsService, updates booking creation to use resolved IDs (or fallback/throw), bumps kernel versions, and adds configuration plus unit tests for canonical, fallback, and strict modes.

Changes

Cohort / File(s)	Summary
Kernel Dependency Update pre-registration-booking-service/pom.xml	Bumps kernel.core.version and kernel.bom.version from 1.2.1-SNAPSHOT to 1.3.0.
User ID Resolution Logic pre-registration-booking-service/src/main/.../BookingServiceUtil.java	Injects UserDetailsService and a piiBackwardCompatibility flag; adds resolveEffectiveCrBy(String) and uses it in bookingEntitySetter(...) to derive RegistrationBookingEntity.crBy, with conditional fallback or exception on unresolved canonical IDs and masked logging.
Configuration Properties pre-registration-booking-service/src/main/resources/bootstrap.properties, pre-registration-booking-service/src/test/resources/application.properties	Adds mosip.prereg.pii.backward.compatibility=false to control fallback behavior.
Test Coverage pre-registration-booking-service/src/test/java/.../BookingServiceUtilTest.java	Mocks UserDetailsService, toggles piiBackwardCompatibility via ReflectionTestUtils, and adds tests for canonical resolution success, canonical-or-identifier fallback, and strict-mode failure; updates imports accordingly.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Caller
    participant Auth as AuthContext
    participant BookingUtil as BookingServiceUtil
    participant UserSvc as UserDetailsService

    Caller->>Auth: obtain rawIdentifier
    Caller->>BookingUtil: bookingEntitySetter(rawIdentifier, ...)
    BookingUtil->>UserSvc: maskIdentifier(rawIdentifier) (log)
    BookingUtil->>UserSvc: resolveCanonicalUserId(rawIdentifier)
    alt canonical resolved
        UserSvc-->>BookingUtil: canonicalId
        BookingUtil-->>Caller: set crBy = canonicalId
    else canonical not resolved
        alt piiBackwardCompatibility == true
            BookingUtil->>UserSvc: resolveCanonicalUserIdOrIdentifier(rawIdentifier)
            UserSvc-->>BookingUtil: identifierOrRaw
            BookingUtil-->>Caller: set crBy = identifierOrRaw
        else piiBackwardCompatibility == false
            BookingUtil-->>Caller: throw AppointmentBookingFailedException
        end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 I hop, I check the canonical way,

masked hops for logs, fallbacks at play,
tests snugly bound, versions tuned tight,
a rabbit applauds this careful write.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% 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 PR title accurately describes the main changes: PII-related fixes with backward compatibility introduced in the booking service through version updates, configuration flags, and refactored user ID resolution logic.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b63858 and 52bf984.

📒 Files selected for processing (5)

• pre-registration-booking-service/pom.xml
• pre-registration-booking-service/src/main/java/io/mosip/preregistration/booking/service/util/BookingServiceUtil.java
• pre-registration-booking-service/src/main/resources/bootstrap.properties
• pre-registration-booking-service/src/test/java/io/mosip/preregistration/booking/test/service/util/BookingServiceUtilTest.java
• pre-registration-booking-service/src/test/resources/application.properties

[coderabbitai]

♻️ Duplicate comments (1)

pre-registration-booking-service/src/main/java/io/mosip/preregistration/booking/service/util/BookingServiceUtil.java (1)

558-575: ⚠️ Potential issue | 🟠 Major

Guard against blank raw fallback in backward-compatibility mode.

On Line [568]-Line [571], fallback returns userId even when it may be null/blank, which can still persist an unattributable crBy. Fail fast unless a non-blank identifier is available.

Proposed fix

 private String resolveEffectiveCrBy(String userId) {
+	if (userId == null || userId.isBlank()) {
+		throw new AppointmentBookingFailedException(
+				ErrorCodes.PRG_BOOK_RCI_005.getCode(),
+				ErrorMessages.APPOINTMENT_BOOKING_FAILED.getMessage());
+	}
 	try {
 		UserDetails userDetail = userDetailsService.findOrCreateByIdentifier(userId);
 		if (userDetail != null && userDetail.getUserId() != null) {
 			return userDetail.getUserId().toString();
 		}
 	} catch (Exception ex) {
 		log.warn("sessionId", "idType", "id",
 				"Failed to resolve canonical user id for booking: " + maskIdentifier(userId));
 	}
-	if (piiBackwardCompatibility) {
+	if (piiBackwardCompatibility) {
 		log.warn("sessionId", "idType", "id",
 				"Falling back to raw identifier for booking due to backward-compatibility mode");
 		return userId;
 	}
 	throw new AppointmentBookingFailedException(ErrorCodes.PRG_BOOK_RCI_005.getCode(),
 			ErrorMessages.APPOINTMENT_BOOKING_FAILED.getMessage());
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@pre-registration-booking-service/src/main/java/io/mosip/preregistration/booking/service/util/BookingServiceUtil.java`
around lines 558 - 575, The resolveEffectiveCrBy method currently falls back to
returning userId when piiBackwardCompatibility is true even if userId is
null/blank; update resolveEffectiveCrBy to validate that the raw identifier is
non-blank before returning it (use a String non-blank check or
StringUtils.isBlank/hasText), log a clear warning (including
maskIdentifier(userId)) when the raw id is blank, and if blank throw
AppointmentBookingFailedException with ErrorCodes.PRG_BOOK_RCI_005 and
ErrorMessages.APPOINTMENT_BOOKING_FAILED; keep the existing catch block and
masking behavior but ensure the fallback only returns a non-empty identifier.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@pre-registration-booking-service/src/main/java/io/mosip/preregistration/booking/service/util/BookingServiceUtil.java`:
- Around line 558-575: The resolveEffectiveCrBy method currently falls back to
returning userId when piiBackwardCompatibility is true even if userId is
null/blank; update resolveEffectiveCrBy to validate that the raw identifier is
non-blank before returning it (use a String non-blank check or
StringUtils.isBlank/hasText), log a clear warning (including
maskIdentifier(userId)) when the raw id is blank, and if blank throw
AppointmentBookingFailedException with ErrorCodes.PRG_BOOK_RCI_005 and
ErrorMessages.APPOINTMENT_BOOKING_FAILED; keep the existing catch block and
masking behavior but ensure the fallback only returns a non-empty identifier.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52bf984 and 5d118e5.

📒 Files selected for processing (2)

• pre-registration-booking-service/src/main/java/io/mosip/preregistration/booking/service/util/BookingServiceUtil.java
• pre-registration-booking-service/src/test/java/io/mosip/preregistration/booking/test/service/util/BookingServiceUtilTest.java

🚧 Files skipped from review as they are similar to previous changes (1)

• pre-registration-booking-service/src/test/java/io/mosip/preregistration/booking/test/service/util/BookingServiceUtilTest.java