-
Notifications
You must be signed in to change notification settings - Fork 107
Commit 1585d4a
![gcf-owl-bot[bot]](https://avatars.githubusercontent.com/in/99011?v=4&size=40){kind=avatar}
feat: Nodejs transaction redesign feature branch (#1235)
* 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)
[](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) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](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.
---
- [ ] <!-- rebase-check -->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).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40Ni4wIiwidXBkYXRlZEluVmVyIjoiMzcuNDYuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->
* 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: googleapis/synthtool@1063ef3
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:e92044720ab3cb6984a70b0c6001081204375959ba3599ef6c42dd99a7783a67
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* 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 <[email protected]>
Co-authored-by: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* 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)
[](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) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](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.
---
- [ ] <!-- rebase-check -->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).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40Ni4wIiwidXBkYXRlZEluVmVyIjoiMzcuNDYuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->
* 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: googleapis/synthtool@1063ef3
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:e92044720ab3cb6984a70b0c6001081204375959ba3599ef6c42dd99a7783a67
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* 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 <[email protected]>
Co-authored-by: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* 🦉 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 <[email protected]>
Co-authored-by: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>](https://renovatebot.com){kind=link}
1 parent 74f1e28 commit 1585d4aCopy full SHA for 1585d4a
File tree
5 files changed
+1871
-147
lines changedFilter options
- src
- system-test
- test
5 files changed
+1871
-147
lines changed+1
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| 46 | + | |
46 | 47 | | |
47 | 48 | | |
48 | 49 | | |
| |||
+17-4
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
82 | | - | |
| 82 | + | |
83 | 83 | | |
84 | 84 | | |
85 | 85 | | |
| |||
546 | 546 | | |
547 | 547 | | |
548 | 548 | | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
549 | 562 | | |
550 | 563 | | |
551 | 564 | | |
| |||
1157 | 1170 | | |
1158 | 1171 | | |
1159 | 1172 | | |
1160 | | - | |
| 1173 | + | |
1161 | 1174 | | |
1162 | 1175 | | |
1163 | 1176 | | |
| |||
1166 | 1179 | | |
1167 | 1180 | | |
1168 | 1181 | | |
1169 | | - | |
| 1182 | + | |
1170 | 1183 | | |
1171 | | - | |
| 1184 | + | |
1172 | 1185 | | |
1173 | 1186 | | |
1174 | 1187 | | |
| |||
0 commit comments