cshannon commented on code in PR #2133:
URL: https://github.com/apache/activemq/pull/2133#discussion_r3934964932
##########
activemq-client/src/main/java/org/apache/activemq/transport/failover/FailoverTransportFactory.java:
##########
@@ -43,6 +45,18 @@ public Transport doConnect(URI location) throws IOException {
}
}
+ @Override
+ public Transport doConnect(URI location, SslContext sslContext) throws
Exception {
Review Comment:
i think you should be able to remove the copy and paste and update the
doConnect(location) method to just delegate to this one now with
doConnect(location, null)
##########
activemq-client/src/main/java/org/apache/activemq/transport/tcp/TcpTransportFactory.java:
##########
@@ -121,8 +149,72 @@ protected boolean isUseInactivityMonitor(Transport
transport) {
return true;
}
+ @Override
+ public Transport doConnect(URI location) throws Exception {
+ return doConnect(location, (SslContext) null);
+ }
+
+ /**
+ * Connects a TCP based transport. The given {@link SslContext} is handed
+ * to {@link #createSocketFactory(SslContext)} so SSL capable subclasses
+ * can derive their socket factory from it; plain TCP ignores it.
+ */
+ @Override
+ public Transport doConnect(URI location, SslContext sslContext) throws
Exception {
+ try {
+ Map<String, String> options = new HashMap<String,
String>(URISupport.parseParameters(location));
+ if (!options.containsKey("wireFormat.host")) {
+ options.put("wireFormat.host", location.getHost());
+ }
+ WireFormat wf = createWireFormat(options);
+ Transport transport = createTransport(location, wf, sslContext);
+ Transport rc = configure(transport, wf, options);
+ //remove auto
+ IntrospectionSupport.extractProperties(options, "auto.");
+
+ if (!options.isEmpty()) {
+ throw new IllegalArgumentException("Invalid connect
parameters: " + options);
+ }
+ return rc;
+ } catch (URISyntaxException e) {
+ throw IOExceptionSupport.create(e);
+ }
+ }
+
+ @Override
+ public Transport doCompositeConnect(URI location) throws Exception {
+ return doCompositeConnect(location, (SslContext) null);
+ }
+
+ @Override
+ public Transport doCompositeConnect(URI location, SslContext sslContext)
throws Exception {
Review Comment:
Same comment here
##########
activemq-client/src/main/java/org/apache/activemq/transport/tcp/TcpTransportFactory.java:
##########
@@ -121,8 +149,72 @@ protected boolean isUseInactivityMonitor(Transport
transport) {
return true;
}
+ @Override
+ public Transport doConnect(URI location) throws Exception {
+ return doConnect(location, (SslContext) null);
+ }
+
+ /**
+ * Connects a TCP based transport. The given {@link SslContext} is handed
+ * to {@link #createSocketFactory(SslContext)} so SSL capable subclasses
+ * can derive their socket factory from it; plain TCP ignores it.
+ */
+ @Override
+ public Transport doConnect(URI location, SslContext sslContext) throws
Exception {
Review Comment:
Is there any way to share this logic with TransportFactory?
This is a
[copy](https://github.com/apache/activemq/pull/2133/changes#diff-e046a9bfc52ea7eb6ec812e84805d8c33a8ecd1a3ac14e68eacd61102ae8b3bcR155)
and we are at risk of getting out of sync by only updating one or the other in
the future
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
