Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: new transaction feature #1239

Merged
Merged
Show file tree
Hide file tree
Changes from 63 commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
f9bb82e
Move the state up, make it protected
danieljbruce Mar 4, 2024
e6c1265
Build new transaction options method
danieljbruce Mar 4, 2024
974eee2
Correct the fn that builds transactions
danieljbruce Mar 4, 2024
9cf180a
Set transaction to readOnly in system test.
danieljbruce Mar 5, 2024
a58d4ec
Change #parseRunSuccess
danieljbruce Mar 5, 2024
bf110eb
Move parseRunSuccess up to protected
danieljbruce Mar 5, 2024
f1cb78d
Saves the transaction id
danieljbruce Mar 5, 2024
8a420c3
Add tests for testing read only transactions
danieljbruce Mar 5, 2024
6ca70ac
Add tests that measure read time
danieljbruce Mar 5, 2024
46121e0
ran linter
danieljbruce Mar 6, 2024
8b5a311
Add a test for testing requests
danieljbruce Mar 6, 2024
5a91c4e
Final changes to make test work
danieljbruce Mar 6, 2024
38d76ff
Add the transaction.run test
danieljbruce Mar 6, 2024
6299bf3
run linter
danieljbruce Mar 6, 2024
e36b158
runQuery, lookup, put, commit
danieljbruce Mar 6, 2024
28a89ed
runAggregationQuery, lookup, put, commit
danieljbruce Mar 6, 2024
fa4f200
put, put, lookup, commit
danieljbruce Mar 6, 2024
6c976e1
Add tests for the commits
danieljbruce Mar 6, 2024
920f282
Add testing to ensure begin tx is called
danieljbruce Mar 7, 2024
7f9c2c9
Document #blockWithMutex
danieljbruce Mar 7, 2024
f762bb1
Document transaction state
danieljbruce Mar 7, 2024
4375378
feat: new transaction feature branch
danieljbruce Mar 7, 2024
599bec1
Add a check for expired
danieljbruce Mar 8, 2024
dc23b45
Add commit and rollback blocks
danieljbruce Mar 8, 2024
dac429a
run the linter
danieljbruce Mar 8, 2024
637ec5d
Don’t allow readtime to be specified in a txn
danieljbruce May 30, 2024
4a5fdae
throw error for both read time and consistency
danieljbruce May 30, 2024
e0ad5e8
Merge branch 'main' of https://github.com/googleapis/nodejs-datastore…
danieljbruce May 30, 2024
06661c4
Add test for specifying readtime
danieljbruce May 30, 2024
30f5b79
Remove the console logs
danieljbruce May 30, 2024
a3872bd
Refactor the test
danieljbruce May 30, 2024
3b03bf7
Improve test to make sure Gapic layer isn’t called
danieljbruce May 30, 2024
5891af8
Run linter
danieljbruce Jun 3, 2024
5fd45e7
Make change to allow the runAggregationQuery go up
danieljbruce Jun 3, 2024
4aedb17
Should error when get is used
danieljbruce Jun 3, 2024
38bd702
Move tests over and refactor initialized datastore
danieljbruce Jun 3, 2024
d8821d9
Remove only and remove imports
danieljbruce Jun 3, 2024
5eed19f
Introduce parameterized testing for errors
danieljbruce Jun 3, 2024
ecb9737
Use parameters in parameterized tests
danieljbruce Jun 3, 2024
c08df99
Migrate error tests to parameterized testing
danieljbruce Jun 3, 2024
316c651
Change description
danieljbruce Jun 3, 2024
974117b
Prepare second describe block for tests
danieljbruce Jun 3, 2024
5edf87a
Revert "Prepare second describe block for tests"
danieljbruce Jun 3, 2024
9f3b5ca
Always return after the error is sent back
danieljbruce Jun 3, 2024
602280e
Add headers
danieljbruce Jun 3, 2024
82f3b03
Tests and implementation for expired on rollback
danieljbruce Jun 3, 2024
57e7575
fix test
danieljbruce Jun 3, 2024
ff7d200
Wrap rollbacks with a withBeginTransaction.
danieljbruce Jun 3, 2024
d43f02f
Update the test so that it begins the tx before
danieljbruce Jun 4, 2024
1c74b6b
Throw error if transaction not started on rollback
danieljbruce Jun 4, 2024
a1a5ba6
Remove only
danieljbruce Jun 4, 2024
52f4199
Add a comment to the test regarding new txn.run()
danieljbruce Jun 4, 2024
13dcb58
Ensure that the errors get bubbled up
danieljbruce Jun 5, 2024
8091062
Run. linter
danieljbruce Jun 5, 2024
4a74f4d
Merge branch 'main' into new-transaction-feature-branch
danieljbruce Jun 5, 2024
d52c773
read time and consistency error
danieljbruce Jun 5, 2024
66b0286
Merge branch 'new-transaction-feature-branch' of https://github.com/d…
danieljbruce Jun 5, 2024
2b93852
Remove unnecessary change
danieljbruce Jun 5, 2024
22e7b9d
Eliminate the call to withBeginTransaction
danieljbruce Jun 5, 2024
a54b6c6
Throw error reported in documentation
danieljbruce Jun 10, 2024
418c43b
Work on streaming errors
danieljbruce Jun 10, 2024
2edd410
Move the error throwing up the stack
danieljbruce Jun 11, 2024
5c1dee5
Fix linting errors
danieljbruce Jun 11, 2024
7bf7a90
Generate unit tests for getTransactionRequest
danieljbruce Jun 20, 2024
2022006
Generate unit tests
danieljbruce Jun 20, 2024
f94c37e
Merge branch 'main' into new-transaction-feature-branch
danieljbruce Jun 20, 2024
03b7cd3
Make function one ternary operation
danieljbruce Jun 20, 2024
b16afbf
Merge branch 'new-transaction-feature-branch' of https://github.com/d…
danieljbruce Jun 20, 2024
9b05013
Add two comments
danieljbruce Jun 20, 2024
f10d1ef
Update the comment
danieljbruce Jun 20, 2024
7e453e1
Run the linter
danieljbruce Jun 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
228 changes: 189 additions & 39 deletions src/request.ts

