Skip to content

Commit 7afc0e2

Browse files
lfkellogglfkellogg
committed
Migrate executors in NewReleaseFetcher (#4534)
1 parent d23a82d commit 7afc0e2

File tree

5 files changed

+100
-47
lines changed

5 files changed

+100
-47
lines changed

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,9 @@ private FirebaseAppDistribution buildFirebaseAppDistribution(
121121
SignInStorage signInStorage = new SignInStorage(context, backgroundExecutor);
122122
FirebaseAppDistributionLifecycleNotifier lifecycleNotifier =
123123
FirebaseAppDistributionLifecycleNotifier.getInstance(uiThreadExecutor);
124-
ReleaseIdentifier releaseIdentifier = new ReleaseIdentifier(firebaseApp, testerApiClient);
124+
ReleaseIdentifier releaseIdentifier =
125+
new ReleaseIdentifier(
126+
firebaseApp, testerApiClient, backgroundExecutor, lightweightExecutor);
125127
FirebaseAppDistribution appDistribution =
126128
new FirebaseAppDistributionImpl(
127129
firebaseApp,

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/NewReleaseFetcher.java

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,31 +62,44 @@ public Task<AppDistributionReleaseInternal> checkForNewRelease() {
6262
.fetchNewRelease()
6363
.onSuccessTask(
6464
lightweightExecutor,
65-
release -> Tasks.forResult(isNewerRelease(release) ? release : null)));
65+
release ->
66+
isNewerRelease(release)
67+
.onSuccessTask(
68+
lightweightExecutor,
69+
isNewer -> Tasks.forResult(isNewer ? release : null))));
6670
}
6771

68-
private boolean isNewerRelease(AppDistributionReleaseInternal retrievedNewRelease)
72+
private Task<Boolean> isNewerRelease(AppDistributionReleaseInternal retrievedNewRelease)
6973
throws FirebaseAppDistributionException {
7074
if (retrievedNewRelease == null) {
7175
LogWrapper.v(TAG, "Tester does not have access to any releases");
72-
return false;
76+
return Tasks.forResult(false);
7377
}
7478

7579
long newReleaseBuildVersion = parseBuildVersion(retrievedNewRelease.getBuildVersion());
7680

7781
if (isOlderBuildVersion(newReleaseBuildVersion)) {
7882
LogWrapper.v(TAG, "New release has lower version code than current release");
79-
return false;
83+
return Tasks.forResult(false);
8084
}
8185

8286
if (isNewerBuildVersion(newReleaseBuildVersion)
83-
|| !isSameAsInstalledRelease(retrievedNewRelease)
8487
|| hasDifferentAppVersionName(retrievedNewRelease)) {
85-
return true;
86-
} else {
87-
LogWrapper.v(TAG, "New release is older or is currently installed");
88-
return false;
88+
return Tasks.forResult(true);
8989
}
90+
91+
return hasSameBinaryIdentifier(retrievedNewRelease)
92+
.onSuccessTask(
93+
lightweightExecutor,
94+
hasSameBinaryIdentifier -> {
95+
if (hasSameBinaryIdentifier) {
96+
// The release has the same app version name and binary identifier, and has either
97+
// an equal or older build version
98+
LogWrapper.v(TAG, "New release is older or is currently installed");
99+
return Tasks.forResult(false);
100+
}
101+
return Tasks.forResult(true);
102+
});
90103
}
91104

