[GitHub] [activemq-artemis] brusdev commented on a diff in pull request #4539: ARTEMIS-4348 Parse connectors on broker.xml for default URI

["brusdev (via GitHub)" <gi...@apache.org>]

brusdev commented on code in PR #4539:
URL: https://github.com/apache/activemq-artemis/pull/4539#discussion_r1255245184


##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java:
##########
@@ -39,6 +39,8 @@ public abstract class ActionAbstract implements Action {
    @Option(name = "--verbose", description = "Print additional information.")
    public boolean verbose;
 
+   String brokerConfiguration = "broker.xml";

Review Comment:
   The name `brokerConfiguration` is already used in the method `protected Configuration getBrokerConfiguration()`
   ```suggestion
      String brokerConfigurationFilename = "broker.xml";
   ```



##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/Transfer.java:
##########
@@ -43,7 +43,7 @@ public class Transfer extends InputAbstract {
    protected String sourceURL = DEFAULT_BROKER_URL;
 
    @Option(name = "--source-acceptor", description = "Acceptor used to build URL towards the broker. Default: 'artemis'.")
-   protected String sourceAcceptor;
+   protected String sourceAcceptor = "artemis";

Review Comment:
   The `DEFAULT_BROKER_ACCEPTOR` could be reused here:
   ```suggestion
      protected String sourceAcceptor = DEFAULT_BROKER_ACCEPTOR;
   ```



##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java:
##########
@@ -113,7 +115,23 @@ public String getBrokerURLInstance(String acceptor) {
                   return new URI(scheme, null, host, port, null, null, null).toString();
                }
             }
+
+            for (TransportConfiguration connectorConfiguration: brokerConfiguration.getConnectorConfigurations().values()) {
+               if (connectorConfiguration.getName().equals(acceptor)) {
+                  Map<String, Object> acceptorParams = connectorConfiguration.getParams();
+                  String scheme = ConfigurationHelper.getStringProperty(TransportConstants.SCHEME_PROP_NAME, SchemaConstants.TCP, acceptorParams);
+                  String host = ConfigurationHelper.getStringProperty(TransportConstants.HOST_PROP_NAME, "localhost", acceptorParams);
+                  int port = ConfigurationHelper.getIntProperty(TransportConstants.PORT_PROP_NAME, 61616, acceptorParams);
+
+                  if (InetAddress.getByName(host).isAnyLocalAddress()) {
+                     host = "localhost";
+                  }
+
+                  return new URI(scheme, null, host, port, null, null, null).toString();
+               }
+            }

Review Comment:
   This block of the code is causing a duplication warning in my IDE because is very similar to the block of code for the acceptors.



##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java:
##########
@@ -113,7 +115,23 @@ public String getBrokerURLInstance(String acceptor) {
                   return new URI(scheme, null, host, port, null, null, null).toString();
                }
             }
+
+            for (TransportConfiguration connectorConfiguration: brokerConfiguration.getConnectorConfigurations().values()) {
+               if (connectorConfiguration.getName().equals(acceptor)) {
+                  Map<String, Object> acceptorParams = connectorConfiguration.getParams();
+                  String scheme = ConfigurationHelper.getStringProperty(TransportConstants.SCHEME_PROP_NAME, SchemaConstants.TCP, acceptorParams);
+                  String host = ConfigurationHelper.getStringProperty(TransportConstants.HOST_PROP_NAME, "localhost", acceptorParams);
+                  int port = ConfigurationHelper.getIntProperty(TransportConstants.PORT_PROP_NAME, 61616, acceptorParams);
+
+                  if (InetAddress.getByName(host).isAnyLocalAddress()) {
+                     host = "localhost";
+                  }
+
+                  return new URI(scheme, null, host, port, null, null, null).toString();
+               }
+            }
          } catch (Exception e) {
+            e.printStackTrace();

Review Comment:
   Why is not the stack trace printed using `getActionContext().out` ?



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org