Large diffs are not rendered by default.

136 changes: 66 additions & 70 deletions src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,34 @@
*/

import {promisifyAll} from '@google-cloud/promisify';
import arrify = require('arrify');
import {CallOptions} from 'google-gax';

import {google} from '../protos/protos';

import {Datastore, TransactionOptions} from '.';
import {entity, Entity, Entities} from './entity';
import {Entities, Entity, entity} from './entity';
import {
Query,
RunQueryCallback,
RunQueryInfo,
RunQueryOptions,
RunQueryResponse,
} from './query';
import {
CommitCallback,
CommitResponse,
DatastoreRequest,
RequestOptions,
PrepareEntityObjectResponse,
CreateReadStreamOptions,
GetResponse,
DatastoreRequest,
GetCallback,
GetResponse,
getTransactionRequest,
PrepareEntityObjectResponse,
RequestCallback,
transactionExpiredError,
TransactionState,
} from './request';
import {AggregateQuery} from './aggregate';
import {Mutex} from 'async-mutex';
import arrify = require('arrify');

/*
* This type matches the value returned by the promise in the
Expand All @@ -53,11 +54,6 @@ interface BeginAsyncResponse {
resp?: google.datastore.v1.IBeginTransactionResponse;
}

enum TransactionState {
NOT_STARTED,
IN_PROGRESS, // IN_PROGRESS currently tracks the expired state as well
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the super class because it is now needed there.

/**
* A transaction is a set of Datastore operations on one or more entities. Each
* transaction is guaranteed to be atomic, which means that transactions are
Expand Down Expand Up @@ -85,7 +81,6 @@ class Transaction extends DatastoreRequest {
modifiedEntities_: ModifiedEntities;
skipCommit?: boolean;
#mutex = new Mutex();
#state = TransactionState.NOT_STARTED;
constructor(datastore: Datastore, options?: TransactionOptions) {
super();
/**
Expand Down Expand Up @@ -115,6 +110,7 @@ class Transaction extends DatastoreRequest {

// Queue the requests to make when we send the transactional commit.
this.requests_ = [];
this.state = TransactionState.NOT_STARTED;
}

/*! Developer Documentation
Expand Down Expand Up @@ -177,6 +173,10 @@ class Transaction extends DatastoreRequest {
: () => {};
const gaxOptions =
typeof gaxOptionsOrCallback === 'object' ? gaxOptionsOrCallback : {};
if (this.state === TransactionState.EXPIRED) {
callback(new Error(transactionExpiredError));
return;
}
// This ensures that the transaction is started before calling runCommit
this.#withBeginTransaction(
gaxOptions,
Expand Down Expand Up @@ -355,13 +355,9 @@ class Transaction extends DatastoreRequest {
const callback =
typeof optionsOrCallback === 'function' ? optionsOrCallback : cb!;
// This ensures that the transaction is started before calling get
this.#withBeginTransaction(
options.gaxOptions,
() => {
super.get(keys, options, callback);
},
callback
);
this.#blockWithMutex(() => {
super.get(keys, options, callback);
});
}

/**
Expand Down Expand Up @@ -434,6 +430,14 @@ class Transaction extends DatastoreRequest {
const callback =
typeof gaxOptionsOrCallback === 'function' ? gaxOptionsOrCallback : cb!;

if (this.state === TransactionState.EXPIRED) {
callback(new Error(transactionExpiredError));
return;
}
if (this.state === TransactionState.NOT_STARTED) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering what are the allowed states here? NOT_TRANSACTION and IN_PROGRESS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOT_TRANSACTION, NOT_STARTED, IN_PROGRESS, EXPIRED.

callback(new Error('Transaction is not started'));
return;
}
this.request_(
{
client: 'DatastoreClient',
Expand All @@ -442,6 +446,7 @@ class Transaction extends DatastoreRequest {
},
(err, resp) => {
this.skipCommit = true;
this.state = TransactionState.EXPIRED;
callback(err || null, resp);
}
);
Expand Down Expand Up @@ -511,7 +516,7 @@ class Transaction extends DatastoreRequest {
const callback =
typeof optionsOrCallback === 'function' ? optionsOrCallback : cb!;
this.#mutex.runExclusive(async () => {
if (this.#state === TransactionState.NOT_STARTED) {
if (this.state === TransactionState.NOT_STARTED) {
const runResults = await this.#beginTransactionAsync(options);
this.#processBeginResults(runResults, callback);
} else {
Expand Down Expand Up @@ -635,6 +640,7 @@ class Transaction extends DatastoreRequest {
return;
}

this.state = TransactionState.EXPIRED;
// The `callbacks` array was built previously. These are the callbacks
// that handle the API response normally when using the
// DatastoreRequest.save and .delete methods.
Expand Down Expand Up @@ -666,24 +672,11 @@ class Transaction extends DatastoreRequest {
if (err) {
callback(err, null, resp);
} else {
this.#parseRunSuccess(runResults);
this.parseTransactionResponse(resp);
callback(null, this, resp);
}
}

/**
* This function saves results from a successful beginTransaction call.
*
* @param {BeginAsyncResponse} [response] The response from a call to
* begin a transaction that completed successfully.
*
**/
#parseRunSuccess(runResults: BeginAsyncResponse) {
const resp = runResults.resp;
this.id = resp!.transaction;
this.#state = TransactionState.IN_PROGRESS;
}

