From 1585d4a4e1b4b16d198307a3e97ffcf156d000b1 Mon Sep 17 00:00:00 2001 From: danieljbruce Date: Mon, 26 Feb 2024 13:09:01 -0500 Subject: [PATCH] feat: Nodejs transaction redesign feature branch (#1235) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: Break transaction.run into smaller pieces for use with async functions and other read/write calls (#1196) * Add a unit test for commit as a non-tx The fact that using a transaction object to do a commit results in a non-transaction should be documented so that if we decide to introduce a change later where this behaves differently then it is well documented. # Conflicts: # test/transaction.ts * use linter * Write tests to capture current behavior of error When begin transaction sends back an error, we want some tests to capture what the behavior is so that when we make changes to the run function then behavior is preserved. * Add tests for passing a response A response should reach the user the right way. Add tests to make sure behavior is preserved. * run async close to working In the run function delegate calls to runAsync and use run async to make promise calls * Add runAsync to promise excludes This allows this function to return a promise instead of a promise wrapped in a promise. This makes the tests pass and behave the way they should. * Remove space * Change to use this instead of self Do not call request from self * Eliminate unused comments * Add two comments Comments should actually explain what is being done * Remove the commit test for this PR The commit test for this PR should be removed because it is not really relevant for the async run functionality. * Clarify types throughout the function The types used should be very specific so that reading the code isn’t confusing. * Add a bit more typing for clarity Add a type to the resolve function just to introduce more clarity * Change types used in the data client callback Make the types more specific in the data client callback so that it is easier to track down the signature and match against the begin transaction function. * run the linter * Add comments to clarify PR * Refactor the parsing logic out of run The parsing logic is going to be needed elsewhere so taking it apart now. * Change interface of request promise callback The interface name should be changed so that it matches what it is. It is the callback used to form a promise. * Hide data completely Change accessors to hide data completely instead of using the private modifier * PR use if/else block Eliminate the early return as suggested in the PR * Add comments to document the new functions The comments capture the parameters and return type. * Update return type in docs * Update the tests to include runAsync runAsync should be in promisfy excludes * refactor: Break transaction.run into smaller pieces for use with async functions and other read/write calls * Rename a function to be more descriptive Make sure it is explicit that we are parsing begin results. * Modify comment Modify comment so that it doesn’t reference the way the code was before. * refactor: Move commit logic and add tests that prepare for transaction function changes (#1202) * Add a unit test for commit as a non-tx The fact that using a transaction object to do a commit results in a non-transaction should be documented so that if we decide to introduce a change later where this behaves differently then it is well documented. # Conflicts: # test/transaction.ts * use linter * Write tests to capture current behavior of error When begin transaction sends back an error, we want some tests to capture what the behavior is so that when we make changes to the run function then behavior is preserved. * Add tests for passing a response A response should reach the user the right way. Add tests to make sure behavior is preserved. * run async close to working In the run function delegate calls to runAsync and use run async to make promise calls * Add runAsync to promise excludes This allows this function to return a promise instead of a promise wrapped in a promise. This makes the tests pass and behave the way they should. * Remove space * Change to use this instead of self Do not call request from self * Eliminate unused comments * Add two comments Comments should actually explain what is being done * Remove the commit test for this PR The commit test for this PR should be removed because it is not really relevant for the async run functionality. * Clarify types throughout the function The types used should be very specific so that reading the code isn’t confusing. * Add a bit more typing for clarity Add a type to the resolve function just to introduce more clarity * Change types used in the data client callback Make the types more specific in the data client callback so that it is easier to track down the signature and match against the begin transaction function. * run the linter * Add comments to clarify PR * Refactor the parsing logic out of run The parsing logic is going to be needed elsewhere so taking it apart now. * Change interface of request promise callback The interface name should be changed so that it matches what it is. It is the callback used to form a promise. * Move commit functionality to a new function The internals of commit should be moved to a new function. This way we can sandwich a commit async call between commit and runCommitAsync. * Add the commit tests Commit tests added to ensure that commit behaves the same way as before. * Fix the tests so that they pass on commit The tests should pass before we make changes to commit. * refactor one of the mocks One of the mocks does not need to be written twice * Hide data completely Change accessors to hide data completely instead of using the private modifier * PR use if/else block Eliminate the early return as suggested in the PR * Add comments to document the new functions The comments capture the parameters and return type. * Update return type in docs * Update the tests to include runAsync runAsync should be in promisfy excludes * refactor: Break transaction.run into smaller pieces for use with async functions and other read/write calls * Rename a function to be more descriptive Make sure it is explicit that we are parsing begin results. * chore(deps): update dependency @types/sinon to v17 (#1197) * chore(deps): update dependency @types/is to v0.0.25 (#1198) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@types/is](https://togithub.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/is) ([source](https://togithub.com/DefinitelyTyped/DefinitelyTyped)) | [`0.0.24` -> `0.0.25`](https://renovatebot.com/diffs/npm/@types%2fis/0.0.24/0.0.25) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@types%2fis/0.0.25?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@types%2fis/0.0.25?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@types%2fis/0.0.24/0.0.25?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@types%2fis/0.0.24/0.0.25?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/googleapis/nodejs-datastore). * Modify comment Modify comment so that it doesn’t reference the way the code was before. * Create a transaction wrapper class for testing This is going to be a useful test for mocking out various layers to make sure they work the same way as before. * Clean up mocked transaction wrapper The mocked transaction wrapper should reset all mocked gapic functions and not have to be told which ones to reset. * Move test information for commit into commit block * Add gapic mocked tests for aggregation query Gapic mocked tests for run aggregation query need to be written that use the mock transaction object. * Fixing up the runQuery test * Finished the runQuery tests The runQuery tests are finished so we can use this to take apart the function from end to end. * Finished the get tests The get tests now make sure that the data coming back from the gapic layer results in the same values for users. * remove only * Add try catch blocks to handle errors in the tests try/catch logic in the test suite is added so that errors bubble up to the test runner and it is easier to see why tests failed * chore: update cloud-rad version to ^0.4.0 (#1199) Source-Link: https://github.com/googleapis/synthtool/commit/1063ef32bfe41b112bade7a2dfad4e84d0058ebd Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:e92044720ab3cb6984a70b0c6001081204375959ba3599ef6c42dd99a7783a67 Co-authored-by: Owl Bot * Add comments and general cleanup Cleanup includes adding comments so that the tests are more readable. * Remove some redundant tests Some of these tests are captured in the various functions describe block. * Correct the comment to be more accurate The comment should talk about what the tests actually intend to do * General improvements to code quality Move declared transactionWrapper and other variables out to shorten code. Remove require and replace with import. Use more specific type. * Add data client check Add data client check and add type for data client. * Eliminate the mocked function variable The mocked function variable is not used so remove it from code. * Move begin transaction setup code The same block of code for setting up begin transaction is repeated twice. Move it into one function and use it from both before blocks. * Replace the TODO for the key Add some try blocks to make the errors more visible also. * Update description Update the description for the test to give a better explanation of what the describe block does. * Add comments to describe the purpose of signaller The comments describing the callback signaller should explain the problem it solves. * Add both dones back in The done functions were lost in the refactors. Let us add them back in. * mockedBeginTransaction should be Function Make a more specific type for mockedBeginTransaction and the functions mocked. * Use ECMA script modifier Make sure this function is completely hidden. * Remove TODOs that no longer apply The TODO statements no longer need to be followed up on so remove them. * beginTransaction type definition Remove the type definition. It is not used. * Add the setupBeginTransaction method Refactor test code to provide better error catching and also not repeat setup code for the run function. * Add a comment for private runCommit method The comment for the runCommit method just captures the fact that the function is used as a pass-through. * Modify comment to be more clear * Update comments The comments should more directly address the problem the tests are meant to solve * Eliminate redundant test code This describe block is now duplicated. Remove it. * revert comment This was a typo * refactor: Move commit logic and add tests that prepare for transaction function changes --------- Co-authored-by: Mend Renovate Co-authored-by: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Owl Bot * feat: Begin transactions before each transaction read/write (#1205) * Add a unit test for commit as a non-tx The fact that using a transaction object to do a commit results in a non-transaction should be documented so that if we decide to introduce a change later where this behaves differently then it is well documented. # Conflicts: # test/transaction.ts * use linter * Write tests to capture current behavior of error When begin transaction sends back an error, we want some tests to capture what the behavior is so that when we make changes to the run function then behavior is preserved. * Add tests for passing a response A response should reach the user the right way. Add tests to make sure behavior is preserved. * run async close to working In the run function delegate calls to runAsync and use run async to make promise calls * Add runAsync to promise excludes This allows this function to return a promise instead of a promise wrapped in a promise. This makes the tests pass and behave the way they should. * Remove space * Change to use this instead of self Do not call request from self * Eliminate unused comments * Add two comments Comments should actually explain what is being done * Remove the commit test for this PR The commit test for this PR should be removed because it is not really relevant for the async run functionality. * Clarify types throughout the function The types used should be very specific so that reading the code isn’t confusing. * Add a bit more typing for clarity Add a type to the resolve function just to introduce more clarity * Change types used in the data client callback Make the types more specific in the data client callback so that it is easier to track down the signature and match against the begin transaction function. * run the linter * Add comments to clarify PR * Refactor the parsing logic out of run The parsing logic is going to be needed elsewhere so taking it apart now. * Change interface of request promise callback The interface name should be changed so that it matches what it is. It is the callback used to form a promise. * Move commit functionality to a new function The internals of commit should be moved to a new function. This way we can sandwich a commit async call between commit and runCommitAsync. * Add the mutex and transaction state etc. Still an issue with system tests. commit still needs tweaks to work with async. * Remove no-op, get commit tests working Remove the no-op, get commit tests in place. * Add mocks and additional debugging Mocks and additional debugging hooks to introspect what is going on. * Add tests for commit Make sure commit behaves the same way as before. * Add the commit tests Commit tests added to ensure that commit behaves the same way as before. * Fix the tests so that they pass on commit The tests should pass before we make changes to commit. * refactor one of the mocks One of the mocks does not need to be written twice * reverting changes to add new test on transaction * Change the promise Make the promise simpler. Change the tests to exclude functions with promisify. * Hide data completely Change accessors to hide data completely instead of using the private modifier * PR use if/else block Eliminate the early return as suggested in the PR * Add comments to document the new functions The comments capture the parameters and return type. * Update return type in docs * Update the tests to include runAsync runAsync should be in promisfy excludes * refactor: Break transaction.run into smaller pieces for use with async functions and other read/write calls * Rename a function to be more descriptive Make sure it is explicit that we are parsing begin results. * Move the mutex and the state down to derived class The mutex and state should be moved down to the derived class. We are going to override get/runQuery/runAggregateQuery there. * chore(deps): update dependency @types/sinon to v17 (#1197) * Add hook to call run before commit Add the hook to call run before calling commit in existing tests. * Add commitAsync to promisify excludes commitAsync should be resolved and then() function should be called as it was not being called before * remove the console logs * Delete run commit * Remove the private identifier Use the private modifier instead to hide data * Add withBeginTransaction withBeginTransaction will be used with all calls that begin transactions and then intend to use the mutex for locking when the begin transaction call is made. * Add another level of abstraction Add #beginWithCallback so that all the read calls can be made with one line of code. * commit async is not needed anymore * This data structure is not needed anymore * Replace types associated with run to use generics The generic parameter should be used for types with run. * Make response type more specific for parseRunAsync More specific types are better and it make code easier to read * chore(deps): update dependency @types/is to v0.0.25 (#1198) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@types/is](https://togithub.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/is) ([source](https://togithub.com/DefinitelyTyped/DefinitelyTyped)) | [`0.0.24` -> `0.0.25`](https://renovatebot.com/diffs/npm/@types%2fis/0.0.24/0.0.25) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@types%2fis/0.0.25?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@types%2fis/0.0.25?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@types%2fis/0.0.24/0.0.25?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@types%2fis/0.0.24/0.0.25?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/googleapis/nodejs-datastore). * Modify comment Modify comment so that it doesn’t reference the way the code was before. * Add implementation for runQuery runQuery should make the call to begin the transaction first if that hasn’t already happened yet. * Making fixes for runAggregationQuery Fixes for run aggregation query. Still getting undefined results. * Write tests for runAggregateQuery Make sure that runAggregateQuery return results make it back to the user. * Get one test case passing for runAggregateQuery runAggregateQuery should not be excluded by promisify. Otherwise it will not return a promise, but we want it to return a promise. * remove console log clutter * Change tests for runAggregationQuery Change the test to use deep strict equal since the objects being compared will not be reference equal. * Add resolver type Eliminate some unused code * Create a transaction wrapper class for testing This is going to be a useful test for mocking out various layers to make sure they work the same way as before. * Clean up mocked transaction wrapper The mocked transaction wrapper should reset all mocked gapic functions and not have to be told which ones to reset. * Move test information for commit into commit block * Add gapic mocked tests for aggregation query Gapic mocked tests for run aggregation query need to be written that use the mock transaction object. * Fixing up the runQuery test * Finished the runQuery tests The runQuery tests are finished so we can use this to take apart the function from end to end. * Finished the get tests The get tests now make sure that the data coming back from the gapic layer results in the same values for users. * remove only * remove only and console log * Try modifications to runQuery Add modifications to runQuery to use some function to return values * Add try catch blocks to handle errors in the tests try/catch logic in the test suite is added so that errors bubble up to the test runner and it is easier to see why tests failed * Modify commit so it doesn’t run early Don’t start commit with a promise or the promise will run early. * Update get with resolver get should use the same pattern as runQuery and runAggregationQuery to use a resolver for the mutex business logic * remove #beginWithCallback #beginWithCallback is no longer used so remove it * Remove #withBeginTransaction This function is no longer used * Rename #someFunction with begin transaction #someFunction should be named differently * Fix promisify. Change to a deepStrictEqual check. A deepStrictEqual check is all that is needed in this test that currently reuses a transaction. * Add setImmediate to the tests If we add a delay to the tests then the mutex has the opportunity to unlock before running the tests. * Added some tests for call ordering We want to make sure that the calls get delivered and received in order * Pack testing tool into an object Put the testing tool in an object so that we can explore more orderings and possibilities. * Eliminate console logs, use expected order Allow expected order to be passed into the tester. This will make the object reusable. * Add a check for transaction not started yet Need a check to be sure that the transaction is not in progress if making another beginTransaction call. * Remove NOT_TRANSACTION Remove the NOT_TRANSACTION and default to NOT_STARTED * Remove only Remove only and let all unit tests run * Use an enum instead of static class members Use an enum to track transaction state. * TODOs are done * Move excludes to proper position Excludes should contain the right functions in the right order. * Simplify the run function a little bit Regroup functionality to simplify the run function * Add comments to tests Explain purpose of testing objects * Modify tests and fix a bug from the merge The merge added a bug because the pound sign wasn’t used and it should have been. * Fix error message * Comments and general cleanup Remove constructor parameter that is not used. Add comments to functions so that the tests are more readable. * Added comments for functions in transaction.ts Functions in transaction.ts need JSdoc comments. * Add a comment for aggregate queries Add JSdoc comment * Add an assertion to test The assertion should make sure the first read returns undefined. * Add tests for lookup, put, commit For calls with run and without run, do lookup, put, commit. Make sure to clean up after the tests too. * refactor the test Test does lookup, put, commit. This is done in an async function. * Add tests for put, lookup, commit A group of tests should be added for put, lookup, commit. One with run and one without run. * chore: update cloud-rad version to ^0.4.0 (#1199) Source-Link: https://github.com/googleapis/synthtool/commit/1063ef32bfe41b112bade7a2dfad4e84d0058ebd Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:e92044720ab3cb6984a70b0c6001081204375959ba3599ef6c42dd99a7783a67 Co-authored-by: Owl Bot * Add comments and general cleanup Cleanup includes adding comments so that the tests are more readable. * Remove some redundant tests Some of these tests are captured in the various functions describe block. * Correct the comment to be more accurate The comment should talk about what the tests actually intend to do * General improvements to code quality Move declared transactionWrapper and other variables out to shorten code. Remove require and replace with import. Use more specific type. * Add data client check Add data client check and add type for data client. * Eliminate the mocked function variable The mocked function variable is not used so remove it from code. * Move begin transaction setup code The same block of code for setting up begin transaction is repeated twice. Move it into one function and use it from both before blocks. * Replace the TODO for the key Add some try blocks to make the errors more visible also. * Update description Update the description for the test to give a better explanation of what the describe block does. * Add comments to describe the purpose of signaller The comments describing the callback signaller should explain the problem it solves. * Add both dones back in The done functions were lost in the refactors. Let us add them back in. * mockedBeginTransaction should be Function Make a more specific type for mockedBeginTransaction and the functions mocked. * Use ECMA script modifier Make sure this function is completely hidden. * Remove TODOs that no longer apply The TODO statements no longer need to be followed up on so remove them. * beginTransaction type definition Remove the type definition. It is not used. * Add the setupBeginTransaction method Refactor test code to provide better error catching and also not repeat setup code for the run function. * Add a comment for private runCommit method The comment for the runCommit method just captures the fact that the function is used as a pass-through. * Modify comment to be more clear * Update comments The comments should more directly address the problem the tests are meant to solve * Eliminate redundant test code This describe block is now duplicated. Remove it. * revert comment This was a typo * refactor: Move commit logic and add tests that prepare for transaction function changes * runQuery, put, commit Another some more integration tests for runQuery, put, commit * run linter and group some tests into a describe block so that they don’t run before some new tests we are going to add. * Add some runAggregationQuery integration tests Add one of the test cases from the document (runAggregationQuery tests) * Add tests for put, runAggregationQuery, commit put, runAggregationQuery, commit tests have been added and now run properly. * Write some latency tests The latency tests measure time taken with and without using the run call. * Add logs to make latency tests run The logs will output the time required for the latency tests. * Add two tests for put, commit The two tests for put, commit should make sure that begin transaction is called. * Build requests into the transaction order tester To meet the needs we want for unit testing we must add some checker that lets us record the requests and verify that they are the right values. * Modify order testing Modify the order tester object to include more calls. Also add expected requests to the existing tests. * Create lookup, lookup, put, commit Create test for a read, read and then commit. * Fix the unit test Unit test should capture the fact that the run callback is used. * Add lookup requests When using get requests without passing consistency, we should see two lookup requests reach the gapic layer. Consistency should be removed from the test because it is not meant to reach the Gapic layer. * Remove the console logs console logs are not needed. Remove them so that the tests are cleaner. * Remove only * remove the latency tests The latency tests are not needed anymore so remove them. * Separate into a section transactions with/without A section name transactions with and without run should be used. * Remove a describe block and run linter This should make the diff a lot cleaner * Re-introduce a test that was there from before We don’t want the diff to say that we have removed tests. Therefore, add the test that was there before. * Update comment Modify the comment so that it is more objective * Add a comment for the runAggregationQuery function The comment for the aggregation query function in transaction.ts is needed here. * Simplify argument type In #withBeginTransaction there is a data type that matches the complex data type in the second argument of this function. * remove unnecessary extra variable The extra variable is not needed * Eliminate redundant definition Eliminate a definition and use the commit response data type here instead. * Add a comment for the #withBeginTransaction fn Need to clearly explain what the withBeginTransactionFunction does * Eliminate a line that was used for debugging * Add space back in * remove indent * Add empty line back in This simplifies the diff a bit * Remove ambiguous pronoun * Shorten comment * Add a comment for begin Add a comment that incorporates how this function interacts with #withBeginTransaction. * Shorten comments Don’t repeat information from the comments in the super class. Just mention the super class comments. * Type is only used once Inline the type to eliminate redundancy in the code. * Inline another promise type Inline another promise type which lets us eliminate one line of code. * Rename type to executor Executor is what it is called in the promise argument. * run linter * Simplify the data types more The types should line up for the executor in the runAsync function * Rename type to describe how it is used This name is clearer * Rename to userCallbackData. Add comments UserCallbackData is a more specific description of the type of data the generic type is. We also need comments to explain what these interfaces should represent. * resolver is a better term to use here * Change the type in the comments The type has updated since to be a more general type. * Flip if/else Flip the if else. Improvements are that the if checks for the affirmative and the longer block comes first * run linter * Add brackets Introduce brackets so that the word matching the parameter is more isolated. * Stronger type checking for runAsync Use more specific types in the runAsync function so that the compiler will complain if anything isn’t right. * Call the function fed into the promise resolver resolver should be the name used just to be consistent with all other code. * Rename variables and add comments Rename the variables so that the relationships between them are easier to understand. Add comments explaining the purpose of the variables. * Add comment for the done function * Make checkForCompletion private To avoid confusion make checkForCompletion private so that the user of the object does not need to think about using it. * Replace all events with enum values The events should not be strings. If they are strings then we might be magic string matching and this should not be done in code. * Make code more succinct Remove the need for a map. Remove occurrences of strings throughout tests and replace them with enum values. * Rename event to UserCodeEvent Call this enum UserCodeEvent as it is a better description of what the values could potentially be. * Remove the map from string names to gapic layer This map is not needed anymore because now we use the Gapic layer function event directly. * More specific type Shouldn’t just be a string. This should be a Gapic Layer function. * todo is done Assertion check is done * Eliminate need to define extra variable This just makes the code a bit more concise. * Add comments to describe test objects The comments describe Gapic layer data passed back. * There is no need to make these functions private Just inline the callbacks. A private function makes their relationship with the code that uses them confusing and each one of them is only used in one place. * Renames all callbacks to just callback Rename all callbacks to callback because that is the variable name inside the client function. * Take requests private * Take expectedRequests private expectedRequests is not used outside of the transaction order tester so we should take it private. * Take expected event order private Expected event order should be taken private. * Take event order private event order is not used outside this object * Fix some warning messages Various warnings show up in the es-lint. Fix them. * Use unknown for Gapic Layer Response Change the type to unknown to address the ES-lint error. * Get rid of any type for runQuery Use the more specific type provided by runQuery instead. * More linter fixes Addressing some of the problems in the new tests according to ES-lint. * Change parameters to match the get callback Stronger type enforcement helping us catch errors later. * Another ESLInt correction Use GetCallback here instead of any for the Get response type. * Remove query variable as it is not used * Take done private When done is passed in and saved it should only be used internally. * Eliminate unused arguments in callback Eliminate the arguments because if they are not used then they cause ES-lint errors. * Add readOnly reserved word Various properties can be readOnly as recommended by the linter. * Make type for done explicit Explicitly call out the fact that done should equal mocha’s done function. * Another place where done should be mentioned The done type should explicitly be mentioned here. * No need to access order tester to get wrapper There is no need to access the transaction wrapper to get the order tester. Just access the transaction wrapper directly. * private variable transaction wrapper Take transaction wrapper private. It does not need to be visible to the user. * Capitalize RequestType Change RequestType to be capitalized and use it whenever a request needs to be made. * request should be a more specific type RequestType should be used here. * Do info checks for error and response cases Add another assertion check. Do the check for the error and response cases. * These are actually results from runQuery * Eliminate extra line of code Add assertion check for info. Eliminate the code that stores all the results. * rename enum The word Layer is redundant. Also emphasize that the enum captures the function name. * Lookup response The variable should include the word lookup since technically that is the name of the Gapic layer endpoint. * Add comments for the transaction order tester Explain each function with a comment describing what it does. * Introduce a new push function Add a push function so that in each test it is easier to see the events line up. * Remove middle layer functions Remove functions in the transactionOrderTester and use functionality from those functions directly because then in the test it is easier to read the test’s intent. Also, add GapicRequestData type to simplify types in the test class. Shorten transactionOrderTester to just tester. * Prefer block comments The comments are multi-line so use block comments instead. * Rename event to CUSTOM_EVENT CUSTOM_EVENT should be used instead of FUNCTIONS_CALLED. FUNCTIONS_CALLED is sort of ambiguous. * Add square brackets to comments square brackets added to make the comment more readable and user friendly. * Remove unused comment * Eliminate the promiseType variable This variable is only used once. We should not define it here. * Eliminate double try/catch blocks The nested try/catch blocks are not necessary. The finally block is going to run even if it returns an error in the catch block. * Add comments to describe what T is Need to make sure the generic type is clear. * Use wrapped promise The wrapped promise and runExclusive should be used instead of releasing the mutex manually. This is safer. * Change name to callback instead Name the variable to match the parameter name being passed into runExclusive. * Eliminate extra variable Use the async function in place. Don’t define an extra variable just for one usage. * Add a test for commit first If commit is first then make sure the calls happen in the right order. * Remove a redundant check for transaction started The results won’t be any different if the first check is removed. Therefore, should remove it just because it makes the code a lot simpler. * Rename variable and use runExclusive runExclusive in the run function will get rid of excess code that is not needed. Also rename a variable to runResults to be more specific. * refactor: remove resolvers, remove promise in withBeginTransaction, simplify code calling withBeginTransaction (#1212) * Change resolvers Change the resolvers so that they all follow the same pattern. * Add a wrapWithBeginTransaction function for get Generalize the calls to withBeginTransaction. Don’t allow the error type to be undefined in UserCallbackData since the error never actually will be undefined. This is necessary to solve various compile time errors. * Use wrapWithBeginTransaction in more places commit and runQuery can now use #wrapWithBeginTransaction and this will reduce the amount of code necessary in each function respectively. * Add callbackWithError function This function replaces a common pattern that occurs in transactions where we provide a callback that accepts an error as the first argument and some generic type that extends an array of any as the rest of the arguments. * Add return type to callbackWithError Adding a return type just makes type checking stronger in the client library. We should introduce it here to make it more likely a compiler error will be thrown if the function is misused. * Delete code that is not used When playing around with code to return a standard callback, code was written that is not required anymore. Also, rename StandardCallbackArgumentsAny. * There is no need to define a separate type The UserCallbackArguments do not need a separate type if they are only used once. Define the type inline where it is needed. * TODO no longer applies * Remove TODO * Inline the error type Error or null is only used in two places so inline it here. * Add a comment for callbackWithError callbackWithError is an abstract function that needs to be explained. This comment helps with that. * Added comments to document sendUserCallbackData sendUserCallbackData needs to be described so that its purpose is clear. * Move function Move function to the right order alphabetically * Do not define a commit type only used once Do not define a separate variable for only one use. * Replace the type in the comment with Function Replace with Function to eliminate references to the generic parameter which make CI tests fail. * Just use function type for the callback No need to mention the argument types and return types. * Remove withBeginTransaction withBeginTransaction is only called by one function so remove it to condense call stack. * Simplify code usage of withBeginTransaction Eliminate the need to build a resolver for all code usages of withBeginTransaction. Eliminate the need for a promise in withBeginTransaction. Eliminate the need for a callbackWithError function. * function should not have any arguments * Remove generic Args parameter It is not used. * Remove TODOs that are not relevant anymore. * Rename to withBeginTransaction Rename the function and eliminate redundant code. * Update comments for withBeginTransaction Update the comments to reflect new parameters. * Simplify diff * Remove error as null * withBeginTransaction Move withBeginTransaction to last alphabetically * Add a comment to indicate error In the code block it is not clear what kind of error will be produced and caught in catch so a comment is added to clarify the type of error that gets produced. * Rename method to beginTxAsync Calling the method `run` is too ambiguous. Use #beginTransactionAsync instead. * Remove abstract types and inline instead With changes from before, these abstract data types are now only used once in the async function that begins a transaction. This change inlines the code that creates the promise so that these don’t have to be used at all. * Change the type of the id The id for a transaction should actually match the value returned by the server. Adding this change also allows us to be more specific with types in function arguments. * For types to align, previous txn must be flexible Previous transaction must accept the Uint8Array type because after all, that is what the id type usually is. * Set transaction type in request options The request options must accept a Uint8Array for the compiler to work now that the ids can accept this type too. * For read options, more types should be accepted The type for read options should line up with the type for other properties that store a transaction id. * Replace UserCallbackData with BeginAsyncResponse UserCallbackData is only used in one place. Therefore, the generic type is not necessary and this can be replaced with a more specific type for readability. * Rewrite description for withBeginTransaction. This description adds a few more details. * Add comments to withBeginTransaction Ensure that the use of withBeginTransaction is clear and that this function contains comments throughout it. * Make the MockedTransactionWrapper class flexible We want to test what happens when sending an error back from the begin transaction call. * Move the after hook out The after hook does the same thing for all four of the describe blocks. Move it out to reduce the amount of code required. * Add beginTransaction error test Add a test to ensure that when beginTransaction sends back an error that the error actually reaches the user when they are using a promise or a callback. * Add comments to withBeginTransaction Comments are needed in withBeginTransaction to clearly indicate the flow of control within the function. * Added a few comments for withBeginTxn Comments will help all functions that are using withBeginTransaction. --------- Co-authored-by: Mend Renovate Co-authored-by: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Owl Bot * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Mend Renovate Co-authored-by: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Owl Bot --- package.json | 1 + src/request.ts | 21 +- src/transaction.ts | 537 ++++++++++++---- system-test/datastore.ts | 197 +++++- test/transaction.ts | 1262 +++++++++++++++++++++++++++++++++++++- 5 files changed, 1871 insertions(+), 147 deletions(-) diff --git a/package.json b/package.json index 06751f1e5..8423fb7b9 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "dependencies": { "@google-cloud/promisify": "^4.0.0", "arrify": "^2.0.1", + "async-mutex": "^0.4.0", "concat-stream": "^2.0.0", "extend": "^3.0.2", "google-gax": "^4.0.5", diff --git a/src/request.ts b/src/request.ts index d39f572ef..047cf0ab6 100644 --- a/src/request.ts +++ b/src/request.ts @@ -79,7 +79,7 @@ const CONSISTENCY_PROTO_CODE: ConsistencyProtoCode = { * @class */ class DatastoreRequest { - id: string | undefined; + id: string | undefined | Uint8Array | null; requests_: | Entity | { @@ -546,6 +546,19 @@ class DatastoreRequest { ); } + /** + * Datastore allows you to run aggregate queries by supplying aggregate fields + * which will determine the type of aggregation that is performed. + * + * The query is run, and the results are returned in the second argument of + * the callback provided. + * + * @param {AggregateQuery} query AggregateQuery object. + * @param {RunQueryOptions} options Optional configuration + * @param {function} [callback] The callback function. If omitted, a promise is + * returned. + * + **/ runAggregationQuery( query: AggregateQuery, options?: RunQueryOptions @@ -1157,7 +1170,7 @@ export interface SharedQueryOptions { partitionId?: google.datastore.v1.IPartitionId | null; readOptions?: { readConsistency?: number; - transaction?: string; + transaction?: string | Uint8Array | null; readTime?: ITimestamp; }; } @@ -1166,9 +1179,9 @@ export interface RequestOptions extends SharedQueryOptions { keys?: Entity; transactionOptions?: { readOnly?: {}; - readWrite?: {previousTransaction?: string}; + readWrite?: {previousTransaction?: string | Uint8Array | null}; } | null; - transaction?: string | null; + transaction?: string | null | Uint8Array; mode?: string; query?: QueryProto; filter?: string; diff --git a/src/transaction.ts b/src/transaction.ts index 9bc760dd3..0b7883b6d 100644 --- a/src/transaction.ts +++ b/src/transaction.ts @@ -22,15 +22,41 @@ import {google} from '../protos/protos'; import {Datastore, TransactionOptions} from '.'; import {entity, Entity, Entities} from './entity'; -import {Query} from './query'; +import { + Query, + RunQueryCallback, + RunQueryInfo, + RunQueryOptions, + RunQueryResponse, +} from './query'; import { CommitCallback, CommitResponse, DatastoreRequest, RequestOptions, PrepareEntityObjectResponse, + CreateReadStreamOptions, + GetResponse, + GetCallback, + RequestCallback, } from './request'; import {AggregateQuery} from './aggregate'; +import {Mutex} from 'async-mutex'; + +/* + * This type matches the value returned by the promise in the + * #beginTransactionAsync function and subsequently passed into various other + * methods in this class. + */ +interface BeginAsyncResponse { + err?: Error | null; + resp?: google.datastore.v1.IBeginTransactionResponse; +} + +enum TransactionState { + NOT_STARTED, + IN_PROGRESS, // IN_PROGRESS currently tracks the expired state as well +} /** * A transaction is a set of Datastore operations on one or more entities. Each @@ -58,6 +84,8 @@ class Transaction extends DatastoreRequest { request: Function; modifiedEntities_: ModifiedEntities; skipCommit?: boolean; + #mutex = new Mutex(); + #state = TransactionState.NOT_STARTED; constructor(datastore: Datastore, options?: TransactionOptions) { super(); /** @@ -149,116 +177,13 @@ class Transaction extends DatastoreRequest { : () => {}; const gaxOptions = typeof gaxOptionsOrCallback === 'object' ? gaxOptionsOrCallback : {}; - - if (this.skipCommit) { - setImmediate(callback); - return; - } - - const keys: Entities = {}; - - this.modifiedEntities_ - // Reverse the order of the queue to respect the "last queued request - // wins" behavior. - .reverse() - // Limit the operations we're going to send through to only the most - // recently queued operations. E.g., if a user tries to save with the - // same key they just asked to be deleted, the delete request will be - // ignored, giving preference to the save operation. - .filter((modifiedEntity: Entity) => { - const key = modifiedEntity.entity.key; - - if (!entity.isKeyComplete(key)) return true; - - const stringifiedKey = JSON.stringify(modifiedEntity.entity.key); - - if (!keys[stringifiedKey]) { - keys[stringifiedKey] = true; - return true; - } - - return false; - }) - // Group entities together by method: `save` mutations, then `delete`. - // Note: `save` mutations being first is required to maintain order when - // assigning IDs to incomplete keys. - .sort((a, b) => { - return a.method < b.method ? 1 : a.method > b.method ? -1 : 0; - }) - // Group arguments together so that we only make one call to each - // method. This is important for `DatastoreRequest.save`, especially, as - // that method handles assigning auto-generated IDs to the original keys - // passed in. When we eventually execute the `save` method's API - // callback, having all the keys together is necessary to maintain - // order. - .reduce((acc: Entities, entityObject: Entity) => { - const lastEntityObject = acc[acc.length - 1]; - const sameMethod = - lastEntityObject && entityObject.method === lastEntityObject.method; - - if (!lastEntityObject || !sameMethod) { - acc.push(entityObject); - } else { - lastEntityObject.args = lastEntityObject.args.concat( - entityObject.args - ); - } - - return acc; - }, []) - // Call each of the mutational methods (DatastoreRequest[save,delete]) - // to build up a `req` array on this instance. This will also build up a - // `callbacks` array, that is the same callback that would run if we - // were using `save` and `delete` outside of a transaction, to process - // the response from the API. - .forEach( - (modifiedEntity: {method: string; args: {reverse: () => void}}) => { - const method = modifiedEntity.method; - const args = modifiedEntity.args.reverse(); - Datastore.prototype[method].call(this, args, () => {}); - } - ); - - // Take the `req` array built previously, and merge them into one request to - // send as the final transactional commit. - const reqOpts = { - mutations: this.requests_ - .map((x: {mutations: google.datastore.v1.Mutation}) => x.mutations) - .reduce( - (a: {concat: (arg0: Entity) => void}, b: Entity) => a.concat(b), - [] - ), - }; - - this.request_( - { - client: 'DatastoreClient', - method: 'commit', - reqOpts, - gaxOpts: gaxOptions || {}, + // This ensures that the transaction is started before calling runCommit + this.#withBeginTransaction( + gaxOptions, + () => { + this.#runCommit(gaxOptions, callback); }, - (err, resp) => { - if (err) { - // Rollback automatically for the user. - this.rollback(() => { - // Provide the error & API response from the failed commit to the - // user. Even a failed rollback should be transparent. RE: - // https://github.com/GoogleCloudPlatform/google-cloud-node/pull/1369#discussion_r66833976 - callback(err, resp); - }); - return; - } - - // The `callbacks` array was built previously. These are the callbacks - // that handle the API response normally when using the - // DatastoreRequest.save and .delete methods. - this.requestCallbacks_.forEach( - (cb: (arg0: null, arg1: Entity) => void) => { - cb(null, resp); - } - ); - callback(null, resp); - } + callback ); } @@ -398,6 +323,47 @@ class Transaction extends DatastoreRequest { }); } + /** + * This function calls get on the super class. If the transaction + * has not been started yet then the transaction is started before the + * get call is made. + * + * @param {Key|Key[]} keys Datastore key object(s). + * @param {object} [options] Optional configuration. + * @param {function} callback The callback function. + * + */ + get( + keys: entity.Key | entity.Key[], + options?: CreateReadStreamOptions + ): Promise; + get(keys: entity.Key | entity.Key[], callback: GetCallback): void; + get( + keys: entity.Key | entity.Key[], + options: CreateReadStreamOptions, + callback: GetCallback + ): void; + get( + keys: entity.Key | entity.Key[], + optionsOrCallback?: CreateReadStreamOptions | GetCallback, + cb?: GetCallback + ): void | Promise { + const options = + typeof optionsOrCallback === 'object' && optionsOrCallback + ? optionsOrCallback + : {}; + 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 + ); + } + /** * Maps to {@link https://cloud.google.com/nodejs/docs/reference/datastore/latest/datastore/transaction#_google_cloud_datastore_Transaction_save_member_1_|Datastore#save}, forcing the method to be `insert`. * @@ -544,10 +510,195 @@ class Transaction extends DatastoreRequest { typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; const callback = typeof optionsOrCallback === 'function' ? optionsOrCallback : cb!; + this.#mutex.runExclusive(async () => { + if (this.#state === TransactionState.NOT_STARTED) { + const runResults = await this.#beginTransactionAsync(options); + this.#processBeginResults(runResults, callback); + } else { + process.emitWarning( + 'run has already been called and should not be called again.' + ); + callback(null, this, {transaction: this.id}); + } + }); + } + + /** + * This function is a pass-through for the transaction.commit method + * It contains the business logic used for committing a transaction + * + * @param {object} [gaxOptions] Request configuration options, outlined here: + * https://googleapis.github.io/gax-nodejs/global.html#CallOptions. + * @param {function} callback The callback function. + * @private + */ + #runCommit( + gaxOptions: CallOptions, + callback: CommitCallback + ): void | Promise { + if (this.skipCommit) { + setImmediate(callback); + return; + } + + const keys: Entities = {}; + + this.modifiedEntities_ + // Reverse the order of the queue to respect the "last queued request + // wins" behavior. + .reverse() + // Limit the operations we're going to send through to only the most + // recently queued operations. E.g., if a user tries to save with the + // same key they just asked to be deleted, the delete request will be + // ignored, giving preference to the save operation. + .filter((modifiedEntity: Entity) => { + const key = modifiedEntity.entity.key; + + if (!entity.isKeyComplete(key)) return true; + + const stringifiedKey = JSON.stringify(modifiedEntity.entity.key); + + if (!keys[stringifiedKey]) { + keys[stringifiedKey] = true; + return true; + } + + return false; + }) + // Group entities together by method: `save` mutations, then `delete`. + // Note: `save` mutations being first is required to maintain order when + // assigning IDs to incomplete keys. + .sort((a, b) => { + return a.method < b.method ? 1 : a.method > b.method ? -1 : 0; + }) + // Group arguments together so that we only make one call to each + // method. This is important for `DatastoreRequest.save`, especially, as + // that method handles assigning auto-generated IDs to the original keys + // passed in. When we eventually execute the `save` method's API + // callback, having all the keys together is necessary to maintain + // order. + .reduce((acc: Entities, entityObject: Entity) => { + const lastEntityObject = acc[acc.length - 1]; + const sameMethod = + lastEntityObject && entityObject.method === lastEntityObject.method; + + if (!lastEntityObject || !sameMethod) { + acc.push(entityObject); + } else { + lastEntityObject.args = lastEntityObject.args.concat( + entityObject.args + ); + } + + return acc; + }, []) + // Call each of the mutational methods (DatastoreRequest[save,delete]) + // to build up a `req` array on this instance. This will also build up a + // `callbacks` array, that is the same callback that would run if we + // were using `save` and `delete` outside of a transaction, to process + // the response from the API. + .forEach( + (modifiedEntity: {method: string; args: {reverse: () => void}}) => { + const method = modifiedEntity.method; + const args = modifiedEntity.args.reverse(); + Datastore.prototype[method].call(this, args, () => {}); + } + ); + // Take the `req` array built previously, and merge them into one request to + // send as the final transactional commit. const reqOpts = { + mutations: this.requests_ + .map((x: {mutations: google.datastore.v1.Mutation}) => x.mutations) + .reduce( + (a: {concat: (arg0: Entity) => void}, b: Entity) => a.concat(b), + [] + ), + }; + + this.request_( + { + client: 'DatastoreClient', + method: 'commit', + reqOpts, + gaxOpts: gaxOptions || {}, + }, + (err, resp) => { + if (err) { + // Rollback automatically for the user. + this.rollback(() => { + // Provide the error & API response from the failed commit to the + // user. Even a failed rollback should be transparent. RE: + // https://github.com/GoogleCloudPlatform/google-cloud-node/pull/1369#discussion_r66833976 + callback(err, resp); + }); + return; + } + + // The `callbacks` array was built previously. These are the callbacks + // that handle the API response normally when using the + // DatastoreRequest.save and .delete methods. + this.requestCallbacks_.forEach( + (cb: (arg0: null, arg1: Entity) => void) => { + cb(null, resp); + } + ); + callback(null, resp); + } + ); + } + + /** + * This function parses results from a beginTransaction call + * + * @param {BeginAsyncResponse} [response] + * The response data from a call to begin a transaction. + * @param {RunCallback} [callback] A callback that accepts an error and a + * response as arguments. + * + **/ + #processBeginResults( + runResults: BeginAsyncResponse, + callback: RunCallback + ): void { + const err = runResults.err; + const resp = runResults.resp; + if (err) { + callback(err, null, resp); + } else { + this.#parseRunSuccess(runResults); + 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. + * + * @param {RunOptions} options The options used for a beginTransaction call. + * @returns {Promise} + * + * + **/ + async #beginTransactionAsync( + options: RunOptions + ): Promise { + const reqOpts: RequestOptions = { transactionOptions: {}, - } as RequestOptions; + }; if (options.readOnly || this.readOnly) { reqOpts.transactionOptions!.readOnly = {}; @@ -562,22 +713,104 @@ class Transaction extends DatastoreRequest { if (options.transactionOptions) { reqOpts.transactionOptions = options.transactionOptions; } + return new Promise((resolve: (value: BeginAsyncResponse) => void) => { + this.request_( + { + client: 'DatastoreClient', + method: 'beginTransaction', + reqOpts, + gaxOpts: options.gaxOptions, + }, + // Always use resolve because then this function can return both the error and the response + (err, resp) => { + resolve({ + err, + resp, + }); + } + ); + }); + } - this.request_( - { - client: 'DatastoreClient', - method: 'beginTransaction', - reqOpts, - gaxOpts: options.gaxOptions, + /** + * + * This function calls runAggregationQuery on the super class. If the transaction + * has not been started yet then the transaction is started before the + * runAggregationQuery call is made. + * + * @param {AggregateQuery} [query] AggregateQuery object. + * @param {RunQueryOptions} [options] Optional configuration + * @param {function} [callback] The callback function. If omitted, a promise is + * returned. + * + **/ + runAggregationQuery( + query: AggregateQuery, + options?: RunQueryOptions + ): Promise; + runAggregationQuery( + query: AggregateQuery, + options: RunQueryOptions, + callback: RequestCallback + ): void; + runAggregationQuery(query: AggregateQuery, callback: RequestCallback): void; + runAggregationQuery( + query: AggregateQuery, + optionsOrCallback?: RunQueryOptions | RequestCallback, + cb?: RequestCallback + ): void | Promise { + const options = + typeof optionsOrCallback === 'object' && optionsOrCallback + ? optionsOrCallback + : {}; + 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); }, - (err, resp) => { - if (err) { - callback(err, null, resp); - return; - } - this.id = resp!.transaction; - callback(null, this, resp); - } + callback + ); + } + + /** + * This function calls runQuery on the super class. If the transaction + * has not been started yet then the transaction is started before the + * runQuery call is made. + * + * @param {Query} query Query object. + * @param {object} [options] Optional configuration. + * @param {function} [callback] The callback function. If omitted, a readable + * stream instance is returned. + * + */ + runQuery(query: Query, options?: RunQueryOptions): Promise; + runQuery( + query: Query, + options: RunQueryOptions, + callback: RunQueryCallback + ): void; + runQuery(query: Query, callback: RunQueryCallback): void; + runQuery( + query: Query, + optionsOrCallback?: RunQueryOptions | RunQueryCallback, + cb?: RunQueryCallback + ): void | Promise { + const options = + typeof optionsOrCallback === 'object' && optionsOrCallback + ? optionsOrCallback + : {}; + 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 ); } @@ -771,6 +1004,59 @@ class Transaction extends DatastoreRequest { this.save(entities); } + + /** + * 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, `#withBeginTransaction` will first + * send an rpc to begin the transaction, and then execute the wrapped + * function. If it has begun, the wrapped function will be called directly + * instead. If an error is encountered during the beginTransaction call, the + * callback will be executed instead of the wrapped function. + * + * @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. + * @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. + * @private + */ + #withBeginTransaction( + gaxOptions: CallOptions | undefined, + fn: () => void, + callback: (...args: [Error | null, ...T] | [Error | null]) => void + ): void { + (async () => { + if (this.#state === TransactionState.NOT_STARTED) { + try { + await this.#mutex.runExclusive(async () => { + if (this.#state === TransactionState.NOT_STARTED) { + // This sends an rpc call to get the transaction id + const runResults = await this.#beginTransactionAsync({ + gaxOptions, + }); + if (runResults.err) { + // The rpc getting the id was unsuccessful. + // Do not call the wrapped function. + throw runResults.err; + } + this.#parseRunSuccess(runResults); + // The rpc saving the transaction id was successful. + // Now the wrapped function fn will be called. + } + }); + } catch (err: any) { + // Handle an error produced by the beginTransactionAsync call + return callback(err); + } + } + return fn(); + })(); + } } export type ModifiedEntities = Array<{ @@ -810,6 +1096,7 @@ promisifyAll(Transaction, { 'createQuery', 'delete', 'insert', + '#runAsync', 'save', 'update', 'upsert', diff --git a/system-test/datastore.ts b/system-test/datastore.ts index dffcc7244..62e1f89f8 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -1749,6 +1749,187 @@ async.each( assert.deepStrictEqual(results, [{property_1: 4}]); }); }); + describe('transactions with and without run', () => { + describe('lookup, put, commit', () => { + const key = datastore.key(['Company', 'Google']); + const obj = { + url: 'www.google.com', + }; + afterEach(async () => { + await datastore.delete(key); + }); + async function doLookupPutCommit(transaction: Transaction) { + const [firstRead] = await transaction.get(key); + assert(!firstRead); + transaction.save({key, data: obj}); + await transaction.commit(); + const [entity] = await datastore.get(key); + delete entity[datastore.KEY]; + assert.deepStrictEqual(entity, obj); + } + it('should run in a transaction', async () => { + const transaction = datastore.transaction(); + await transaction.run(); + await doLookupPutCommit(transaction); + }); + it('should run in a transaction without run', async () => { + const transaction = datastore.transaction(); + await doLookupPutCommit(transaction); + }); + }); + describe('put, lookup, commit', () => { + const key = datastore.key(['Company', 'Google']); + const obj = { + url: 'www.google.com', + }; + afterEach(async () => { + await datastore.delete(key); + }); + async function doPutLookupCommit(transaction: Transaction) { + transaction.save({key, data: obj}); + const [firstRead] = await transaction.get(key); + assert(!firstRead); + await transaction.commit(); + const [entity] = await datastore.get(key); + delete entity[datastore.KEY]; + assert.deepStrictEqual(entity, obj); + } + it('should run in a transaction', async () => { + const transaction = datastore.transaction(); + await transaction.run(); + await doPutLookupCommit(transaction); + }); + it('should run in a transaction without run', async () => { + const transaction = datastore.transaction(); + await doPutLookupCommit(transaction); + }); + }); + describe('runQuery, put, commit', () => { + const key = datastore.key(['Company', 'Google']); + const obj = { + url: 'www.google.com', + }; + afterEach(async () => { + await datastore.delete(key); + }); + async function doRunQueryPutCommit(transaction: Transaction) { + const query = transaction.createQuery('Company'); + const [results] = await transaction.runQuery(query); + assert.deepStrictEqual(results, []); + transaction.save({key, data: obj}); + await transaction.commit(); + const [entity] = await datastore.get(key); + delete entity[datastore.KEY]; + assert.deepStrictEqual(entity, obj); + } + it('should run in a transaction', async () => { + const transaction = datastore.transaction(); + await transaction.run(); + await doRunQueryPutCommit(transaction); + }); + it('should run in a transaction without run', async () => { + const transaction = datastore.transaction(); + await doRunQueryPutCommit(transaction); + }); + }); + describe('put, runQuery, commit', () => { + const key = datastore.key(['Company', 'Google']); + const obj = { + url: 'www.google.com', + }; + afterEach(async () => { + await datastore.delete(key); + }); + async function doPutRunQueryCommit(transaction: Transaction) { + transaction.save({key, data: obj}); + const query = transaction.createQuery('Company'); + const [results] = await transaction.runQuery(query); + assert.deepStrictEqual(results, []); + await transaction.commit(); + const [entity] = await datastore.get(key); + delete entity[datastore.KEY]; + assert.deepStrictEqual(entity, obj); + } + it('should run in a transaction', async () => { + const transaction = datastore.transaction(); + await transaction.run(); + await doPutRunQueryCommit(transaction); + }); + it('should run in a transaction without run', async () => { + const transaction = datastore.transaction(); + await doPutRunQueryCommit(transaction); + }); + }); + + describe('runAggregationQuery, put, commit', () => { + const key = datastore.key(['Company', 'Google']); + const obj = { + url: 'www.google.com', + }; + afterEach(async () => { + await datastore.delete(key); + }); + async function doRunAggregationQueryPutCommit( + transaction: Transaction + ) { + const query = transaction.createQuery('Company'); + const aggregateQuery = transaction + .createAggregationQuery(query) + .count('total'); + const [results] = + await transaction.runAggregationQuery(aggregateQuery); + assert.deepStrictEqual(results, [{total: 0}]); + transaction.save({key, data: obj}); + await transaction.commit(); + const [entity] = await datastore.get(key); + delete entity[datastore.KEY]; + assert.deepStrictEqual(entity, obj); + } + it('should run in a transaction', async () => { + const transaction = datastore.transaction(); + await transaction.run(); + await doRunAggregationQueryPutCommit(transaction); + }); + it('should run in a transaction without run', async () => { + const transaction = datastore.transaction(); + await doRunAggregationQueryPutCommit(transaction); + }); + }); + describe('put, runAggregationQuery, commit', () => { + const key = datastore.key(['Company', 'Google']); + const obj = { + url: 'www.google.com', + }; + afterEach(async () => { + await datastore.delete(key); + }); + async function doPutRunAggregationQueryCommit( + transaction: Transaction + ) { + transaction.save({key, data: obj}); + const query = transaction.createQuery('Company'); + const aggregateQuery = transaction + .createAggregationQuery(query) + .count('total'); + const [results] = + await transaction.runAggregationQuery(aggregateQuery); + assert.deepStrictEqual(results, [{total: 0}]); + await transaction.commit(); + const [entity] = await datastore.get(key); + delete entity[datastore.KEY]; + assert.deepStrictEqual(entity, obj); + } + it('should run in a transaction', async () => { + const transaction = datastore.transaction(); + await transaction.run(); + await doPutRunAggregationQueryCommit(transaction); + }); + it('should run in a transaction without run', async () => { + const transaction = datastore.transaction(); + await doPutRunAggregationQueryCommit(transaction); + }); + }); + }); describe('transactions', () => { it('should run in a transaction', async () => { const key = datastore.key(['Company', 'Google']); @@ -1873,9 +2054,7 @@ async.each( [result] = await aggregateQuery.run(); } catch (e) { await transaction.rollback(); - assert.fail( - 'The aggregation query run should have been successful' - ); + throw e; } assert.deepStrictEqual(result, [{total: 2}]); await transaction.commit(); @@ -1892,9 +2071,7 @@ async.each( [result] = await aggregateQuery.run(); } catch (e) { await transaction.rollback(); - assert.fail( - 'The aggregation query run should have been successful' - ); + throw e; } assert.deepStrictEqual(result, [{'total rating': 200}]); await transaction.commit(); @@ -1911,9 +2088,7 @@ async.each( [result] = await aggregateQuery.run(); } catch (e) { await transaction.rollback(); - assert.fail( - 'The aggregation query run should have been successful' - ); + throw e; } assert.deepStrictEqual(result, [{'average rating': 100}]); await transaction.commit(); @@ -1929,9 +2104,7 @@ async.each( [result] = await aggregateQuery.run(); } catch (e) { await transaction.rollback(); - assert.fail( - 'The aggregation query run should have been successful' - ); + throw e; } return result; } diff --git a/test/transaction.ts b/test/transaction.ts index 06faf3421..c1346f3df 100644 --- a/test/transaction.ts +++ b/test/transaction.ts @@ -24,21 +24,33 @@ import { DatastoreRequest, Query, TransactionOptions, + Transaction, + AggregateField, } from '../src'; -import {Entity} from '../src/entity'; +import {Entities, Entity, entity} from '../src/entity'; import * as tsTypes from '../src/transaction'; import * as sinon from 'sinon'; -import {RequestConfig} from '../src/request'; +import {Callback, CallOptions, ClientStub} from 'google-gax'; +import { + CommitCallback, + CreateReadStreamOptions, + GetCallback, + RequestCallback, + RequestConfig, +} from '../src/request'; import {SECOND_DATABASE_ID} from './index'; +import {google} from '../protos/protos'; +import {RunCallback} from '../src/transaction'; +import * as protos from '../protos/protos'; +import {AggregateQuery} from '../src/aggregate'; +import {RunQueryCallback, RunQueryInfo, RunQueryOptions} from '../src/query'; +import * as mocha from 'mocha'; const async = require('async'); // eslint-disable-next-line @typescript-eslint/no-explicit-any type Any = any; type Path = string | [string] | [string, number]; -// eslint-disable-next-line @typescript-eslint/no-var-requires -const {entity} = require('../src/entity'); - let promisified = false; const fakePfy = Object.assign({}, pfy, { promisifyAll(klass: Function, options: pfy.PromisifyAllOptions) { @@ -51,6 +63,7 @@ const fakePfy = Object.assign({}, pfy, { 'createQuery', 'delete', 'insert', + '#runAsync', 'save', 'update', 'upsert', @@ -147,9 +160,1246 @@ async.each( }); }); + describe('testing various transaction functions when transaction.run returns a response', () => { + type RequestType = + | protos.google.datastore.v1.ICommitRequest + | protos.google.datastore.v1.IBeginTransactionRequest + | protos.google.datastore.v1.ILookupRequest + | protos.google.datastore.v1.IRunQueryRequest + | protos.google.datastore.v1.IRunAggregationQueryRequest; + // These tests were created to ensure that various transaction functions work correctly after run is called. + // This allows us to catch any breaking changes to code usages that should remain the same. + const testRunResp = { + transaction: Buffer.from(Array.from(Array(100).keys())), + }; + enum GapicFunctionName { + BEGIN_TRANSACTION = 'beginTransaction', + LOOKUP = 'lookup', + RUN_QUERY = 'runQuery', + RUN_AGGREGATION_QUERY = 'runAggregationQuery', + COMMIT = 'commit', + } + + // MockedTransactionWrapper is a helper class for mocking out various + // Gapic functions and ensuring that responses and errors actually make it + // back to the user. + class MockedTransactionWrapper { + datastore: Datastore; + transaction: Transaction; + dataClient?: ClientStub; + mockedBeginTransaction: Function; + functionsMocked: { + name: GapicFunctionName; + mockedFunction: Function; + }[]; + // The callBackSignaler lets the user of this object get a signal when the mocked function is called. + // This is useful for tests that need to know when the mocked function is called. + callBackSignaler: ( + callbackReached: GapicFunctionName, + request?: RequestType + ) => void = () => {}; + + constructor( + err: Error | null = null, + resp: google.datastore.v1.IBeginTransactionResponse = testRunResp + ) { + const namespace = 'run-without-mock'; + const projectId = 'project-id'; + const options = { + projectId, + namespace, + }; + const datastore = new Datastore(options); + const dataClientName = 'DatastoreClient'; + // Create a fresh transaction for each test because transaction state changes after a commit. + this.transaction = datastore.transaction(); + // In this before hook, save the original beginTransaction method in a variable. + // After tests are finished, reassign beginTransaction to the variable. + // This way, mocking beginTransaction in this block doesn't affect other tests. + const gapic = Object.freeze({ + v1: require('../src/v1'), + }); + // Datastore Gapic clients haven't been initialized yet, so we initialize them here. + datastore.clients_.set( + dataClientName, + new gapic.v1[dataClientName](options) + ); + const dataClient = datastore.clients_.get(dataClientName); + // Mock begin transaction + this.mockedBeginTransaction = () => {}; + if (dataClient && dataClient.beginTransaction) { + this.mockedBeginTransaction = dataClient.beginTransaction; + } + if (dataClient && dataClient.beginTransaction) { + dataClient.beginTransaction = ( + request: protos.google.datastore.v1.IBeginTransactionRequest, + options: CallOptions, + callback: Callback< + protos.google.datastore.v1.IBeginTransactionResponse, + | protos.google.datastore.v1.IBeginTransactionRequest + | null + | undefined, + {} | null | undefined + > + ) => { + // Calls a user provided function that will receive this string + // Usually used to track when this code was reached relative to other code + this.callBackSignaler( + GapicFunctionName.BEGIN_TRANSACTION, + request + ); + callback(err, resp); + }; + } + this.dataClient = dataClient; + this.functionsMocked = []; + this.datastore = datastore; + } + + // This mocks out a gapic function to just call the callback received in the Gapic function. + // The callback will send back the error and response arguments provided as parameters. + mockGapicFunction( + functionName: GapicFunctionName, + response: ResponseType, + error: Error | null + ) { + const dataClient = this.dataClient; + // Check here that function hasn't been mocked out already + // Ensures that this mocking object is not being misused. + this.functionsMocked.forEach(fn => { + if (fn.name === functionName) { + throw Error(`${functionName} has already been mocked out`); + } + }); + if (dataClient && dataClient[functionName]) { + this.functionsMocked.push({ + name: functionName, + mockedFunction: dataClient[functionName], + }); + } + if (dataClient && dataClient[functionName]) { + dataClient[functionName] = ( + request: RequestType, + options: CallOptions, + callback: Callback< + ResponseType, + RequestType | null | undefined, + {} | null | undefined + > + ) => { + this.callBackSignaler(functionName, request); + callback(error, response); + }; + } + } + + // This resets beginTransaction from the Gapic layer to what it originally was. + // Resetting beginTransaction ensures other tests don't use the beginTransaction mock. + resetBeginTransaction() { + if (this.dataClient && this.dataClient.beginTransaction) { + this.dataClient.beginTransaction = this.mockedBeginTransaction; + } + } + + // This resets Gapic functions mocked out by the tests to what they originally were. + // Resetting mocked out Gapic functions ensures other tests don't use these mocked out functions. + resetGapicFunctions() { + this.functionsMocked.forEach(functionMocked => { + if (this.dataClient) { + this.dataClient[functionMocked.name] = + functionMocked.mockedFunction; + } + }); + } + } + + let transactionWrapper: MockedTransactionWrapper; + let transaction: Transaction; + + afterEach(() => { + transactionWrapper.resetBeginTransaction(); + transactionWrapper.resetGapicFunctions(); + }); + + describe('sending an error back from the beginTransaction gapic function', () => { + const testErrorMessage = 'test-beginTransaction-error'; + beforeEach(async () => { + transactionWrapper = new MockedTransactionWrapper( + new Error(testErrorMessage), + undefined + ); + }); + it('should send back the error when awaiting a promise', async () => { + try { + await transactionWrapper.transaction.commit(); + assert.fail('The run call should have failed.'); + } catch (error: any) { + assert.strictEqual(error['message'], testErrorMessage); + } + }); + it('should send back the error when using a callback', done => { + const commitCallback: CommitCallback = ( + error: Error | null | undefined, + response?: google.datastore.v1.ICommitResponse + ) => { + try { + assert(error); + assert.strictEqual(error.message, testErrorMessage); + assert.deepStrictEqual(response, undefined); + done(); + } catch (e) { + done(e); + } + }; + transactionWrapper.transaction.commit(commitCallback); + }); + }); + + describe('commit', () => { + // These tests were created to catch regressions for transaction.commit changes. + const testCommitResp = { + mutationResults: [ + { + key: { + path: [ + { + kind: 'some-kind', + }, + ], + }, + }, + ], + }; + const testErrorMessage = 'test-commit-error'; + + beforeEach(async () => { + transactionWrapper = new MockedTransactionWrapper(); + }); + + describe('should pass error back to the user', async () => { + beforeEach(() => { + transactionWrapper.mockGapicFunction( + GapicFunctionName.COMMIT, + testCommitResp, + new Error(testErrorMessage) + ); + }); + + it('should send back the error when awaiting a promise', async () => { + try { + await transactionWrapper.transaction.run(); + await transactionWrapper.transaction.commit(); + assert.fail('The run call should have failed.'); + } catch (error: any) { + assert.strictEqual(error['message'], testErrorMessage); + } + }); + it('should send back the error when using a callback', done => { + const commitCallback: CommitCallback = ( + error: Error | null | undefined, + response?: google.datastore.v1.ICommitResponse + ) => { + try { + assert(error); + assert.strictEqual(error.message, testErrorMessage); + assert.strictEqual(response, testCommitResp); + done(); + } catch (e) { + done(e); + } + }; + transactionWrapper.transaction.run(() => { + transactionWrapper.transaction.commit(commitCallback); + }); + }); + }); + describe('should pass response back to the user', async () => { + beforeEach(() => { + transactionWrapper.mockGapicFunction( + GapicFunctionName.COMMIT, + testCommitResp, + null + ); + }); + it('should send back the response when awaiting a promise', async () => { + await transactionWrapper.transaction.run(); + const [commitResults] = + await transactionWrapper.transaction.commit(); + assert.strictEqual(commitResults, testCommitResp); + }); + it('should send back the response when using a callback', done => { + const commitCallback: CommitCallback = ( + error: Error | null | undefined, + response?: google.datastore.v1.ICommitResponse + ) => { + try { + assert.strictEqual(error, null); + assert.strictEqual(response, testCommitResp); + done(); + } catch (e) { + done(e); + } + }; + transactionWrapper.transaction.run(() => { + transactionWrapper.transaction.commit(commitCallback); + }); + }); + }); + }); + describe('runAggregationQuery', () => { + // These tests were created to catch regressions for transaction.runAggregationQuery changes. + const runAggregationQueryUserResp = [{'average rating': 100}]; + const runAggregationQueryResp = { + batch: { + aggregationResults: [ + { + aggregateProperties: { + 'average rating': { + meaning: 0, + excludeFromIndexes: false, + doubleValue: 100, + valueType: 'doubleValue', + }, + }, + }, + ], + moreResults: + google.datastore.v1.QueryResultBatch.MoreResultsType + .NO_MORE_RESULTS, + readTime: {seconds: '1699390681', nanos: 961667000}, + }, + query: null, + transaction: testRunResp.transaction, + }; + const testErrorMessage = 'test-run-Aggregate-Query-error'; + let aggregate: AggregateQuery; + + beforeEach(async () => { + transactionWrapper = new MockedTransactionWrapper(); + transaction = transactionWrapper.transaction; + const q = transactionWrapper.datastore.createQuery('Character'); + aggregate = transactionWrapper.datastore + .createAggregationQuery(q) + .addAggregation(AggregateField.average('appearances')); + }); + + describe('should pass error back to the user', async () => { + beforeEach(() => { + transactionWrapper.mockGapicFunction( + GapicFunctionName.RUN_AGGREGATION_QUERY, + runAggregationQueryResp, + new Error(testErrorMessage) + ); + }); + + it('should send back the error when awaiting a promise', async () => { + try { + await transaction.run(); + await transaction.runAggregationQuery(aggregate); + assert.fail('The run call should have failed.'); + } catch (error: any) { + assert.strictEqual(error['message'], testErrorMessage); + } + }); + it('should send back the error when using a callback', done => { + const runAggregateQueryCallback: RequestCallback = ( + error: Error | null | undefined, + response?: unknown + ) => { + try { + assert(error); + assert.strictEqual(error.message, testErrorMessage); + assert.deepStrictEqual(response, runAggregationQueryUserResp); + done(); + } catch (e) { + done(e); + } + }; + transaction.run(() => { + transaction.runAggregationQuery( + aggregate, + runAggregateQueryCallback + ); + }); + }); + }); + describe('should pass response back to the user', async () => { + beforeEach(() => { + transactionWrapper.mockGapicFunction( + GapicFunctionName.RUN_AGGREGATION_QUERY, + runAggregationQueryResp, + null + ); + }); + it('should send back the response when awaiting a promise', async () => { + await transaction.run(); + const allResults = + await transaction.runAggregationQuery(aggregate); + const [runAggregateQueryResults] = allResults; + assert.deepStrictEqual( + runAggregateQueryResults, + runAggregationQueryUserResp + ); + }); + it('should send back the response when using a callback', done => { + const runAggregateQueryCallback: CommitCallback = ( + error: Error | null | undefined, + response?: unknown + ) => { + try { + assert.strictEqual(error, null); + assert.deepStrictEqual(response, runAggregationQueryUserResp); + done(); + } catch (e) { + done(e); + } + }; + transaction.run(() => { + transaction.runAggregationQuery( + aggregate, + runAggregateQueryCallback + ); + }); + }); + }); + }); + describe('runQuery', () => { + // These tests were created to catch regressions for transaction.runQuery changes. + const runQueryResp = { + batch: { + entityResults: [], + endCursor: { + type: 'Buffer', + data: Buffer.from(Array.from(Array(100).keys())), + }, + }, + }; + const runQueryUserResp: Entity[] = []; + const runQueryUserInfo: RunQueryInfo = { + moreResults: undefined, + endCursor: '[object Object]', + }; + const testErrorMessage = 'test-run-Query-error'; + let q: Query; + + beforeEach(async () => { + transactionWrapper = new MockedTransactionWrapper(); + transaction = transactionWrapper.transaction; + q = transactionWrapper.datastore.createQuery('Character'); + }); + + describe('should pass error back to the user', async () => { + beforeEach(() => { + transactionWrapper.mockGapicFunction( + GapicFunctionName.RUN_QUERY, + runQueryResp, + new Error(testErrorMessage) + ); + }); + + it('should send back the error when awaiting a promise', async () => { + try { + await transaction.run(); + await transaction.runQuery(q); + assert.fail('The run call should have failed.'); + } catch (error: any) { + assert.strictEqual(error['message'], testErrorMessage); + } + }); + it('should send back the error when using a callback', done => { + const callback: RunQueryCallback = ( + error: Error | null | undefined, + entities?: Entity[], + info?: RunQueryInfo + ) => { + try { + assert(error); + assert.strictEqual(error.message, testErrorMessage); + assert.deepStrictEqual(entities, undefined); + assert.deepStrictEqual(info, undefined); + done(); + } catch (e) { + done(e); + } + }; + transaction.run(() => { + transaction.runQuery(q, callback); + }); + }); + }); + describe('should pass response back to the user', async () => { + beforeEach(() => { + transactionWrapper.mockGapicFunction( + GapicFunctionName.RUN_QUERY, + runQueryResp, + null + ); + }); + it('should send back the response when awaiting a promise', async () => { + await transaction.run(); + const [runQueryResults, info] = await transaction.runQuery(q); + assert.deepStrictEqual(runQueryResults, runQueryUserResp); + assert.deepStrictEqual(info, runQueryUserInfo); + }); + it('should send back the response when using a callback', done => { + const callback: RunQueryCallback = ( + error: Error | null | undefined, + entities?: Entity[], + info?: RunQueryInfo + ) => { + try { + assert.strictEqual(error, null); + assert.deepStrictEqual(entities, runQueryUserResp); + assert.deepStrictEqual(info, runQueryUserInfo); + done(); + } catch (e) { + done(e); + } + }; + transaction.run(() => { + transaction.runQuery(q, callback); + }); + }); + }); + }); + describe('get', () => { + // These tests were created to catch regressions for transaction.get changes. + const getResp = { + found: [ + { + entity: { + key: { + path: [ + { + kind: 'Post', + name: 'post1', + idType: 'name', + }, + ], + partitionId: { + projectId: 'projectId', + databaseId: 'databaseId', + namespaceId: 'namespaceId', + }, + }, + excludeFromIndexes: false, + properties: {}, + }, + }, + ], + missing: [], + deferred: [], + transaction: testRunResp.transaction, + readTime: { + seconds: '1699470605', + nanos: 201398000, + }, + }; + const getUserResp = 'post1'; + const testErrorMessage = 'test-run-Query-error'; + let key: entity.Key; + + beforeEach(async () => { + transactionWrapper = new MockedTransactionWrapper(); + transaction = transactionWrapper.transaction; + key = transactionWrapper.datastore.key(['Company', 'Google']); + }); + + describe('should pass error back to the user', async () => { + beforeEach(() => { + transactionWrapper.mockGapicFunction( + GapicFunctionName.LOOKUP, + getResp, + new Error(testErrorMessage) + ); + }); + + it('should send back the error when awaiting a promise', async () => { + try { + await transaction.run(); + await transaction.get(key); + assert.fail('The run call should have failed.'); + } catch (error: any) { + assert.strictEqual(error['message'], testErrorMessage); + } + }); + it('should send back the error when using a callback', done => { + const callback: GetCallback = ( + err?: Error | null, + entity?: Entities + ) => { + try { + assert(err); + assert.strictEqual(err.message, testErrorMessage); + assert.deepStrictEqual(entity, undefined); + done(); + } catch (e) { + done(e); + } + }; + transaction.run(() => { + transaction.get(key, callback); + }); + }); + }); + describe('should pass response back to the user', async () => { + beforeEach(() => { + transactionWrapper.mockGapicFunction( + GapicFunctionName.LOOKUP, + getResp, + null + ); + }); + it('should send back the response when awaiting a promise', async () => { + await transaction.run(); + const [results] = await transaction.get(key); + const result = results[transactionWrapper.datastore.KEY]; + assert.deepStrictEqual(result.name, getUserResp); + }); + it('should send back the response when using a callback', done => { + const callback: GetCallback = ( + err?: Error | null, + entity?: Entities + ) => { + try { + const result = entity[transactionWrapper.datastore.KEY]; + assert.strictEqual(err, null); + assert.deepStrictEqual(result.name, getUserResp); + done(); + } catch (e) { + done(e); + } + }; + transaction.run(() => { + transaction.get(key, callback); + }); + }); + }); + }); + describe('concurrency', async () => { + // Items in this enum represent different points in time in the user code. + enum UserCodeEvent { + RUN_CALLBACK, + COMMIT_CALLBACK, + GET_CALLBACK, + RUN_QUERY_CALLBACK, + RUN_AGGREGATION_QUERY_CALLBACK, + CUSTOM_EVENT, + } + // A transaction event represents a point in time particular code is reached + // when running code that uses a transaction. + type TransactionEvent = GapicFunctionName | UserCodeEvent; + + // This object is a sample response from 'commit' in the Gapic layer. + const testCommitResp = { + mutationResults: [ + { + key: { + path: [ + { + kind: 'some-kind', + }, + ], + }, + }, + ], + }; + // This object is a sample response from 'lookup' in the Gapic layer. + const testLookupResp = { + found: [ + { + entity: { + key: { + path: [ + { + kind: 'Post', + name: 'post1', + idType: 'name', + }, + ], + partitionId: { + projectId: 'projectId', + databaseId: 'databaseId', + namespaceId: 'namespaceId', + }, + }, + excludeFromIndexes: false, + properties: {}, + }, + }, + ], + missing: [], + deferred: [], + transaction: testRunResp.transaction, + readTime: { + seconds: '1699470605', + nanos: 201398000, + }, + }; + // This object is a sample response from 'runQuery' in the Gapic layer. + const testRunQueryResp = { + batch: { + entityResults: [], + endCursor: { + type: 'Buffer', + data: Buffer.from(Array.from(Array(100).keys())), + }, + }, + }; + // This object is a sample response from 'runAggregationQuery' in the Gapic layer. + const testRunAggregationQueryResp = { + batch: { + aggregationResults: [ + { + aggregateProperties: { + 'average rating': { + meaning: 0, + excludeFromIndexes: false, + doubleValue: 100, + valueType: 'doubleValue', + }, + }, + }, + ], + moreResults: + google.datastore.v1.QueryResultBatch.MoreResultsType + .NO_MORE_RESULTS, + readTime: {seconds: '1699390681', nanos: 961667000}, + }, + query: null, + transaction: testRunResp.transaction, + }; + let transactionWrapper: MockedTransactionWrapper; + let transaction: Transaction; + + beforeEach(async () => { + transactionWrapper = new MockedTransactionWrapper(); + transaction = transactionWrapper.transaction; + }); + + afterEach(() => { + transactionWrapper.resetBeginTransaction(); + transactionWrapper.resetGapicFunctions(); + }); + + type GapicRequestData = { + call: GapicFunctionName; + request?: RequestType; + }; + + /** + * This object is used for testing the order that different events occur. + * The events can include user code reached, gapic code reached and callbacks called. + * + * @param {MockedTransactionWrapper} [transactionWrapper] A TransactionWrapper instance. + * @param {mocha.Done} [done] A function for signalling the test is complete. + * @param {TransactionEvent[]} [expectedOrder] The order events are expected to occur. + * @param {MockedTransactionWrapper} [transactionWrapper] A TransactionWrapper instance. + */ + class TransactionOrderTester { + /** + * expectedRequests equal the request data in the order they are expected to + * be passed into the Gapic layer. + * @private + */ + readonly #expectedRequests?: GapicRequestData[]; + /** + * requests are the actual order of the requests that are passed into the + * gapic layer + * @private + */ + readonly #requests: GapicRequestData[] = []; + /** + * expectedEventOrder is the order the test expects different events to occur + * such as a callback being called, Gapic functions being called or user + * code being run. + */ + readonly #expectedEventOrder: TransactionEvent[] = []; + /** + * eventOrder is the order events actually occur in the test and will be compared with + * expectedEventOrder. + * @private + */ + #eventOrder: TransactionEvent[] = []; + // A transaction wrapper object is used to contain the transaction and mocked Gapic functions. + #transactionWrapper: MockedTransactionWrapper; + // Stores the mocha done function so that it can be called from this object. + readonly #done: mocha.Done; + + /** + * Each time an event occurs this function is called to check to see if all + * events happened that were supposed to happen. If all events in the test + * happened then this function passes tests if the events happened in the + * right order. + */ + #checkForCompletion() { + if (this.#eventOrder.length >= this.#expectedEventOrder.length) { + try { + assert.deepStrictEqual( + this.#eventOrder, + this.#expectedEventOrder + ); + if (this.#expectedRequests) { + assert.deepStrictEqual( + this.#requests, + this.#expectedRequests + ); + } + this.#done(); + } catch (e) { + this.#done(e); + } + } + } + + constructor( + transactionWrapper: MockedTransactionWrapper, + done: mocha.Done, + expectedOrder: TransactionEvent[], + expectedRequests?: { + call: GapicFunctionName; + request?: RequestType; + }[] + ) { + this.#expectedEventOrder = expectedOrder; + this.#expectedRequests = expectedRequests; + this.#done = done; + transactionWrapper.callBackSignaler = ( + call: GapicFunctionName, + request?: RequestType + ) => { + try { + this.#requests.push({call, request}); + this.#eventOrder.push(call); + this.#checkForCompletion(); + } catch (e) { + done(e); + } + }; + this.#transactionWrapper = transactionWrapper; + } + + /** + * Returns a callback that will record an event so that order of events + * can be compared later. + * + * @param {UserCodeEvent} [event] The event that should be recorded. + */ + push(event: UserCodeEvent) { + return () => { + try { + this.#eventOrder.push(event); + this.#checkForCompletion(); + } catch (e) { + this.#done(e); + } + }; + } + } + + describe('should pass response back to the user', async () => { + beforeEach(() => { + transactionWrapper.mockGapicFunction( + GapicFunctionName.COMMIT, + testCommitResp, + null + ); + }); + + it('should call the callbacks in the proper order with run and commit', done => { + const tester = new TransactionOrderTester( + transactionWrapper, + done, + [ + UserCodeEvent.CUSTOM_EVENT, + GapicFunctionName.BEGIN_TRANSACTION, + UserCodeEvent.RUN_CALLBACK, + GapicFunctionName.COMMIT, + UserCodeEvent.COMMIT_CALLBACK, + ] + ); + transaction.run(tester.push(UserCodeEvent.RUN_CALLBACK)); + transaction.commit(tester.push(UserCodeEvent.COMMIT_CALLBACK)); + tester.push(UserCodeEvent.CUSTOM_EVENT)(); + }); + it('should call the callbacks in the proper order with commit', done => { + const tester = new TransactionOrderTester( + transactionWrapper, + done, + [ + UserCodeEvent.CUSTOM_EVENT, + GapicFunctionName.BEGIN_TRANSACTION, + GapicFunctionName.COMMIT, + UserCodeEvent.COMMIT_CALLBACK, + ] + ); + transaction.commit(tester.push(UserCodeEvent.COMMIT_CALLBACK)); + tester.push(UserCodeEvent.CUSTOM_EVENT)(); + }); + it('should call the callbacks in the proper order with two run calls', done => { + const tester = new TransactionOrderTester( + transactionWrapper, + done, + [ + UserCodeEvent.CUSTOM_EVENT, + GapicFunctionName.BEGIN_TRANSACTION, + UserCodeEvent.RUN_CALLBACK, + UserCodeEvent.RUN_CALLBACK, + ] + ); + transaction.run(tester.push(UserCodeEvent.RUN_CALLBACK)); + transaction.run(tester.push(UserCodeEvent.RUN_CALLBACK)); + tester.push(UserCodeEvent.CUSTOM_EVENT)(); + }); + it('should call the callbacks in the proper order with commit and then run', done => { + const tester = new TransactionOrderTester( + transactionWrapper, + done, + [ + UserCodeEvent.CUSTOM_EVENT, + GapicFunctionName.BEGIN_TRANSACTION, + UserCodeEvent.RUN_CALLBACK, + GapicFunctionName.COMMIT, + UserCodeEvent.COMMIT_CALLBACK, + ] + ); + transaction.commit(tester.push(UserCodeEvent.COMMIT_CALLBACK)); + transaction.run(tester.push(UserCodeEvent.RUN_CALLBACK)); + tester.push(UserCodeEvent.CUSTOM_EVENT)(); + }); + }); + describe('should pass response back to the user and check the request', async () => { + let key: entity.Key; + beforeEach(() => { + key = transactionWrapper.datastore.key(['Company', 'Google']); + transactionWrapper.mockGapicFunction( + GapicFunctionName.COMMIT, + testCommitResp, + null + ); + transactionWrapper.mockGapicFunction( + GapicFunctionName.LOOKUP, + testLookupResp, + null + ); + transactionWrapper.mockGapicFunction( + GapicFunctionName.RUN_QUERY, + testRunQueryResp, + null + ); + transactionWrapper.mockGapicFunction( + GapicFunctionName.RUN_AGGREGATION_QUERY, + testRunAggregationQueryResp, + null + ); + }); + const beginTransactionRequest = { + transactionOptions: {}, + projectId: 'project-id', + }; + const commitRequest = { + mode: 'TRANSACTIONAL', + transaction: testRunResp.transaction, + projectId: 'project-id', + mutations: [ + { + upsert: { + properties: {}, + key: { + partitionId: { + namespaceId: 'run-without-mock', + }, + path: [ + { + kind: 'Company', + name: 'Google', + }, + ], + }, + }, + }, + ], + }; + const lookupTransactionRequest = { + keys: [ + { + partitionId: { + namespaceId: 'run-without-mock', + }, + path: [ + { + kind: 'Company', + name: 'Google', + }, + ], + }, + ], + projectId: 'project-id', + readOptions: { + transaction: testRunResp.transaction, + }, + }; + describe('put, commit', () => { + const expectedRequests = [ + { + call: GapicFunctionName.BEGIN_TRANSACTION, + request: beginTransactionRequest, + }, + { + call: GapicFunctionName.COMMIT, + request: commitRequest, + }, + ]; + it('should verify that there is a BeginTransaction call while beginning later', done => { + const tester = new TransactionOrderTester( + transactionWrapper, + done, + [ + GapicFunctionName.BEGIN_TRANSACTION, + GapicFunctionName.COMMIT, + UserCodeEvent.COMMIT_CALLBACK, + ], + expectedRequests + ); + transaction.save({ + key, + data: '', + }); + transaction.commit(tester.push(UserCodeEvent.COMMIT_CALLBACK)); + }); + it('should verify that there is a BeginTransaction call while beginning early', done => { + const tester = new TransactionOrderTester( + transactionWrapper, + done, + [ + GapicFunctionName.BEGIN_TRANSACTION, + UserCodeEvent.RUN_CALLBACK, + GapicFunctionName.COMMIT, + UserCodeEvent.COMMIT_CALLBACK, + ], + expectedRequests + ); + transaction.save({ + key, + data: '', + }); + transaction.run(tester.push(UserCodeEvent.RUN_CALLBACK)); + transaction.commit(tester.push(UserCodeEvent.COMMIT_CALLBACK)); + }); + }); + describe('lookup, lookup, put, commit', () => { + const expectedRequests = [ + { + call: GapicFunctionName.BEGIN_TRANSACTION, + request: beginTransactionRequest, + }, + { + call: GapicFunctionName.COMMIT, + request: commitRequest, + }, + { + call: GapicFunctionName.LOOKUP, + request: lookupTransactionRequest, + }, + { + call: GapicFunctionName.LOOKUP, + request: lookupTransactionRequest, + }, + ]; + it('should verify that there is a BeginTransaction call while beginning later', done => { + const tester = new TransactionOrderTester( + transactionWrapper, + done, + [ + GapicFunctionName.BEGIN_TRANSACTION, + GapicFunctionName.COMMIT, + UserCodeEvent.COMMIT_CALLBACK, + GapicFunctionName.LOOKUP, + GapicFunctionName.LOOKUP, + UserCodeEvent.GET_CALLBACK, + UserCodeEvent.GET_CALLBACK, + ], + expectedRequests + ); + transaction.get(key, tester.push(UserCodeEvent.GET_CALLBACK)); + transaction.get(key, tester.push(UserCodeEvent.GET_CALLBACK)); + transactionWrapper.transaction.save({ + key, + data: '', + }); + transaction.commit(tester.push(UserCodeEvent.COMMIT_CALLBACK)); + }); + it('should verify that there is a BeginTransaction call while beginning early', done => { + const tester = new TransactionOrderTester( + transactionWrapper, + done, + [ + GapicFunctionName.BEGIN_TRANSACTION, + UserCodeEvent.RUN_CALLBACK, + GapicFunctionName.COMMIT, + UserCodeEvent.COMMIT_CALLBACK, + GapicFunctionName.LOOKUP, + GapicFunctionName.LOOKUP, + UserCodeEvent.GET_CALLBACK, + UserCodeEvent.GET_CALLBACK, + ], + expectedRequests + ); + transaction.run(tester.push(UserCodeEvent.RUN_CALLBACK)); + transaction.get(key, tester.push(UserCodeEvent.GET_CALLBACK)); + transaction.get(key, tester.push(UserCodeEvent.GET_CALLBACK)); + transactionWrapper.transaction.save({ + key, + data: '', + }); + transaction.commit(tester.push(UserCodeEvent.COMMIT_CALLBACK)); + }); + }); + }); + }); + }); + + describe('run without setting up transaction id', () => { + // These tests were created so that when transaction.run is restructured we + // can be confident that it works the same way as before. + const testRunResp = { + transaction: Buffer.from(Array.from(Array(100).keys())), + }; + const namespace = 'run-without-mock'; + const projectId = 'project-id'; + const testErrorMessage = 'test-error'; + const options = { + projectId, + namespace, + }; + const datastore = new Datastore(options); + const transactionWithoutMock = datastore.transaction(); + const dataClientName = 'DatastoreClient'; + let dataClient: ClientStub | undefined; + let originalBeginTransactionMethod: Function; + + beforeEach(async () => { + // In this before hook, save the original beginTransaction method in a variable. + // After tests are finished, reassign beginTransaction to the variable. + // This way, mocking beginTransaction in this block doesn't affect other tests. + const gapic = Object.freeze({ + v1: require('../src/v1'), + }); + // Datastore Gapic clients haven't been initialized yet, so we initialize them here. + datastore.clients_.set( + dataClientName, + new gapic.v1[dataClientName](options) + ); + dataClient = datastore.clients_.get(dataClientName); + if (dataClient && dataClient.beginTransaction) { + originalBeginTransactionMethod = dataClient.beginTransaction; + } + }); + + afterEach(() => { + // beginTransaction has likely been mocked out in these tests. + // We should reassign beginTransaction back to its original value for tests outside this block. + if (dataClient && originalBeginTransactionMethod) { + dataClient.beginTransaction = originalBeginTransactionMethod; + } + }); + + function setupBeginTransaction(err: Error | null | undefined) { + if (dataClient) { + dataClient.beginTransaction = ( + request: protos.google.datastore.v1.IBeginTransactionRequest, + options: CallOptions, + callback: Callback< + protos.google.datastore.v1.IBeginTransactionResponse, + | protos.google.datastore.v1.IBeginTransactionRequest + | null + | undefined, + {} | null | undefined + > + ) => { + callback(err, testRunResp); + }; + } + } + + describe('should pass error back to the user', async () => { + beforeEach(() => { + // Mock out begin transaction and send error back to the user + // from the Gapic layer. + setupBeginTransaction(new Error(testErrorMessage)); + }); + + it('should send back the error when awaiting a promise', async () => { + try { + await transactionWithoutMock.run(); + assert.fail('The run call should have failed.'); + } catch (error: any) { + assert.strictEqual(error['message'], testErrorMessage); + } + }); + it('should send back the error when using a callback', done => { + const runCallback: RunCallback = ( + error: Error | null, + transaction: Transaction | null, + response?: google.datastore.v1.IBeginTransactionResponse + ) => { + try { + assert(error); + assert.strictEqual(error.message, testErrorMessage); + assert.strictEqual(transaction, null); + assert.strictEqual(response, testRunResp); + done(); + } catch (e) { + done(e); + } + }; + transactionWithoutMock.run({}, runCallback); + }); + }); + describe('should pass response back to the user', async () => { + beforeEach(() => { + // Mock out begin transaction and send a response + // back to the user from the Gapic layer. + setupBeginTransaction(null); + }); + it('should send back the response when awaiting a promise', async () => { + const [transaction, resp] = await transactionWithoutMock.run(); + assert.strictEqual(transaction, transactionWithoutMock); + assert.strictEqual(resp, testRunResp); + }); + it('should send back the response when using a callback', done => { + const runCallback: RunCallback = ( + error: Error | null, + transaction: Transaction | null, + response?: google.datastore.v1.IBeginTransactionResponse + ) => { + try { + assert.strictEqual(error, null); + assert.deepStrictEqual(response, testRunResp); + assert.strictEqual(transaction, transactionWithoutMock); + done(); + } catch (e) { + done(e); + } + }; + transactionWithoutMock.run({}, runCallback); + }); + }); + }); + describe('commit', () => { - beforeEach(() => { + beforeEach(done => { transaction.id = TRANSACTION_ID; + transaction.request_ = (config, callback) => { + callback(null, { + transaction: Buffer.from(Array.from(Array(100).keys())), + }); + // Delay to give the transaction mutex the opportunity to unlock before running tests. + setImmediate(() => { + done(); + }); + }; + transaction.run(); }); afterEach(() => {