Skip to content

Commit a81aa21

Browse files
rosalyntanrosalyntan
committed
Address review comments.
1 parent e755e6b commit a81aa21

File tree

6 files changed

+33
-25
lines changed

6 files changed

+33
-25
lines changed

‎appcheck/firebase-appcheck-debug/src/test/java/com/google/firebase/appcheck/debug/internal/DebugAppCheckProviderTest.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ public class DebugAppCheckProviderTest {
7474
private StorageHelper storageHelper;
7575
private SharedPreferences sharedPreferences;
7676
// TODO(b/258273630): Use TestOnlyExecutors instead of MoreExecutors.directExecutor().
77-
private Executor executor = MoreExecutors.directExecutor();
77+
private Executor liteExecutor = MoreExecutors.directExecutor();
78+
private Executor backgroundExecutor = MoreExecutors.directExecutor();
79+
private Executor blockingExecutor = MoreExecutors.directExecutor();
7880

7981
@Before
8082
public void setup() {
@@ -117,7 +119,7 @@ public void testDetermineDebugSecret_noStoredSecret_createsNewSecret() {
117119
assertThat(storageHelper.retrieveDebugSecret()).isNull();
118120

119121
Task<String> debugSecretTask =
120-
DebugAppCheckProvider.determineDebugSecret(mockFirebaseApp, executor);
122+
DebugAppCheckProvider.determineDebugSecret(mockFirebaseApp, backgroundExecutor);
121123
assertThat(storageHelper.retrieveDebugSecret()).isNotNull();
122124
assertThat(storageHelper.retrieveDebugSecret()).isEqualTo(debugSecretTask.getResult());
123125
}
@@ -127,7 +129,7 @@ public void testDetermineDebugSecret_storedSecret_usesExistingSecret() {
127129
storageHelper.saveDebugSecret(DEBUG_SECRET);
128130

129131
Task<String> debugSecretTask =
130-
DebugAppCheckProvider.determineDebugSecret(mockFirebaseApp, executor);
132+
DebugAppCheckProvider.determineDebugSecret(mockFirebaseApp, backgroundExecutor);
131133
assertThat(debugSecretTask.getResult()).isEqualTo(DEBUG_SECRET);
132134
assertThat(storageHelper.retrieveDebugSecret()).isEqualTo(DEBUG_SECRET);
133135
}
@@ -142,11 +144,7 @@ public void exchangeDebugToken_onSuccess_setsTaskResult() throws Exception {
142144

143145
DebugAppCheckProvider provider =
144146
new DebugAppCheckProvider(
145-
DEBUG_SECRET,
146-
mockNetworkClient,
147-
/* liteExecutor= */ executor,
148-
/* blockingExecutor= */ executor,
149-
mockRetryManager);
147+
DEBUG_SECRET, mockNetworkClient, liteExecutor, blockingExecutor, mockRetryManager);
150148
Task<AppCheckToken> task = provider.getToken();
151149

152150
verify(mockNetworkClient)
@@ -165,11 +163,7 @@ public void exchangeDebugToken_onFailure_setsTaskException() throws Exception {
165163

166164
DebugAppCheckProvider provider =
167165
new DebugAppCheckProvider(
168-
DEBUG_SECRET,
169-
mockNetworkClient,
170-
/* liteExecutor= */ executor,
171-
/* blockingExecutor= */ executor,
172-
mockRetryManager);
166+
DEBUG_SECRET, mockNetworkClient, liteExecutor, blockingExecutor, mockRetryManager);
173167
Task<AppCheckToken> task = provider.getToken();
174168

175169
verify(mockNetworkClient)

‎appcheck/firebase-appcheck-playintegrity/src/test/java/com/google/firebase/appcheck/playintegrity/internal/PlayIntegrityAppCheckProviderTest.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ public class PlayIntegrityAppCheckProviderTest {
7272
@Captor private ArgumentCaptor<byte[]> exchangePlayIntegrityTokenRequestCaptor;
7373

7474
// TODO(b/258273630): Use TestOnlyExecutors instead of MoreExecutors.directExecutor().
75-
private Executor executor = MoreExecutors.directExecutor();
75+
private Executor liteExecutor = MoreExecutors.directExecutor();
76+
private Executor blockingExecutor = MoreExecutors.directExecutor();
7677

7778
@Before
7879
public void setup() {
@@ -107,8 +108,8 @@ public void getToken_onSuccess_setsTaskResult() throws Exception {
107108
PROJECT_NUMBER,
108109
mockIntegrityManager,
109110
mockNetworkClient,
110-
/* liteExecutor= */ executor,
111-
/* blockingExecutor= */ executor,
111+
liteExecutor,
112+
blockingExecutor,
112113
mockRetryManager);
113114
Task<AppCheckToken> task = provider.getToken();
114115

@@ -143,8 +144,8 @@ public void getToken_generateChallengeFails_setsTaskException() throws Exception
143144
PROJECT_NUMBER,
144145
mockIntegrityManager,
145146
mockNetworkClient,
146-
/* liteExecutor= */ executor,
147-
/* blockingExecutor= */ executor,
147+
liteExecutor,
148+
blockingExecutor,
148149
mockRetryManager);
149150
Task<AppCheckToken> task = provider.getToken();
150151

@@ -168,8 +169,8 @@ public void getToken_requestIntegrityTokenFails_setsTaskException() throws Excep
168169
PROJECT_NUMBER,
169170
mockIntegrityManager,
170171
mockNetworkClient,
171-
/* liteExecutor= */ executor,
172-
/* blockingExecutor= */ executor,
172+
liteExecutor,
173+
blockingExecutor,
173174
mockRetryManager);
174175
Task<AppCheckToken> task = provider.getToken();
175176

@@ -200,8 +201,8 @@ public void getToken_tokenExchangeFails_setsTaskException() throws Exception {
200201
PROJECT_NUMBER,
201202
mockIntegrityManager,
202203
mockNetworkClient,
203-
/* liteExecutor= */ executor,
204-
/* blockingExecutor= */ executor,
204+
liteExecutor,
205+
blockingExecutor,
205206
mockRetryManager);
206207
Task<AppCheckToken> task = provider.getToken();
207208

‎appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.firebase.annotations.concurrent.Background;
2020
import com.google.firebase.annotations.concurrent.Blocking;
2121
import com.google.firebase.annotations.concurrent.Lightweight;
22+
import com.google.firebase.annotations.concurrent.UiThread;
2223
import com.google.firebase.appcheck.internal.DefaultFirebaseAppCheck;
2324
import com.google.firebase.appcheck.interop.InternalAppCheckTokenProvider;
2425
import com.google.firebase.components.Component;
@@ -45,6 +46,7 @@ public class FirebaseAppCheckRegistrar implements ComponentRegistrar {
4546

4647
@Override
4748
public List<Component<?>> getComponents() {
49+
Qualified<Executor> uiExecutor = Qualified.qualified(UiThread.class, Executor.class);
4850
Qualified<Executor> liteExecutor = Qualified.qualified(Lightweight.class, Executor.class);
4951
Qualified<Executor> backgroundExecutor = Qualified.qualified(Background.class, Executor.class);
5052
Qualified<ScheduledExecutorService> blockingScheduledExecutorService =
@@ -54,6 +56,7 @@ public List<Component<?>> getComponents() {
5456
Component.builder(FirebaseAppCheck.class, (InternalAppCheckTokenProvider.class))
5557
.name(LIBRARY_NAME)
5658
.add(Dependency.required(FirebaseApp.class))
59+
.add(Dependency.required(uiExecutor))
5760
.add(Dependency.required(liteExecutor))
5861
.add(Dependency.required(backgroundExecutor))
5962
.add(Dependency.required(blockingScheduledExecutorService))
@@ -63,6 +66,7 @@ public List<Component<?>> getComponents() {
6366
new DefaultFirebaseAppCheck(
6467
container.get(FirebaseApp.class),
6568
container.getProvider(HeartBeatController.class),
69+
container.get(uiExecutor),
6670
container.get(liteExecutor),
6771
container.get(backgroundExecutor),
6872
container.get(blockingScheduledExecutorService)))

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.google.firebase.annotations.concurrent.Background;
2828
import com.google.firebase.annotations.concurrent.Blocking;
2929
import com.google.firebase.annotations.concurrent.Lightweight;
30+
import com.google.firebase.annotations.concurrent.UiThread;
3031
import com.google.firebase.appcheck.AppCheckProvider;
3132
import com.google.firebase.appcheck.AppCheckProviderFactory;
3233
import com.google.firebase.appcheck.AppCheckToken;
@@ -51,6 +52,7 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck {
5152
private final List<AppCheckListener> appCheckListenerList;
5253
private final StorageHelper storageHelper;
5354
private final TokenRefreshManager tokenRefreshManager;
55+
private final Executor uiExecutor;
5456
private final Executor liteExecutor;
5557
private final Executor backgroundExecutor;
5658
private final Task<Void> retrieveStoredTokenTask;
@@ -63,6 +65,7 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck {
6365
public DefaultFirebaseAppCheck(
6466
@NonNull FirebaseApp firebaseApp,
6567
@NonNull Provider<HeartBeatController> heartBeatController,
68+
@UiThread Executor uiExecutor,
6669
@Lightweight Executor liteExecutor,
6770
@Background Executor backgroundExecutor,
6871
@Blocking ScheduledExecutorService scheduledExecutorService) {
@@ -80,6 +83,7 @@ public DefaultFirebaseAppCheck(
8083
/* firebaseAppCheck= */ this,
8184
liteExecutor,
8285
scheduledExecutorService);
86+
this.uiExecutor = uiExecutor;
8387
this.liteExecutor = liteExecutor;
8488
this.backgroundExecutor = backgroundExecutor;
8589
this.retrieveStoredTokenTask = retrieveStoredAppCheckTokenInBackground(backgroundExecutor);
@@ -230,7 +234,7 @@ Task<AppCheckToken> fetchTokenFromProvider() {
230234
return appCheckProvider
231235
.getToken()
232236
.onSuccessTask(
233-
liteExecutor,
237+
uiExecutor,
234238
token -> {
235239
updateStoredToken(token);
236240
for (AppCheckListener listener : appCheckListenerList) {

‎appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.firebase.annotations.concurrent.Background;
2121
import com.google.firebase.annotations.concurrent.Blocking;
2222
import com.google.firebase.annotations.concurrent.Lightweight;
23+
import com.google.firebase.annotations.concurrent.UiThread;
2324
import com.google.firebase.components.Component;
2425
import com.google.firebase.components.Dependency;
2526
import com.google.firebase.components.Qualified;
@@ -45,6 +46,7 @@ public void testGetComponents() {
4546
assertThat(firebaseAppCheckComponent.getDependencies())
4647
.containsExactly(
4748
Dependency.required(FirebaseApp.class),
49+
Dependency.required(Qualified.qualified(UiThread.class, Executor.class)),
4850
Dependency.required(Qualified.qualified(Lightweight.class, Executor.class)),
4951
Dependency.required(Qualified.qualified(Background.class, Executor.class)),
5052
Dependency.required(

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,9 @@ public void setup() {
8282
new DefaultFirebaseAppCheck(
8383
mockFirebaseApp,
8484
() -> mockHeartBeatController,
85-
MoreExecutors.directExecutor(),
86-
MoreExecutors.directExecutor(),
85+
TestOnlyExecutors.ui(),
86+
/* liteExecutor= */ MoreExecutors.directExecutor(),
87+
/* backgroundExecutor= */ MoreExecutors.directExecutor(),
8788
TestOnlyExecutors.blocking());
8889
}
8990

@@ -95,6 +96,7 @@ public void testConstructor_nullFirebaseApp_expectThrows() {
9596
new DefaultFirebaseAppCheck(
9697
null,
9798
() -> mockHeartBeatController,
99+
TestOnlyExecutors.ui(),
98100
TestOnlyExecutors.lite(),
99101
TestOnlyExecutors.background(),
100102
TestOnlyExecutors.blocking());
@@ -109,6 +111,7 @@ public void testConstructor_nullHeartBeatControllerProvider_expectThrows() {
109111
new DefaultFirebaseAppCheck(
110112
mockFirebaseApp,
111113
null,
114+
TestOnlyExecutors.ui(),
112115
TestOnlyExecutors.lite(),
113116
TestOnlyExecutors.background(),
114117
TestOnlyExecutors.blocking());

0 commit comments

Comments
 (0)