Skip to content

Improve connection/protocol layers #659

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 1 commit into from
Jan 14, 2021

Conversation

bigmontz
Copy link
Contributor

@bigmontz bigmontz commented Dec 23, 2020

The implementation of the connection channel, the protocol and handshake are dependent in the other in cross mutual dependency situation. This cross dependency situation could leave to unexpected behaviours when something change on this layers.

The solution for this issue was separate the bolt protocol with it handshake process, creation, observers and so on in a different package to hide the details and make easier to spot the unwanted dependencies.

The following changes were done to achieve these goals:

  • Extract a factory for the connection removing the create method
  • Extract the handshake process and separate in two, the handshake message exchange and the protocol initialisation
  • Remove the response parsing and response observers management from the connection and put this in the bolt package (response-handler.js)
  • Move the message write responsibility to the protocol object
  • Remove connection dependency from the stream-observers
  • Fix the connection integration tests
  • Improve the unit tests
  • Do not touch or break test kit tests and stress tests

@bigmontz bigmontz force-pushed the 4.3-improve-channel-connection branch 2 times, most recently from a077aa4 to d614d13 Compare December 28, 2020 16:33
Copy link
Contributor

@2hdddg 2hdddg left a comment

Choose a reason for hiding this comment

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

Looks like good improvements! Got halfway through the changes, need coffee!!

@bigmontz bigmontz force-pushed the 4.3-improve-channel-connection branch from 48ed401 to ae15f60 Compare January 8, 2021 13:55
@bigmontz bigmontz marked this pull request as ready for review January 13, 2021 09:35
@bigmontz bigmontz force-pushed the 4.3-improve-channel-connection branch 2 times, most recently from a646648 to dccc3b4 Compare January 13, 2021 13:21
The implementation of the connection channel, the protocol and handshake are dependent in the other in cross mutual dependency situation. This cross dependency situation could leave to unexpected behaviours when something change on this layers.

The solution for this issue was separate the bolt protocol with it handshake process, creation, observers and so on in a different package to hide the details and make easier to spot the unwanted dependencies.

The following changes were done to achieve these goals:
* [x] Extract a factory for the connection removing the create method
* [x] Extract the handshake process and separate in two, the handshake message exchange and the protocol initialisation
* [x] Remove the response parsing and response observers management from the connection and put this in the bolt package (`response-handler.js`)
* [x] Move the message write responsibility to the protocol object
* [x] Remove connection dependency from the stream-observers
* [x] Fix the connection integration tests
* [x] Improve the unit tests
* [x] Do not touch or break test kit tests and stress tests
@bigmontz bigmontz force-pushed the 4.3-improve-channel-connection branch from dccc3b4 to 99d192d Compare January 14, 2021 09:59
@bigmontz bigmontz merged commit 50a3424 into neo4j:4.3 Jan 14, 2021
@bigmontz bigmontz deleted the 4.3-improve-channel-connection branch January 14, 2021 10:43
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