Skip to content

Deprecate Session.(read|write)Transaction in favor of execute(Read|Write) methods #911

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 7, 2022

Conversation

bigmontz
Copy link
Contributor

@bigmontz bigmontz commented Apr 4, 2022

The new methods provides a ManagedTransaction objects to the transaction functions. These
transaction objects don't have commit, rollback and close capabilities exposed in the API.

…d|Write)" methods

The new methods provides a `ManagedTransaction` objects to the transaction functions. These
transaction objects don't have `commit`, `rollback` and `close` capabilities exposed in the API.
@bigmontz bigmontz changed the title Deprecate Session.(read|write)Transaction in favor for execute(Read|Write) methods Deprecate Session.(read|write)Transaction in favor of execute(Read|Write) methods Apr 4, 2022
@bigmontz bigmontz marked this pull request as ready for review April 5, 2022 08:50
* @returns {Observable}
*/
_executeInTransaction (accessMode, work, transactionConfig) {
const wrapper = txc => new RxManagedTransaction({
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't it be easier if RxManagedTransaction constructor accepted txc as its argument instead of an arbitrary object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The bind will only change places at the end. I don't want to keep reference from the transaction inside the ManagedTransaction to avoid the client code by pass the encapsulation and commit/rollback/close it anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

what about:

class RxManagedTransaction {

constructor(txc) {
    this._run = txc.run.bind(txc)
    this._isOpen = tcx.isOpen.bind(txc)
  }

so you don't have to repeat the destructuring every time you call the constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved to the binding to a static factory method called fromTransaction. It makes easier to pass the function as parameter.

* @returns {Observable}
*/
_executeInTransaction (accessMode, work, transactionConfig) {
const wrapper = txc => new RxManagedTransaction({
Copy link
Contributor

Choose a reason for hiding this comment

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

what about:

class RxManagedTransaction {

constructor(txc) {
    this._run = txc.run.bind(txc)
    this._isOpen = tcx.isOpen.bind(txc)
  }

so you don't have to repeat the destructuring every time you call the constructor

@bigmontz bigmontz merged commit 8d7b671 into neo4j:5.0 Apr 7, 2022
@bigmontz bigmontz deleted the 5.0-execute-tx branch April 7, 2022 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants