chore: refactor ProxyServer to use channels instead of sockets #184
Conversation
Refactors the ProxyServer to use the SocketChannel interfaces instead of Sockets. This makes the sockets replaceable for the new Unix domain socket implementation in Java16, which again means that we can conditionally load that when we know that we have Java16 or higher as the target language.
Codecov Report
@@ Coverage Diff @@
## postgresql-dialect #184 +/- ##
========================================================
- Coverage 84.09% 84.07% -0.03%
- Complexity 1614 1622 +8
========================================================
Files 114 115 +1
Lines 5470 5525 +55
Branches 733 738 +5
========================================================
+ Hits 4600 4645 +45
- Misses 644 651 +7
- Partials 226 229 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…/pgadapter into refactor-to-channels
…/pgadapter into refactor-to-channels
| GOOGLE_CLOUD_PROJECT: "span-cloud-testing" | ||
| GOOGLE_CLOUD_INSTANCE: "pgadapter-testing" | ||
| GOOGLE_CLOUD_DATABASE: "testdb_integration" | ||
| GOOGLE_CLOUD_ENDPOINT: "spanner.googleapis.com" |
There was a problem hiding this comment.
Is it fine to to put these in public repo?
There was a problem hiding this comment.
The endpoint is public information. It's for example also included in the generated client for each programming language.
The project, instance and database names are also included in many other repositories.
| @@ -80,7 +83,9 @@ public class ConnectionHandler extends Thread { | |||
| private static final String CHANNEL_PROVIDER_PROPERTY = "CHANNEL_PROVIDER"; | |||
|
|
|||
| private final ProxyServer server; | |||
| private final Socket socket; | |||
| private final ByteChannel socketChannel; | |||
| private final InetSocketAddress remoteAddress; | |||
There was a problem hiding this comment.
Should we add a comment that this is nullable?
There was a problem hiding this comment.
Good point, added a @Nullable annotation and a comment.
| super("ConnectionHandler-" + CONNECTION_HANDLER_ID_GENERATOR.incrementAndGet()); | ||
| this.server = server; | ||
| this.socket = socket; | ||
| this.socketChannel = byteChannel; | ||
| if (byteChannel instanceof SocketChannel |
There was a problem hiding this comment.
What are the cases when byteChannel is not of SocketChannel type?
There was a problem hiding this comment.
I've added a comment to explain. SocketChannel is only used by TCP connections. Unix domain sockets use other implementations.
|
There's a good chance that this will not be needed, as the Unix domain socket factory that we are currently using has added support for GraalVM, which was the initial reason to add this. |
Refactors the ProxyServer to use the SocketChannel interfaces instead of Sockets. This makes the sockets replaceable for the new Unix domain socket implementation in Java16, which again means that we can conditionally load that when we know that we have Java16 or higher as the target language.
Also adds a Maven profile for building a .jar that can be used for a native image. This profile loads a Socket factory that uses the default Java Unix domain sockets implementation instead of a third-party dependency that requires a native library.