[
https://issues.apache.org/jira/browse/QPID-3401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13127664#comment-13127664
]
[email protected] commented on QPID-3401:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2366/#review2589
-----------------------------------------------------------
There are a number of operations performed based on the Destination during
client execution, e.g. during Consumer creation and closing, Producer creation
and closing amongst others. Each of these might be implemented differently
depending on the destination syntax and settings in use, and then further
differences can emerge based on any protocol-specific requirements. Currently,
everywhere differences relating to the syntax in use occur we generally have an
if-else statement. This is not a maintainable implementation approach, and
needs to be abstracted away by a common Destination model that allows grouping
of syntax specific operations into separate classes rather than spreading
decisions throughout the code.
The design alterations here improve on some of the deficiencies of the previous
implementation (e.g. the policy validation no longer being within the main
session etc implementation code), but in other ways seems worse. For example,
it appears to result in us having Destination implementations that are both
syntax AND protocol specific, and which have, and require, intimate knowledge
of the Session implementation. This does not seem like progress and still
leaves us with all the if-else statements throughout the client. Also, it seems
that as AddressBasedDestination is not abstract, it will now be possible for
our Destination objects to contain a delegate which implement either the JMS
Topic or Queue interfaces, but not implement it themselves.
In order to eliminate the if-else statements for behaviour which varies
depending on the destination syntax, we can isolate that code in specific
delegate classes for use with Destination objects. Such delegates would
implement a shared interface that declares the varying address related
operations, and could exist for both BindingURL and Address based Destinations.
The different behaviours could be contained within the destination delegates,
however specific implementations of operations such as queue creation, queue
deletion, subscription creation, would be would found within the
protocol-specific session implementations that actually perform these
operations.
The class diagram at
https://issues.apache.org/jira/secure/attachment/12499057/model2.gif depicts
what some of such a Destination model would look like. AMQDestination would be
refactored such that BindingURL specific functionality and Address specific
functionality was moved to subclasses, with all 3 being abstract, and these
used as the basis for implementations of the Queue and Topic interfaces
specific to each URL syntax.
Additionally, if would be nice to have immutable destination object in order to
resolve current issues with thread safety which in turn will allow storing and
reusing them via JNDI.
- Oleksandr
On 2011-10-12 21:02:31, rajith attapattu wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2366/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-12 21:02:31)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Robbie Gemmell, Weston Price, and
Keith Wall.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This patch contains the new class structure used for retrieving
information from addressing and creating destinations and managing it's
lifecycle.
bq. How this code is tied to the main client is illustrated with the patch put
up for review at https://reviews.apache.org/r/2364/
bq.
bq. A basic class diagram for this can be found here [
https://issues.apache.org/jira/secure/attachment/12498753/class_diagram.png ]
bq.
bq. In summary the goals are,
bq. ===========================
bq. 1. Provide a proper abstraction of Queue and Topic concepts
bq.
bq. 2. Provide an address format based implementation of Queue and Topic
bq.
bq. 3. Hide the implementatio of the life cycle of a destination (create,
delete, createSubscription, deleteSubscription)
bq.
bq. 4. Create a top level AddressBasedDestination class (extending from
AMQDestination),
bq. 4.1 To separate the address based details from AMQQueue, AMQTopic ..etc
bq. 4.2 To bridge btw the new code and the AMQDestination interface
bq.
bq. 4. Improve the code that retrievs data from an address (a.k.a
AddressHelper)
bq.
bq. 5. Provide a fix for QPID-3265, QPID-3317, QPID-3271
bq.
bq. 6. Implement the above with minimum disruption to regular client code.
bq.
bq.
bq. This addresses bug QPID-3401.
bq. https://issues.apache.org/jira/browse/QPID-3401
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AddressBasedDestination.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AddressBasedQueue.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AddressBasedTopic.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/QpidDestination.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/QpidQueue.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/QpidTopic.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/Session.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/SubscriptionSettings.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/AddressException.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/AddressHelper.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/AddressProperty.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/AddressResolver.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/Link.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/Node.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/AddressHelper_0_10.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/AddressProperty_0_10.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/AddressResolver_0_10.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/Binding.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/ExchangeNode.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/Link_0_10.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/Node_0_10.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/QpidQueue_0_10.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/QpidTopic_0_10.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/QueueNode.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/address/amqp_0_10/SubscriptionSettings_0_10.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/messaging/amqp_0_10/Session_0_10.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/2366/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Changes are verified using the existing tests in
AddressBasedDestinationTest.java
bq. I am planning to add more coverage and possibly refactor the above class
to cover more cases with less code.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> Refactor address resolution code
> --------------------------------
>
> Key: QPID-3401
> URL: https://issues.apache.org/jira/browse/QPID-3401
> Project: Qpid
> Issue Type: Improvement
> Components: Java Client
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Fix For: 0.14
>
> Attachments: QPID-3401-systests.patch, QPID-3401.patch,
> class_diagram.png, model2.gif
>
>
> After some thought it seems that the following JIRA's would benefit from some
> reworking of the address resolution code as the original design had a few
> flaws based on incorrect understanding of the address syntax.
> QPID-3265
> QPID-3317
> QPID-3271
> The redesign would be minimal and not very disruptive. The goal is to fix
> certain design flaws in the current code, rather than a complete redesign. I
> am planning to reuse as much code as possible to ensure we don't throw away
> tested code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]
