[
https://issues.apache.org/jira/browse/QPID-4659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13614095#comment-13614095
]
Robbie Gemmell commented on QPID-4659:
--------------------------------------
Looks good overall, and involved far less actual change than I expected, which
is nice (the diff itself is large though :P).
Some nitpicks/questions:
It seemed preferable when there was an enum for MessageMetaDataType to
reference the ordinals from rather than each type just choosing the int
directly. Obviously if we retained an Enum it would have to go somewhere common
that all the impls could reference it though, and adding new plugins later
would still involve choosing the new int and updating the enum at some point,
so I can kind of see why you just removed it, it jsut felt a little nicer when
it was there.
MessageMetaDataTypeRegistry.java doesnt check whether there were no
MessageMetaDataType protocol creator implementations (the service loader has a
method to validate that for you), or whether diffrent types accidentally (or
not) used the same ordinal, it should probably do both?
Continuing that theme, should we throw an exception in
MultiVersionProtocolEngineFactory.java if there are no protocol creator
implementations, or more than one was to specify the same header value?
Commented out code in MessageConverter_0_10_to_1_0.java ?
and MessageConverter_0_8_to_1_0.java?
Should MessageConverter_1_0_to_0_10.java return null from convert, or perhaps
throw an Unsupported exception? Will it will just NPE after using that method?
The new subscription interest checking seems like it would never try to convert
the messages if we just removed the not-implemented converter.. ?
Why move this line? Does it log connections to the vhost earlier now as a
result, and then subsequently fail if its denied ACL access or the Vhost isnt
the Master in a HA cluster? If so, do we really want it to do that?
{noformat}
@@ -78,6 +78,8 @@ public class ConnectionOpenMethodHandler
}
else
{
+ session.setVirtualHost(
virtualHost);
+
// Check virtualhost access
if
(!virtualHost.getSecurityManager().accessVirtualhost(virtualHostName,
session.getRemoteAddress()))
{
@@ -88,7 +90,6 @@ public class ConnectionOpenMethodHandler
throw
body.getConnectionException(AMQConstant.CONNECTION_FORCED, "Virtual host '" +
virtualHost.getName() + "' is not active");
}
- session.setVirtualHost(virtualHost);
{noformat}
> [Java Broker] Refactor broker to separate protocol independent from protocol
> specific classes
> ---------------------------------------------------------------------------------------------
>
> Key: QPID-4659
> URL: https://issues.apache.org/jira/browse/QPID-4659
> Project: Qpid
> Issue Type: Improvement
> Components: Java Broker
> Reporter: Rob Godfrey
> Assignee: Rob Godfrey
>
> The Java Broker currently supports all versions of the AMQP protocol from 0-8
> to 1.0, however the current structure of the code within the broker makes it
> hard to distinguish between code which is specific to a version of the
> protocol and code which is common across all protocols.
> By refactoring we can separate the protocol dependent and independent parts
> and allow for the possibility of separating out the different protocol
> implementations into independently loadable libraries.
> Fundamentally the refactoring takes the form of moving protocol specific
> classes into org.apache.qpid.server.protocol.v{0-8,0-10,1-0} and sub-packages
> and using the QpidClassLoader to load the protocol implementations (there are
> three separate implementations to load - the protocol delegate creators that
> interface to the IO code; the MessageMetaDataTypes used to (de)serialize the
> message data to stores; and MessageConverters used to convert between message
> formats and allow 0-8 messages to be received by 1-0 consumers.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
