Skip to content

Commit ebad628

Browse files
rpdomerpdome
committed
wip
1 parent b412e0d commit ebad628

File tree

4 files changed

+185
-144
lines changed

4 files changed

+185
-144
lines changed

‎changelog.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
vNext
22
----------
3+
- [MINOR] Cache Active Broker In Memory (BrokerDiscoveryClient) (#2817)
34
- [MINOR] Share SharedPreferencesInMemoryCache across instances of BrokerOAuth2TokenCache
45
- [MINOR] Use SharedPreferencesInMemoryCache implementation in Broker (#2802)
56

‎common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt‎

Lines changed: 122 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,57 @@ class BrokerDiscoveryClient(private val brokerCandidates: Set<BrokerData>,
206206
}
207207
}
208208

209+
data class CachedBrokerData (
210+
val brokerData: BrokerData?
211+
)
212+
213+
@Volatile
214+
var cachedData: CachedBrokerData? = null
215+
216+
private fun validateInMemoryCacheValue(
217+
data: BrokerData,
218+
telemetryCallback: IBrokerDiscoveryClientTelemetryCallback?,
219+
): Boolean {
220+
val timeStartIsValidBroker = System.nanoTime()
221+
val isValidBroker = isValidBroker(data)
222+
telemetryCallback?.onFinishCheckingIfValidBroker(System.nanoTime() - timeStartIsValidBroker)
223+
return isValidBroker
224+
}
225+
226+
override fun getActiveBrokerWithInMemoryCache(telemetryCallback: IBrokerDiscoveryClientTelemetryCallback?): BrokerData?{
227+
cachedData?.let {
228+
if (it.brokerData == null) {
229+
return null
230+
}
231+
if (validateInMemoryCacheValue(it.brokerData, telemetryCallback)){
232+
return it.brokerData
233+
}
234+
// Even if the value is not valid, we don't touch the class variable here since we're not in the lock.
235+
// This is also an edge case.
236+
}
237+
238+
return runBlocking {
239+
val timeStartAcquiringLock = System.nanoTime()
240+
classLevelLock.withLock {
241+
telemetryCallback?.onLockAcquired(System.nanoTime() - timeStartAcquiringLock)
242+
243+
// just in case the value is already populated while waiting for the lock.
244+
cachedData?.let {
245+
if (it.brokerData == null) {
246+
return@runBlocking null
247+
}
248+
if (validateInMemoryCacheValue(it.brokerData, telemetryCallback)){
249+
return@runBlocking it.brokerData
250+
}
251+
}
252+
253+
val brokerData = getActiveBrokerAsync(shouldSkipCache = false, telemetryCallback)
254+
cachedData = CachedBrokerData(brokerData)
255+
return@runBlocking brokerData
256+
}
257+
}
258+
}
259+
209260
constructor(context: Context,
210261
components: IPlatformComponents,
211262
cache: IClientActiveBrokerCache) : this(
@@ -275,7 +326,9 @@ class BrokerDiscoveryClient(private val brokerCandidates: Set<BrokerData>,
275326

276327
override fun getActiveBroker(shouldSkipCache: Boolean): BrokerData? {
277328
return runBlocking {
278-
return@runBlocking getActiveBrokerAsync(shouldSkipCache, null)
329+
classLevelLock.withLock {
330+
return@runBlocking getActiveBrokerAsync(shouldSkipCache, null)
331+
}
279332
}
280333
}
281334

@@ -284,102 +337,88 @@ class BrokerDiscoveryClient(private val brokerCandidates: Set<BrokerData>,
284337
telemetryCallback: IBrokerDiscoveryClientTelemetryCallback
285338
): BrokerData? {
286339
return runBlocking {
287-
return@runBlocking getActiveBrokerAsync(shouldSkipCache, telemetryCallback)
340+
val timeStartAcquiringLock = System.nanoTime()
341+
classLevelLock.withLock {
342+
telemetryCallback.onLockAcquired(System.nanoTime() - timeStartAcquiringLock)
343+
return@runBlocking getActiveBrokerAsync(shouldSkipCache, telemetryCallback)
344+
}
288345
}
289346
}
290347

291348
private suspend fun getActiveBrokerAsync(shouldSkipCache:Boolean,
292349
telemetryCallback: IBrokerDiscoveryClientTelemetryCallback?): BrokerData?{
293350
val methodTag = "$TAG:getActiveBrokerAsync"
294-
295-
val timeStartAcquiringLock = System.nanoTime()
296-
classLevelLock.withLock {
297-
telemetryCallback?.onLockAcquired(System.nanoTime() - timeStartAcquiringLock)
298-
if (!shouldSkipCache) {
299-
if (cache.shouldUseAccountManager()) {
300-
telemetryCallback?.onUseAccountManager()
301-
return getActiveBrokerFromAccountManager()
351+
if (!shouldSkipCache) {
352+
if (cache.shouldUseAccountManager()) {
353+
telemetryCallback?.onUseAccountManager()
354+
return getActiveBrokerFromAccountManager()
355+
}
356+
val timeStartReadingFromCache = System.nanoTime()
357+
cache.getCachedActiveBroker()?.let {
358+
telemetryCallback?.onReadFromCache(System.nanoTime() - timeStartReadingFromCache)
359+
360+
val timeStartIsPackageInstalled = System.nanoTime()
361+
val isPackageInstalled = isPackageInstalled(it)
362+
telemetryCallback?.onFinishCheckingIfPackageIsInstalled(System.nanoTime() - timeStartIsPackageInstalled)
363+
if (!isPackageInstalled) {
364+
Logger.info(
365+
methodTag,
366+
"There is a cached broker: $it, but the app is no longer installed."
367+
)
368+
cache.clearCachedActiveBroker()
369+
return@let
302370
}
303-
val timeStartReadingFromCache = System.nanoTime()
304-
cache.getCachedActiveBroker()?.let {
305-
telemetryCallback?.onReadFromCache(System.nanoTime() - timeStartReadingFromCache)
306-
307-
val timeStartIsPackageInstalled = System.nanoTime()
308-
val isPackageInstalled = isPackageInstalled(it)
309-
telemetryCallback?.onFinishCheckingIfPackageIsInstalled(System.nanoTime() - timeStartIsPackageInstalled)
310-
if (!isPackageInstalled) {
311-
Logger.info(
312-
methodTag,
313-
"There is a cached broker: $it, but the app is no longer installed."
314-
)
315-
cache.clearCachedActiveBroker()
316-
return@let
317-
}
318-
319-
val timeStartIsValidBroker = System.nanoTime()
320-
val isValidBroker = isValidBroker(it)
321-
telemetryCallback?.onFinishCheckingIfValidBroker(System.nanoTime() - timeStartIsValidBroker)
322-
if (!isValidBroker) {
323-
Logger.info(
324-
methodTag,
325-
"Clearing cache as the installed app does not have a matching signature hash."
326-
)
327-
cache.clearCachedActiveBroker()
328-
return@let
329-
}
330-
331-
val timeStartIsSupportedByTargetedBroker = System.nanoTime()
332-
val isSupportedByTargetedBroker =
333-
ipcStrategy.isSupportedByTargetedBroker(it.packageName)
334-
telemetryCallback?.onFinishCheckingIfSupportedByTargetedBroker(System.nanoTime() - timeStartIsSupportedByTargetedBroker)
335-
if(!isSupportedByTargetedBroker){
336-
Logger.info(
337-
methodTag,
338-
"Clearing cache as the installed app does not provide any IPC mechanism to communicate to. (e.g. the broker code isn't shipped with this apk)"
339-
)
340-
cache.clearCachedActiveBroker()
341-
return@let
342-
}
343371

344-
Logger.info(methodTag, "Returning cached broker: $it")
345-
return it
372+
val timeStartIsValidBroker = System.nanoTime()
373+
val isValidBroker = isValidBroker(it)
374+
telemetryCallback?.onFinishCheckingIfValidBroker(System.nanoTime() - timeStartIsValidBroker)
375+
if (!isValidBroker) {
376+
Logger.info(
377+
methodTag,
378+
"Clearing cache as the installed app does not have a matching signature hash."
379+
)
380+
cache.clearCachedActiveBroker()
381+
return@let
346382
}
347-
}
348383

349-
val timeStartQueryFromBroker = System.nanoTime()
350-
val brokerData = queryFromBroker(
351-
brokerCandidates = brokerCandidates,
352-
ipcStrategy = ipcStrategy,
353-
isPackageInstalled = isPackageInstalled,
354-
isValidBroker = isValidBroker
355-
)
356-
telemetryCallback?.onFinishQueryingResultFromBroker(System.nanoTime() - timeStartQueryFromBroker)
357-
358-
if (brokerData != null) {
359-
cache.setCachedActiveBroker(brokerData)
360-
return brokerData
384+
Logger.info(methodTag, "Returning cached broker: $it")
385+
return it
361386
}
387+
}
362388

363-
Logger.info(
364-
methodTag,
365-
"Will skip broker discovery via IPC and fall back to AccountManager " +
366-
"for the next 60 minutes."
367-
)
368-
cache.clearCachedActiveBroker()
369-
cache.setShouldUseAccountManagerForTheNextMilliseconds(
370-
TimeUnit.MINUTES.toMillis(
371-
60
372-
)
373-
)
389+
val timeStartQueryFromBroker = System.nanoTime()
390+
val brokerData = queryFromBroker(
391+
brokerCandidates = brokerCandidates,
392+
ipcStrategy = ipcStrategy,
393+
isPackageInstalled = isPackageInstalled,
394+
isValidBroker = isValidBroker
395+
)
396+
telemetryCallback?.onFinishQueryingResultFromBroker(System.nanoTime() - timeStartQueryFromBroker)
397+
398+
if (brokerData != null) {
399+
cache.setCachedActiveBroker(brokerData)
400+
return brokerData
401+
}
374402

375-
telemetryCallback?.onUseAccountManager()
376-
val accountManagerResult = getActiveBrokerFromAccountManager()
377-
Logger.info(
378-
methodTag, "Tried getting active broker from account manager, " +
379-
"get ${accountManagerResult?.packageName}."
403+
Logger.info(
404+
methodTag,
405+
"Will skip broker discovery via IPC and fall back to AccountManager " +
406+
"for the next 60 minutes."
407+
)
408+
cache.clearCachedActiveBroker()
409+
cache.setShouldUseAccountManagerForTheNextMilliseconds(
410+
TimeUnit.MINUTES.toMillis(
411+
60
380412
)
413+
)
381414

382-
return accountManagerResult
383-
}
415+
telemetryCallback?.onUseAccountManager()
416+
val accountManagerResult = getActiveBrokerFromAccountManager()
417+
Logger.info(
418+
methodTag, "Tried getting active broker from account manager, " +
419+
"get ${accountManagerResult?.packageName}."
420+
)
421+
422+
return accountManagerResult
384423
}
385424
}

‎common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClient.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,6 @@ interface IBrokerDiscoveryClient {
6565
* **/
6666
@kotlin.jvm.Throws(ClientException::class)
6767
fun forceBrokerRediscovery(brokerCandidate: BrokerData): BrokerData
68+
69+
fun getActiveBrokerWithInMemoryCache(telemetryCallback: IBrokerDiscoveryClientTelemetryCallback?): BrokerData?
6870
}

0 commit comments

Comments
 (0)