[ 
https://issues.apache.org/jira/browse/QPID-4659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13615264#comment-13615264
 ] 

Rob Godfrey commented on QPID-4659:
-----------------------------------

{quote}
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?
{quote}
Updated to use the method which checks for at least one instance, also throws 
an exception if two types are defined for the same ordinal.

{quote}
Commented out code in MessageConverter_0_10_to_1_0.java ?
and MessageConverter_0_8_to_1_0.java?
{quote}

These were the properties not currently being set.  I have changed the comments 
to make this clear and no longer make it look like code.

{quote}
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.. ?
{quote}

These converters were not actually registered and thus never used.  Obviously 
in time they should be implemented and registered, but for now I have simply 
removed them.

{quote}
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?
{quote}

I have reverted this change and instead fixed the getVirtualHostName() method 
on the various implementations of AMQConnectionModel that were giving NPE with 
the setVirtualHost() in its original position.

Not (yet?) addresses:

{quote}
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?
{quote}

Possibly, there probably should be some sort of validation to check that the 
intersection of supported versions and available creators is non empty.  

{quote}
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.
{quote}

Yeah - it's unfortunate... but I think having an Enum there would sort of 
defeat the point of Pluggability.  I have modified the code so that each of the 
ordinals is now defined as a public constant in the implementing class so they 
can be more easily referred to.

                
> [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: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to