TcpCommunicationSpi split to independent classes [IGNITE-13098]

[Maksim Stepachev]

Hi, I'm almost done donating these changes from the GridGain repository. It
has been tested and published in several releases. I would like to
encourage you to make the same changes because the product testing process
is very difficult. These changes usually do not affect the behavior and
public API. In the next patch, I will pass a Dependencyresolver that avoids
using reflection in tests.

Issues - https://issues.apache.org/jira/browse/IGNITE-13098
Draft here - https://github.com/apache/ignite/pull/8057
Description

This ticket describes  requirements for TcpCommunicationSpi refactoring.
The main goal is to split the class without changing behavior and public
API.

*Actual problem:*

CurrentlyTcpCommunicationSpi has over 5K lines and includes about15+ inner
classes like:

   1. ShmemAcceptWorker
   2. SHMemHandshakeClosure
   3. ShmemWorker
   4. CommunicationDiscoveryEventListener
   5. CommunicationWorker
   6. ConnectFuture
   7. ConnectGateway
   8. ConnectionKey
   9. ConnectionPolicy
   10. DisconnectedSessionInfo
   11. FirstConnectionPolicy
   12. HandshakeTimeoutObject
   13. RoundRobinConnectionPolicy
   14. TcpCommunicationConnectionCheckFuture
   15. TcpCommunicationSpiMBeanImpl

In addition, it contains logic of client connection life cycle, nio server
handler, and handshake handler.

The classes above have cyclic dependencies and high coupling.The whole
mechanism works because classes have access to each other via parent class
references. As a result, initialization of class isn't consistent. By
consistent I mean that class created via constructor is ready to be used.
All of the classes work with context and shareproperties everywhere.

Many methods of TcpCommunicationSpi don’t have a single responsibility.
Example is getNodeAttribute:,it makes client reservation,  takes the IP
address of the node and provides attributes.

It works fine and we usually don’t have reasons to change anything. But if
you want to create a test that has a little different behavior than a
blocking message, you can't mock or change the behavior of inner classes.
For example, test covering change in the handshake process. Some people
make test methods in public API like "closeConnections" or
"openSocketChannel" because the current design isn't fine for it. It also
takes a lot of time for test development for minor changes.

*Solution:*

The scope of work is big and communication spi is place which should be
changed carefully. I recommend to make this refactoring step by step.

   - The first idea is to split the parent class into independent classes
   and move them to the internal package. We should achieveSOLID when it’s
   done.
   - Extract spread logic to appropriate classes like ClientPool,
   HandshakeHandler, etc.
   - Make a common transfer object for TCSpi configuration.
   - Make dependencies direct if it is possible.
   - Initialize all dependencies in one place.
   - Make child classes context-free.
   - Try to do classes more testable.
   - Use the idea of dependency injection without a framework for it.

*Benefits:*

With the ability to write truly jUnit-style tests and cover functionality
with better testing we get a way to easier develop new features and
optimizations needed in such low-level components as TcpCommunicationSpi.

Examples of features that improve usability of Apache Ignite a lot are:
inverse communication connection with optimizations and connection
multiplexing. Both of the features could be used in environments with
restricted network connectivity (e.g. when connections between nodes could
be established only in one direction).