92105
private long parseBuildVersion(String buildVersion) throws FirebaseAppDistributionException {
@@ -114,12 +127,15 @@ private boolean hasDifferentAppVersionName(AppDistributionReleaseInternal newRel
114127
}
115128

116129
@VisibleForTesting
117-
boolean isSameAsInstalledRelease(AppDistributionReleaseInternal newRelease)
130+
Task<Boolean> hasSameBinaryIdentifier(AppDistributionReleaseInternal newRelease)
118131
throws FirebaseAppDistributionException {
119132
if (newRelease.getBinaryType().equals(BinaryType.APK)) {
120133
return hasSameHashAsInstalledRelease(newRelease);
121134
}
135+
return Tasks.forResult(hasSameIasArtifactId(newRelease));
136+
}
122137

138+
boolean hasSameIasArtifactId(AppDistributionReleaseInternal newRelease) {
123139
if (newRelease.getIasArtifactId() == null || newRelease.getIasArtifactId().isEmpty()) {
124140
LogWrapper.w(TAG, "AAB release missing IAS Artifact ID. Assuming new release is different.");
125141
return false;
@@ -146,12 +162,17 @@ private String getInstalledAppVersionName(Context context)
146162
return getPackageInfo(context).versionName;
147163
}
148164

149-
private boolean hasSameHashAsInstalledRelease(AppDistributionReleaseInternal newRelease)
165+
private Task<Boolean> hasSameHashAsInstalledRelease(AppDistributionReleaseInternal newRelease)
150166
throws FirebaseAppDistributionException {
151167
if (newRelease.getApkHash().isEmpty()) {
152-
throw new FirebaseAppDistributionException(
153-
"Missing APK hash from new release", Status.UNKNOWN);
168+
return Tasks.forException(
169+
new FirebaseAppDistributionException(
170+
"Missing APK hash from new release", Status.UNKNOWN));
154171
}
155-
return releaseIdentifier.extractApkHash().equals(newRelease.getApkHash());
172+
return releaseIdentifier
173+
.extractApkHash()
174+
.onSuccessTask(
175+
lightweightExecutor,
176+
apkHash -> Tasks.forResult(apkHash.equals(newRelease.getApkHash())));
156177
}
157178
}

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/ReleaseIdentifier.java

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package com.google.firebase.appdistribution.impl;
1616

1717
import static com.google.firebase.appdistribution.impl.PackageInfoUtils.getPackageInfoWithMetadata;
18+
import static com.google.firebase.appdistribution.impl.TaskUtils.runAsyncInTask;
1819

1920
import android.content.pm.PackageInfo;
2021
import androidx.annotation.NonNull;
@@ -23,6 +24,8 @@
2324
import com.google.android.gms.tasks.Task;
2425
import com.google.android.gms.tasks.Tasks;
2526
import com.google.firebase.FirebaseApp;
27+
import com.google.firebase.annotations.concurrent.Background;
28+
import com.google.firebase.annotations.concurrent.Lightweight;
2629
import com.google.firebase.appdistribution.FirebaseAppDistributionException;
2730
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
2831
import java.io.File;
@@ -36,6 +39,7 @@
3639
import java.util.Locale;
3740
import java.util.concurrent.ConcurrentHashMap;
3841
import java.util.concurrent.ConcurrentMap;
42+
import java.util.concurrent.Executor;
3943
import java.util.zip.ZipEntry;
4044
import java.util.zip.ZipFile;
4145

@@ -51,10 +55,18 @@ class ReleaseIdentifier {
5155
private final FirebaseApp firebaseApp;
5256
private final FirebaseAppDistributionTesterApiClient testerApiClient;
5357

58+
@Background private final Executor backgroundExecutor;
59+
@Lightweight private final Executor lightweightExecutor;
60+
5461
ReleaseIdentifier(
55-
FirebaseApp firebaseApp, FirebaseAppDistributionTesterApiClient testerApiClient) {
62+
FirebaseApp firebaseApp,
63+
FirebaseAppDistributionTesterApiClient testerApiClient,
64+
@Background Executor backgroundExecutor,
65+
@Lightweight Executor lightweightExecutor) {
5666
this.firebaseApp = firebaseApp;
5767
this.testerApiClient = testerApiClient;
68+
this.backgroundExecutor = backgroundExecutor;
69+
this.lightweightExecutor = lightweightExecutor;
5870
}
5971

6072
/**
@@ -82,13 +94,8 @@ Task<String> identifyRelease() {
8294
}
8395

8496
// If we can't find an IAS artifact ID, we assume the installed release is an APK
85-
String apkHash;
86-
try {
87-
apkHash = extractApkHash();
88-
} catch (FirebaseAppDistributionException e) {
89-
return Tasks.forException(e);
90-
}
91-
return testerApiClient.findReleaseUsingApkHash(apkHash);
97+
return extractApkHash()
98+
.onSuccessTask(lightweightExecutor, testerApiClient::findReleaseUsingApkHash);
9299
}
93100

94101
/**
@@ -111,15 +118,19 @@ String extractInternalAppSharingArtifactId() throws FirebaseAppDistributionExcep
111118
*
112119
* <p>The result is stored in an in-memory cache to avoid computing it repeatedly.
113120
*/
114-
String extractApkHash() throws FirebaseAppDistributionException {
115-
PackageInfo metadataPackageInfo =
116-
getPackageInfoWithMetadata(firebaseApp.getApplicationContext());
117-
String installedReleaseApkHash = extractApkHash(metadataPackageInfo);
118-
if (installedReleaseApkHash == null || installedReleaseApkHash.isEmpty()) {
119-
throw new FirebaseAppDistributionException(
120-
"Could not calculate hash of installed APK", Status.UNKNOWN);
121-
}
122-
return installedReleaseApkHash;
121+
Task<String> extractApkHash() {
122+
return runAsyncInTask(
123+
backgroundExecutor,
124+
() -> {
125+
PackageInfo metadataPackageInfo =
126+
getPackageInfoWithMetadata(firebaseApp.getApplicationContext());
127+
String installedReleaseApkHash = extractApkHash(metadataPackageInfo);
128+
if (installedReleaseApkHash == null || installedReleaseApkHash.isEmpty()) {
129+
throw new FirebaseAppDistributionException(
130+
"Could not calculate hash of installed APK", Status.UNKNOWN);
131+
}
132+
return installedReleaseApkHash;
133+
});
123134
}
124135

125136
private String extractApkHash(PackageInfo packageInfo) {

‎firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/NewReleaseFetcherTest.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static android.os.Looper.getMainLooper;
1818
import static com.google.common.truth.Truth.assertThat;
1919
import static com.google.firebase.appdistribution.BinaryType.APK;
20+
import static com.google.firebase.appdistribution.impl.TestUtils.awaitAsyncOperations;
2021
import static com.google.firebase.appdistribution.impl.TestUtils.awaitTask;
2122
import static com.google.firebase.appdistribution.impl.TestUtils.awaitTaskFailure;
2223
import static org.junit.Assert.assertEquals;
@@ -38,7 +39,7 @@
3839
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
3940
import com.google.firebase.concurrent.TestOnlyExecutors;
4041
import java.util.concurrent.ExecutionException;
41-
import java.util.concurrent.Executor;
42+
import java.util.concurrent.ExecutorService;
4243
import org.junit.Before;
4344
import org.junit.Test;
4445
import org.junit.runner.RunWith;
@@ -64,7 +65,7 @@ public class NewReleaseFetcherTest {
6465
@Mock private FirebaseAppDistributionTesterApiClient mockFirebaseAppDistributionTesterApiClient;
6566
@Mock private ReleaseIdentifier mockReleaseIdentifier;
6667

67-
@Lightweight Executor lightweightExecutor = TestOnlyExecutors.lite();
68+
@Lightweight ExecutorService lightweightExecutor = TestOnlyExecutors.lite();
6869

6970
@Before
7071
public void setup() throws FirebaseAppDistributionException {
@@ -81,7 +82,7 @@ public void setup() throws FirebaseAppDistributionException {
8182
packageInfo.versionName = "1.0";
8283
shadowPackageManager.installPackage(packageInfo);
8384

84-
when(mockReleaseIdentifier.extractApkHash()).thenReturn(CURRENT_APK_HASH);
85+
when(mockReleaseIdentifier.extractApkHash()).thenReturn(Tasks.forResult(CURRENT_APK_HASH));
8586
when(mockReleaseIdentifier.extractInternalAppSharingArtifactId())
8687
.thenReturn(TEST_IAS_ARTIFACT_ID);
8788

@@ -147,6 +148,7 @@ public void checkForNewRelease_whenNewReleaseIsSameRelease_returnsNull()
147148
.thenReturn(Tasks.forResult(getTestInstalledRelease().build()));
148149

149150
Task<AppDistributionReleaseInternal> releaseTask = newReleaseFetcher.checkForNewRelease();
151+
awaitAsyncOperations(lightweightExecutor);
150152

151153
assertThat(awaitTask(releaseTask)).isNull();
152154
}

‎firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/ReleaseIdentifierTest.java

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package com.google.firebase.appdistribution.impl;
1616

1717
import static com.google.common.truth.Truth.assertThat;
18+
import static com.google.firebase.appdistribution.impl.TestUtils.awaitTask;
1819
import static org.junit.Assert.assertThrows;
1920
import static org.mockito.ArgumentMatchers.any;
2021
import static org.mockito.Mockito.doReturn;
@@ -33,7 +34,12 @@
3334
import com.google.android.gms.tasks.Tasks;
3435
import com.google.firebase.FirebaseApp;
3536
import com.google.firebase.FirebaseOptions;
37+
import com.google.firebase.annotations.concurrent.Background;
38+
import com.google.firebase.annotations.concurrent.Lightweight;
3639
import com.google.firebase.appdistribution.FirebaseAppDistributionException;
40+
import com.google.firebase.concurrent.TestOnlyExecutors;
41+
import java.util.concurrent.ExecutionException;
42+
import java.util.concurrent.Executor;
3743
import org.junit.Before;
3844
import org.junit.Test;
3945
import org.junit.runner.RunWith;
@@ -56,6 +62,9 @@ public class ReleaseIdentifierTest {
5662
private static final String CURRENT_APK_HASH = "currentApkHash";
5763
private static final long INSTALLED_VERSION_CODE = 1;
5864

65+
@Background private final Executor backgroundExecutor = TestOnlyExecutors.background();
66+
@Lightweight private final Executor lightweightExecutor = TestOnlyExecutors.lite();
67+
5968
private FirebaseApp firebaseApp;
6069
private ShadowPackageManager shadowPackageManager;
6170
@Mock private FirebaseAppDistributionTesterApiClient mockTesterApiClient;
@@ -79,44 +88,52 @@ public void setup() {
7988

8089
shadowPackageManager =
8190
shadowOf(ApplicationProvider.getApplicationContext().getPackageManager());
82-
releaseIdentifier = Mockito.spy(new ReleaseIdentifier(firebaseApp, mockTesterApiClient));
91+
releaseIdentifier =
92+
Mockito.spy(
93+
new ReleaseIdentifier(
94+
firebaseApp, mockTesterApiClient, backgroundExecutor, lightweightExecutor));
8395
}
8496

8597
@Test
86-
public void identifyRelease_apk_returnsReleaseName() {
98+
public void identifyRelease_apk_returnsReleaseName()
99+
throws FirebaseAppDistributionException, ExecutionException, InterruptedException {
87100
installTestApk();
88101
doReturn(CURRENT_APK_HASH).when(releaseIdentifier).calculateApkHash(any());
89102
when(mockTesterApiClient.findReleaseUsingApkHash(CURRENT_APK_HASH))
90103
.thenReturn(Tasks.forResult(TEST_RELEASE_NAME));
91104

92105
Task<String> task = releaseIdentifier.identifyRelease();
93106

94-
assertThat(task.isSuccessful()).isTrue();
95-
assertThat(task.getResult()).isEqualTo(TEST_RELEASE_NAME);
107+
assertThat(awaitTask(task)).isEqualTo(TEST_RELEASE_NAME);
96108
}
97109

98110
@Test
99-
public void identifyRelease_aab_returnsReleaseName() {
111+
public void identifyRelease_aab_returnsReleaseName()
112+
throws FirebaseAppDistributionException, ExecutionException, InterruptedException {
100113
installTestAab();
101114
when(mockTesterApiClient.findReleaseUsingIasArtifactId(TEST_IAS_ARTIFACT_ID))
102115
.thenReturn(Tasks.forResult(TEST_RELEASE_NAME));
103116

104117
Task<String> task = releaseIdentifier.identifyRelease();
105118

106-
assertThat(task.isSuccessful()).isTrue();
107-
assertThat(task.getResult()).isEqualTo(TEST_RELEASE_NAME);
119+
assertThat(awaitTask(task)).isEqualTo(TEST_RELEASE_NAME);
108120
}
109121

110122
@Test
111-
public void extractApkHash_returnsAHash() throws FirebaseAppDistributionException {
112-
assertThat(releaseIdentifier.extractApkHash().matches("^[0-9a-fA-F]+$")).isTrue();
123+
public void extractApkHash_returnsAHash()
124+
throws FirebaseAppDistributionException, ExecutionException, InterruptedException {
125+
Task<String> task = releaseIdentifier.extractApkHash();
126+
127+
assertThat(awaitTask(task).matches("^[0-9a-fA-F]+$")).isTrue();
113128
}
114129

115130
@Test
116131
public void extractApkHash_ifKeyInCachedApkHashes_doesNotRecalculateZipHash()
117-
throws FirebaseAppDistributionException {
118-
releaseIdentifier.extractApkHash();
119-
releaseIdentifier.extractApkHash();
132+
throws FirebaseAppDistributionException, ExecutionException, InterruptedException {
133+
Task<String> task1 = releaseIdentifier.extractApkHash();
134+
awaitTask(task1);
135+
Task<String> task2 = releaseIdentifier.extractApkHash();
136+
awaitTask(task2);
120137

121138
// asserts that that calculateApkHash is only called once
122139
verify(releaseIdentifier).calculateApkHash(any());

0 commit comments

Comments
 (0)