gemmellr commented on code in PR #4186:
URL: https://github.com/apache/activemq-artemis/pull/4186#discussion_r954993212
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpOutboundConnectionTest.java:
##########
@@ -132,6 +132,7 @@ public void connectionFailed(ActiveMQException exception,
boolean failedOver, St
}
Wait.assertEquals(0, remote::getConnectionCount);
+ System.out.println(remotingConnection.getClass());
Review Comment:
Leftover? Logger or remove.
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/spi/core/protocol/AbstractRemotingConnection.java:
##########
@@ -45,6 +45,7 @@ public abstract class AbstractRemotingConnection implements
RemotingConnection {
protected final Connection transportConnection;
protected final Executor executor;
protected final long creationTime;
+ protected volatile boolean destroyed = false;
Review Comment:
= false seems superfluous, and looks odd when none of the others have
similar initialisations.
##########
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireProtocolManager.java:
##########
@@ -451,6 +451,7 @@ public void fireAdvisory(AMQConnectionContext context,
Command command,
ConsumerId targetConsumerId,
String originalConnectionId) throws Exception {
+ System.out.println("FireAdvisory: " + command);
Review Comment:
Leftover? Logger or remove.
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/spi/core/protocol/RemotingConnection.java:
##########
@@ -35,9 +35,11 @@
/**
* A RemotingConnection is a connection between a client and a server.
*
- *
* Perhaps a better name for this class now would be ProtocolConnection as this
* represents the link with the used protocol
+ *
+ * A default noop implementation is provided. This, for example, makes test
+ * implementations much simpler.
Review Comment:
Though they can also make managing multiple 'real implementations' as the
codebase has a little more awkward, and can have an overhead vs concrete method
impl. The interface becomes less streamlined also.
There only looks to be 1 test impl this actually changed (which could
perhaps have just used mocking instead, maybe using even less lines than the
defaultings) and then also 1 'fake' management connection. I'd personally use
the abstract one as proper base for all the real impls like you did and leave
the management one closer to as-is than convert all the interface methods to
default to largely return false/null just so the fake one doesnt have to.
--
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]
