Qpid
  1. Qpid
  2. QPID-3688

AMQDestination and children should conform to the JavaBean pattern

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.12
    • Fix Version/s: 0.15
    • Component/s: Java Client
    • Environment:

      All platforms.

      Description

      In order to support deploying and managing AMQDestination's in JEE application servers, conformance to the JavaBean design pattern is preferred as many appellation servers have their own management infrastructure for supporting Administrative objects that require, at the very least, a default constructor to create and bootstrap the destination into the server address space.

      The AMQDestination and children classes should support this to make JEE integration easier and more consistent.

      1. QPID-3688.patch
        3 kB
        Rajith Attapattu

        Issue Links

          Activity

          Hide
          Rajith Attapattu added a comment -

          This patch doesn't change the AMQDestination into a Java bean as I'm not sure if we should allow fields like routing-key, exchange-name etc modifiable via setter methods.

          Infact the Destination implementations should be immutable once a BURL or an Address string is set.

          However in order to satisfy the app server situation, I think it's a reasonable compromise to introduce a default constructor and a setter method for BURL and Address String. The rest of the fields should be immutable.

          Show
          Rajith Attapattu added a comment - This patch doesn't change the AMQDestination into a Java bean as I'm not sure if we should allow fields like routing-key, exchange-name etc modifiable via setter methods. Infact the Destination implementations should be immutable once a BURL or an Address string is set. However in order to satisfy the app server situation, I think it's a reasonable compromise to introduce a default constructor and a setter method for BURL and Address String. The rest of the fields should be immutable.
          Hide
          Robbie Gemmell added a comment -

          I agree that the destinations should be immutable, and given what they store they really should be just getting created based on Strings (as e.g the session.createQueue() or JNDI properties file methods would)

          I take it the intention is that the protected default constructors would only be used by subclassing the various destination objects?

          The new AMQDestination constructor added seems like it should possibly use the 'default syntax' value located in the class rather than just assuming it is ADDR, and the 'setAddressString' method could then set _destSyntax if it is ever used or throw an exception if the two conflicted.

          I'm possibly missing something, but if the calling code can be altered such that it is able to use the 'setAddressString' method, whats stopping it being made to use the existing String constructors for the objects?

          Show
          Robbie Gemmell added a comment - I agree that the destinations should be immutable, and given what they store they really should be just getting created based on Strings (as e.g the session.createQueue() or JNDI properties file methods would) I take it the intention is that the protected default constructors would only be used by subclassing the various destination objects? The new AMQDestination constructor added seems like it should possibly use the 'default syntax' value located in the class rather than just assuming it is ADDR, and the 'setAddressString' method could then set _destSyntax if it is ever used or throw an exception if the two conflicted. I'm possibly missing something, but if the calling code can be altered such that it is able to use the 'setAddressString' method, whats stopping it being made to use the existing String constructors for the objects?
          Hide
          Weston M. Price added a comment -

          The issue is that in JEE environments we have no control over the calling code as the application server is constructing the object and requires a no-arg constructor. Once the object is constructed, properties are set and then the object is bound into JNDI. Again, the app server is managing this entire process so we have no control over how the object is constructed.

          Show
          Weston M. Price added a comment - The issue is that in JEE environments we have no control over the calling code as the application server is constructing the object and requires a no-arg constructor. Once the object is constructed, properties are set and then the object is bound into JNDI. Again, the app server is managing this entire process so we have no control over how the object is constructed.
          Hide
          Robbie Gemmell added a comment -

          Ok, I had thought that initially, but how does it know what properties are to be set to what values...properties files? (In case it doesnt show I have no experience of application servers )

          Show
          Robbie Gemmell added a comment - Ok, I had thought that initially, but how does it know what properties are to be set to what values...properties files? (In case it doesnt show I have no experience of application servers )
          Hide
          Weston M. Price added a comment -

          It's a good question actually. In terms of JCA, there is an ra.xml file that is used as a 'template' by the application server. Listed in the ra.xml file are the properties an AdminObject supports. Something along the lines of:

          <adminobject>
          <adminobject-interface>javax.jms.Destination</adminobject-interface>
          <adminobject-class> org.apache.qpid.ra.admin.QpidDestinationProxy</adminobject-class>
          <config-property>
          <config-property-name>destinationAddress </config-property-name>
          <config-property-type>java.lang.String </config-property-type>
          </config-property>
          <config-property>
          <config-property-name>destinationType</config-property-name>
          <config-property-type>java.lang.String </config-property-type>
          </config-property>
          </adminobject>

          So, based on this definition, when you configure/deploy an admin object, the app server knows what properties are exposed and what to set after constructing the AdminObject, which of course is the reason a default constructor is required since the app server depends on the AdminObject conforming to the JavaBean specification.

          Show
          Weston M. Price added a comment - It's a good question actually. In terms of JCA, there is an ra.xml file that is used as a 'template' by the application server. Listed in the ra.xml file are the properties an AdminObject supports. Something along the lines of: <adminobject> <adminobject-interface>javax.jms.Destination</adminobject-interface> <adminobject-class> org.apache.qpid.ra.admin.QpidDestinationProxy</adminobject-class> <config-property> <config-property-name>destinationAddress </config-property-name> <config-property-type>java.lang.String </config-property-type> </config-property> <config-property> <config-property-name>destinationType</config-property-name> <config-property-type>java.lang.String </config-property-type> </config-property> </adminobject> So, based on this definition, when you configure/deploy an admin object, the app server knows what properties are exposed and what to set after constructing the AdminObject, which of course is the reason a default constructor is required since the app server depends on the AdminObject conforming to the JavaBean specification.
          Hide
          Rajith Attapattu added a comment -

          "The new AMQDestination constructor added seems like it should possibly use the 'default syntax' value located in the class rather than just assuming it is ADDR, and the 'setAddressString' method could then set _destSyntax if it is ever used or throw an exception if the two conflicted."

          I think we could do that and possibly add a getter and setter for BURL as well.

          Robbie, I believe Weston addressed the other concern you had.

          Show
          Rajith Attapattu added a comment - "The new AMQDestination constructor added seems like it should possibly use the 'default syntax' value located in the class rather than just assuming it is ADDR, and the 'setAddressString' method could then set _destSyntax if it is ever used or throw an exception if the two conflicted." I think we could do that and possibly add a getter and setter for BURL as well. Robbie, I believe Weston addressed the other concern you had.
          Hide
          Robbie Gemmell added a comment -

          Weston, that makes things clearer thanks

          Rajith, I would probably just use a single setter for ADDR and BURL Strings and use the existing support code to distinguish them, given thats how it works everywhere else. Possibly changing the method to something more generic like 'setDestinationString'.

          Show
          Robbie Gemmell added a comment - Weston, that makes things clearer thanks Rajith, I would probably just use a single setter for ADDR and BURL Strings and use the existing support code to distinguish them, given thats how it works everywhere else. Possibly changing the method to something more generic like 'setDestinationString'.
          Hide
          Rajith Attapattu added a comment -

          Agreed, that make sense.

          Show
          Rajith Attapattu added a comment - Agreed, that make sense.
          Hide
          Weston M. Price added a comment -

          'Weston, that makes things clearer thanks'

          My pleasure.

          I am in agreement as well with the above comments.

          Show
          Weston M. Price added a comment - 'Weston, that makes things clearer thanks' My pleasure. I am in agreement as well with the above comments.

            People

            • Assignee:
              Rajith Attapattu
              Reporter:
              Weston M. Price
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development