Skip to content

Commit dd12700

Browse files
Copilotsiddhijain
Copilot
and
siddhijain
authored
Cherry-pick in-memory cache implementation for release 23.1.1 Fixes AB#3444129 (#2822)
Fixes [AB#3444129](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3444129) Cherry-picks two commits from dev to enable in-memory caching for accounts and credentials in the broker. ## Commits - **f904c60** - Use in-memory cache for accounts and credentials ([AB#3417272](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3417272), #2802) - **228db78** - Share SharedPreferencesInMemoryCache across cache instances ([AB#3428107](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3428107), #2813) ## Changes **Core Implementation** (`BrokerOAuth2TokenCache.java`) - Added static `ConcurrentHashMap` to share in-memory cache instances across `BrokerOAuth2TokenCache` instances - Reduces disk I/O and serialization overhead by reusing cache layers for the same underlying storage **Feature Flag** (`CommonFlight.java`) - Added `USE_IN_MEMORY_CACHE_FOR_ACCOUNTS_AND_CREDENTIALS` (default: false) **Telemetry** (`AttributeName.java`) - Added `in_memory_cache_used_for_accounts_and_credentials` attribute **Tests** - New `BrokerOAuth2TokenCacheWithInMemoryCacheTest.java` (1240 lines) - Extended `BrokerOAuth2TokenCacheTest.java` with cache sharing tests ## Conflicts Resolved `changelog.txt` had conflicts due to version markers in the release branch. Resolved by adding entries under vNext: ``` - [MINOR] Share SharedPreferencesInMemoryCache across instances of BrokerOAuth2TokenCache (#2813) - [MINOR] Use SharedPreferencesInMemoryCache implementation in Broker (#2802) ``` > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `dl.google.com` > - Triggering command: `/usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=1024m -XX:&#43;HeapDumpOnOutOfMemoryError -Xmx3072m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant` (dns block) > - `www.puppycrawl.com` > - Triggering command: `/opt/hostedtoolcache/CodeQL/2.23.3/x64/codeql/tools/linux64/java/bin/java /opt/hostedtoolcache/CodeQL/2.23.3/x64/codeql/tools/linux64/java/bin/java -jar /opt/hostedtoolcache/CodeQL/2.23.3/x64/codeql/xml/tools/xml-extractor.jar --fileList=/home/REDACTED/work/microsoft-authentication-library-common-for-android/.codeql-scratch/dbs/java/working/files-to-index2294403091429273239.list --sourceArchiveDir=/home/REDACTED/work/microsoft-authentication-library-common-for-android/.codeql-scratch/dbs/java/src --outputDir=/home/REDACTED/work/microsoft-authentication-library-common-for-android/.codeql-scratch/dbs/java/trap/java` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/AzureAD/microsoft-authentication-library-common-for-android/settings/copilot/coding_agent) (admins only) > > </details> <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > Cherry-pick the following commits from the dev branch to the working/release/23.1.1 branch: > > - f904c60 > - 228db78 > > Include all files and changes from the specified commits. Ensure that any conflicts are noted in the pull request description. Provide a detailed summary of what was cherry-picked in the PR body. </details> *This pull request was created as a result of the following prompt from Copilot chat.* > Cherry-pick the following commits from the dev branch to the working/release/23.1.1 branch: > > - f904c60 > - 228db78 > > Include all files and changes from the specified commits. Ensure that any conflicts are noted in the pull request description. Provide a detailed summary of what was cherry-picked in the PR body. <!-- START COPILOT CODING AGENT TIPS --> --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: siddhijain <[email protected]> Co-authored-by: Siddhi <[email protected]>
1 parent a15adf5 commit dd12700

File tree

6 files changed

+1459
-79
lines changed

6 files changed

+1459
-79
lines changed

‎changelog.txt‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
vNext
1+
Version 23.1.1-RC1
22
----------
3+
- [PATCH] Share SharedPreferencesInMemoryCache across instances of BrokerOAuth2TokenCache (#2813)
4+
- [PATCH] Use SharedPreferencesInMemoryCache implementation in Broker (#2802)
35

4-
Version 23.1.1-RC1
5-
----------
6-
76
Version 23.1.0
87
----------
98
- [MINOR] Add OpenTelemetry support for passkey operations (#2795)
@@ -1199,4 +1198,4 @@ Version 0.0.3
11991198
* Separate methods for PII/OII logging
12001199
- Initial Exception model implemented
12011200
* BaseException + Client & Service subclasses
1202-
- Substantial portions of HTTP/S networking code migrated from ADAL & MSAL to this module
1201+
- Substantial portions of HTTP/S networking code migrated from ADAL & MSAL to this module

‎common/src/test/java/com/microsoft/identity/common/BrokerOAuth2TokenCacheTest.java‎

Lines changed: 127 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,17 @@
4242
import static org.junit.Assert.assertEquals;
4343
import static org.junit.Assert.assertFalse;
4444
import static org.junit.Assert.assertNotNull;
45+
import static org.junit.Assert.assertNotSame;
4546
import static org.junit.Assert.assertNull;
47+
import static org.junit.Assert.assertSame;
4648
import static org.junit.Assert.assertTrue;
4749
import static org.junit.Assert.fail;
4850
import static org.mockito.Mockito.when;
4951

50-
import android.content.Context;
5152

53+
import androidx.annotation.NonNull;
5254
import androidx.test.core.app.ApplicationProvider;
5355

54-
import com.microsoft.identity.common.components.AndroidPlatformComponentsFactory;
5556
import com.microsoft.identity.common.components.MockPlatformComponentsFactory;
5657
import com.microsoft.identity.common.internal.platform.AndroidPlatformUtil;
5758
import com.microsoft.identity.common.java.cache.BrokerApplicationMetadata;
@@ -66,10 +67,15 @@
6667
import com.microsoft.identity.common.java.cache.SharedPreferencesAccountCredentialCache;
6768
import com.microsoft.identity.common.java.cache.AccountDeletionRecord;
6869
import com.microsoft.identity.common.java.cache.ICacheRecord;
70+
import com.microsoft.identity.common.java.cache.SharedPreferencesAccountCredentialCacheWithMemoryCache;
6971
import com.microsoft.identity.common.java.dto.AccountRecord;
7072
import com.microsoft.identity.common.java.dto.Credential;
7173
import com.microsoft.identity.common.java.dto.CredentialType;
7274
import com.microsoft.identity.common.java.exception.ClientException;
75+
import com.microsoft.identity.common.java.flighting.CommonFlight;
76+
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
77+
import com.microsoft.identity.common.java.flighting.IFlightsManager;
78+
import com.microsoft.identity.common.java.flighting.IFlightsProvider;
7379
import com.microsoft.identity.common.java.interfaces.INameValueStorage;
7480
import com.microsoft.identity.common.java.interfaces.IPlatformComponents;
7581
import com.microsoft.identity.common.java.providers.microsoft.MicrosoftAccount;
@@ -79,14 +85,15 @@
7985
import com.microsoft.identity.common.java.providers.oauth2.OAuth2TokenCache;
8086
import com.microsoft.identity.common.shadows.ShadowAndroidSdkStorageEncryptionManager;
8187

88+
import org.jetbrains.annotations.NotNull;
8289
import org.junit.After;
8390
import org.junit.Before;
8491
import org.junit.Test;
8592
import org.junit.runner.RunWith;
93+
import org.mockito.Mockito;
8694
import org.powermock.api.mockito.PowerMockito;
8795
import org.robolectric.RobolectricTestRunner;
8896
import org.robolectric.annotation.Config;
89-
import org.robolectric.shadows.ShadowSharedPreferences;
9097

9198
import java.util.ArrayList;
9299
import java.util.List;
@@ -236,6 +243,7 @@ public void tearDown() throws Exception {
236243
}
237244

238245
mApplicationMetadataCache.clear();
246+
CommonFlightsManager.INSTANCE.resetFlightsManager();
239247
}
240248

241249
private void initOtherCaches(final IPlatformComponents components) {
@@ -1241,4 +1249,120 @@ public void testClearAll() throws ClientException {
12411249
assertEquals(false, mBrokerOAuth2TokenCache.isClientIdKnownToCache(clientId));
12421250
}
12431251
}
1252+
1253+
@Test
1254+
public void testSingleCacheInstancePerStoreName_FlightEnabled() {
1255+
// Enable the flight
1256+
updateUseInMemoryCacheFlight(true);
1257+
1258+
final String storeName = "test_store_name";
1259+
final IPlatformComponents components1 = mPlatformComponents;
1260+
final IPlatformComponents components2 = mPlatformComponents;
1261+
1262+
// Call getCacheToBeUsed twice with the same storeName
1263+
final IAccountCredentialCache cache1 = BrokerOAuth2TokenCache.getCacheToBeUsed(components1, storeName);
1264+
final IAccountCredentialCache cache2 = BrokerOAuth2TokenCache.getCacheToBeUsed(components2, storeName);
1265+
1266+
// Verify both references point to the same instance
1267+
assertNotNull(cache1);
1268+
assertNotNull(cache2);
1269+
assertSame("Expected same cache instance for same storeName", cache1, cache2);
1270+
assertTrue("Cache should be of type SharedPreferencesAccountCredentialCacheWithMemoryCache",
1271+
cache1 instanceof SharedPreferencesAccountCredentialCacheWithMemoryCache);
1272+
}
1273+
1274+
@Test
1275+
public void testDifferentCacheInstancesPerStoreName_FlightEnabled() {
1276+
// Enable the flight
1277+
updateUseInMemoryCacheFlight(true);
1278+
1279+
final String storeName1 = "test_store_name_1";
1280+
final String storeName2 = "test_store_name_2";
1281+
final IPlatformComponents components = mPlatformComponents;
1282+
1283+
// Call getCacheToBeUsed with different storeNames
1284+
final IAccountCredentialCache cache1 = BrokerOAuth2TokenCache.getCacheToBeUsed(components, storeName1);
1285+
final IAccountCredentialCache cache2 = BrokerOAuth2TokenCache.getCacheToBeUsed(components, storeName2);
1286+
1287+
// Verify both are valid but different instances
1288+
assertNotNull(cache1);
1289+
assertNotNull(cache2);
1290+
assertNotSame("Expected different cache instances for different storeNames", cache1, cache2);
1291+
assertTrue("Cache should be of type SharedPreferencesAccountCredentialCacheWithMemoryCache",
1292+
cache1 instanceof SharedPreferencesAccountCredentialCacheWithMemoryCache);
1293+
assertTrue("Cache should be of type SharedPreferencesAccountCredentialCacheWithMemoryCache",
1294+
cache2 instanceof SharedPreferencesAccountCredentialCacheWithMemoryCache);
1295+
}
1296+
1297+
@Test
1298+
public void testCacheInstanceReusedAcrossMultipleBrokerTokenCaches_FlightEnabled() {
1299+
// Enable the flight
1300+
updateUseInMemoryCacheFlight(true);
1301+
1302+
final String storeName = getBrokerUidSequesteredFilename(TEST_APP_UID);
1303+
1304+
// Create multiple BrokerOAuth2TokenCache instances
1305+
final BrokerOAuth2TokenCache tokenCache1 = new BrokerOAuth2TokenCache
1306+
(mPlatformComponents,
1307+
TEST_APP_UID,
1308+
new NameValueStorageBrokerApplicationMetadataCache(mPlatformComponents));
1309+
final BrokerOAuth2TokenCache tokenCache2 = new BrokerOAuth2TokenCache(mPlatformComponents, TEST_APP_UID,
1310+
new NameValueStorageBrokerApplicationMetadataCache(mPlatformComponents));
1311+
1312+
// Get the underlying account credential caches
1313+
final IAccountCredentialCache cache1 = tokenCache1.getCacheToBeUsed(mPlatformComponents, storeName);
1314+
final IAccountCredentialCache cache2 = tokenCache2.getCacheToBeUsed(mPlatformComponents, storeName);
1315+
1316+
// Verify same instance is reused
1317+
assertNotNull(cache1);
1318+
assertNotNull(cache2);
1319+
assertSame(cache1, cache2);
1320+
}
1321+
1322+
@Test
1323+
public void testFociCacheInstanceReused_FlightEnabled() {
1324+
// Enable the flight
1325+
updateUseInMemoryCacheFlight(true);
1326+
1327+
final String fociStoreName = BROKER_FOCI_ACCOUNT_CREDENTIAL_SHARED_PREFERENCES;
1328+
1329+
// Call getCacheToBeUsed multiple times for FOCI cache
1330+
final IAccountCredentialCache fociCache1 = BrokerOAuth2TokenCache.getCacheToBeUsed(mPlatformComponents, fociStoreName);
1331+
final IAccountCredentialCache fociCache2 = BrokerOAuth2TokenCache.getCacheToBeUsed(mPlatformComponents, fociStoreName);
1332+
1333+
// Verify same FOCI cache instance is reused
1334+
assertNotNull(fociCache1);
1335+
assertNotNull(fociCache2);
1336+
assertSame(fociCache1, fociCache2);
1337+
}
1338+
1339+
private void updateUseInMemoryCacheFlight(boolean enabled) {
1340+
final IFlightsProvider mockFlightsProvider = Mockito.mock(IFlightsProvider.class);
1341+
Mockito.when(mockFlightsProvider.isFlightEnabled(CommonFlight.USE_IN_MEMORY_CACHE_FOR_ACCOUNTS_AND_CREDENTIALS))
1342+
.thenReturn(enabled);
1343+
1344+
// Create anonymous IFlightsManager
1345+
IFlightsManager anonymousFlightsManager = new IFlightsManager() {
1346+
@Override
1347+
public @NotNull IFlightsProvider getFlightsProvider(long waitForConfigsWithTimeoutInMs) {
1348+
return mockFlightsProvider;
1349+
}
1350+
@Override
1351+
public @NotNull IFlightsProvider getFlightsProviderForTenant(@NotNull String tenantId, long waitForConfigsWithTimeoutInMs) {
1352+
return mockFlightsProvider;
1353+
}
1354+
@Override
1355+
public @NotNull IFlightsProvider getFlightsProviderForTenant(@NotNull String tenantId) {
1356+
return mockFlightsProvider;
1357+
}
1358+
@NonNull
1359+
@Override
1360+
public IFlightsProvider getFlightsProvider() {
1361+
return mockFlightsProvider;
1362+
}
1363+
};
1364+
1365+
// Initialize CommonFlightsManager with the anonymous implementation
1366+
CommonFlightsManager.INSTANCE.initializeCommonFlightsManager(anonymousFlightsManager);
1367+
}
12441368
}

0 commit comments

Comments
 (0)