From 2846f47a32534f59b4f53cf89e93b51f8038a8be Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Mon, 26 Sep 2022 16:57:10 -0400 Subject: [PATCH 1/7] Implement MultiDB Support --- pom.xml | 8 ++ .../firebase/cloud/FirestoreClient.java | 83 ++++++++++-- .../firebase/cloud/FirestoreClientTest.java | 122 +++++++++++------- 3 files changed, 154 insertions(+), 59 deletions(-) diff --git a/pom.xml b/pom.xml index e76b2dc10..3be7a002f 100644 --- a/pom.xml +++ b/pom.xml @@ -360,6 +360,14 @@ true + + org.apache.maven.plugins + maven-compiler-plugin + + 8 + 8 + + diff --git a/src/main/java/com/google/firebase/cloud/FirestoreClient.java b/src/main/java/com/google/firebase/cloud/FirestoreClient.java index fddf3320d..234db17c5 100644 --- a/src/main/java/com/google/firebase/cloud/FirestoreClient.java +++ b/src/main/java/com/google/firebase/cloud/FirestoreClient.java @@ -14,6 +14,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + /** * {@code FirestoreClient} provides access to Google Cloud Firestore. Use this API to obtain a * {@code Firestore} @@ -32,7 +36,7 @@ public class FirestoreClient { private final Firestore firestore; - private FirestoreClient(FirebaseApp app) { + private FirestoreClient(FirebaseApp app, String databaseId) { checkNotNull(app, "FirebaseApp must not be null"); String projectId = ImplFirebaseTrampolines.getProjectId(app); checkArgument(!Strings.isNullOrEmpty(projectId), @@ -47,12 +51,13 @@ private FirestoreClient(FirebaseApp app) { .setCredentialsProvider( FixedCredentialsProvider.create(ImplFirebaseTrampolines.getCredentials(app))) .setProjectId(projectId) + .setDatabaseId(databaseId) .build() .getService(); } /** - * Returns the Firestore instance associated with the default Firebase app. Returns the same + * Returns the default Firestore instance associated with the default Firebase app. Returns the same * instance for all invocations. The Firestore instance and all references obtained from it * becomes unusable, once the default app is deleted. * @@ -65,7 +70,7 @@ public static Firestore getFirestore() { } /** - * Returns the Firestore instance associated with the specified Firebase app. For a given app, + * Returns the default Firestore instance associated with the specified Firebase app. For a given app, * always returns the same instance. The Firestore instance and all references obtained from it * becomes unusable, once the specified app is deleted. * @@ -75,32 +80,86 @@ public static Firestore getFirestore() { */ @NonNull public static Firestore getFirestore(FirebaseApp app) { - return getInstance(app).firestore; + return getFirestore(app, ImplFirebaseTrampolines.getFirestoreOptions(app).getDatabaseId()); + } + + /** + * Returns the Firestore instance associated with the specified Firebase app. For a given app, + * always returns the same instance. The Firestore instance and all references obtained from it + * becomes unusable, once the specified app is deleted. + * + * @param app A non-null {@link FirebaseApp}. + * @param database - The name of database. + * @return A non-null {@code Firestore} + * instance. + */ + @NonNull + public static Firestore getFirestore(FirebaseApp app, String database) { + return getInstance(app, database).firestore; + } + + /** + * Returns the Firestore instance associated with the default Firebase app. Returns the same + * instance for all invocations. The Firestore instance and all references obtained from it + * becomes unusable, once the default app is deleted. + * + * @param database - The name of database. + * @return A non-null {@code Firestore} + * instance. + */ + @NonNull + public static Firestore getFirestore(String database) { + return getFirestore(FirebaseApp.getInstance(), database); } - private static synchronized FirestoreClient getInstance(FirebaseApp app) { + private static synchronized FirestoreClient getInstance(FirebaseApp app, String database) { FirestoreClientService service = ImplFirebaseTrampolines.getService(app, SERVICE_ID, FirestoreClientService.class); if (service == null) { service = ImplFirebaseTrampolines.addService(app, new FirestoreClientService(app)); } - return service.getInstance(); + return service.getInstance().get(database); } private static final String SERVICE_ID = FirestoreClient.class.getName(); - private static class FirestoreClientService extends FirebaseService { + private static class FirestoreClientService extends FirebaseService { FirestoreClientService(FirebaseApp app) { - super(SERVICE_ID, new FirestoreClient(app)); + super(SERVICE_ID, new FirestoreInstances(app)); } @Override public void destroy() { - try { - instance.firestore.close(); - } catch (Exception e) { - logger.warn("Error while closing the Firestore instance", e); + instance.destroy(); + } + } + + private static class FirestoreInstances { + + private final FirebaseApp app; + + private final Map clients = + Collections.synchronizedMap(new HashMap<>()); + + private FirestoreInstances(FirebaseApp app) { + this.app = app; + } + + FirestoreClient get(String databaseId) { + return clients.computeIfAbsent(databaseId, id -> new FirestoreClient(app, id)); + } + + void destroy() { + synchronized (clients) { + for (FirestoreClient client : clients.values()) { + try { + client.firestore.close(); + } catch (Exception e) { + logger.warn("Error while closing the Firestore instance", e); + } + } + clients.clear(); } } } diff --git a/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java b/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java index 018d0e45b..1f99d8260 100644 --- a/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java +++ b/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java @@ -1,11 +1,5 @@ package com.google.firebase.cloud; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.firestore.DocumentReference; import com.google.cloud.firestore.Firestore; @@ -20,12 +14,15 @@ import org.junit.After; import org.junit.Test; +import static org.junit.Assert.*; + public class FirestoreClientTest { private static final FirestoreOptions FIRESTORE_OPTIONS = FirestoreOptions.newBuilder() // Setting credentials is not required (they get overridden by Admin SDK), but without // this Firestore logs an ugly warning during tests. .setCredentials(new MockGoogleCredentials("test-token")) + .setDatabaseId("differedDefaultDatabaseId") .build(); @After @@ -35,47 +32,75 @@ public void tearDown() { @Test public void testExplicitProjectId() throws IOException { + String databaseId = "databaseIdInTestExplicitProjectId"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("explicit-project-id") .setFirestoreOptions(FIRESTORE_OPTIONS) .build()); - Firestore firestore = FirestoreClient.getFirestore(app); - assertEquals("explicit-project-id", firestore.getOptions().getProjectId()); + Firestore firestore1 = FirestoreClient.getFirestore(app); + assertEquals("explicit-project-id", firestore1.getOptions().getProjectId()); + assertEquals(FIRESTORE_OPTIONS.getDatabaseId(), firestore1.getOptions().getDatabaseId()); + + assertSame(firestore1, FirestoreClient.getFirestore()); + + Firestore firestore2 = FirestoreClient.getFirestore(app, databaseId); + assertEquals("explicit-project-id", firestore2.getOptions().getProjectId()); + assertEquals(databaseId, firestore2.getOptions().getDatabaseId()); + + assertSame(firestore2, FirestoreClient.getFirestore(databaseId)); - firestore = FirestoreClient.getFirestore(); - assertEquals("explicit-project-id", firestore.getOptions().getProjectId()); + assertNotSame(firestore1, firestore2); } @Test public void testServiceAccountProjectId() throws IOException { + String databaseId = "databaseIdInTestServiceAccountProjectId"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setFirestoreOptions(FIRESTORE_OPTIONS) .build()); - Firestore firestore = FirestoreClient.getFirestore(app); - assertEquals("mock-project-id", firestore.getOptions().getProjectId()); + Firestore firestore1 = FirestoreClient.getFirestore(app); + assertEquals("mock-project-id", firestore1.getOptions().getProjectId()); + assertEquals(FIRESTORE_OPTIONS.getDatabaseId(), firestore1.getOptions().getDatabaseId()); - firestore = FirestoreClient.getFirestore(); - assertEquals("mock-project-id", firestore.getOptions().getProjectId()); + assertSame(firestore1, FirestoreClient.getFirestore()); + + Firestore firestore2 = FirestoreClient.getFirestore(app, databaseId); + assertEquals("mock-project-id", firestore2.getOptions().getProjectId()); + assertEquals(databaseId, firestore2.getOptions().getDatabaseId()); + + assertSame(firestore2, FirestoreClient.getFirestore(databaseId)); + + assertNotSame(firestore1, firestore2); } @Test public void testFirestoreOptions() throws IOException { + String databaseId = "databaseIdInTestFirestoreOptions"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("explicit-project-id") .setFirestoreOptions(FIRESTORE_OPTIONS) .build()); - Firestore firestore = FirestoreClient.getFirestore(app); - assertEquals("explicit-project-id", firestore.getOptions().getProjectId()); + Firestore firestore1 = FirestoreClient.getFirestore(app); + assertEquals("explicit-project-id", firestore1.getOptions().getProjectId()); + assertEquals(FIRESTORE_OPTIONS.getDatabaseId(), firestore1.getOptions().getDatabaseId()); + + assertSame(firestore1, FirestoreClient.getFirestore()); - firestore = FirestoreClient.getFirestore(); - assertEquals("explicit-project-id", firestore.getOptions().getProjectId()); + Firestore firestore2 = FirestoreClient.getFirestore(app, databaseId); + assertEquals("explicit-project-id", firestore2.getOptions().getProjectId()); + assertEquals(databaseId, firestore2.getOptions().getDatabaseId()); + + assertSame(firestore2, FirestoreClient.getFirestore(databaseId)); + + assertNotSame(firestore1, firestore2); } @Test public void testFirestoreOptionsOverride() throws IOException { + String databaseId = "databaseIdInTestFirestoreOptions"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("explicit-project-id") @@ -84,48 +109,51 @@ public void testFirestoreOptionsOverride() throws IOException { .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .build()) .build()); - Firestore firestore = FirestoreClient.getFirestore(app); - assertEquals("explicit-project-id", firestore.getOptions().getProjectId()); + Firestore firestore1 = FirestoreClient.getFirestore(app); + assertEquals("explicit-project-id", firestore1.getOptions().getProjectId()); assertSame(ImplFirebaseTrampolines.getCredentials(app), - firestore.getOptions().getCredentialsProvider().getCredentials()); + firestore1.getOptions().getCredentialsProvider().getCredentials()); + assertEquals("(default)", firestore1.getOptions().getDatabaseId()); + + assertSame(firestore1, FirestoreClient.getFirestore()); - firestore = FirestoreClient.getFirestore(); - assertEquals("explicit-project-id", firestore.getOptions().getProjectId()); + Firestore firestore2 = FirestoreClient.getFirestore(app, databaseId); + assertEquals("explicit-project-id", firestore2.getOptions().getProjectId()); assertSame(ImplFirebaseTrampolines.getCredentials(app), - firestore.getOptions().getCredentialsProvider().getCredentials()); + firestore2.getOptions().getCredentialsProvider().getCredentials()); + assertEquals(databaseId, firestore2.getOptions().getDatabaseId()); + + assertSame(firestore2, FirestoreClient.getFirestore(databaseId)); + + assertNotSame(firestore1, firestore2); } @Test public void testAppDelete() throws IOException { + String databaseId = "databaseIdInTestAppDelete"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("mock-project-id") .setFirestoreOptions(FIRESTORE_OPTIONS) .build()); - Firestore firestore = FirestoreClient.getFirestore(app); - assertNotNull(firestore); - DocumentReference document = firestore.collection("collection").document("doc"); + Firestore firestore1 = FirestoreClient.getFirestore(app); + assertNotNull(firestore1); + assertSame(firestore1, FirestoreClient.getFirestore()); + + Firestore firestore2 = FirestoreClient.getFirestore(app, databaseId); + assertNotNull(firestore2); + assertSame(firestore2, FirestoreClient.getFirestore(databaseId)); + + assertNotSame(firestore1, firestore2); + + DocumentReference document = firestore1.collection("collection").document("doc"); app.delete(); - try { - FirestoreClient.getFirestore(app); - fail("No error thrown for deleted app"); - } catch (IllegalStateException expected) { - // ignore - } - - try { - document.get(); - fail("No error thrown for deleted app"); - } catch (IllegalStateException expected) { - // ignore - } - - try { - FirestoreClient.getFirestore(); - fail("No error thrown for deleted app"); - } catch (IllegalStateException expected) { - // ignore - } + + assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore(app)); + assertThrows(IllegalStateException.class, () -> document.get()); + assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore()); + assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore(app, databaseId)); + assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore(databaseId)); } } From 6bf22d79284ccebd622e0e46746de902cdd1f21f Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Tue, 27 Sep 2022 15:29:52 -0400 Subject: [PATCH 2/7] Fix Checkstyle --- .../firebase/cloud/FirestoreClient.java | 19 +++++++++--------- .../firebase/cloud/FirestoreClientTest.java | 20 +++++++++++-------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/google/firebase/cloud/FirestoreClient.java b/src/main/java/com/google/firebase/cloud/FirestoreClient.java index 234db17c5..a13d15430 100644 --- a/src/main/java/com/google/firebase/cloud/FirestoreClient.java +++ b/src/main/java/com/google/firebase/cloud/FirestoreClient.java @@ -11,12 +11,11 @@ import com.google.firebase.ImplFirebaseTrampolines; import com.google.firebase.internal.FirebaseService; import com.google.firebase.internal.NonNull; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.util.Collections; import java.util.HashMap; import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * {@code FirestoreClient} provides access to Google Cloud Firestore. Use this API to obtain a @@ -57,8 +56,8 @@ private FirestoreClient(FirebaseApp app, String databaseId) { } /** - * Returns the default Firestore instance associated with the default Firebase app. Returns the same - * instance for all invocations. The Firestore instance and all references obtained from it + * Returns the default Firestore instance associated with the default Firebase app. Returns the + * same instance for all invocations. The Firestore instance and all references obtained from it * becomes unusable, once the default app is deleted. * * @return A non-null {@code Firestore} @@ -70,9 +69,9 @@ public static Firestore getFirestore() { } /** - * Returns the default Firestore instance associated with the specified Firebase app. For a given app, - * always returns the same instance. The Firestore instance and all references obtained from it - * becomes unusable, once the specified app is deleted. + * Returns the default Firestore instance associated with the specified Firebase app. For a given + * app, always returns the same instance. The Firestore instance and all references obtained from + * it becomes unusable, once the specified app is deleted. * * @param app A non-null {@link FirebaseApp}. * @return A non-null {@code Firestore} @@ -88,7 +87,7 @@ public static Firestore getFirestore(FirebaseApp app) { * always returns the same instance. The Firestore instance and all references obtained from it * becomes unusable, once the specified app is deleted. * - * @param app A non-null {@link FirebaseApp}. + * @param app A non-null {@link FirebaseApp}. * @param database - The name of database. * @return A non-null {@code Firestore} * instance. @@ -140,7 +139,7 @@ private static class FirestoreInstances { private final FirebaseApp app; private final Map clients = - Collections.synchronizedMap(new HashMap<>()); + Collections.synchronizedMap(new HashMap<>()); private FirestoreInstances(FirebaseApp app) { this.app = app; diff --git a/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java b/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java index 1f99d8260..a5f617ce6 100644 --- a/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java +++ b/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java @@ -1,5 +1,11 @@ package com.google.firebase.cloud; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; + import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.firestore.DocumentReference; import com.google.cloud.firestore.Firestore; @@ -14,8 +20,6 @@ import org.junit.After; import org.junit.Test; -import static org.junit.Assert.*; - public class FirestoreClientTest { private static final FirestoreOptions FIRESTORE_OPTIONS = FirestoreOptions.newBuilder() @@ -32,7 +36,7 @@ public void tearDown() { @Test public void testExplicitProjectId() throws IOException { - String databaseId = "databaseIdInTestExplicitProjectId"; + final String databaseId = "databaseIdInTestExplicitProjectId"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("explicit-project-id") @@ -55,7 +59,7 @@ public void testExplicitProjectId() throws IOException { @Test public void testServiceAccountProjectId() throws IOException { - String databaseId = "databaseIdInTestServiceAccountProjectId"; + final String databaseId = "databaseIdInTestServiceAccountProjectId"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setFirestoreOptions(FIRESTORE_OPTIONS) @@ -77,7 +81,7 @@ public void testServiceAccountProjectId() throws IOException { @Test public void testFirestoreOptions() throws IOException { - String databaseId = "databaseIdInTestFirestoreOptions"; + final String databaseId = "databaseIdInTestFirestoreOptions"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("explicit-project-id") @@ -100,7 +104,7 @@ public void testFirestoreOptions() throws IOException { @Test public void testFirestoreOptionsOverride() throws IOException { - String databaseId = "databaseIdInTestFirestoreOptions"; + final String databaseId = "databaseIdInTestFirestoreOptions"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("explicit-project-id") @@ -120,7 +124,7 @@ public void testFirestoreOptionsOverride() throws IOException { Firestore firestore2 = FirestoreClient.getFirestore(app, databaseId); assertEquals("explicit-project-id", firestore2.getOptions().getProjectId()); assertSame(ImplFirebaseTrampolines.getCredentials(app), - firestore2.getOptions().getCredentialsProvider().getCredentials()); + firestore2.getOptions().getCredentialsProvider().getCredentials()); assertEquals(databaseId, firestore2.getOptions().getDatabaseId()); assertSame(firestore2, FirestoreClient.getFirestore(databaseId)); @@ -130,7 +134,7 @@ public void testFirestoreOptionsOverride() throws IOException { @Test public void testAppDelete() throws IOException { - String databaseId = "databaseIdInTestAppDelete"; + final String databaseId = "databaseIdInTestAppDelete"; FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("mock-project-id") From 11aa7ad5f2e24a0686826298bfae0e4aba411715 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Tue, 27 Sep 2022 15:32:37 -0400 Subject: [PATCH 3/7] Revert accidental commit --- pom.xml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pom.xml b/pom.xml index 3be7a002f..e76b2dc10 100644 --- a/pom.xml +++ b/pom.xml @@ -360,14 +360,6 @@ true - - org.apache.maven.plugins - maven-compiler-plugin - - 8 - 8 - - From 24813d0e21b31cd67d4128c2aecfda02049bee80 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Tue, 27 Sep 2022 16:06:28 -0400 Subject: [PATCH 4/7] Remove usage of Java 8 features (lambdas) --- .../firebase/cloud/FirestoreClient.java | 13 +++++-- .../firebase/cloud/FirestoreClientTest.java | 35 +++++++++++++++---- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/firebase/cloud/FirestoreClient.java b/src/main/java/com/google/firebase/cloud/FirestoreClient.java index a13d15430..9835f1d0a 100644 --- a/src/main/java/com/google/firebase/cloud/FirestoreClient.java +++ b/src/main/java/com/google/firebase/cloud/FirestoreClient.java @@ -14,6 +14,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.function.Function; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -139,16 +140,24 @@ private static class FirestoreInstances { private final FirebaseApp app; private final Map clients = - Collections.synchronizedMap(new HashMap<>()); + Collections.synchronizedMap(new HashMap()); private FirestoreInstances(FirebaseApp app) { this.app = app; } FirestoreClient get(String databaseId) { - return clients.computeIfAbsent(databaseId, id -> new FirestoreClient(app, id)); + return clients.computeIfAbsent(databaseId, newFirestoreInstance); } + private final Function newFirestoreInstance = + new Function() { + @Override + public FirestoreClient apply(String id) { + return new FirestoreClient(app, id); + } + }; + void destroy() { synchronized (clients) { for (FirestoreClient client : clients.values()) { diff --git a/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java b/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java index a5f617ce6..d7635cb94 100644 --- a/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java +++ b/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java @@ -19,6 +19,7 @@ import java.io.IOException; import org.junit.After; import org.junit.Test; +import org.junit.function.ThrowingRunnable; public class FirestoreClientTest { @@ -135,7 +136,7 @@ public void testFirestoreOptionsOverride() throws IOException { @Test public void testAppDelete() throws IOException { final String databaseId = "databaseIdInTestAppDelete"; - FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() + final FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("mock-project-id") .setFirestoreOptions(FIRESTORE_OPTIONS) @@ -151,13 +152,33 @@ public void testAppDelete() throws IOException { assertNotSame(firestore1, firestore2); - DocumentReference document = firestore1.collection("collection").document("doc"); + final DocumentReference document = firestore1.collection("collection").document("doc"); app.delete(); - assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore(app)); - assertThrows(IllegalStateException.class, () -> document.get()); - assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore()); - assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore(app, databaseId)); - assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore(databaseId)); + assertThrows(IllegalStateException.class, new ThrowingRunnable() { + public void run() { + FirestoreClient.getFirestore(app); + } + }); + assertThrows(IllegalStateException.class, new ThrowingRunnable() { + public void run() throws Throwable { + document.get(); + } + }); + assertThrows(IllegalStateException.class, new ThrowingRunnable() { + public void run() throws Throwable { + FirestoreClient.getFirestore(); + } + }); + assertThrows(IllegalStateException.class, new ThrowingRunnable() { + public void run() throws Throwable { + FirestoreClient.getFirestore(app, databaseId); + } + }); + assertThrows(IllegalStateException.class, new ThrowingRunnable() { + public void run() throws Throwable { + FirestoreClient.getFirestore(databaseId); + } + }); } } From 30651de464d947e267c9b4db3378dee553b1ae84 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Tue, 27 Sep 2022 16:41:18 -0400 Subject: [PATCH 5/7] Revert "Remove usage of Java 8 features (lambdas)" This reverts commit 24813d0e21b31cd67d4128c2aecfda02049bee80. --- .../firebase/cloud/FirestoreClient.java | 13 ++----- .../firebase/cloud/FirestoreClientTest.java | 35 ++++--------------- 2 files changed, 9 insertions(+), 39 deletions(-) diff --git a/src/main/java/com/google/firebase/cloud/FirestoreClient.java b/src/main/java/com/google/firebase/cloud/FirestoreClient.java index 9835f1d0a..a13d15430 100644 --- a/src/main/java/com/google/firebase/cloud/FirestoreClient.java +++ b/src/main/java/com/google/firebase/cloud/FirestoreClient.java @@ -14,7 +14,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.function.Function; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -140,24 +139,16 @@ private static class FirestoreInstances { private final FirebaseApp app; private final Map clients = - Collections.synchronizedMap(new HashMap()); + Collections.synchronizedMap(new HashMap<>()); private FirestoreInstances(FirebaseApp app) { this.app = app; } FirestoreClient get(String databaseId) { - return clients.computeIfAbsent(databaseId, newFirestoreInstance); + return clients.computeIfAbsent(databaseId, id -> new FirestoreClient(app, id)); } - private final Function newFirestoreInstance = - new Function() { - @Override - public FirestoreClient apply(String id) { - return new FirestoreClient(app, id); - } - }; - void destroy() { synchronized (clients) { for (FirestoreClient client : clients.values()) { diff --git a/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java b/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java index d7635cb94..a5f617ce6 100644 --- a/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java +++ b/src/test/java/com/google/firebase/cloud/FirestoreClientTest.java @@ -19,7 +19,6 @@ import java.io.IOException; import org.junit.After; import org.junit.Test; -import org.junit.function.ThrowingRunnable; public class FirestoreClientTest { @@ -136,7 +135,7 @@ public void testFirestoreOptionsOverride() throws IOException { @Test public void testAppDelete() throws IOException { final String databaseId = "databaseIdInTestAppDelete"; - final FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() + FirebaseApp app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream())) .setProjectId("mock-project-id") .setFirestoreOptions(FIRESTORE_OPTIONS) @@ -152,33 +151,13 @@ public void testAppDelete() throws IOException { assertNotSame(firestore1, firestore2); - final DocumentReference document = firestore1.collection("collection").document("doc"); + DocumentReference document = firestore1.collection("collection").document("doc"); app.delete(); - assertThrows(IllegalStateException.class, new ThrowingRunnable() { - public void run() { - FirestoreClient.getFirestore(app); - } - }); - assertThrows(IllegalStateException.class, new ThrowingRunnable() { - public void run() throws Throwable { - document.get(); - } - }); - assertThrows(IllegalStateException.class, new ThrowingRunnable() { - public void run() throws Throwable { - FirestoreClient.getFirestore(); - } - }); - assertThrows(IllegalStateException.class, new ThrowingRunnable() { - public void run() throws Throwable { - FirestoreClient.getFirestore(app, databaseId); - } - }); - assertThrows(IllegalStateException.class, new ThrowingRunnable() { - public void run() throws Throwable { - FirestoreClient.getFirestore(databaseId); - } - }); + assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore(app)); + assertThrows(IllegalStateException.class, () -> document.get()); + assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore()); + assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore(app, databaseId)); + assertThrows(IllegalStateException.class, () -> FirestoreClient.getFirestore(databaseId)); } } From cd5e0e12735d56307871b54937b2f597963c0bf2 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Tue, 27 Sep 2022 16:43:15 -0400 Subject: [PATCH 6/7] Target Java 8 --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index e76b2dc10..51ea68480 100644 --- a/pom.xml +++ b/pom.xml @@ -272,8 +272,8 @@ maven-compiler-plugin 3.10.1 - 1.7 - 1.7 + 1.8 + 1.8 From 4d39f966d6c597ab28d5d0eed0103054617d84a7 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 21 Oct 2022 15:57:18 -0400 Subject: [PATCH 7/7] Fix API description --- .../google/firebase/cloud/FirestoreClient.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/firebase/cloud/FirestoreClient.java b/src/main/java/com/google/firebase/cloud/FirestoreClient.java index a13d15430..101201781 100644 --- a/src/main/java/com/google/firebase/cloud/FirestoreClient.java +++ b/src/main/java/com/google/firebase/cloud/FirestoreClient.java @@ -70,8 +70,8 @@ public static Firestore getFirestore() { /** * Returns the default Firestore instance associated with the specified Firebase app. For a given - * app, always returns the same instance. The Firestore instance and all references obtained from - * it becomes unusable, once the specified app is deleted. + * app, invocation always returns the same instance. The Firestore instance and all references + * obtained from it becomes unusable, once the specified app is deleted. * * @param app A non-null {@link FirebaseApp}. * @return A non-null {@code Firestore} @@ -83,9 +83,9 @@ public static Firestore getFirestore(FirebaseApp app) { } /** - * Returns the Firestore instance associated with the specified Firebase app. For a given app, - * always returns the same instance. The Firestore instance and all references obtained from it - * becomes unusable, once the specified app is deleted. + * Returns the Firestore instance associated with the specified Firebase app. Returns the same + * instance for all invocations given the same app and database parameter. The Firestore instance + * and all references obtained from it becomes unusable, once the specified app is deleted. * * @param app A non-null {@link FirebaseApp}. * @param database - The name of database. @@ -93,21 +93,21 @@ public static Firestore getFirestore(FirebaseApp app) { * instance. */ @NonNull - public static Firestore getFirestore(FirebaseApp app, String database) { + static Firestore getFirestore(FirebaseApp app, String database) { return getInstance(app, database).firestore; } /** * Returns the Firestore instance associated with the default Firebase app. Returns the same - * instance for all invocations. The Firestore instance and all references obtained from it - * becomes unusable, once the default app is deleted. + * instance for all invocations given the same database parameter. The Firestore instance and all + * references obtained from it becomes unusable, once the default app is deleted. * * @param database - The name of database. * @return A non-null {@code Firestore} * instance. */ @NonNull - public static Firestore getFirestore(String database) { + static Firestore getFirestore(String database) { return getFirestore(FirebaseApp.getInstance(), database); }