diff options
3 files changed, 13 insertions, 209 deletions
diff --git a/service/java/com/android/safetycenter/SafetyCenterDataTracker.java b/service/java/com/android/safetycenter/SafetyCenterDataTracker.java index 4681b4799..c83734b3c 100644 --- a/service/java/com/android/safetycenter/SafetyCenterDataTracker.java +++ b/service/java/com/android/safetycenter/SafetyCenterDataTracker.java @@ -69,7 +69,6 @@ import com.android.safetycenter.persistence.PersistedSafetyCenterIssue; import com.android.safetycenter.resources.SafetyCenterResourcesContext; import java.io.PrintWriter; -import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Comparator; @@ -359,13 +358,11 @@ final class SafetyCenterDataTracker { */ void dismissSafetyCenterIssue(@NonNull SafetyCenterIssueKey safetyCenterIssueKey) { IssueData issueData = mSafetyCenterIssueCache.get(safetyCenterIssueKey); - if (issueData == null) { - Log.w(TAG, "Issue missing when writing to cache for key: " + safetyCenterIssueKey); - return; + if (issueData != null) { + issueData.setDismissedAt(Instant.now()); + issueData.setDismissCount(issueData.getDismissCount() + 1); + mSafetyCenterIssueCacheDirty = true; } - issueData.setDismissedAt(Instant.now()); - issueData.setDismissCount(issueData.getDismissCount() + 1); - mSafetyCenterIssueCacheDirty = true; } /** @@ -376,6 +373,10 @@ final class SafetyCenterDataTracker { */ @Nullable SafetySourceIssue getSafetySourceIssue(@NonNull SafetyCenterIssueKey safetyCenterIssueKey) { + if (isDismissed(safetyCenterIssueKey)) { + return null; + } + SafetySourceKey key = SafetySourceKey.of( safetyCenterIssueKey.getSafetySourceId(), safetyCenterIssueKey.getUserId()); @@ -385,24 +386,15 @@ final class SafetyCenterDataTracker { } List<SafetySourceIssue> safetySourceIssues = safetySourceData.getIssues(); - SafetySourceIssue targetIssue = null; for (int i = 0; i < safetySourceIssues.size(); i++) { SafetySourceIssue safetySourceIssue = safetySourceIssues.get(i); if (safetyCenterIssueKey.getSafetySourceIssueId().equals(safetySourceIssue.getId())) { - targetIssue = safetySourceIssue; - break; + return safetySourceIssue; } } - if (targetIssue == null) { - return null; - } - - if (isDismissed(safetyCenterIssueKey, targetIssue.getSeverityLevel())) { - return null; - } - return targetIssue; + return null; } /** @@ -568,37 +560,12 @@ final class SafetyCenterDataTracker { } } - private boolean isDismissed( - @NonNull SafetyCenterIssueKey safetyCenterIssueKey, - @SafetySourceData.SeverityLevel int safetySourceIssueSeverityLevel) { + private boolean isDismissed(@NonNull SafetyCenterIssueKey safetyCenterIssueKey) { IssueData issueData = mSafetyCenterIssueCache.get(safetyCenterIssueKey); if (issueData == null) { - Log.w(TAG, "Issue missing when reading from cache for key: " + safetyCenterIssueKey); - return false; - } - - Instant dismissedAt = issueData.getDismissedAt(); - boolean hasNeverBeenDismissed = dismissedAt == null; - if (hasNeverBeenDismissed) { - return false; - } - - long maxCount = SafetyCenterFlags.getResurfaceIssueMaxCount(safetySourceIssueSeverityLevel); - Duration delay = SafetyCenterFlags.getResurfaceIssueDelay(safetySourceIssueSeverityLevel); - - boolean hasAlreadyResurfacedTheMaxAllowedNumberOfTimes = - issueData.getDismissCount() > maxCount; - if (hasAlreadyResurfacedTheMaxAllowedNumberOfTimes) { - return true; - } - - Duration timeSinceLastDismissal = Duration.between(dismissedAt, Instant.now()); - boolean isTimeToResurface = timeSinceLastDismissal.compareTo(delay) >= 0; - if (isTimeToResurface) { return false; } - - return true; + return issueData.getDismissedAt() != null; } private boolean isInFlight(@NonNull SafetyCenterIssueActionId safetyCenterIssueActionId) { @@ -975,9 +942,7 @@ final class SafetyCenterDataTracker { .setIssueTypeId(safetySourceIssue.getIssueTypeId()) .build(); - if (isDismissed( - safetyCenterIssueId.getSafetyCenterIssueKey(), - safetySourceIssue.getSeverityLevel())) { + if (isDismissed(safetyCenterIssueId.getSafetyCenterIssueKey())) { return null; } diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterManagerTest.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterManagerTest.kt index 2a44bd1d1..ac92a0bed 100644 --- a/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterManagerTest.kt +++ b/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterManagerTest.kt @@ -60,7 +60,6 @@ import android.safetycenter.SafetySourceErrorDetails import android.safetycenter.SafetySourceIssue import android.safetycenter.cts.testing.Coroutines.TIMEOUT_LONG import android.safetycenter.cts.testing.Coroutines.TIMEOUT_SHORT -import android.safetycenter.cts.testing.Coroutines.waitForWithTimeout import android.safetycenter.cts.testing.FakeExecutor import android.safetycenter.cts.testing.SafetyCenterApisWithShellPermissions.addOnSafetyCenterDataChangedListenerWithPermission import android.safetycenter.cts.testing.SafetyCenterApisWithShellPermissions.clearAllSafetySourceDataForTestsWithPermission @@ -139,10 +138,8 @@ import androidx.test.core.app.ApplicationProvider.getApplicationContext import androidx.test.ext.junit.runners.AndroidJUnit4 import com.android.compatibility.common.preconditions.ScreenLockHelper import com.android.safetycenter.resources.SafetyCenterResourcesContext -import com.google.common.base.Preconditions.checkState import com.google.common.truth.Truth.assertThat import com.google.common.util.concurrent.MoreExecutors.directExecutor -import java.time.Duration import java.util.Locale import kotlin.test.assertFailsWith import kotlinx.coroutines.TimeoutCancellationException @@ -2499,132 +2496,6 @@ class SafetyCenterManagerTest { } @Test - fun dismissSafetyCenterIssue_withEmptyMaxCountMap_doesNotResurface() { - SafetyCenterFlags.resurfaceIssueMaxCounts = emptyMap() - SafetyCenterFlags.resurfaceIssueDelays = mapOf(SEVERITY_LEVEL_INFORMATION to Duration.ZERO) - safetyCenterCtsHelper.setConfig(SINGLE_SOURCE_CONFIG) - safetyCenterCtsHelper.setData(SINGLE_SOURCE_ID, safetySourceCtsData.informationWithIssue) - val apiSafetyCenterData = safetyCenterManager.getSafetyCenterDataWithPermission() - checkState(apiSafetyCenterData == safetyCenterDataOkOneAlert) - - safetyCenterManager.dismissSafetyCenterIssueWithPermission( - SafetyCenterCtsData.issueId(SINGLE_SOURCE_ID, INFORMATION_ISSUE_ID)) - - assertFailsWith(TimeoutCancellationException::class) { - waitForWithTimeout(timeout = TIMEOUT_SHORT) { - val hasResurfaced = - safetyCenterManager.getSafetyCenterDataWithPermission() != safetyCenterDataOk - hasResurfaced - } - } - } - - @Test - fun dismissSafetyCenterIssue_withZeroMaxCount_doesNotResurface() { - SafetyCenterFlags.resurfaceIssueMaxCounts = - mapOf( - SEVERITY_LEVEL_INFORMATION to 0L, - SEVERITY_LEVEL_RECOMMENDATION to 99L, - SEVERITY_LEVEL_CRITICAL_WARNING to 99L) - SafetyCenterFlags.resurfaceIssueDelays = - mapOf( - SEVERITY_LEVEL_INFORMATION to Duration.ZERO, - SEVERITY_LEVEL_RECOMMENDATION to Duration.ZERO, - SEVERITY_LEVEL_CRITICAL_WARNING to Duration.ZERO) - safetyCenterCtsHelper.setConfig(SINGLE_SOURCE_CONFIG) - safetyCenterCtsHelper.setData(SINGLE_SOURCE_ID, safetySourceCtsData.informationWithIssue) - val apiSafetyCenterData = safetyCenterManager.getSafetyCenterDataWithPermission() - checkState(apiSafetyCenterData == safetyCenterDataOkOneAlert) - - safetyCenterManager.dismissSafetyCenterIssueWithPermission( - SafetyCenterCtsData.issueId(SINGLE_SOURCE_ID, INFORMATION_ISSUE_ID)) - - assertFailsWith(TimeoutCancellationException::class) { - waitForWithTimeout(timeout = TIMEOUT_SHORT) { - val hasResurfaced = - safetyCenterManager.getSafetyCenterDataWithPermission() != safetyCenterDataOk - hasResurfaced - } - } - } - - @Test - fun dismissSafetyCenterIssue_withTwoMaxCount_resurfacesTwice() { - SafetyCenterFlags.resurfaceIssueMaxCounts = - mapOf( - SEVERITY_LEVEL_INFORMATION to 0L, - SEVERITY_LEVEL_RECOMMENDATION to 0L, - SEVERITY_LEVEL_CRITICAL_WARNING to 2L) - SafetyCenterFlags.resurfaceIssueDelays = - mapOf( - SEVERITY_LEVEL_INFORMATION to RESURFACE_DELAY, - SEVERITY_LEVEL_RECOMMENDATION to RESURFACE_DELAY, - SEVERITY_LEVEL_CRITICAL_WARNING to Duration.ZERO) - safetyCenterCtsHelper.setConfig(SINGLE_SOURCE_CONFIG) - safetyCenterCtsHelper.setData( - SINGLE_SOURCE_ID, safetySourceCtsData.criticalWithResolvingIssue) - val apiSafetyCenterData = safetyCenterManager.getSafetyCenterDataWithPermission() - checkState(apiSafetyCenterData == safetyCenterDataCriticalOneAlert) - - safetyCenterManager.dismissSafetyCenterIssueWithPermission( - SafetyCenterCtsData.issueId(SINGLE_SOURCE_ID, CRITICAL_ISSUE_ID)) - val apiSafetyCenterDataResurface = safetyCenterManager.getSafetyCenterDataWithPermission() - checkState(apiSafetyCenterDataResurface == safetyCenterDataCriticalOneAlert) - safetyCenterManager.dismissSafetyCenterIssueWithPermission( - SafetyCenterCtsData.issueId(SINGLE_SOURCE_ID, CRITICAL_ISSUE_ID)) - val apiSafetyCenterDataResurfaceAgain = - safetyCenterManager.getSafetyCenterDataWithPermission() - checkState(apiSafetyCenterDataResurfaceAgain == safetyCenterDataCriticalOneAlert) - safetyCenterManager.dismissSafetyCenterIssueWithPermission( - SafetyCenterCtsData.issueId(SINGLE_SOURCE_ID, CRITICAL_ISSUE_ID)) - - assertFailsWith(TimeoutCancellationException::class) { - waitForWithTimeout(timeout = TIMEOUT_SHORT) { - val hasResurfaced = - safetyCenterManager.getSafetyCenterDataWithPermission() != - safetyCenterDataOkReviewCriticalEntry - hasResurfaced - } - } - } - - @Test - fun dismissSafetyCenterIssue_withNonZeroMaxCountAndNonZeroDelay_resurfacesAfterDelay() { - // We cannot rely on a listener in this test to assert on the API content at all times! - // The listener will not receive an update when a dismissed issue resurfaces, and it will - // not receive an update after subsequent dismissals because as far as the listener cache is - // concerned the dismissed issue never resurfaced. This is working as intended. - SafetyCenterFlags.resurfaceIssueMaxCounts = - mapOf( - SEVERITY_LEVEL_INFORMATION to 0L, - SEVERITY_LEVEL_RECOMMENDATION to 99L, - SEVERITY_LEVEL_CRITICAL_WARNING to 0L) - SafetyCenterFlags.resurfaceIssueDelays = - mapOf( - SEVERITY_LEVEL_INFORMATION to Duration.ZERO, - SEVERITY_LEVEL_RECOMMENDATION to RESURFACE_DELAY, - SEVERITY_LEVEL_CRITICAL_WARNING to Duration.ZERO) - safetyCenterCtsHelper.setConfig(SINGLE_SOURCE_CONFIG) - safetyCenterCtsHelper.setData(SINGLE_SOURCE_ID, safetySourceCtsData.recommendationWithIssue) - val apiSafetyCenterData = safetyCenterManager.getSafetyCenterDataWithPermission() - checkState(apiSafetyCenterData == safetyCenterDataRecommendationOneAlert) - val listener = safetyCenterCtsHelper.addListener() - - safetyCenterManager.dismissSafetyCenterIssueWithPermission( - SafetyCenterCtsData.issueId(SINGLE_SOURCE_ID, RECOMMENDATION_ISSUE_ID)) - - val safetyCenterDataAfterDismissal = listener.receiveSafetyCenterData() - assertThat(safetyCenterDataAfterDismissal) - .isEqualTo(safetyCenterDataOkReviewRecommendationEntry) - waitForWithTimeout(timeout = RESURFACE_TIMEOUT, checkPeriod = RESURFACE_CHECK) { - val hasResurfacedExactly = - safetyCenterManager.getSafetyCenterDataWithPermission() == - safetyCenterDataRecommendationOneAlert - hasResurfacedExactly - } - } - - @Test fun dismissSafetyCenterIssue_withFlagDisabled_doesntCallListenerOrDismiss() { safetyCenterCtsHelper.setConfig(SINGLE_SOURCE_CONFIG) safetyCenterCtsHelper.setData( @@ -3266,15 +3137,4 @@ class SafetyCenterManagerTest { arguments["count"] = count return messageFormat.format(arguments) } - - companion object { - private val RESURFACE_DELAY = Duration.ofMillis(500) - // Wait 1.5 times the RESURFACE_DELAY before asserting whether an issue has or has not - // resurfaced. Use a constant additive error buffer if we increase the delay considerably. - private val RESURFACE_TIMEOUT = RESURFACE_DELAY.multipliedBy(3).dividedBy(2) - // Check more than once during a RESURFACE_DELAY before asserting whether an issue has or - // has not resurfaced. Use a different check logic (focused at the expected resurface time) - // if we increase the delay considerably. - private val RESURFACE_CHECK = RESURFACE_DELAY.dividedBy(4) - } } diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/Coroutines.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/Coroutines.kt index af916a950..6325dbcbb 100644 --- a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/Coroutines.kt +++ b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/Coroutines.kt @@ -17,12 +17,10 @@ package android.safetycenter.cts.testing import java.time.Duration -import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout /** A class that facilitates interacting with coroutines. */ -// TODO(b/228823159) Consolidate with other Coroutines helper functions object Coroutines { /** Behaves in the same way as [runBlocking], but with a timeout. */ @@ -31,25 +29,6 @@ object Coroutines { withTimeout(timeout.toMillis()) { block() } } - /** Check a condition using coroutines with a timeout. */ - fun waitForWithTimeout( - timeout: Duration = TIMEOUT_LONG, - checkPeriod: Duration = CHECK_PERIOD, - condition: () -> Boolean - ) { - runBlockingWithTimeout(timeout) { waitFor(checkPeriod, condition) } - } - - /** Check a condition using coroutines. */ - private suspend fun waitFor(checkPeriod: Duration = CHECK_PERIOD, condition: () -> Boolean) { - while (!condition()) { - delay(checkPeriod.toMillis()) - } - } - - /** A medium period, to be used for conditions that are expected to change. */ - private val CHECK_PERIOD = Duration.ofMillis(250) - /** A long timeout, to be used for actions that are expected to complete. */ val TIMEOUT_LONG: Duration = Duration.ofSeconds(15) |