-
Notifications
You must be signed in to change notification settings - Fork 46
Refactor get-account-by-local-account-id FixesAB#3397557, Fixes AB#3397557 #2781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor get-account-by-local-account-id FixesAB#3397557, Fixes AB#3397557 #2781
Conversation
|
❌ Work item link check failed. Description does not contain AB#{ID}. Click here to Learn more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors and optimizes the account and credential filtering logic in the token cache implementation to improve performance and readability. The changes focus on streamlining filtering conditions and reducing unnecessary checks when retrieving accounts and credentials.
- Optimized filtering logic by using early returns instead of accumulating match state
- Refactored account retrieval to filter accounts and credentials separately before checking associations
- Added new imports and removed unused ones for better maintainability
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 18 comments.
| File | Description |
|---|---|
| MsalOAuth2TokenCache.java | Refactored getAccountByLocalAccountId method to filter accounts and credentials separately, added new imports, and removed unused stream import |
| AbstractAccountCredentialCache.java | Optimized filtering logic with early continue statements to improve performance and readability |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
common4j/src/main/com/microsoft/identity/common/java/cache/MsalOAuth2TokenCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/MsalOAuth2TokenCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/MsalOAuth2TokenCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
…lOAuth2TokenCache.java Co-authored-by: Copilot <[email protected]>
|
✅ Work item link check complete. Description contains link AB#3397557 to an Azure Boards work item. |
common4j/src/main/com/microsoft/identity/common/java/cache/MsalOAuth2TokenCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
Show resolved
Hide resolved
melissaahn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
common4j/src/main/com/microsoft/identity/common/java/cache/MsalOAuth2TokenCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/MsalOAuth2TokenCache.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/cache/MsalOAuth2TokenCache.java
Show resolved
Hide resolved
mohitc1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
…AB#3397557 (#2797) Fixes[AB#3397557](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3397557) Recently refactored getAccountByLocalAccountId method to first filter both accounts and credentials on environment and then, check their homeAccountId to match the accounts with credentials. However, that refactoring does not work when environment passed is **null**. This resulted in test failures in the broker repo. #2781 This PR is to revert some of the changes made as part of that commit and fix the failing tests.
FixesAB#3397557
This pull request refactors and optimizes account and credential filtering logic within the token cache implementation. The main focus is on improving performance and readability by streamlining filtering conditions and reducing unnecessary checks, especially in the methods that retrieve accounts and credentials. Additionally, some redundant imports are removed and logging is enhanced for better traceability.
Filtering Logic Improvements
AbstractAccountCredentialCache.javaby immediately continuing to the next iteration when a match condition fails, rather than accumulating match state across conditions. This makes the code more efficient and easier to read.Account Retrieval Logic
MsalOAuth2TokenCache.java, now filtering accounts and credentials separately and checking for associated credentials before returning an account. This avoids unnecessary environment checks and improves clarity.Codebase Maintenance
MsalOAuth2TokenCache.javafor better maintainability.These changes collectively improve the efficiency, readability, and maintainability of the account and credential filtering logic in the token cache.