Skip to content

Hide protocol object behind the Connection #1139

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 9 commits into from
Sep 18, 2023

Conversation

bigmontz
Copy link
Contributor

@bigmontz bigmontz commented Sep 7, 2023

The connection in the driver is exposing the protocol object, which is not great. Improve this part of the code can be done by make the connection object have methods to do high level requests to the server.

List of calls the core driver does using the protocol, by passing the connection:

  • version
  • run
  • beginTransaction
  • commitTransaction
  • rollbackTransaction

Methods is present in the Connection interface (used by core in bold):

  • id
  • database
  • server
  • authToken
  • address
  • version
  • supportsReAuth
  • isOpen
  • protocol
  • connect
  • write
  • resetAndFlush
  • hasOngoingObservableRequests
  • _release

So, isOpen, resetAndFlush and hasOngoingObservableRequests are the methods which will stay in the connection along with the new methods. The method release will move the a Releasable interface, which will be composed with Connection when returning the connection from the provider.

The Releasable interface is also defined to enable the ConnectionProvider returns a connection which can be released back to the pool.

Internally, bolt-connection can keep exposing the internal of the connection outside the connection in a first moment. The full encapsulation of the protocol should be done in the next phase of refactoring.

The connection in the driver is exposing the protocol object, which is not great. Improve this part of the code can be done by make the connection object have methods to do high level requests to the server.

List of calls the core driver does using the protocol, by passing the connection:

* `version`
* `run`
* `beginTransaction`
* `commitTransaction`
* `rollbackTransaction`

Methods is present in the `Connection` interface (used by core in **bold**):

* `id`
* `database`
* `server`
* `authToken`
* `address`
* `version`
* `supportsReAuth`
* **`isOpen`**
* **`protocol`**
* `connect`
* `write`
* **`resetAndFlush`**
* **`hasOngoingObservableRequests`**
* **`_release`**

So, `isOpen`, `resetAndFlush` and `hasOngoingObservableRequests`  are the methods which will stay in the connection along with the new methods. The method `release` will move the a `Releasable` interface, which will be composed with `Connection` when returning the connection from the provider.

The `Releasable` interface is also defined to enable the `ConnectionProvider` returns a connection which can be released back to the pool.

Internally, `bolt-connection` can keep exposing the internal of the connection outside the connection in a first moment. The full encapsulation of the `protocol` should be done in the next phase of refactoring.
@bigmontz bigmontz force-pushed the 5.13-refactory-connection branch from 2174247 to ae5043a Compare September 7, 2023 15:03
@bigmontz bigmontz marked this pull request as ready for review September 7, 2023 15:03
Copy link
Member

@robsdedude robsdedude left a comment

Choose a reason for hiding this comment

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

As an added bonus, I like how the new structure makes the mocking in tests cleaner as well 🙌

bigmontz and others added 4 commits September 8, 2023 16:14
…re connection

The method _acquireConnection and _run methods were replicating the same logic.
The logic was extracted to a common method avoid duplication.
@bigmontz bigmontz merged commit 48f6af6 into neo4j:5.0 Sep 18, 2023
@bigmontz bigmontz deleted the 5.13-refactory-connection branch September 18, 2023 12:31
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