Skip to content

Commit 68c0da8

Browse files
rosalyntanrosalyntan
committed
Move continuations off main thread for firebase-appcheck-safetynet.
1 parent de26ef4 commit 68c0da8

File tree

4 files changed

+37
-41
lines changed

4 files changed

+37
-41
lines changed

‎appcheck/firebase-appcheck-safetynet/src/main/java/com/google/firebase/appcheck/safetynet/FirebaseAppCheckSafetyNetRegistrar.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.google.firebase.FirebaseApp;
1919
import com.google.firebase.annotations.concurrent.Background;
2020
import com.google.firebase.annotations.concurrent.Blocking;
21+
import com.google.firebase.annotations.concurrent.Lightweight;
2122
import com.google.firebase.appcheck.safetynet.internal.SafetyNetAppCheckProvider;
2223
import com.google.firebase.components.Component;
2324
import com.google.firebase.components.ComponentRegistrar;
@@ -40,19 +41,22 @@ public class FirebaseAppCheckSafetyNetRegistrar implements ComponentRegistrar {
4041

4142
@Override
4243
public List<Component<?>> getComponents() {
44+
Qualified<Executor> liteExecutor = Qualified.qualified(Lightweight.class, Executor.class);
4345
Qualified<Executor> backgroundExecutor = Qualified.qualified(Background.class, Executor.class);
4446
Qualified<Executor> blockingExecutor = Qualified.qualified(Blocking.class, Executor.class);
4547

4648
return Arrays.asList(
4749
Component.builder(SafetyNetAppCheckProvider.class)
4850
.name(LIBRARY_NAME)
4951
.add(Dependency.required(FirebaseApp.class))
52+
.add(Dependency.required(liteExecutor))
5053
.add(Dependency.required(backgroundExecutor))
5154
.add(Dependency.required(blockingExecutor))
5255
.factory(
5356
(container) ->
5457
new SafetyNetAppCheckProvider(
5558
container.get(FirebaseApp.class),
59+
container.get(liteExecutor),
5660
container.get(backgroundExecutor),
5761
container.get(blockingExecutor)))
5862
.build(),

‎appcheck/firebase-appcheck-safetynet/src/main/java/com/google/firebase/appcheck/safetynet/internal/SafetyNetAppCheckProvider.java

Lines changed: 17 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import static com.google.android.gms.common.internal.Preconditions.checkNotEmpty;
1818
import static com.google.android.gms.common.internal.Preconditions.checkNotNull;
1919

20-
import android.annotation.SuppressLint;
2120
import android.content.Context;
2221
import androidx.annotation.NonNull;
2322
import androidx.annotation.VisibleForTesting;
@@ -32,9 +31,9 @@
3231
import com.google.firebase.FirebaseApp;
3332
import com.google.firebase.annotations.concurrent.Background;
3433
import com.google.firebase.annotations.concurrent.Blocking;
34+
import com.google.firebase.annotations.concurrent.Lightweight;
3535
import com.google.firebase.appcheck.AppCheckProvider;
3636
import com.google.firebase.appcheck.AppCheckToken;
37-
import com.google.firebase.appcheck.internal.AppCheckTokenResponse;
3837
import com.google.firebase.appcheck.internal.DefaultAppCheckToken;
3938
import com.google.firebase.appcheck.internal.NetworkClient;
4039
import com.google.firebase.appcheck.internal.RetryManager;
@@ -51,19 +50,22 @@ public class SafetyNetAppCheckProvider implements AppCheckProvider {
5150

5251
private final Task<SafetyNetClient> safetyNetClientTask;
5352
private final NetworkClient networkClient;
53+
private final Executor liteExecutor;
5454
private final Executor blockingExecutor;
5555
private final RetryManager retryManager;
5656
private final String apiKey;
5757

5858
/** @param firebaseApp the FirebaseApp to which this Factory is tied. */
5959
public SafetyNetAppCheckProvider(
6060
@NonNull FirebaseApp firebaseApp,
61+
@Lightweight Executor liteExecutor,
6162
@Background Executor backgroundExecutor,
6263
@Blocking Executor blockingExecutor) {
6364
this(
6465
firebaseApp,
6566
new NetworkClient(firebaseApp),
6667
GoogleApiAvailability.getInstance(),
68+
liteExecutor,
6769
backgroundExecutor,
6870
blockingExecutor);
6971
}
@@ -73,13 +75,15 @@ public SafetyNetAppCheckProvider(
7375
@NonNull FirebaseApp firebaseApp,
7476
@NonNull NetworkClient networkClient,
7577
@NonNull GoogleApiAvailability googleApiAvailability,
78+
@NonNull Executor liteExecutor,
7679
@NonNull Executor backgroundExecutor,
7780
@NonNull Executor blockingExecutor) {
7881
checkNotNull(firebaseApp);
7982
checkNotNull(networkClient);
8083
checkNotNull(googleApiAvailability);
8184
checkNotNull(backgroundExecutor);
8285
this.apiKey = firebaseApp.getOptions().getApiKey();
86+
this.liteExecutor = liteExecutor;
8387
this.blockingExecutor = blockingExecutor;
8488
this.safetyNetClientTask =
8589
initSafetyNetClient(
@@ -93,11 +97,13 @@ public SafetyNetAppCheckProvider(
9397
@NonNull FirebaseApp firebaseApp,
9498
@NonNull SafetyNetClient safetyNetClient,
9599
@NonNull NetworkClient networkClient,
100+
@NonNull Executor liteExecutor,
96101
@NonNull Executor blockingExecutor,
97102
@NonNull RetryManager retryManager) {
98103
this.apiKey = firebaseApp.getOptions().getApiKey();
99104
this.safetyNetClientTask = Tasks.forResult(safetyNetClient);
100105
this.networkClient = networkClient;
106+
this.liteExecutor = liteExecutor;
101107
this.blockingExecutor = blockingExecutor;
102108
this.retryManager = retryManager;
103109
}
@@ -142,35 +148,15 @@ Task<SafetyNetClient> getSafetyNetClientTask() {
142148
return safetyNetClientTask;
143149
}
144150

145-
// TODO(b/261013814): Use an explicit executor in continuations.
146-
@SuppressLint("TaskMainThread")
147151
@NonNull
148152
@Override
149153
public Task<AppCheckToken> getToken() {
150154
return safetyNetClientTask
151-
.continueWithTask(
152-
task -> {
153-
if (task.isSuccessful()) {
154-
return task.getResult().attest(NONCE.getBytes(), apiKey);
155-
}
156-
return Tasks.forException(task.getException());
157-
})
158-
.continueWithTask(
159-
task -> {
160-
if (!task.isSuccessful()) {
161-
// Proxies errors to the client directly; need to wrap to get the
162-
// types right.
163-
// TODO: more specific error mapping to help clients debug more
164-
// easily.
165-
return Tasks.forException(task.getException());
166-
} else {
167-
return exchangeSafetyNetAttestationResponseForToken(task.getResult());
168-
}
169-
});
155+
.onSuccessTask(
156+
liteExecutor, safetyNetClient -> safetyNetClient.attest(NONCE.getBytes(), apiKey))
157+
.onSuccessTask(liteExecutor, this::exchangeSafetyNetAttestationResponseForToken);
170158
}
171159

172-
// TODO(b/261013814): Use an explicit executor in continuations.
173-
@SuppressLint("TaskMainThread")
174160
@NonNull
175161
Task<AppCheckToken> exchangeSafetyNetAttestationResponseForToken(
176162
@NonNull SafetyNetApi.AttestationResponse attestationResponse) {
@@ -180,22 +166,14 @@ Task<AppCheckToken> exchangeSafetyNetAttestationResponseForToken(
180166

181167
ExchangeSafetyNetTokenRequest request = new ExchangeSafetyNetTokenRequest(safetyNetJwsResult);
182168

183-
Task<AppCheckTokenResponse> networkTask =
184-
Tasks.call(
169+
return Tasks.call(
185170
blockingExecutor,
186171
() ->
187172
networkClient.exchangeAttestationForAppCheckToken(
188-
request.toJsonString().getBytes(UTF_8),
189-
NetworkClient.SAFETY_NET,
190-
retryManager));
191-
return networkTask.continueWithTask(
192-
task -> {
193-
if (task.isSuccessful()) {
194-
return Tasks.forResult(
195-
DefaultAppCheckToken.constructFromAppCheckTokenResponse(task.getResult()));
196-
}
197-
// TODO: Surface more error details.
198-
return Tasks.forException(task.getException());
199-
});
173+
request.toJsonString().getBytes(UTF_8), NetworkClient.SAFETY_NET, retryManager))
174+
.onSuccessTask(
175+
liteExecutor,
176+
response ->
177+
Tasks.forResult(DefaultAppCheckToken.constructFromAppCheckTokenResponse(response)));
200178
}
201179
}

‎appcheck/firebase-appcheck-safetynet/src/test/java/com/google/firebase/appcheck/safetynet/FirebaseAppCheckSafetyNetRegistrarTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.firebase.FirebaseApp;
2020
import com.google.firebase.annotations.concurrent.Background;
2121
import com.google.firebase.annotations.concurrent.Blocking;
22+
import com.google.firebase.annotations.concurrent.Lightweight;
2223
import com.google.firebase.components.Component;
2324
import com.google.firebase.components.Dependency;
2425
import com.google.firebase.components.Qualified;
@@ -41,6 +42,7 @@ public void testGetComponents() {
4142
assertThat(appCheckSafetyNetComponent.getDependencies())
4243
.containsExactly(
4344
Dependency.required(FirebaseApp.class),
45+
Dependency.required(Qualified.qualified(Lightweight.class, Executor.class)),
4446
Dependency.required(Qualified.qualified(Background.class, Executor.class)),
4547
Dependency.required(Qualified.qualified(Blocking.class, Executor.class)));
4648
assertThat(appCheckSafetyNetComponent.isLazy()).isTrue();

‎appcheck/firebase-appcheck-safetynet/src/test/java/com/google/firebase/appcheck/safetynet/internal/SafetyNetAppCheckProviderTest.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ public void testPublicConstructor_nullFirebaseApp_expectThrows() {
8484
NullPointerException.class,
8585
() -> {
8686
new SafetyNetAppCheckProvider(
87-
null, TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
87+
null,
88+
TestOnlyExecutors.lite(),
89+
TestOnlyExecutors.background(),
90+
TestOnlyExecutors.blocking());
8891
});
8992
}
9093

@@ -98,6 +101,7 @@ public void testPublicConstructor_nullFirebaseApp_expectThrows() {
98101
firebaseApp,
99102
mockNetworkClient,
100103
mockGoogleApiAvailability,
104+
TestOnlyExecutors.lite(),
101105
TestOnlyExecutors.background(),
102106
TestOnlyExecutors.blocking());
103107
assertThat(provider.getSafetyNetClientTask().isSuccessful()).isFalse();
@@ -112,6 +116,7 @@ public void testGetToken_googlePlayServicesIsNotAvailable_expectGetTokenTaskExce
112116
firebaseApp,
113117
mockNetworkClient,
114118
mockGoogleApiAvailability,
119+
TestOnlyExecutors.lite(),
115120
TestOnlyExecutors.background(),
116121
TestOnlyExecutors.blocking());
117122
assertThat(provider.getSafetyNetClientTask().isSuccessful()).isFalse();
@@ -123,11 +128,13 @@ public void testGetToken_googlePlayServicesIsNotAvailable_expectGetTokenTaskExce
123128

124129
@Test
125130
public void testGetToken_nonNullSafetyNetClient_expectCallsSafetyNetForAttestation() {
131+
// TODO(b/258273630): Use TestOnlyExecutors instead of MoreExecutors.directExecutor().
126132
SafetyNetAppCheckProvider provider =
127133
new SafetyNetAppCheckProvider(
128134
firebaseApp,
129135
mockSafetyNetClient,
130136
mockNetworkClient,
137+
MoreExecutors.directExecutor(),
131138
TestOnlyExecutors.blocking(),
132139
mockRetryManager);
133140
assertThat(provider.getSafetyNetClientTask().getResult()).isEqualTo(mockSafetyNetClient);
@@ -150,6 +157,7 @@ public void testExchangeSafetyNetJwsForToken_nullAttestationResponse_expectThrow
150157
firebaseApp,
151158
mockSafetyNetClient,
152159
mockNetworkClient,
160+
TestOnlyExecutors.lite(),
153161
TestOnlyExecutors.blocking(),
154162
mockRetryManager);
155163
assertThrows(
@@ -168,6 +176,7 @@ public void testExchangeSafetyNetJwsForToken_emptySafetyNetJwsResult_expectThrow
168176
firebaseApp,
169177
mockSafetyNetClient,
170178
mockNetworkClient,
179+
TestOnlyExecutors.lite(),
171180
TestOnlyExecutors.blocking(),
172181
mockRetryManager);
173182
assertThrows(
@@ -185,6 +194,7 @@ public void testExchangeSafetyNetJwsForToken_validFields_expectReturnsTask() {
185194
firebaseApp,
186195
mockSafetyNetClient,
187196
mockNetworkClient,
197+
TestOnlyExecutors.lite(),
188198
TestOnlyExecutors.blocking(),
189199
mockRetryManager);
190200
Task<AppCheckToken> task =
@@ -207,7 +217,8 @@ public void exchangeSafetyNetJwsForToken_onSuccess_setsTaskResult() throws Excep
207217
firebaseApp,
208218
mockSafetyNetClient,
209219
mockNetworkClient,
210-
MoreExecutors.directExecutor(),
220+
/* liteExecutor= */ MoreExecutors.directExecutor(),
221+
/* blockingExecutor= */ MoreExecutors.directExecutor(),
211222
mockRetryManager);
212223
Task<AppCheckToken> task =
213224
provider.exchangeSafetyNetAttestationResponseForToken(mockSafetyNetAttestationResponse);
@@ -234,6 +245,7 @@ public void exchangeSafetyNetJwsForToken_onFailure_setsTaskException() throws Ex
234245
firebaseApp,
235246
mockSafetyNetClient,
236247
mockNetworkClient,
248+
TestOnlyExecutors.lite(),
237249
MoreExecutors.directExecutor(),
238250
mockRetryManager);
239251
Task<AppCheckToken> task =

0 commit comments

Comments
 (0)