/**
* This async function makes a beginTransaction call and returns a promise with
* the information returned from the call that was made.
Expand All @@ -696,24 +689,10 @@ class Transaction extends DatastoreRequest {
async #beginTransactionAsync(
options: RunOptions
): Promise<BeginAsyncResponse> {
const reqOpts: RequestOptions = {
transactionOptions: {},
};

if (options.readOnly || this.readOnly) {
reqOpts.transactionOptions!.readOnly = {};
}

if (options.transactionId || this.id) {
reqOpts.transactionOptions!.readWrite = {
previousTransaction: options.transactionId || this.id,
};
}

if (options.transactionOptions) {
reqOpts.transactionOptions = options.transactionOptions;
}
return new Promise((resolve: (value: BeginAsyncResponse) => void) => {
const reqOpts = {
transactionOptions: getTransactionRequest(this, options),
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getTransactionRequest does what the removed code does here.

this.request_(
{
client: 'DatastoreClient',
Expand Down Expand Up @@ -766,13 +745,9 @@ class Transaction extends DatastoreRequest {
const callback =
typeof optionsOrCallback === 'function' ? optionsOrCallback : cb!;
// This ensures that the transaction is started before calling runAggregationQuery
this.#withBeginTransaction(
options.gaxOptions,
() => {
super.runAggregationQuery(query, options, callback);
},
callback
);
this.#blockWithMutex(() => {
super.runAggregationQuery(query, options, callback);
});
}

/**
Expand Down Expand Up @@ -805,13 +780,9 @@ class Transaction extends DatastoreRequest {
const callback =
typeof optionsOrCallback === 'function' ? optionsOrCallback : cb!;
// This ensures that the transaction is started before calling runQuery
this.#withBeginTransaction(
options.gaxOptions,
() => {
super.runQuery(query, options, callback);
},
callback
);
this.#blockWithMutex(() => {
super.runQuery(query, options, callback);
});
}

/**
Expand Down Expand Up @@ -1019,7 +990,7 @@ class Transaction extends DatastoreRequest {
* @param {CallOptions | undefined} [gaxOptions] Gax options provided by the
* user that are used for the beginTransaction grpc call.
* @param {function} [fn] A function which is run after ensuring a
* beginTransaction call is made.
* transaction has begun.
* @param {function} [callback] A callback provided by the user that expects
* an error in the first argument and a custom data type for the rest of the
* arguments.
Expand All @@ -1031,10 +1002,10 @@ class Transaction extends DatastoreRequest {
callback: (...args: [Error | null, ...T] | [Error | null]) => void
): void {
(async () => {
if (this.#state === TransactionState.NOT_STARTED) {
if (this.state === TransactionState.NOT_STARTED) {
try {
await this.#mutex.runExclusive(async () => {
if (this.#state === TransactionState.NOT_STARTED) {
if (this.state === TransactionState.NOT_STARTED) {
// This sends an rpc call to get the transaction id
const runResults = await this.#beginTransactionAsync({
gaxOptions,
Expand All @@ -1044,7 +1015,7 @@ class Transaction extends DatastoreRequest {
// Do not call the wrapped function.
throw runResults.err;
}
this.#parseRunSuccess(runResults);
this.parseTransactionResponse(runResults.resp);
// The rpc saving the transaction id was successful.
// Now the wrapped function fn will be called.
}
Expand All @@ -1057,6 +1028,31 @@ class Transaction extends DatastoreRequest {
return fn();
})();
}

/*
* Some rpc calls require that the transaction has been started (i.e, has a
* valid id) before they can be sent. #withBeginTransaction acts as a wrapper
* over those functions.
*
* If the transaction has not begun yet, `#blockWithMutex` will call the
* wrapped function which will begin the transaction in the rpc call it sends.
* If the transaction has begun, the wrapped function will be called, but it
* will not begin a transaction.
*
* @param {function} [fn] A function which is run after ensuring a
* transaction has begun.
*/
#blockWithMutex(fn: () => void) {
(async () => {
if (this.state === TransactionState.NOT_STARTED) {
await this.#mutex.runExclusive(async () => {
fn();
});
} else {
fn();
}
})();
}
}

export type ModifiedEntities = Array<{
Expand Down
Loading
Loading