Add new APIs to DataStore to allow for int storage

The failure count for CTLogListUpdateFailed is an integer, not a long.
We need APIs in DataStore to handle this different data type, as
otherwise it will result in an unnecessary cast from long <-> int, which
is a lossy conversion.

Bug: 384473786
Test: atest NetworkSecurityUnitTests
Change-Id: I3b31fc9faef4ac0dec77bc5adb8235a155a4a3d1
This commit is contained in:
Bessie Jiang 2024-12-18 18:20:21 +00:00
commit 8d0c2c0b5f
4 changed files with 38 additions and 30 deletions

View file

@ -221,7 +221,7 @@ class CertificateTransparencyDownloader extends BroadcastReceiver {
mDataStore.setProperty(Config.VERSION, version);
// Reset the number of consecutive log list failure updates back to zero.
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* value= */ 0L);
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* value= */ 0);
mDataStore.store();
} else {
if (updateFailureCount()) {
@ -244,11 +244,11 @@ class CertificateTransparencyDownloader extends BroadcastReceiver {
* @return whether the failure count exceeds the threshold and should be logged.
*/
private boolean updateFailureCount() {
long failure_count = mDataStore.getPropertyLong(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L);
long new_failure_count = failure_count + 1L;
int failure_count = mDataStore.getPropertyInt(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0);
int new_failure_count = failure_count + 1;
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, new_failure_count);
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, new_failure_count);
mDataStore.store();
boolean shouldReport = new_failure_count >= Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD;

View file

@ -63,5 +63,5 @@ final class Config {
static final String URL_PUBLIC_KEY = URL_PREFIX + "log_list.pub";
// Threshold amounts
static final long LOG_LIST_UPDATE_FAILURE_THRESHOLD = 10L;
static final int LOG_LIST_UPDATE_FAILURE_THRESHOLD = 10;
}

View file

@ -64,4 +64,12 @@ class DataStore extends Properties {
Object setPropertyLong(String key, long value) {
return setProperty(key, Long.toString(value));
}
int getPropertyInt(String key, int defaultValue) {
return Optional.ofNullable(getProperty(key)).map(Integer::parseInt).orElse(defaultValue);
}
Object setPropertyInt(String key, int value) {
return setProperty(key, Integer.toString(value));
}
}

View file

@ -190,7 +190,7 @@ public class CertificateTransparencyDownloaderTest {
throws Exception {
long publicKeyId = mCertificateTransparencyDownloader.startPublicKeyDownload();
// Set the failure count to just below the threshold
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT,
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT,
Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD - 1);
setFailedDownload(
publicKeyId, // Failure cases where we give up on the download.
@ -200,8 +200,8 @@ public class CertificateTransparencyDownloaderTest {
mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
assertThat(mDataStore.getPropertyLong(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L))
assertThat(mDataStore.getPropertyInt(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0))
.isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD);
// TODO(378626065): Verify logged failure via statsd.
}
@ -211,7 +211,7 @@ public class CertificateTransparencyDownloaderTest {
throws Exception {
long publicKeyId = mCertificateTransparencyDownloader.startPublicKeyDownload();
// Set the failure count to just below the threshold
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0);
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0);
setFailedDownload(
publicKeyId, // Failure cases where we give up on the download.
DownloadManager.ERROR_INSUFFICIENT_SPACE,
@ -220,8 +220,8 @@ public class CertificateTransparencyDownloaderTest {
mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
assertThat(mDataStore.getPropertyLong(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L))
assertThat(mDataStore.getPropertyInt(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0))
.isEqualTo(1);
// TODO(378626065): Verify no failure logged via statsd.
}
@ -260,7 +260,7 @@ public class CertificateTransparencyDownloaderTest {
throws Exception {
long metadataId = mCertificateTransparencyDownloader.startMetadataDownload();
// Set the failure count to just below the threshold
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT,
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT,
Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD - 1);
setFailedDownload(
metadataId,
@ -271,8 +271,8 @@ public class CertificateTransparencyDownloaderTest {
mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
assertThat(mDataStore.getPropertyLong(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L))
assertThat(mDataStore.getPropertyInt(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0))
.isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD);
// TODO(378626065): Verify logged failure via statsd.
}
@ -282,7 +282,7 @@ public class CertificateTransparencyDownloaderTest {
throws Exception {
long metadataId = mCertificateTransparencyDownloader.startMetadataDownload();
// Set the failure count to just below the threshold
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0);
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0);
setFailedDownload(
metadataId,
// Failure cases where we give up on the download.
@ -292,8 +292,8 @@ public class CertificateTransparencyDownloaderTest {
mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
assertThat(mDataStore.getPropertyLong(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L))
assertThat(mDataStore.getPropertyInt(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0))
.isEqualTo(1);
// TODO(378626065): Verify no failure logged via statsd.
}
@ -342,7 +342,7 @@ public class CertificateTransparencyDownloaderTest {
throws Exception {
long contentId = mCertificateTransparencyDownloader.startContentDownload();
// Set the failure count to just below the threshold
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT,
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT,
Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD - 1);
setFailedDownload(
contentId,
@ -353,8 +353,8 @@ public class CertificateTransparencyDownloaderTest {
mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
assertThat(mDataStore.getPropertyLong(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L))
assertThat(mDataStore.getPropertyInt(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0))
.isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD);
// TODO(378626065): Verify logged failure via statsd.
}
@ -364,7 +364,7 @@ public class CertificateTransparencyDownloaderTest {
throws Exception {
long contentId = mCertificateTransparencyDownloader.startContentDownload();
// Set the failure count to just below the threshold
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0);
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0);
setFailedDownload(
contentId,
// Failure cases where we give up on the download.
@ -374,8 +374,8 @@ public class CertificateTransparencyDownloaderTest {
mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
assertThat(mDataStore.getPropertyLong(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L))
assertThat(mDataStore.getPropertyInt(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0))
.isEqualTo(1);
// TODO(378626065): Verify no failure logged via statsd.
}
@ -413,7 +413,7 @@ public class CertificateTransparencyDownloaderTest {
long contentId = mCertificateTransparencyDownloader.startContentDownload();
setSuccessfulDownload(contentId, logListFile);
// Set the failure count to just below the threshold
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT,
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT,
Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD - 1);
when(mCertificateTransparencyInstaller.install(
eq(Config.COMPATIBILITY_VERSION), any(), anyString()))
@ -422,8 +422,8 @@ public class CertificateTransparencyDownloaderTest {
mCertificateTransparencyDownloader.onReceive(
mContext, makeDownloadCompleteIntent(contentId));
assertThat(mDataStore.getPropertyLong(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L))
assertThat(mDataStore.getPropertyInt(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0))
.isEqualTo(Config.LOG_LIST_UPDATE_FAILURE_THRESHOLD);
// TODO(378626065): Verify logged failure via statsd.
}
@ -440,7 +440,7 @@ public class CertificateTransparencyDownloaderTest {
long contentId = mCertificateTransparencyDownloader.startContentDownload();
setSuccessfulDownload(contentId, logListFile);
// Set the failure count to just below the threshold
mDataStore.setPropertyLong(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0);
mDataStore.setPropertyInt(Config.LOG_LIST_UPDATE_FAILURE_COUNT, 0);
when(mCertificateTransparencyInstaller.install(
eq(Config.COMPATIBILITY_VERSION), any(), anyString()))
.thenReturn(false);
@ -448,8 +448,8 @@ public class CertificateTransparencyDownloaderTest {
mCertificateTransparencyDownloader.onReceive(
mContext, makeDownloadCompleteIntent(contentId));
assertThat(mDataStore.getPropertyLong(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0L))
assertThat(mDataStore.getPropertyInt(
Config.LOG_LIST_UPDATE_FAILURE_COUNT, /* defaultValue= */ 0))
.isEqualTo(1);
// TODO(378626065): Verify no failure logged via statsd.
}