From d0a30ce3bf51abe65875f8e951156a2649164be5 Mon Sep 17 00:00:00 2001 From: jonathanedey Date: Thu, 4 Dec 2025 12:35:14 -0500 Subject: [PATCH 1/6] feat(firestore): Add Firestore Multi Database Support --- firebase.go | 9 ++ firebase_test.go | 16 +++ integration/firestore/firestore_test.go | 153 +++++++++++++++++++++++- 3 files changed, 172 insertions(+), 6 deletions(-) diff --git a/firebase.go b/firebase.go index 9373ae23..a4712f8d 100644 --- a/firebase.go +++ b/firebase.go @@ -111,6 +111,15 @@ func (a *App) Firestore(ctx context.Context) (*firestore.Client, error) { return firestore.NewClient(ctx, a.projectID, a.opts...) } +// FirestoreWithDatabase returns a new firestore.Client instance with the specified named database from the +// https://godoc.org/cloud.google.com/go/firestore package. +func (a *App) FirestoreWithDatabase(ctx context.Context, databaseID string) (*firestore.Client, error) { + if a.projectID == "" { + return nil, errors.New("project id is required to access Firestore") + } + return firestore.NewClientWithDatabase(ctx, a.projectID, databaseID, a.opts...) +} + // InstanceID returns an instance of iid.Client. func (a *App) InstanceID(ctx context.Context) (*iid.Client, error) { conf := &internal.InstanceIDConfig{ diff --git a/firebase_test.go b/firebase_test.go index 2699146a..8225cf5a 100644 --- a/firebase_test.go +++ b/firebase_test.go @@ -287,6 +287,18 @@ func TestFirestore(t *testing.T) { } } +func TestFirestoreWithDatabase(t *testing.T) { + ctx := context.Background() + app, err := NewApp(ctx, nil, option.WithCredentialsFile("testdata/service_account.json")) + if err != nil { + t.Fatal(err) + } + + if c, err := app.FirestoreWithDatabase(ctx, "other-db"); c == nil || err != nil { + t.Errorf("FirestoreWithDatabase() = (%v, %v); want (client, nil)", c, err) + } +} + func TestFirestoreWithProjectID(t *testing.T) { verify := func(varName string) { current := os.Getenv(varName) @@ -336,6 +348,10 @@ func TestFirestoreWithNoProjectID(t *testing.T) { if c, err := app.Firestore(ctx); c != nil || err == nil { t.Errorf("Firestore() = (%v, %v); want (nil, error)", c, err) } + + if c, err := app.FirestoreWithDatabase(ctx, "other-db"); c != nil || err == nil { + t.Errorf("FirestoreWithDatabase() = (%v, %v); want (nil, error)", c, err) + } } func TestInstanceID(t *testing.T) { diff --git a/integration/firestore/firestore_test.go b/integration/firestore/firestore_test.go index 3e6d2da9..0299bad3 100644 --- a/integration/firestore/firestore_test.go +++ b/integration/firestore/firestore_test.go @@ -20,9 +20,27 @@ import ( "reflect" "testing" + "time" + + "cloud.google.com/go/firestore" "firebase.google.com/go/v4/integration/internal" ) +var ( + cityData = map[string]interface{}{ + "name": "Mountain View", + "country": "USA", + "population": int64(77846), + "capital": false, + } + movieData = map[string]interface{}{ + "Name": "Interstellar", + "Year": int64(2014), + "Runtime": "2h 49m", + "Academy Award Winner": true, + } +) + func TestFirestore(t *testing.T) { if testing.Short() { log.Println("skipping Firestore integration tests in short mode.") @@ -40,11 +58,130 @@ func TestFirestore(t *testing.T) { } doc := client.Collection("cities").Doc("Mountain View") + if _, err := doc.Set(ctx, cityData); err != nil { + t.Fatal(err) + } + snap, err := doc.Get(ctx) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(snap.Data(), cityData) { + t.Errorf("Get() = %v; want %v", snap.Data(), cityData) + } + if _, err := doc.Delete(ctx); err != nil { + t.Fatal(err) + } +} + +func TestFirestoreWithDatabase(t *testing.T) { + if testing.Short() { + log.Println("skipping Firestore integration tests in short mode.") + return + } + ctx := context.Background() + app, err := internal.NewTestApp(ctx, nil) + if err != nil { + t.Fatal(err) + } + + // This test requires a non-default database to exist in the project. + // If it doesn't exist, this test will fail. + client, err := app.FirestoreWithDatabase(ctx, "testing-database") + if err != nil { + t.Fatal(err) + } + + doc := client.Collection("cities").NewDoc() + if _, err := doc.Set(ctx, cityData); err != nil { + t.Fatal(err) + } + snap, err := doc.Get(ctx) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(snap.Data(), cityData) { + t.Errorf("Get() = %v; want %v", snap.Data(), cityData) + } + if _, err := doc.Delete(ctx); err != nil { + t.Fatal(err) + } +} + +func TestFirestoreMultiDB(t *testing.T) { + if testing.Short() { + log.Println("skipping Firestore integration tests in short mode.") + return + } + ctx := context.Background() + app, err := internal.NewTestApp(ctx, nil) + if err != nil { + t.Fatal(err) + } + + cityClient, err := app.Firestore(ctx) + if err != nil { + t.Fatal(err) + } + // This test requires a non-default database to exist in the project. + movieClient, err := app.FirestoreWithDatabase(ctx, "testing-database") + if err != nil { + t.Fatal(err) + } + + cityDoc := cityClient.Collection("cities").NewDoc() + movieDoc := movieClient.Collection("movies").NewDoc() + + if _, err := cityDoc.Set(ctx, cityData); err != nil { + t.Fatal(err) + } + if _, err := movieDoc.Set(ctx, movieData); err != nil { + t.Fatal(err) + } + + citySnap, err := cityDoc.Get(ctx) + if err != nil { + t.Fatal(err) + } + movieSnap, err := movieDoc.Get(ctx) + if err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(citySnap.Data(), cityData) { + t.Errorf("City Get() = %v; want %v", citySnap.Data(), cityData) + } + if !reflect.DeepEqual(movieSnap.Data(), movieData) { + t.Errorf("Movie Get() = %v; want %v", movieSnap.Data(), movieData) + } + + if _, err := cityDoc.Delete(ctx); err != nil { + t.Fatal(err) + } + if _, err := movieDoc.Delete(ctx); err != nil { + t.Fatal(err) + } +} + +func TestServerTimestamp(t *testing.T) { + if testing.Short() { + log.Println("skipping Firestore integration tests in short mode.") + return + } + ctx := context.Background() + app, err := internal.NewTestApp(ctx, nil) + if err != nil { + t.Fatal(err) + } + + client, err := app.Firestore(ctx) + if err != nil { + t.Fatal(err) + } + + doc := client.Collection("cities").NewDoc() data := map[string]interface{}{ - "name": "Mountain View", - "country": "USA", - "population": int64(77846), - "capital": false, + "name": "Mountain View", + "timestamp": firestore.ServerTimestamp, } if _, err := doc.Set(ctx, data); err != nil { t.Fatal(err) @@ -53,8 +190,12 @@ func TestFirestore(t *testing.T) { if err != nil { t.Fatal(err) } - if !reflect.DeepEqual(snap.Data(), data) { - t.Errorf("Get() = %v; want %v", snap.Data(), data) + got := snap.Data() + if got["name"] != "Mountain View" { + t.Errorf("Name = %v; want Mountain View", got["name"]) + } + if _, ok := got["timestamp"].(time.Time); !ok { + t.Errorf("Timestamp is not a time.Time: %v", got["timestamp"]) } if _, err := doc.Delete(ctx); err != nil { t.Fatal(err) From bb2a59b062d92135cc608575aa2f8e3371afb42b Mon Sep 17 00:00:00 2001 From: jonathanedey Date: Mon, 8 Dec 2025 10:49:24 -0500 Subject: [PATCH 2/6] fix: Address gemini review --- firebase.go | 5 +---- integration/firestore/firestore_test.go | 25 +++++++++---------------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/firebase.go b/firebase.go index a4712f8d..edb76937 100644 --- a/firebase.go +++ b/firebase.go @@ -105,10 +105,7 @@ func (a *App) Storage(ctx context.Context) (*storage.Client, error) { // Firestore returns a new firestore.Client instance from the https://godoc.org/cloud.google.com/go/firestore // package. func (a *App) Firestore(ctx context.Context) (*firestore.Client, error) { - if a.projectID == "" { - return nil, errors.New("project id is required to access Firestore") - } - return firestore.NewClient(ctx, a.projectID, a.opts...) + return a.FirestoreWithDatabase(ctx, firestore.DefaultDatabaseID) } // FirestoreWithDatabase returns a new firestore.Client instance with the specified named database from the diff --git a/integration/firestore/firestore_test.go b/integration/firestore/firestore_test.go index 0299bad3..873d0092 100644 --- a/integration/firestore/firestore_test.go +++ b/integration/firestore/firestore_test.go @@ -61,6 +61,8 @@ func TestFirestore(t *testing.T) { if _, err := doc.Set(ctx, cityData); err != nil { t.Fatal(err) } + defer doc.Delete(ctx) + snap, err := doc.Get(ctx) if err != nil { t.Fatal(err) @@ -68,9 +70,6 @@ func TestFirestore(t *testing.T) { if !reflect.DeepEqual(snap.Data(), cityData) { t.Errorf("Get() = %v; want %v", snap.Data(), cityData) } - if _, err := doc.Delete(ctx); err != nil { - t.Fatal(err) - } } func TestFirestoreWithDatabase(t *testing.T) { @@ -95,6 +94,8 @@ func TestFirestoreWithDatabase(t *testing.T) { if _, err := doc.Set(ctx, cityData); err != nil { t.Fatal(err) } + defer doc.Delete(ctx) + snap, err := doc.Get(ctx) if err != nil { t.Fatal(err) @@ -102,9 +103,6 @@ func TestFirestoreWithDatabase(t *testing.T) { if !reflect.DeepEqual(snap.Data(), cityData) { t.Errorf("Get() = %v; want %v", snap.Data(), cityData) } - if _, err := doc.Delete(ctx); err != nil { - t.Fatal(err) - } } func TestFirestoreMultiDB(t *testing.T) { @@ -134,9 +132,12 @@ func TestFirestoreMultiDB(t *testing.T) { if _, err := cityDoc.Set(ctx, cityData); err != nil { t.Fatal(err) } + defer cityDoc.Delete(ctx) + if _, err := movieDoc.Set(ctx, movieData); err != nil { t.Fatal(err) } + defer movieDoc.Delete(ctx) citySnap, err := cityDoc.Get(ctx) if err != nil { @@ -153,13 +154,6 @@ func TestFirestoreMultiDB(t *testing.T) { if !reflect.DeepEqual(movieSnap.Data(), movieData) { t.Errorf("Movie Get() = %v; want %v", movieSnap.Data(), movieData) } - - if _, err := cityDoc.Delete(ctx); err != nil { - t.Fatal(err) - } - if _, err := movieDoc.Delete(ctx); err != nil { - t.Fatal(err) - } } func TestServerTimestamp(t *testing.T) { @@ -186,6 +180,8 @@ func TestServerTimestamp(t *testing.T) { if _, err := doc.Set(ctx, data); err != nil { t.Fatal(err) } + defer doc.Delete(ctx) + snap, err := doc.Get(ctx) if err != nil { t.Fatal(err) @@ -197,7 +193,4 @@ func TestServerTimestamp(t *testing.T) { if _, ok := got["timestamp"].(time.Time); !ok { t.Errorf("Timestamp is not a time.Time: %v", got["timestamp"]) } - if _, err := doc.Delete(ctx); err != nil { - t.Fatal(err) - } } From e5e21e57fc1889b566af0652d4ff90d15a27d9bc Mon Sep 17 00:00:00 2001 From: jonathanedey Date: Mon, 8 Dec 2025 14:18:15 -0500 Subject: [PATCH 3/6] fix: gemini review --- integration/firestore/firestore_test.go | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/integration/firestore/firestore_test.go b/integration/firestore/firestore_test.go index 873d0092..20e93818 100644 --- a/integration/firestore/firestore_test.go +++ b/integration/firestore/firestore_test.go @@ -16,7 +16,9 @@ package firestore import ( "context" + "flag" "log" + "os" "reflect" "testing" @@ -41,11 +43,16 @@ var ( } ) -func TestFirestore(t *testing.T) { +func TestMain(m *testing.M) { + flag.Parse() if testing.Short() { log.Println("skipping Firestore integration tests in short mode.") - return + os.Exit(0) } + os.Exit(m.Run()) +} + +func TestFirestore(t *testing.T) { ctx := context.Background() app, err := internal.NewTestApp(ctx, nil) if err != nil { @@ -73,10 +80,6 @@ func TestFirestore(t *testing.T) { } func TestFirestoreWithDatabase(t *testing.T) { - if testing.Short() { - log.Println("skipping Firestore integration tests in short mode.") - return - } ctx := context.Background() app, err := internal.NewTestApp(ctx, nil) if err != nil { @@ -106,10 +109,6 @@ func TestFirestoreWithDatabase(t *testing.T) { } func TestFirestoreMultiDB(t *testing.T) { - if testing.Short() { - log.Println("skipping Firestore integration tests in short mode.") - return - } ctx := context.Background() app, err := internal.NewTestApp(ctx, nil) if err != nil { @@ -157,10 +156,6 @@ func TestFirestoreMultiDB(t *testing.T) { } func TestServerTimestamp(t *testing.T) { - if testing.Short() { - log.Println("skipping Firestore integration tests in short mode.") - return - } ctx := context.Background() app, err := internal.NewTestApp(ctx, nil) if err != nil { From a8f72e5f23d19c2c68a3a33a3019ba83ccbc84d8 Mon Sep 17 00:00:00 2001 From: jonathanedey Date: Mon, 8 Dec 2025 17:17:17 -0500 Subject: [PATCH 4/6] fix: Update name from API review --- firebase.go | 6 +++--- firebase_test.go | 10 +++++----- integration/firestore/firestore_test.go | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/firebase.go b/firebase.go index edb76937..33a0ae65 100644 --- a/firebase.go +++ b/firebase.go @@ -105,12 +105,12 @@ func (a *App) Storage(ctx context.Context) (*storage.Client, error) { // Firestore returns a new firestore.Client instance from the https://godoc.org/cloud.google.com/go/firestore // package. func (a *App) Firestore(ctx context.Context) (*firestore.Client, error) { - return a.FirestoreWithDatabase(ctx, firestore.DefaultDatabaseID) + return a.FirestoreWithDatabaseID(ctx, firestore.DefaultDatabaseID) } -// FirestoreWithDatabase returns a new firestore.Client instance with the specified named database from the +// FirestoreWithDatabaseID returns a new firestore.Client instance with the specified named database from the // https://godoc.org/cloud.google.com/go/firestore package. -func (a *App) FirestoreWithDatabase(ctx context.Context, databaseID string) (*firestore.Client, error) { +func (a *App) FirestoreWithDatabaseID(ctx context.Context, databaseID string) (*firestore.Client, error) { if a.projectID == "" { return nil, errors.New("project id is required to access Firestore") } diff --git a/firebase_test.go b/firebase_test.go index 8225cf5a..ddc130a1 100644 --- a/firebase_test.go +++ b/firebase_test.go @@ -287,15 +287,15 @@ func TestFirestore(t *testing.T) { } } -func TestFirestoreWithDatabase(t *testing.T) { +func TestFirestoreWithDatabaseID(t *testing.T) { ctx := context.Background() app, err := NewApp(ctx, nil, option.WithCredentialsFile("testdata/service_account.json")) if err != nil { t.Fatal(err) } - if c, err := app.FirestoreWithDatabase(ctx, "other-db"); c == nil || err != nil { - t.Errorf("FirestoreWithDatabase() = (%v, %v); want (client, nil)", c, err) + if c, err := app.FirestoreWithDatabaseID(ctx, "other-db"); c == nil || err != nil { + t.Errorf("FirestoreWithDatabaseID() = (%v, %v); want (client, nil)", c, err) } } @@ -349,8 +349,8 @@ func TestFirestoreWithNoProjectID(t *testing.T) { t.Errorf("Firestore() = (%v, %v); want (nil, error)", c, err) } - if c, err := app.FirestoreWithDatabase(ctx, "other-db"); c != nil || err == nil { - t.Errorf("FirestoreWithDatabase() = (%v, %v); want (nil, error)", c, err) + if c, err := app.FirestoreWithDatabaseID(ctx, "other-db"); c != nil || err == nil { + t.Errorf("FirestoreWithDatabaseID() = (%v, %v); want (nil, error)", c, err) } } diff --git a/integration/firestore/firestore_test.go b/integration/firestore/firestore_test.go index 20e93818..6f468e1c 100644 --- a/integration/firestore/firestore_test.go +++ b/integration/firestore/firestore_test.go @@ -79,7 +79,7 @@ func TestFirestore(t *testing.T) { } } -func TestFirestoreWithDatabase(t *testing.T) { +func TestFirestoreWithDatabaseID(t *testing.T) { ctx := context.Background() app, err := internal.NewTestApp(ctx, nil) if err != nil { @@ -88,7 +88,7 @@ func TestFirestoreWithDatabase(t *testing.T) { // This test requires a non-default database to exist in the project. // If it doesn't exist, this test will fail. - client, err := app.FirestoreWithDatabase(ctx, "testing-database") + client, err := app.FirestoreWithDatabaseID(ctx, "testing-database") if err != nil { t.Fatal(err) } @@ -120,7 +120,7 @@ func TestFirestoreMultiDB(t *testing.T) { t.Fatal(err) } // This test requires a non-default database to exist in the project. - movieClient, err := app.FirestoreWithDatabase(ctx, "testing-database") + movieClient, err := app.FirestoreWithDatabaseID(ctx, "testing-database") if err != nil { t.Fatal(err) } From 52d89af9f78921adc336f8f806e5a810155cf3aa Mon Sep 17 00:00:00 2001 From: jonathanedey Date: Tue, 9 Dec 2025 15:30:03 -0500 Subject: [PATCH 5/6] fix: Address review comments --- CONTRIBUTING.md | 8 +++- integration/firestore/firestore_test.go | 49 ++++--------------------- 2 files changed, 13 insertions(+), 44 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index eacfcda4..df96fae1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -148,8 +148,12 @@ Set up your Firebase project as follows: 2. Enable Firestore: 1. Go to the Firebase Console, and select **Firestore Database** from the **Build** menu. - 2. Click on the **Create database** button. You can choose to set up Firestore either in - the production mode or in the test mode. + 2. Click on the **Create database** button and create a default database. You can choose + to set up Firestore either in the production mode or in the test mode. + > **Note:** Integration tests are run against both the default database and an additional + database named "testing-database". + 3. After the bedault datbase is created, click the **Add database** button to create a + second database named "testing-database". 3. Enable Realtime Database: diff --git a/integration/firestore/firestore_test.go b/integration/firestore/firestore_test.go index 6f468e1c..3300f882 100644 --- a/integration/firestore/firestore_test.go +++ b/integration/firestore/firestore_test.go @@ -22,12 +22,11 @@ import ( "reflect" "testing" - "time" - - "cloud.google.com/go/firestore" "firebase.google.com/go/v4/integration/internal" ) +const testDatabaseID = "testing-database" + var ( cityData = map[string]interface{}{ "name": "Mountain View", @@ -86,9 +85,9 @@ func TestFirestoreWithDatabaseID(t *testing.T) { t.Fatal(err) } - // This test requires a non-default database to exist in the project. + // This test requires the target non-default database to exist in the project. // If it doesn't exist, this test will fail. - client, err := app.FirestoreWithDatabaseID(ctx, "testing-database") + client, err := app.FirestoreWithDatabaseID(ctx, testDatabaseID) if err != nil { t.Fatal(err) } @@ -119,8 +118,9 @@ func TestFirestoreMultiDB(t *testing.T) { if err != nil { t.Fatal(err) } - // This test requires a non-default database to exist in the project. - movieClient, err := app.FirestoreWithDatabaseID(ctx, "testing-database") + // This test requires the target non-default database to exist in the project. + // If it doesn't exist, this test will fail. + movieClient, err := app.FirestoreWithDatabaseID(ctx, testDatabaseID) if err != nil { t.Fatal(err) } @@ -154,38 +154,3 @@ func TestFirestoreMultiDB(t *testing.T) { t.Errorf("Movie Get() = %v; want %v", movieSnap.Data(), movieData) } } - -func TestServerTimestamp(t *testing.T) { - ctx := context.Background() - app, err := internal.NewTestApp(ctx, nil) - if err != nil { - t.Fatal(err) - } - - client, err := app.Firestore(ctx) - if err != nil { - t.Fatal(err) - } - - doc := client.Collection("cities").NewDoc() - data := map[string]interface{}{ - "name": "Mountain View", - "timestamp": firestore.ServerTimestamp, - } - if _, err := doc.Set(ctx, data); err != nil { - t.Fatal(err) - } - defer doc.Delete(ctx) - - snap, err := doc.Get(ctx) - if err != nil { - t.Fatal(err) - } - got := snap.Data() - if got["name"] != "Mountain View" { - t.Errorf("Name = %v; want Mountain View", got["name"]) - } - if _, ok := got["timestamp"].(time.Time); !ok { - t.Errorf("Timestamp is not a time.Time: %v", got["timestamp"]) - } -} From ed2bea1204b762892e6907d8eb6c3a6d7ac257db Mon Sep 17 00:00:00 2001 From: Jonathan Edey <145066863+jonathanedey@users.noreply.github.com> Date: Wed, 10 Dec 2025 13:47:14 -0500 Subject: [PATCH 6/6] chore: Fix CONTRIBUTING.md typo Co-authored-by: Lahiru Maramba --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index df96fae1..901fad26 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -152,7 +152,7 @@ Set up your Firebase project as follows: to set up Firestore either in the production mode or in the test mode. > **Note:** Integration tests are run against both the default database and an additional database named "testing-database". - 3. After the bedault datbase is created, click the **Add database** button to create a + 3. After the default database is created, click the **Add database** button to create a second database named "testing-database".