From 76cd8ecfd76932be2f97c2213747d43edfc24ac7 Mon Sep 17 00:00:00 2001 From: Baha Aiman Date: Thu, 29 Jun 2023 11:37:10 -0700 Subject: [PATCH 1/5] feat(datastore): Multiple databases support --- datastore/datastore.go | 44 +++++++++++++++++++++++++++++-------- datastore/datastore_test.go | 31 ++++++++++++++++++++++++++ datastore/key.go | 5 +++-- datastore/query.go | 6 +++-- datastore/transaction.go | 7 +++++- 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/datastore/datastore.go b/datastore/datastore.go index f32522ebb36b..04170857d69b 100644 --- a/datastore/datastore.go +++ b/datastore/datastore.go @@ -58,6 +58,7 @@ type Client struct { connPool gtransport.ConnPool client pb.DatastoreClient dataset string // Called dataset by the datastore API, synonym for project ID. + databaseID string // Default value is empty string readSettings *readSettings } @@ -128,6 +129,27 @@ func NewClient(ctx context.Context, projectID string, opts ...option.ClientOptio }, nil } +// NewClientWithDatabase creates a new Client for given dataset and database. If the project ID is +// empty, it is derived from the DATASTORE_PROJECT_ID environment variable. +// If the DATASTORE_EMULATOR_HOST environment variable is set, client will use +// its value to connect to a locally-running datastore emulator. +// DetectProjectID can be passed as the projectID argument to instruct +// NewClientWithDatabase to detect the project ID from the credentials. +// Call (*Client).Close() when done with the client. +func NewClientWithDatabase(ctx context.Context, projectID, databaseID string, opts ...option.ClientOption) (*Client, error) { + if databaseID == "" { + return nil, errors.New("firestore: databaseID is empty") + } + + client, err := NewClient(ctx, projectID, opts...) + if err != nil { + return nil, err + } + + client.databaseID = databaseID + return client, nil +} + func detectProjectID(ctx context.Context, opts ...option.ClientOption) (string, error) { creds, err := transport.Creds(ctx, opts...) if err != nil { @@ -468,6 +490,7 @@ func (c *Client) get(ctx context.Context, keys []*Key, dst interface{}, opts *pb } req := &pb.LookupRequest{ ProjectId: c.dataset, + DatabaseId: c.databaseID, Keys: pbKeys, ReadOptions: opts, } @@ -565,9 +588,10 @@ func (c *Client) PutMulti(ctx context.Context, keys []*Key, src interface{}) (re // Make the request. req := &pb.CommitRequest{ - ProjectId: c.dataset, - Mutations: mutations, - Mode: pb.CommitRequest_NON_TRANSACTIONAL, + ProjectId: c.dataset, + DatabaseId: c.databaseID, + Mutations: mutations, + Mode: pb.CommitRequest_NON_TRANSACTIONAL, } resp, err := c.client.Commit(ctx, req) if err != nil { @@ -688,9 +712,10 @@ func (c *Client) DeleteMulti(ctx context.Context, keys []*Key) (err error) { } req := &pb.CommitRequest{ - ProjectId: c.dataset, - Mutations: mutations, - Mode: pb.CommitRequest_NON_TRANSACTIONAL, + ProjectId: c.dataset, + DatabaseId: c.databaseID, + Mutations: mutations, + Mode: pb.CommitRequest_NON_TRANSACTIONAL, } _, err = c.client.Commit(ctx, req) return err @@ -740,9 +765,10 @@ func (c *Client) Mutate(ctx context.Context, muts ...*Mutation) (ret []*Key, err return nil, err } req := &pb.CommitRequest{ - ProjectId: c.dataset, - Mutations: pmuts, - Mode: pb.CommitRequest_NON_TRANSACTIONAL, + ProjectId: c.dataset, + DatabaseId: c.databaseID, + Mutations: pmuts, + Mode: pb.CommitRequest_NON_TRANSACTIONAL, } resp, err := c.client.Commit(ctx, req) if err != nil { diff --git a/datastore/datastore_test.go b/datastore/datastore_test.go index 5fde4e718861..e29e5d653ea9 100644 --- a/datastore/datastore_test.go +++ b/datastore/datastore_test.go @@ -29,6 +29,37 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" ) +func TestNewClientWithDatabase(t *testing.T) { + tests := []struct { + desc string + databaseId string + wantErr string + }{ + { + desc: "Emmpty databaseID", + databaseId: "", + wantErr: "firestore: databaseID is empty", + }, + } + + for _, test := range tests { + _, gotErr := NewClientWithDatabase(context.Background(), "my-project", "") + if gotErr != nil && test.wantErr == "" { + t.Errorf("%s: got error %v, but none was expected", test.desc, gotErr) + continue + } + if gotErr == nil && test.wantErr != "" { + t.Errorf("%s: wanted error, but none returned", test.desc) + continue + } + + if gotErr.Error() != test.wantErr { + t.Errorf("%s: error mismatch: got %q want something containing %q", test.desc, gotErr, test.wantErr) + continue + } + } +} + func TestQueryConstruction(t *testing.T) { tests := []struct { q, exp *Query diff --git a/datastore/key.go b/datastore/key.go index a25edc88fd94..5807eedd30a6 100644 --- a/datastore/key.go +++ b/datastore/key.go @@ -247,8 +247,9 @@ func (c *Client) AllocateIDs(ctx context.Context, keys []*Key) ([]*Key, error) { } req := &pb.AllocateIdsRequest{ - ProjectId: c.dataset, - Keys: multiKeyToProto(keys), + ProjectId: c.dataset, + DatabaseId: c.databaseID, + Keys: multiKeyToProto(keys), } resp, err := c.client.AllocateIds(ctx, req) if err != nil { diff --git a/datastore/query.go b/datastore/query.go index fb83f1c2cfea..8c087998ba21 100644 --- a/datastore/query.go +++ b/datastore/query.go @@ -727,7 +727,8 @@ func (c *Client) Run(ctx context.Context, q *Query) *Iterator { pageCursor: q.start, entityCursor: q.start, req: &pb.RunQueryRequest{ - ProjectId: c.dataset, + ProjectId: c.dataset, + DatabaseId: c.databaseID, }, } @@ -763,7 +764,8 @@ func (c *Client) RunAggregationQuery(ctx context.Context, aq *AggregationQuery) } req := &pb.RunAggregationQueryRequest{ - ProjectId: c.dataset, + ProjectId: c.dataset, + DatabaseId: c.databaseID, QueryType: &pb.RunAggregationQueryRequest_AggregationQuery{ AggregationQuery: &pb.AggregationQuery{ QueryType: &pb.AggregationQuery_NestedQuery{ diff --git a/datastore/transaction.go b/datastore/transaction.go index 0f2c1686afe0..579ab56adcac 100644 --- a/datastore/transaction.go +++ b/datastore/transaction.go @@ -130,7 +130,10 @@ func (c *Client) NewTransaction(ctx context.Context, opts ...TransactionOption) } func (c *Client) newTransaction(ctx context.Context, s *transactionSettings) (_ *Transaction, err error) { - req := &pb.BeginTransactionRequest{ProjectId: c.dataset} + req := &pb.BeginTransactionRequest{ + ProjectId: c.dataset, + DatabaseId: c.databaseID, + } if s.readOnly { ctx = trace.StartSpan(ctx, "cloud.google.com/go/datastore.Transaction.ReadOnlyTransaction") defer func() { trace.EndSpan(ctx, err) }() @@ -225,6 +228,7 @@ func (t *Transaction) Commit() (c *Commit, err error) { } req := &pb.CommitRequest{ ProjectId: t.client.dataset, + DatabaseId: t.client.databaseID, TransactionSelector: &pb.CommitRequest_Transaction{Transaction: t.id}, Mutations: t.mutations, Mode: pb.CommitRequest_TRANSACTIONAL, @@ -267,6 +271,7 @@ func (t *Transaction) Rollback() (err error) { t.id = nil _, err = t.client.client.Rollback(t.ctx, &pb.RollbackRequest{ ProjectId: t.client.dataset, + DatabaseId: t.client.databaseID, Transaction: id, }) return err From d5222063e01f8de1af1641dfd0376f179510328f Mon Sep 17 00:00:00 2001 From: Baha Aiman Date: Mon, 17 Jul 2023 17:54:08 -0700 Subject: [PATCH 2/5] feat(datastore): Multi DB support --- CONTRIBUTING.md | 10 ++++-- datastore/client.go | 8 +++-- datastore/datastore.go | 53 ++++++++++++++++--------------- datastore/datastore_test.go | 59 ++++++++++++++++++++++++----------- datastore/integration_test.go | 53 +++++++++++++++++++++++++------ internal/kokoro/continuous.sh | 1 + 6 files changed, 127 insertions(+), 57 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6d6e48b65bd7..69c31dbb0642 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -125,6 +125,7 @@ variables: bamboo-shift-455) for the general project. - `GCLOUD_TESTS_GOLANG_KEY`: The path to the JSON key file of the general project's service account. +- `GCLOUD_TESTS_GOLANG_DATASTORE_DATABASES`: Comma separated list of developer's Datastore databases. If not provided, default database i.e. empty string is used. - `GCLOUD_TESTS_GOLANG_FIRESTORE_PROJECT_ID`: Developers Console project's ID (e.g. doorway-cliff-677) for the Firestore project. - `GCLOUD_TESTS_GOLANG_FIRESTORE_KEY`: The path to the JSON key file of the @@ -153,8 +154,9 @@ $ gcloud config set project $GCLOUD_TESTS_GOLANG_PROJECT_ID # Authenticates the gcloud tool with your account. $ gcloud auth login -# Create the indexes used in the datastore integration tests. -$ gcloud datastore indexes create datastore/testdata/index.yaml +# Create the indexes for all the databases you want to use in the datastore integration tests. +# Use empty string as databaseID or skip database flag for default database. +$ gcloud alpha datastore indexes create --database=your-databaseID-1 --project=cndb-sdk-golang-general testdata/index.yaml # Creates a Google Cloud storage bucket with the same name as your test project, # and with the Cloud Logging service account as owner, for the sink @@ -219,6 +221,10 @@ export GCLOUD_TESTS_GOLANG_PROJECT_ID=your-project # The path to the JSON key file of the general project's service account. export GCLOUD_TESTS_GOLANG_KEY=~/directory/your-project-abcd1234.json +# Comma separated list of developer's Datastore databases. If not provided, +# default database i.e. empty string is used. +export GCLOUD_TESTS_GOLANG_DATASTORE_DATABASES=your-database-1,your-database-2 + # Developers Console project's ID (e.g. doorway-cliff-677) for the Firestore project. export GCLOUD_TESTS_GOLANG_FIRESTORE_PROJECT_ID=your-firestore-project diff --git a/datastore/client.go b/datastore/client.go index c10791294f7e..a6c3c540e992 100644 --- a/datastore/client.go +++ b/datastore/client.go @@ -42,11 +42,15 @@ type datastoreClient struct { md metadata.MD } -func newDatastoreClient(conn grpc.ClientConnInterface, projectID string) pb.DatastoreClient { +func newDatastoreClient(conn grpc.ClientConnInterface, projectID, databaseID string) pb.DatastoreClient { + resourcePrefixHeaderVal := "projects/" + projectID + if databaseID != "" { + resourcePrefixHeaderVal += "/databases/" + databaseID + } return &datastoreClient{ c: pb.NewDatastoreClient(conn), md: metadata.Pairs( - resourcePrefixHeader, "projects/"+projectID, + resourcePrefixHeader, resourcePrefixHeaderVal, "x-goog-api-client", fmt.Sprintf("gl-go/%s gccl/%s grpc/", version.Go(), internal.Version)), } } diff --git a/datastore/datastore.go b/datastore/datastore.go index 04170857d69b..24fcc2102ebb 100644 --- a/datastore/datastore.go +++ b/datastore/datastore.go @@ -53,6 +53,14 @@ const DetectProjectID = "*detect-project-id*" // the resource being operated on. const resourcePrefixHeader = "google-cloud-resource-prefix" +// DefaultDatabaseID is ID of the default database +const DefaultDatabaseID = "" + +var ( + gtransportDialPoolFn = gtransport.DialPool + detectProjectIDFn = detectProjectID +) + // Client is a client for reading and writing data in a datastore dataset. type Client struct { connPool gtransport.ConnPool @@ -70,6 +78,21 @@ type Client struct { // NewClient to detect the project ID from the credentials. // Call (*Client).Close() when done with the client. func NewClient(ctx context.Context, projectID string, opts ...option.ClientOption) (*Client, error) { + client, err := NewClientWithDatabase(ctx, projectID, DefaultDatabaseID, opts...) + if err != nil { + return nil, err + } + return client, nil +} + +// NewClientWithDatabase creates a new Client for given dataset and database. If the project ID is +// empty, it is derived from the DATASTORE_PROJECT_ID environment variable. +// If the DATASTORE_EMULATOR_HOST environment variable is set, client will use +// its value to connect to a locally-running datastore emulator. +// DetectProjectID can be passed as the projectID argument to instruct +// NewClientWithDatabase to detect the project ID from the credentials. +// Call (*Client).Close() when done with the client. +func NewClientWithDatabase(ctx context.Context, projectID, databaseID string, opts ...option.ClientOption) (*Client, error) { var o []option.ClientOption // Environment variables for gcd emulator: // https://cloud.google.com/datastore/docs/tools/datastore-emulator @@ -81,7 +104,7 @@ func NewClient(ctx context.Context, projectID string, opts ...option.ClientOptio option.WithGRPCDialOption(grpc.WithInsecure()), } if projectID == DetectProjectID { - projectID, _ = detectProjectID(ctx, opts...) + projectID, _ = detectProjectIDFn(ctx, opts...) if projectID == "" { projectID = "dummy-emulator-datastore-project" } @@ -107,7 +130,7 @@ func NewClient(ctx context.Context, projectID string, opts ...option.ClientOptio o = append(o, opts...) if projectID == DetectProjectID { - detected, err := detectProjectID(ctx, opts...) + detected, err := detectProjectIDFn(ctx, opts...) if err != nil { return nil, err } @@ -117,39 +140,19 @@ func NewClient(ctx context.Context, projectID string, opts ...option.ClientOptio if projectID == "" { return nil, errors.New("datastore: missing project/dataset id") } - connPool, err := gtransport.DialPool(ctx, o...) + connPool, err := gtransportDialPoolFn(ctx, o...) if err != nil { return nil, fmt.Errorf("dialing: %w", err) } return &Client{ connPool: connPool, - client: newDatastoreClient(connPool, projectID), + client: newDatastoreClient(connPool, projectID, databaseID), dataset: projectID, readSettings: &readSettings{}, + databaseID: databaseID, }, nil } -// NewClientWithDatabase creates a new Client for given dataset and database. If the project ID is -// empty, it is derived from the DATASTORE_PROJECT_ID environment variable. -// If the DATASTORE_EMULATOR_HOST environment variable is set, client will use -// its value to connect to a locally-running datastore emulator. -// DetectProjectID can be passed as the projectID argument to instruct -// NewClientWithDatabase to detect the project ID from the credentials. -// Call (*Client).Close() when done with the client. -func NewClientWithDatabase(ctx context.Context, projectID, databaseID string, opts ...option.ClientOption) (*Client, error) { - if databaseID == "" { - return nil, errors.New("firestore: databaseID is empty") - } - - client, err := NewClient(ctx, projectID, opts...) - if err != nil { - return nil, err - } - - client.databaseID = databaseID - return client, nil -} - func detectProjectID(ctx context.Context, opts ...option.ClientOption) (string, error) { creds, err := transport.Creds(ctx, opts...) if err != nil { diff --git a/datastore/datastore_test.go b/datastore/datastore_test.go index e29e5d653ea9..e44c4a49ecef 100644 --- a/datastore/datastore_test.go +++ b/datastore/datastore_test.go @@ -24,38 +24,59 @@ import ( "cloud.google.com/go/internal/testutil" "github.com/google/go-cmp/cmp" + "google.golang.org/api/option" + "google.golang.org/api/transport/grpc" pb "google.golang.org/genproto/googleapis/datastore/v1" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/timestamppb" ) func TestNewClientWithDatabase(t *testing.T) { + origDetectProjectIDFn := detectProjectIDFn + origGtransportDialPoolFn := gtransportDialPoolFn + defer func() { + detectProjectIDFn = origDetectProjectIDFn + gtransportDialPoolFn = origGtransportDialPoolFn + }() + tests := []struct { - desc string - databaseId string - wantErr string + desc string + databaseId string + wantErr bool + detectProjectIDFn func(ctx context.Context, opts ...option.ClientOption) (string, error) + gtransportDialPoolFn func(ctx context.Context, opts ...option.ClientOption) (grpc.ConnPool, error) }{ { - desc: "Emmpty databaseID", - databaseId: "", - wantErr: "firestore: databaseID is empty", + desc: "Error from detect project ID should not fail NewClientWithDatabase", + databaseId: "db1", + wantErr: false, + detectProjectIDFn: func(ctx context.Context, opts ...option.ClientOption) (string, error) { + return "", errors.New("mock error from detect project ID") + }, + gtransportDialPoolFn: origGtransportDialPoolFn, + }, + { + desc: "Error from DialPool", + databaseId: "db1", + wantErr: true, + detectProjectIDFn: origDetectProjectIDFn, + gtransportDialPoolFn: func(ctx context.Context, opts ...option.ClientOption) (grpc.ConnPool, error) { + return nil, errors.New("mock error from DialPool") + }, }, } - for _, test := range tests { - _, gotErr := NewClientWithDatabase(context.Background(), "my-project", "") - if gotErr != nil && test.wantErr == "" { - t.Errorf("%s: got error %v, but none was expected", test.desc, gotErr) - continue - } - if gotErr == nil && test.wantErr != "" { - t.Errorf("%s: wanted error, but none returned", test.desc) - continue - } + for _, tc := range tests { + detectProjectIDFn = tc.detectProjectIDFn + gtransportDialPoolFn = tc.gtransportDialPoolFn - if gotErr.Error() != test.wantErr { - t.Errorf("%s: error mismatch: got %q want something containing %q", test.desc, gotErr, test.wantErr) - continue + client, gotErr := NewClientWithDatabase(context.Background(), "my-project", tc.databaseId) + if gotErr != nil && !tc.wantErr { + t.Errorf("%s: got error %v, but none was expected", tc.desc, gotErr) + } else if gotErr == nil && tc.wantErr { + t.Errorf("%s: wanted error, but none returned", tc.desc) + } else if gotErr == nil && client.databaseID != tc.databaseId { + t.Errorf("%s: got %s, want %s", tc.desc, client.databaseID, tc.databaseId) } } } diff --git a/datastore/integration_test.go b/datastore/integration_test.go index 39783578e1a2..5e2ca39b8bb4 100644 --- a/datastore/integration_test.go +++ b/datastore/integration_test.go @@ -49,7 +49,10 @@ var timeNow = time.Now() // when the tests are run in parallel. var suffix string -const replayFilename = "datastore.replay" +const ( + replayFilename = "datastore.replay" + envDatabases = "GCLOUD_TESTS_GOLANG_DATASTORE_DATABASES" +) type replayInfo struct { ProjectID string @@ -62,6 +65,7 @@ var ( newTestClient = func(ctx context.Context, t *testing.T) *Client { return newClient(ctx, t, nil) } + testParams map[string]interface{} ) func TestMain(m *testing.M) { @@ -103,7 +107,25 @@ func testMain(m *testing.M) int { log.Printf("recording to %s", replayFilename) } suffix = fmt.Sprintf("-t%d", timeNow.UnixNano()) - return m.Run() + + // Run tests on multiple databases + databasesStr, ok := os.LookupEnv(envDatabases) + if !ok { + databasesStr = "" + } + databaseIDs := strings.Split(databasesStr, ",") + testParams = make(map[string]interface{}) + + for _, databaseID := range databaseIDs { + log.Printf("Setting up tests to run on databaseID: %q\n", databaseID) + testParams["databaseID"] = databaseID + status := m.Run() + if status != 0 { + return status + } + } + + return 0 } func initReplay() { @@ -133,9 +155,9 @@ func initReplay() { } opts := append(grpcHeadersEnforcer.CallOptions(), option.WithGRPCConn(conn)) - client, err := NewClient(ctx, ri.ProjectID, opts...) + client, err := NewClientWithDatabase(ctx, ri.ProjectID, testParams["databaseID"].(string), opts...) if err != nil { - t.Fatalf("NewClient: %v", err) + t.Fatalf("NewClientWithDatabase: %v", err) } return client } @@ -161,13 +183,26 @@ func newClient(ctx context.Context, t *testing.T, dialOpts []grpc.DialOption) *C for _, opt := range dialOpts { opts = append(opts, option.WithGRPCDialOption(opt)) } - client, err := NewClient(ctx, testutil.ProjID(), opts...) + client, err := NewClientWithDatabase(ctx, testutil.ProjID(), testParams["databaseID"].(string), opts...) if err != nil { - t.Fatalf("NewClient: %v", err) + t.Fatalf("NewClientWithDatabase: %v", err) } return client } +func TestIntegration_NewClient(t *testing.T) { + if testing.Short() { + t.Skip("Integration tests skipped in short mode") + } + ctx := context.Background() + client, err := NewClient(ctx, testutil.ProjID()) + if err != nil { + t.Errorf("NewClient: %v", err) + } else if client.databaseID != DefaultDatabaseID { + t.Errorf("NewClient: got %s, want %s", client.databaseID, DefaultDatabaseID) + } +} + func TestIntegration_Basics(t *testing.T) { ctx, _ := context.WithTimeout(context.Background(), time.Second*20) client := newTestClient(ctx, t) @@ -1487,13 +1522,13 @@ func TestIntegration_DetectProjectID(t *testing.T) { } // Use creds with project ID. - if _, err := NewClient(ctx, DetectProjectID, option.WithCredentials(creds)); err != nil { - t.Errorf("NewClient: %v", err) + if _, err := NewClientWithDatabase(ctx, DetectProjectID, testParams["databaseID"].(string), option.WithCredentials(creds)); err != nil { + t.Errorf("NewClientWithDatabase: %v", err) } ts := testutil.ErroringTokenSource{} // Try to use creds without project ID. - _, err := NewClient(ctx, DetectProjectID, option.WithTokenSource(ts)) + _, err := NewClientWithDatabase(ctx, DetectProjectID, testParams["databaseID"].(string), option.WithTokenSource(ts)) if err == nil || err.Error() != "datastore: see the docs on DetectProjectID" { t.Errorf("expected an error while using TokenSource that does not have a project ID") } diff --git a/internal/kokoro/continuous.sh b/internal/kokoro/continuous.sh index b9e5f0472a78..62c1c43bb881 100755 --- a/internal/kokoro/continuous.sh +++ b/internal/kokoro/continuous.sh @@ -31,6 +31,7 @@ export GOOGLE_APPLICATION_CREDENTIALS=$KOKORO_KEYSTORE_DIR/72523_go_integration_ export GCLOUD_TESTS_GOLANG_PROJECT_ID=dulcet-port-762 export GCLOUD_TESTS_GOLANG_KEY=$GOOGLE_APPLICATION_CREDENTIALS +export GCLOUD_TESTS_GOLANG_DATASTORE_DATABASES=,database-01 export GCLOUD_TESTS_GOLANG_FIRESTORE_PROJECT_ID=gcloud-golang-firestore-tests export GCLOUD_TESTS_GOLANG_FIRESTORE_KEY=$KOKORO_KEYSTORE_DIR/72523_go_firestore_integration_service_account export GCLOUD_TESTS_API_KEY=`cat $KOKORO_KEYSTORE_DIR/72523_go_gcloud_tests_api_key` From cc8cbc14e5f21033eeabe844daeb4a6385656215 Mon Sep 17 00:00:00 2001 From: Baha Aiman Date: Mon, 17 Jul 2023 22:16:05 -0700 Subject: [PATCH 3/5] feat(datastore): Multi DB support --- datastore/datastore_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/datastore/datastore_test.go b/datastore/datastore_test.go index e44c4a49ecef..3ffdd60f76ed 100644 --- a/datastore/datastore_test.go +++ b/datastore/datastore_test.go @@ -41,14 +41,14 @@ func TestNewClientWithDatabase(t *testing.T) { tests := []struct { desc string - databaseId string + databaseID string wantErr bool detectProjectIDFn func(ctx context.Context, opts ...option.ClientOption) (string, error) gtransportDialPoolFn func(ctx context.Context, opts ...option.ClientOption) (grpc.ConnPool, error) }{ { desc: "Error from detect project ID should not fail NewClientWithDatabase", - databaseId: "db1", + databaseID: "db1", wantErr: false, detectProjectIDFn: func(ctx context.Context, opts ...option.ClientOption) (string, error) { return "", errors.New("mock error from detect project ID") @@ -57,7 +57,7 @@ func TestNewClientWithDatabase(t *testing.T) { }, { desc: "Error from DialPool", - databaseId: "db1", + databaseID: "db1", wantErr: true, detectProjectIDFn: origDetectProjectIDFn, gtransportDialPoolFn: func(ctx context.Context, opts ...option.ClientOption) (grpc.ConnPool, error) { @@ -70,13 +70,13 @@ func TestNewClientWithDatabase(t *testing.T) { detectProjectIDFn = tc.detectProjectIDFn gtransportDialPoolFn = tc.gtransportDialPoolFn - client, gotErr := NewClientWithDatabase(context.Background(), "my-project", tc.databaseId) + client, gotErr := NewClientWithDatabase(context.Background(), "my-project", tc.databaseID) if gotErr != nil && !tc.wantErr { t.Errorf("%s: got error %v, but none was expected", tc.desc, gotErr) } else if gotErr == nil && tc.wantErr { t.Errorf("%s: wanted error, but none returned", tc.desc) - } else if gotErr == nil && client.databaseID != tc.databaseId { - t.Errorf("%s: got %s, want %s", tc.desc, client.databaseID, tc.databaseId) + } else if gotErr == nil && client.databaseID != tc.databaseID { + t.Errorf("%s: got %s, want %s", tc.desc, client.databaseID, tc.databaseID) } } } From 2c7c92aa6f7e18ebf4f338c910a575cdb7651d38 Mon Sep 17 00:00:00 2001 From: Baha Aiman Date: Fri, 21 Jul 2023 12:22:30 -0700 Subject: [PATCH 4/5] feat(datastore): Correcting documentation --- datastore/client.go | 6 +++--- datastore/datastore.go | 6 +++--- datastore/datastore_test.go | 24 +++++++++++++----------- datastore/integration_test.go | 7 ++++--- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/datastore/client.go b/datastore/client.go index a6c3c540e992..fa0aa386e7bf 100644 --- a/datastore/client.go +++ b/datastore/client.go @@ -43,14 +43,14 @@ type datastoreClient struct { } func newDatastoreClient(conn grpc.ClientConnInterface, projectID, databaseID string) pb.DatastoreClient { - resourcePrefixHeaderVal := "projects/" + projectID + resourcePrefixValue := "projects/" + projectID if databaseID != "" { - resourcePrefixHeaderVal += "/databases/" + databaseID + resourcePrefixValue += "/databases/" + databaseID } return &datastoreClient{ c: pb.NewDatastoreClient(conn), md: metadata.Pairs( - resourcePrefixHeader, resourcePrefixHeaderVal, + resourcePrefixHeader, resourcePrefixValue, "x-goog-api-client", fmt.Sprintf("gl-go/%s gccl/%s grpc/", version.Go(), internal.Version)), } } diff --git a/datastore/datastore.go b/datastore/datastore.go index 24fcc2102ebb..e7fb14afa8ef 100644 --- a/datastore/datastore.go +++ b/datastore/datastore.go @@ -53,7 +53,7 @@ const DetectProjectID = "*detect-project-id*" // the resource being operated on. const resourcePrefixHeader = "google-cloud-resource-prefix" -// DefaultDatabaseID is ID of the default database +// DefaultDatabaseID is ID of the default database denoted by an empty string const DefaultDatabaseID = "" var ( @@ -85,8 +85,8 @@ func NewClient(ctx context.Context, projectID string, opts ...option.ClientOptio return client, nil } -// NewClientWithDatabase creates a new Client for given dataset and database. If the project ID is -// empty, it is derived from the DATASTORE_PROJECT_ID environment variable. +// NewClientWithDatabase creates a new Client for given dataset and database. +// If the project ID is empty, it is derived from the DATASTORE_PROJECT_ID environment variable. // If the DATASTORE_EMULATOR_HOST environment variable is set, client will use // its value to connect to a locally-running datastore emulator. // DetectProjectID can be passed as the projectID argument to instruct diff --git a/datastore/datastore_test.go b/datastore/datastore_test.go index 3ffdd60f76ed..550336319e53 100644 --- a/datastore/datastore_test.go +++ b/datastore/datastore_test.go @@ -67,17 +67,19 @@ func TestNewClientWithDatabase(t *testing.T) { } for _, tc := range tests { - detectProjectIDFn = tc.detectProjectIDFn - gtransportDialPoolFn = tc.gtransportDialPoolFn - - client, gotErr := NewClientWithDatabase(context.Background(), "my-project", tc.databaseID) - if gotErr != nil && !tc.wantErr { - t.Errorf("%s: got error %v, but none was expected", tc.desc, gotErr) - } else if gotErr == nil && tc.wantErr { - t.Errorf("%s: wanted error, but none returned", tc.desc) - } else if gotErr == nil && client.databaseID != tc.databaseID { - t.Errorf("%s: got %s, want %s", tc.desc, client.databaseID, tc.databaseID) - } + t.Run(tc.desc, func(t *testing.T) { + detectProjectIDFn = tc.detectProjectIDFn + gtransportDialPoolFn = tc.gtransportDialPoolFn + + client, gotErr := NewClientWithDatabase(context.Background(), "my-project", tc.databaseID) + if gotErr != nil && !tc.wantErr { + t.Errorf("got error %v, but none was expected", gotErr) + } else if gotErr == nil && tc.wantErr { + t.Errorf("wanted error, but none returned") + } else if gotErr == nil && client.databaseID != tc.databaseID { + t.Errorf("got %s, want %s", client.databaseID, tc.databaseID) + } + }) } } diff --git a/datastore/integration_test.go b/datastore/integration_test.go index 5e2ca39b8bb4..b16b26e41f47 100644 --- a/datastore/integration_test.go +++ b/datastore/integration_test.go @@ -197,9 +197,10 @@ func TestIntegration_NewClient(t *testing.T) { ctx := context.Background() client, err := NewClient(ctx, testutil.ProjID()) if err != nil { - t.Errorf("NewClient: %v", err) - } else if client.databaseID != DefaultDatabaseID { - t.Errorf("NewClient: got %s, want %s", client.databaseID, DefaultDatabaseID) + t.Fatalf("NewClient: %v", err) + } + if client.databaseID != DefaultDatabaseID { + t.Fatalf("NewClient: got %s, want %s", client.databaseID, DefaultDatabaseID) } } From 5849abc2a199b06f91a8e0f3ac7cb88eaa5f2c99 Mon Sep 17 00:00:00 2001 From: Baha Aiman Date: Mon, 24 Jul 2023 14:57:03 -0700 Subject: [PATCH 5/5] feat(datastore): Run integration tests on DBs --- datastore/integration_test.go | 8 ++++---- internal/kokoro/continuous.sh | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/datastore/integration_test.go b/datastore/integration_test.go index b16b26e41f47..44f1164e042e 100644 --- a/datastore/integration_test.go +++ b/datastore/integration_test.go @@ -109,13 +109,13 @@ func testMain(m *testing.M) int { suffix = fmt.Sprintf("-t%d", timeNow.UnixNano()) // Run tests on multiple databases + databaseIDs := []string{DefaultDatabaseID} databasesStr, ok := os.LookupEnv(envDatabases) - if !ok { - databasesStr = "" + if ok { + databaseIDs = append(databaseIDs, strings.Split(databasesStr, ",")...) } - databaseIDs := strings.Split(databasesStr, ",") - testParams = make(map[string]interface{}) + testParams = make(map[string]interface{}) for _, databaseID := range databaseIDs { log.Printf("Setting up tests to run on databaseID: %q\n", databaseID) testParams["databaseID"] = databaseID diff --git a/internal/kokoro/continuous.sh b/internal/kokoro/continuous.sh index 62c1c43bb881..a6342f82db98 100755 --- a/internal/kokoro/continuous.sh +++ b/internal/kokoro/continuous.sh @@ -31,7 +31,7 @@ export GOOGLE_APPLICATION_CREDENTIALS=$KOKORO_KEYSTORE_DIR/72523_go_integration_ export GCLOUD_TESTS_GOLANG_PROJECT_ID=dulcet-port-762 export GCLOUD_TESTS_GOLANG_KEY=$GOOGLE_APPLICATION_CREDENTIALS -export GCLOUD_TESTS_GOLANG_DATASTORE_DATABASES=,database-01 +export GCLOUD_TESTS_GOLANG_DATASTORE_DATABASES=database-01 export GCLOUD_TESTS_GOLANG_FIRESTORE_PROJECT_ID=gcloud-golang-firestore-tests export GCLOUD_TESTS_GOLANG_FIRESTORE_KEY=$KOKORO_KEYSTORE_DIR/72523_go_firestore_integration_service_account export GCLOUD_TESTS_API_KEY=`cat $KOKORO_KEYSTORE_DIR/72523_go_gcloud_tests_api_key`