Skip to content

Fix thread-safe auth token storage - #245

Open
Tomeshwari-02 wants to merge 2 commits into
PSMRI:mainfrom
Tomeshwari-02:fix/auth-token-thread-safety
Open

Fix thread-safe auth token storage#245
Tomeshwari-02 wants to merge 2 commits into
PSMRI:mainfrom
Tomeshwari-02:fix/auth-token-thread-safety

Conversation

@Tomeshwari-02

@Tomeshwari-02 Tomeshwari-02 commented Apr 19, 2026

Copy link
Copy Markdown

Description

Make the auth token storage used by the network interceptors thread-safe so concurrent logins, sync jobs, and logout flows do not overwrite each other through shared mutable companion-object state.

Related Issue

Fixes #127

PR Category

  • Bug Fix � Fixes a bug or incorrect behavior
  • Tests � Adds or updates test coverage

Changes Made

  • Added a thread-safe AuthTokenManager backed by AtomicReference values.
  • Updated the TMC, ABHA, and eSanjeevani interceptors to read tokens from the shared manager.
  • Added a unit test suite for independent token storage and concurrent writes.

Testing Performed

  • Attempted ./gradlew app:testNiramayDebugUnitTest --tests org.piramalswasthya.cho.network.interceptors.AuthTokenManagerTest --no-daemon
  • The local environment currently blocks a full Gradle test run because of kapt/JDK and generated Firebase config issues in this checkout.

Checklist

  • I have tested these changes locally as far as this environment allows.
  • I have added/updated tests that prove the effectiveness of these changes.
  • I have followed the project's coding style guidelines.

Additional Notes for Reviewers

The change is intentionally small and keeps the existing interceptor API shape so the rest of the app does not need a larger refactor.

Summary by CodeRabbit

  • Refactor

    • Consolidated authentication token management into a centralized system, enhancing consistency and reliability across multiple authentication interceptors.
  • Tests

    • Added test coverage for concurrent token handling and independent token storage validation.

@coderabbitai

coderabbitai Bot commented Apr 19, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@Tomeshwari-02 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 48 minutes and 16 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 16 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1cd22677-07a3-4104-a33c-21448b9fc199

📥 Commits

Reviewing files that changed from the base of the PR and between fd77d2a and 0e7ef80.

📒 Files selected for processing (3)
  • app/src/main/java/org/piramalswasthya/cho/network/interceptors/AuthTokenManager.kt
  • app/src/main/java/org/piramalswasthya/cho/network/interceptors/TokenInsertTmcInterceptor.kt
  • app/src/test/java/org/piramalswasthya/cho/network/interceptors/AuthTokenManagerTest.kt
📝 Walkthrough

Walkthrough

This PR refactors token management across three interceptors by introducing a centralized, thread-safe AuthTokenManager singleton that stores authentication tokens using AtomicReference. Three existing token interceptors delegate token reads/writes to the manager instead of maintaining local companion-object storage, eliminating direct state mutation patterns.

Changes

Cohort / File(s) Summary
New Token Manager
app/src/main/java/org/piramalswasthya/cho/network/interceptors/AuthTokenManager.kt
New singleton AuthTokenManager with thread-safe storage for five distinct tokens (tmcToken, tmcJwt, abhaToken, abhaXToken, eSanjeevaniToken) via AtomicReference, exposing getter/setter pairs and a clear() method. Nullable ABHA tokens normalized to empty strings.
Token Interceptor Refactoring
app/src/main/java/org/piramalswasthya/cho/network/interceptors/TokenInsertTmcInterceptor.kt, TokenInsertAbhaInterceptor.kt, TokenESanjeevaniInterceptor.kt
Removed local companion-object token fields and updated setter/getter methods to delegate to AuthTokenManager. Request header insertion now sources tokens dynamically from the manager at interception time instead of cached in-class variables.
AuthTokenManager Tests
app/src/test/java/org/piramalswasthya/cho/network/interceptors/AuthTokenManagerTest.kt
New test class verifying independent storage of five token types and concurrent thread-safety via 32 concurrent write tasks using CountDownLatch and thread pool executor.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • Release 4.0 rmnch git glich resolve v1 #127 – Token management refactoring for thread-safety improvements aligns with centralizing authentication state handling across multiple interceptors to eliminate race conditions in concurrent token access patterns.

Poem

A rabbit hops with joy today,
Tokens centralized, thread-safe way,
No more scattered, racing fears,
One manager guards them all through years! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix thread-safe auth token storage' directly and clearly summarizes the main change—introducing AuthTokenManager for thread-safe token management across interceptors.
Linked Issues check ✅ Passed Issue #127 is a release template requesting a summary of changes and type of change designation. The PR provides the required summary and indicates this is a bug fix and test addition, meeting the template's informational requirements.
Out of Scope Changes check ✅ Passed All changes are scoped to implementing thread-safe token storage via AuthTokenManager and updating interceptors to use it. No unrelated refactoring, formatting changes beyond file termination fixes, or extraneous modifications are present.

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

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

@Tomeshwari-02
Tomeshwari-02 marked this pull request as ready for review April 19, 2026 18:33

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@app/src/main/java/org/piramalswasthya/cho/network/interceptors/AuthTokenManager.kt`:
- Around line 12-28: tmcToken and tmcJwt must be stored and read as one atomic
snapshot to avoid interleaved reads/writes; replace the two
AtomicReference<String> fields (tmcToken, tmcJwt) and the separate
setters/getters (setTmcToken, setTmcJwt, getTmcToken, getTmcJwt) with a single
AtomicReference holding an immutable TmcCredentials data class and add
setTmcCredentials(token, jwt) and getTmcCredentials() methods; update the TMC
interceptor to call getTmcCredentials() once and migrate all callers that
currently call setTmcJwt()/setTmcToken() separately to use
setTmcCredentials(token, jwt) so token and jwt are always consistent atomically.

In
`@app/src/test/java/org/piramalswasthya/cho/network/interceptors/AuthTokenManagerTest.kt`:
- Around line 40-63: Update the test concurrentWritesRemainReadable to (1)
ensure TMC token and JWT are read atomically from an API on AuthTokenManager
(add or use a method like getTmcTokenJwtSnapshot()/getTmcCredentialsSnapshot()
that returns both values together) and assert that the numeric suffix of the
returned token ("tmc-N") matches the numeric suffix of the returned jwt
("jwt-N"), and (2) wrap executor shutdown/awaitTermination in a finally block so
the executor is always shut down even on timeouts/failures; keep the existing
per-field prefix assertions if you want, but replace cross-field consistency
checks with the atomic-snapshot based equality check using
AuthTokenManager.getTmcToken/getTmcJwt only if they are returned together
atomically via the new snapshot API.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aa308064-d7f9-41f9-a94e-a8cb1acbc5f6

📥 Commits

Reviewing files that changed from the base of the PR and between 4cfb776 and fd77d2a.

📒 Files selected for processing (5)
  • app/src/main/java/org/piramalswasthya/cho/network/interceptors/AuthTokenManager.kt
  • app/src/main/java/org/piramalswasthya/cho/network/interceptors/TokenESanjeevaniInterceptor.kt
  • app/src/main/java/org/piramalswasthya/cho/network/interceptors/TokenInsertAbhaInterceptor.kt
  • app/src/main/java/org/piramalswasthya/cho/network/interceptors/TokenInsertTmcInterceptor.kt
  • app/src/test/java/org/piramalswasthya/cho/network/interceptors/AuthTokenManagerTest.kt

@sonarqubecloud

Copy link
Copy Markdown

@drtechie

Copy link
Copy Markdown
Member

@sushant-bizbrolly please review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants