Skip to content

Commit c346de1

Browse files
micahstairsmicahstairs
committed
Address pull request feedback.
1 parent 68edeb3 commit c346de1

File tree

5 files changed

+47
-45
lines changed

5 files changed

+47
-45
lines changed

‎src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@
2929
import com.google.firebase.FirebaseApp;
3030
import com.google.firebase.auth.FirebaseUserManager.EmailLinkType;
3131
import com.google.firebase.auth.FirebaseUserManager.UserImportRequest;
32+
import com.google.firebase.auth.ListProviderConfigsPage;
3233
import com.google.firebase.auth.ListProviderConfigsPage.DefaultOidcProviderConfigSource;
33-
import com.google.firebase.auth.ListProviderConfigsPage.ProviderConfigPageFactory;
34+
import com.google.firebase.auth.ListUsersPage;
3435
import com.google.firebase.auth.ListUsersPage.DefaultUserSource;
35-
import com.google.firebase.auth.ListUsersPage.UserPageFactory;
3636
import com.google.firebase.auth.UserRecord;
3737
import com.google.firebase.auth.internal.FirebaseTokenFactory;
3838
import com.google.firebase.internal.CallableOperation;
@@ -503,7 +503,7 @@ private CallableOperation<ListUsersPage, FirebaseAuthException> listUsersOp(
503503
checkNotDestroyed();
504504
final FirebaseUserManager userManager = getUserManager();
505505
final DefaultUserSource source = new DefaultUserSource(userManager, jsonFactory);
506-
final UserPageFactory factory = new UserPageFactory(source, maxResults, pageToken);
506+
final ListUsersPage.Factory factory = new ListUsersPage.Factory(source, maxResults, pageToken);
507507
return new CallableOperation<ListUsersPage, FirebaseAuthException>() {
508508
@Override
509509
protected ListUsersPage execute() throws FirebaseAuthException {
@@ -1035,6 +1035,7 @@ public OidcProviderConfig getOidcProviderConfig(@NonNull String providerId)
10351035

10361036
/**
10371037
* Similar to {@link #getOidcProviderConfig(String)} but performs the operation asynchronously.
1038+
* Page size will be limited to 100 provider configs.
10381039
*
10391040
* @param providerId A provider ID string.
10401041
* @return An {@code ApiFuture} which will complete successfully with an
@@ -1079,7 +1080,7 @@ public ListProviderConfigsPage<OidcProviderConfig> listOidcProviderConfigs(
10791080

10801081
/**
10811082
* Similar to {@link #listlistOidcProviderConfigs(String)} but performs the operation
1082-
* asynchronously.
1083+
* asynchronously. Page size will be limited to 100 provider configs.
10831084
*
10841085
* @param pageToken A non-empty page token string, or null to retrieve the first page of provider
10851086
* configs.
@@ -1120,8 +1121,8 @@ public ApiFuture<ListProviderConfigsPage<OidcProviderConfig>> listOidcProviderCo
11201121
checkNotDestroyed();
11211122
final FirebaseUserManager userManager = getUserManager();
11221123
final DefaultOidcProviderConfigSource source = new DefaultOidcProviderConfigSource(userManager);
1123-
final ProviderConfigPageFactory<OidcProviderConfig> factory =
1124-
new ProviderConfigPageFactory<OidcProviderConfig>(source, maxResults, pageToken);
1124+
final ListProviderConfigsPage.Factory<OidcProviderConfig> factory =
1125+
new ListProviderConfigsPage.Factory<OidcProviderConfig>(source, maxResults, pageToken);
11251126
return
11261127
new CallableOperation<ListProviderConfigsPage<OidcProviderConfig>, FirebaseAuthException>() {
11271128
@Override

‎src/main/java/com/google/firebase/auth/ListProviderConfigsPage.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ public String getNextPageToken() {
8989
@Override
9090
public ListProviderConfigsPage<T> getNextPage() {
9191
if (hasNextPage()) {
92-
ProviderConfigPageFactory<T> factory =
93-
new ProviderConfigPageFactory<T>(source, maxResults, currentBatch.getPageToken());
92+
Factory<T> factory = new Factory<T>(source, maxResults, currentBatch.getPageToken());
9493
try {
9594
return factory.create();
9695
} catch (FirebaseAuthException e) {
@@ -224,17 +223,17 @@ public ListOidcProviderConfigsResponse fetch(int maxResults, String pageToken)
224223
* <p>Performs argument validation before attempting to load any provider config data (which is
225224
* expensive, and hence may be performed asynchronously on a separate thread).
226225
*/
227-
static class ProviderConfigPageFactory<T extends ProviderConfig> {
226+
static class Factory<T extends ProviderConfig> {
228227

229228
private final ProviderConfigSource<T> source;
230229
private final int maxResults;
231230
private final String pageToken;
232231

233-
ProviderConfigPageFactory(@NonNull ProviderConfigSource<T> source) {
232+
Factory(@NonNull ProviderConfigSource<T> source) {
234233
this(source, FirebaseUserManager.MAX_LIST_PROVIDER_CONFIGS_RESULTS, null);
235234
}
236235

237-
ProviderConfigPageFactory(
236+
Factory(
238237
@NonNull ProviderConfigSource source,
239238
int maxResults,
240239
@Nullable String pageToken) {

‎src/main/java/com/google/firebase/auth/ListUsersPage.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,7 @@ public String getNextPageToken() {
8080
@Override
8181
public ListUsersPage getNextPage() {
8282
if (hasNextPage()) {
83-
UserPageFactory factory =
84-
new UserPageFactory(source, maxResults, currentBatch.getNextPageToken());
83+
Factory factory = new Factory(source, maxResults, currentBatch.getNextPageToken());
8584
try {
8685
return factory.create();
8786
} catch (FirebaseAuthException e) {
@@ -238,17 +237,17 @@ String getNextPageToken() {
238237
* before attempting to load any user data (which is expensive, and hence may be performed
239238
* asynchronously on a separate thread).
240239
*/
241-
static class UserPageFactory {
240+
static class Factory {
242241

243242
private final UserSource source;
244243
private final int maxResults;
245244
private final String pageToken;
246245

247-
UserPageFactory(@NonNull UserSource source) {
246+
Factory(@NonNull UserSource source) {
248247
this(source, FirebaseUserManager.MAX_LIST_USERS_RESULTS, null);
249248
}
250249

251-
UserPageFactory(@NonNull UserSource source, int maxResults, @Nullable String pageToken) {
250+
Factory(@NonNull UserSource source, int maxResults, @Nullable String pageToken) {
252251
checkArgument(maxResults > 0 && maxResults <= FirebaseUserManager.MAX_LIST_USERS_RESULTS,
253252
"maxResults must be a positive integer that does not exceed %s",
254253
FirebaseUserManager.MAX_LIST_USERS_RESULTS);

‎src/test/java/com/google/firebase/auth/FirebaseAuthIT.java

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@
6666
import org.junit.BeforeClass;
6767
import org.junit.Test;
6868

69+
// TODO(micahstairs): Move tenant-aware tests into a seperate class, so that we only need to
70+
// create and destroy the tenant once.
6971
public class FirebaseAuthIT {
7072

7173
private static final String VERIFY_CUSTOM_TOKEN_URL =
@@ -1088,10 +1090,8 @@ public void testListOidcProviderConfigs() throws Exception {
10881090
auth.listOidcProviderConfigsAsync(null).get();
10891091
while (page != null) {
10901092
for (OidcProviderConfig providerConfig : page.getValues()) {
1091-
if (providerIds.contains(providerConfig.getProviderId())) {
1093+
if (checkProviderConfig(providerIds, providerConfig)) {
10921094
collected.incrementAndGet();
1093-
assertEquals("CLIENT_ID", providerConfig.getClientId());
1094-
assertEquals("https://oidc.com/issuer", providerConfig.getIssuer());
10951095
}
10961096
}
10971097
page = page.getNextPage();
@@ -1102,10 +1102,8 @@ public void testListOidcProviderConfigs() throws Exception {
11021102
collected.set(0);
11031103
page = auth.listOidcProviderConfigsAsync(null).get();
11041104
for (OidcProviderConfig providerConfig : page.iterateAll()) {
1105-
if (providerIds.contains(providerConfig.getProviderId())) {
1105+
if (checkProviderConfig(providerIds, providerConfig)) {
11061106
collected.incrementAndGet();
1107-
assertEquals("CLIENT_ID", providerConfig.getClientId());
1108-
assertEquals("https://oidc.com/issuer", providerConfig.getIssuer());
11091107
}
11101108
}
11111109
assertEquals(providerIds.size(), collected.get());
@@ -1128,10 +1126,8 @@ public void onFailure(Throwable t) {
11281126
@Override
11291127
public void onSuccess(ListProviderConfigsPage<OidcProviderConfig> result) {
11301128
for (OidcProviderConfig providerConfig : result.iterateAll()) {
1131-
if (providerIds.contains(providerConfig.getProviderId())) {
1129+
if (checkProviderConfig(providerIds, providerConfig)) {
11321130
collected.incrementAndGet();
1133-
assertEquals("CLIENT_ID", providerConfig.getClientId());
1134-
assertEquals("https://oidc.com/issuer", providerConfig.getIssuer());
11351131
}
11361132
}
11371133
semaphore.release();
@@ -1177,10 +1173,8 @@ public void testTenantAwareListOidcProviderConfigs() throws Exception {
11771173
ListProviderConfigsPage<OidcProviderConfig> page =
11781174
tenantAwareAuth.listOidcProviderConfigsAsync(null).get();
11791175
for (OidcProviderConfig providerConfig : page.iterateAll()) {
1180-
if (providerIds.contains(providerConfig.getProviderId())) {
1176+
if (checkProviderConfig(providerIds, providerConfig)) {
11811177
collected.incrementAndGet();
1182-
assertEquals("CLIENT_ID", providerConfig.getClientId());
1183-
assertEquals("https://oidc.com/issuer", providerConfig.getIssuer());
11841178
}
11851179
}
11861180
assertEquals(providerIds.size(), collected.get());
@@ -1327,6 +1321,15 @@ static RandomUser create() {
13271321
}
13281322
}
13291323

1324+
private boolean checkProviderConfig(List<String> providerIds, OidcProviderConfig config) {
1325+
if (providerIds.contains(config.getProviderId())) {
1326+
assertEquals("CLIENT_ID", config.getClientId());
1327+
assertEquals("https://oidc.com/issuer", config.getIssuer());
1328+
return true;
1329+
}
1330+
return false;
1331+
}
1332+
13301333

13311334
private static void assertOidcProviderConfigDoesNotExist(
13321335
AbstractFirebaseAuth firebaseAuth, String providerId) throws Exception {

‎src/test/java/com/google/firebase/auth/ListUsersPageTest.java

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
import com.google.api.client.json.JsonFactory;
2828
import com.google.common.collect.ImmutableList;
2929
import com.google.common.io.BaseEncoding;
30+
import com.google.firebase.auth.ListUsersPage;
3031
import com.google.firebase.auth.ListUsersPage.ListUsersResult;
31-
import com.google.firebase.auth.ListUsersPage.UserPageFactory;
3232
import com.google.firebase.auth.internal.DownloadAccountResponse;
3333
import java.io.IOException;
3434
import java.util.ArrayList;
@@ -46,7 +46,7 @@ public class ListUsersPageTest {
4646
@Test
4747
public void testSinglePage() throws FirebaseAuthException, IOException {
4848
TestUserSource source = new TestUserSource(3);
49-
ListUsersPage page = new UserPageFactory(source).create();
49+
ListUsersPage page = new ListUsersPage.Factory(source).create();
5050
assertFalse(page.hasNextPage());
5151
assertEquals(ListUsersPage.END_OF_LIST, page.getNextPageToken());
5252
assertNull(page.getNextPage());
@@ -69,7 +69,7 @@ public void testRedactedPasswords() throws FirebaseAuthException, IOException {
6969
newUser("user2", REDACTED_BASE64)),
7070
ListUsersPage.END_OF_LIST);
7171
TestUserSource source = new TestUserSource(result);
72-
ListUsersPage page = new UserPageFactory(source).create();
72+
ListUsersPage page = new ListUsersPage.Factory(source).create();
7373
assertFalse(page.hasNextPage());
7474
assertEquals(ListUsersPage.END_OF_LIST, page.getNextPageToken());
7575
assertNull(page.getNextPage());
@@ -90,7 +90,7 @@ public void testMultiplePages() throws FirebaseAuthException, IOException {
9090
ImmutableList.of(newUser("user0"), newUser("user1"), newUser("user2")),
9191
"token");
9292
TestUserSource source = new TestUserSource(result);
93-
ListUsersPage page1 = new UserPageFactory(source).create();
93+
ListUsersPage page1 = new ListUsersPage.Factory(source).create();
9494
assertTrue(page1.hasNextPage());
9595
assertEquals("token", page1.getNextPageToken());
9696
ImmutableList<ExportedUserRecord> users = ImmutableList.copyOf(page1.getValues());
@@ -137,7 +137,7 @@ public void testMultiplePages() throws FirebaseAuthException, IOException {
137137
@Test
138138
public void testListUsersIterable() throws FirebaseAuthException, IOException {
139139
TestUserSource source = new TestUserSource(3);
140-
ListUsersPage page = new UserPageFactory(source).create();
140+
ListUsersPage page = new ListUsersPage.Factory(source).create();
141141
Iterable<ExportedUserRecord> users = page.iterateAll();
142142

143143
int iterations = 0;
@@ -163,7 +163,7 @@ public void testListUsersIterable() throws FirebaseAuthException, IOException {
163163
@Test
164164
public void testListUsersIterator() throws FirebaseAuthException, IOException {
165165
TestUserSource source = new TestUserSource(3);
166-
ListUsersPage page = new UserPageFactory(source).create();
166+
ListUsersPage page = new ListUsersPage.Factory(source).create();
167167
Iterable<ExportedUserRecord> users = page.iterateAll();
168168
Iterator<ExportedUserRecord> iterator = users.iterator();
169169
int iterations = 0;
@@ -193,7 +193,7 @@ public void testListUsersPagedIterable() throws FirebaseAuthException, IOExcepti
193193
ImmutableList.of(newUser("user0"), newUser("user1"), newUser("user2")),
194194
"token");
195195
TestUserSource source = new TestUserSource(result);
196-
ListUsersPage page = new UserPageFactory(source).create();
196+
ListUsersPage page = new ListUsersPage.Factory(source).create();
197197
int iterations = 0;
198198
for (ExportedUserRecord user : page.iterateAll()) {
199199
assertEquals("user" + iterations, user.getUid());
@@ -219,7 +219,7 @@ public void testListUsersPagedIterator() throws FirebaseAuthException, IOExcepti
219219
ImmutableList.of(newUser("user0"), newUser("user1"), newUser("user2")),
220220
"token");
221221
TestUserSource source = new TestUserSource(result);
222-
ListUsersPage page = new UserPageFactory(source).create();
222+
ListUsersPage page = new ListUsersPage.Factory(source).create();
223223
Iterator<ExportedUserRecord> users = page.iterateAll().iterator();
224224
int iterations = 0;
225225
while (users.hasNext()) {
@@ -252,7 +252,7 @@ public void testPageWithNoUsers() throws FirebaseAuthException {
252252
ImmutableList.<ExportedUserRecord>of(),
253253
ListUsersPage.END_OF_LIST);
254254
TestUserSource source = new TestUserSource(result);
255-
ListUsersPage page = new UserPageFactory(source).create();
255+
ListUsersPage page = new ListUsersPage.Factory(source).create();
256256
assertFalse(page.hasNextPage());
257257
assertEquals(ListUsersPage.END_OF_LIST, page.getNextPageToken());
258258
assertNull(page.getNextPage());
@@ -266,7 +266,7 @@ public void testIterableWithNoUsers() throws FirebaseAuthException {
266266
ImmutableList.<ExportedUserRecord>of(),
267267
ListUsersPage.END_OF_LIST);
268268
TestUserSource source = new TestUserSource(result);
269-
ListUsersPage page = new UserPageFactory(source).create();
269+
ListUsersPage page = new ListUsersPage.Factory(source).create();
270270
for (ExportedUserRecord user : page.iterateAll()) {
271271
fail("Should not be able to iterate, but got: " + user);
272272
}
@@ -280,7 +280,7 @@ public void testIteratorWithNoUsers() throws FirebaseAuthException {
280280
ListUsersPage.END_OF_LIST);
281281
TestUserSource source = new TestUserSource(result);
282282

283-
ListUsersPage page = new UserPageFactory(source).create();
283+
ListUsersPage page = new ListUsersPage.Factory(source).create();
284284
Iterator<ExportedUserRecord> iterator = page.iterateAll().iterator();
285285
while (iterator.hasNext()) {
286286
fail("Should not be able to iterate");
@@ -295,7 +295,7 @@ public void testRemove() throws FirebaseAuthException, IOException {
295295
ListUsersPage.END_OF_LIST);
296296
TestUserSource source = new TestUserSource(result);
297297

298-
ListUsersPage page = new UserPageFactory(source).create();
298+
ListUsersPage page = new ListUsersPage.Factory(source).create();
299299
Iterator<ExportedUserRecord> iterator = page.iterateAll().iterator();
300300
while (iterator.hasNext()) {
301301
assertNotNull(iterator.next());
@@ -309,14 +309,14 @@ public void testRemove() throws FirebaseAuthException, IOException {
309309

310310
@Test(expected = NullPointerException.class)
311311
public void testNullSource() {
312-
new UserPageFactory(null);
312+
new ListUsersPage.Factory(null);
313313
}
314314

315315
@Test
316316
public void testInvalidPageToken() throws IOException {
317317
TestUserSource source = new TestUserSource(1);
318318
try {
319-
new UserPageFactory(source, 1000, "");
319+
new ListUsersPage.Factory(source, 1000, "");
320320
fail("No error thrown for empty page token");
321321
} catch (IllegalArgumentException expected) {
322322
// expected
@@ -327,21 +327,21 @@ public void testInvalidPageToken() throws IOException {
327327
public void testInvalidMaxResults() throws IOException {
328328
TestUserSource source = new TestUserSource(1);
329329
try {
330-
new UserPageFactory(source, 1001, "");
330+
new ListUsersPage.Factory(source, 1001, "");
331331
fail("No error thrown for maxResult > 1000");
332332
} catch (IllegalArgumentException expected) {
333333
// expected
334334
}
335335

336336
try {
337-
new UserPageFactory(source, 0, "next");
337+
new ListUsersPage.Factory(source, 0, "next");
338338
fail("No error thrown for maxResult = 0");
339339
} catch (IllegalArgumentException expected) {
340340
// expected
341341
}
342342

343343
try {
344-
new UserPageFactory(source, -1, "next");
344+
new ListUsersPage.Factory(source, -1, "next");
345345
fail("No error thrown for maxResult < 0");
346346
} catch (IllegalArgumentException expected) {
347347
// expected

0 commit comments

Comments
 (0)