feat: Support for using multiple databases in datastore (#1090)

* A simple test with multidb support

* A simple test with multidb support

Adds databaseId to the request options, client options and code for carrying over the client options to the request options as well as a test to make sure it actually works.

* Add additional checks with get

The test should include additional checks to ensure get is running the way it is supposed to.

* Add a test for saving with another database

* other datastore should delete the entity

Since we are saving things to another datastore we need to delete the data from that datastore.

* Add a test for the getDatastore method

The user needs a way to get the database Id from the client so that they can keep track of their clients more easily.

* Add a unit test for database id

Ensure that the database id gets passed all the way down to the generated layer and gets included in the request.

* Do an additional check for the sake of tests

We should check for the existance of datastore.options so that the code doesn’t fail if they are not provided at all.

* initialize generated client so it can be mocked

The test on kokoro is failing because in that environment a project id isn’t defined. The solution is to eliminate the get request so that a call never reaches the backend and initialize the gapic client so it can be mocked out and then catch any call that reaches the mock.

* comments and setDatabaseId

Add docs comments for newly added functions and add a test that ensures setDatabaseId works correctly.

* Refactor of database name

Don’t use the same string multiple times. When the database name we are using for testing changes then we should only have to make that change in one place.

* correction to test against old database

This was a typo. We want to test against the database we were using before

* getRequestWithDatabaseId function

Add the getRequestWithDatabaseId function and change it so that it can easily be used in multiple places and the tests still pass.

* Change function name to addDatabaseIdToRequest

The request options should contain databaseId so rename the function as appropriate.

* Comments in all the places to add reqOpts

This commit contains a comment in all the places we want to add the reqOpts change.

* Add a warning to a certain test for easier debug

Add a warning because the way the assert checking was done before, the test suite was not receiving any indication of which test was failing which makes debugging really hard.

* This fixed the bug for the save operation

* datastore request id

Add request ids back in all the places they are required

* addDatabaseIdToRequest

Add addDatabaseIdToRequest to the transaction test framework so that it can be used in the tests.

* Solve all the test errors for createReadStream

Mock out add database id for the tests in createReadStream that correspond to createReadStream.

* addDatabaseIdToRequest mock for runQueryStream

Add the addDatabaseIdToRequest mock to the tests for every runQueryStream test so that the tests pass.

* By default, mock out addDatabaseIdToRequest

addDatabaseIdToRequest should be a default function in datastore object of request so that we don’t have to mock it out in many different places.

* Revert "addDatabaseIdToRequest mock for runQueryStream"

This reverts commit a3bdb92.

* Revert "Solve all the test errors for createReadStream"

This reverts commit 7b5f902.

* linting

lint an import

* Revert "datastore request id"

This reverts commit f167ce3.

* Revert "This fixed the bug for the save operation"

This reverts commit dd7b06c.

* Revert "Comments in all the places to add reqOpts"

This reverts commit 8ba512a.

* Introduce addDatabaseIdRequest in request

Introduce adding the databaseId in request layer again.

* Declare a constant so that tests don’t run

Importing test into the system-test folder causes the system-test command to also run the tests. Eliminate this import.

* Remove setDatabaseId

Remove setDatabaseId as it provides a way for the user to get confused about the current state of the client.

* Add comment for addDatabaseIdToRequest function

The function needs a comment to be consistent with docs.

* Address header issue

The new util file needs a header

* Add databaseId for encoding and decoding keys

The databaseId should be encoded on keys so that the user can write on a record by record basis

* Revert "Add databaseId for encoding and decoding keys"

This reverts commit 0ebabd5.

* Try a test with the namespace

Try out a new test that looks at data details

* Use the other datastore in the test

Use another datastore with key

* Create a test that looks at specific details

The test should look at specific details in the key and should return the data with the right author.

* Add additional checks to existing test

Add additional checks to existing test to show default database is unaffected.

* Revert "Revert "Add databaseId for encoding and decoding keys""

This reverts commit 6449d10.

* Revert "Revert "Revert "Add databaseId for encoding and decoding keys"""

This reverts commit e694341.

* Rename the second database to secondDatabase

* Use the database called multidb-test

multidb-test is the name of the database set up on the project used in Github automation so we need to change this name in code to use that new database.

* Eliminate function on index that adds the id

Currently a function is used as a passthrough to the function on the util file. This unnecessarily exposes a function to the user that they shouldn’t be concerned with.

* Fix the test so that the assertion error bubbles

The assertion error should bubble up to the user if we send it through the callback.

* Add types so that purpose of function is clear

Types currently set to any should be more specific. This will help clarify the purpose of the function that is used.

* Add types and names so that the function works

The types should be more specific and the names should be more explicit and clear.

* Eliminate repeated code fragments

The test case has the same lines for each secondary database key and these lines should not be repeated so we regroup them into better data structures for the job

* Better names and code organization

The names of some variables were changed to align with others and code was moved so that code pertaining to the same function is grouped together

* Change the variable name of the default key

There is only one default key so we change the variable name so that it doesn’t mention 1

* Refactor the post key hierarchy out

Refactor the hierarchy out for a cleaner test

* Add only to the test we are interested in

* Revert "Add only to the test we are interested in"

This reverts commit 0378dea.

* Add second database id

In the transaction tests we add the databaseId parameter. The databaseId parameter will run tests against the secondary database.

* Remove unnecessary mocks

The mocks for addDatabaseIdToRequest are not necessary for these two files since they were only useful for the way the code was written before so should be removed.

* Remove unused imports

Imports that were used in old mocks no longer exist in the file so should be removed.

* Add parameterized testing to the test/index file

Parameterized testing can be added to the test/index file to test to see how the tests behave with databaseId set to the second database and without this setting too.

* Add parameterized testing to test/transaction.ts

Parameterized testing should be used on transaction.ts. This commit adds the testing with async.

* This ensures that the tests still run on the deft

Tests need to run on the default database as well as the secondary database. This commit ensures that they do.

* Add parameterized testing for the system tests

Parameterized testing for the system tests will ensure that tests run with the default database as well as with the secondary database.

* Use default datastore variable in new tests

The intent of the datastore variable in the new tests is to be the default database so that we are able to see if reads/writes affect the default or secondary database in these tests. They should not use the parameterized version of datastore.

* inline add database id

Now that the tests don’t need to mock out adding database id function, we can inline the code that adds the database id to the request options.

* Modify the namespace in parameterized tests

Parameterized tests should have a different namespace for the second set of tests for sure so we add this prefix.

* Move databaseId to SharedQueryOptions for alignme

The databaseId should be moved to shared query options as discussed.

* Run the linter

Linting was broken so that the PR could be read. Ran the linter again.

Author: danieljbruce

package.json

@@ -59,6 +59,7 @@
     "@types/node": "^20.4.9",
     "@types/proxyquire": "^1.3.28",
     "@types/sinon": "^10.0.0",
+    "async": "^3.2.4",
     "c8": "^8.0.1",
     "gapic-tools": "^0.2.0",
     "gts": "^5.0.0",

src/index.ts

@@ -700,6 +700,16 @@ class Datastore extends DatastoreRequest {
     );
   }
 
+  /**
+   * Gets the database id that all requests will be run against.
+   *
+   * @returns {string} The database id that the current client is set to that
+   *    requests will run against.
+   */
+  getDatabaseId(): string | undefined {
+    return this.options.databaseId;
+  }
+
   getProjectId(): Promise<string> {
     return this.auth.getProjectId();
   }
@@ -1818,6 +1828,7 @@ promisifyAll(Datastore, {
     'double',
     'isDouble',
     'geoPoint',
+    'getDatabaseId',
     'getProjectId',
     'isGeoPoint',
     'index',
@@ -1898,6 +1909,7 @@ export interface DatastoreOptions extends GoogleAuthOptions {
   namespace?: string;
   apiEndpoint?: string;
   sslCreds?: ChannelCredentials;
+  databaseId?: string;
 }
 
 export interface KeyToLegacyUrlSafeCallback {

src/request.ts

@@ -992,6 +992,10 @@ class DatastoreRequest {
       }
     }
 
+    if (datastore.options && datastore.options.databaseId) {
+      reqOpts.databaseId = datastore.options.databaseId;
+    }
+
     if (method === 'rollback') {
       reqOpts.transaction = this.id;
     }
@@ -1143,6 +1147,7 @@ export interface RequestConfig {
   reqOpts?: RequestOptions;
 }
 export interface SharedQueryOptions {
+  databaseId?: string;
   projectId?: string;
   partitionId?: google.datastore.v1.IPartitionId | null;
   readOptions?: {