Cargo
  1. Cargo
  2. CARGO-937

Add support for more JBoss specific deployables (HAR and JBoss AOP)

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.6
    • Fix Version/s: 1.1.0
    • Component/s: JBoss
    • Labels:
      None
    • Environment:
      n/a
    • Complexity:
      Intermediate
    • Number of attachments :
      0

      Description

      There is currently no support for deploying some JBoss specific deployables. We should add support for all JBoss specific archives. The ones that come to mind are the ones supported by jboss-packaging-maven-plugin.

        Issue Links

          Activity

          Hide
          Savas Ali Tokmen added a comment -

          The only fear I have is the complexity for testing and of course the number of users interested in these.

          Show
          Savas Ali Tokmen added a comment - The only fear I have is the complexity for testing and of course the number of users interested in these.
          Hide
          Anders Hammar added a comment -

          Well, I personally need the support for .aop deploables so I'll verify that...

          Show
          Anders Hammar added a comment - Well, I personally need the support for .aop deploables so I'll verify that...
          Hide
          Savas Ali Tokmen added a comment -

          OK; well in this case you can also implement based on how eager you feel

          Show
          Savas Ali Tokmen added a comment - OK; well in this case you can also implement based on how eager you feel
          Hide
          Anders Hammar added a comment -

          The issue I see is that the design in Cargo doesn't allow for container extensions in this area. All deployables need to go into the general container API. Not very nice, but not possible to change without a major redesign (which isn't desirable within a 1.x release).

          Show
          Anders Hammar added a comment - The issue I see is that the design in Cargo doesn't allow for container extensions in this area. All deployables need to go into the general container API. Not very nice, but not possible to change without a major redesign (which isn't desirable within a 1.x release).
          Hide
          Anders Hammar added a comment -

          Need to update documentation as well, for example:
          http://cargo.codehaus.org/JBoss+4.x
          Note that info about existing SAR deployable is curently missing.

          Show
          Anders Hammar added a comment - Need to update documentation as well, for example: http://cargo.codehaus.org/JBoss+4.x Note that info about existing SAR deployable is curently missing.
          Hide
          Savas Ali Tokmen added a comment -

          Anders, are you sure of your comment on adding container extensions? For example, the Tomcat container defines the TomcatWAR type, registered in the TomcatFactoryRegistry class. This is normally what the DeployableFactory is here for.

          Show
          Savas Ali Tokmen added a comment - Anders, are you sure of your comment on adding container extensions? For example, the Tomcat container defines the TomcatWAR type, registered in the TomcatFactoryRegistry class. This is normally what the DeployableFactory is here for.
          Hide
          Anders Hammar added a comment -

          Yes. The TomcatWAR (as well as JBossWAR and some others) are subclasses of the existing WAR deployable and of the WAR deployable type. However, adding a completely new deployable type in the container isn't possible as DeployableType (in Cargo core API) needs to list it. And DeployableType is final and the design simply doesn't have a SPI on this level.
          So, I'll just add this to the Cargo Core API in the same way as SAR already is. Not very nice, but the support of the containers is handled by the container's ContainerCapability implementation.

          Show
          Anders Hammar added a comment - Yes. The TomcatWAR (as well as JBossWAR and some others) are subclasses of the existing WAR deployable and of the WAR deployable type. However, adding a completely new deployable type in the container isn't possible as DeployableType (in Cargo core API) needs to list it. And DeployableType is final and the design simply doesn't have a SPI on this level. So, I'll just add this to the Cargo Core API in the same way as SAR already is. Not very nice, but the support of the containers is handled by the container's ContainerCapability implementation.
          Hide
          Savas Ali Tokmen added a comment -

          Aah, got it licked now. I agree, not very nice.

          Perhaps opening another ticket and targeting 1.2.0 could be an idea?

          Show
          Savas Ali Tokmen added a comment - Aah, got it licked now. I agree, not very nice. Perhaps opening another ticket and targeting 1.2.0 could be an idea?
          Hide
          Anders Hammar added a comment -

          I'm thinking that this would be such a major redesign that it would need to wait for 2.0. Also, it would be very, very nice to add enums instead of these static constants emulating enums...

          I've also found a circular package dependency (in cargo-core-api-container), for example:
          org.codehaus.cargo.container.WAR extends org.codehaus.cargo.container.spi.deployable.AbstractDeployable which implements org.codehaus.cargo.container.Deployable.

          Show
          Anders Hammar added a comment - I'm thinking that this would be such a major redesign that it would need to wait for 2.0. Also, it would be very, very nice to add enums instead of these static constants emulating enums... I've also found a circular package dependency (in cargo-core-api-container), for example: org.codehaus.cargo.container.WAR extends org.codehaus.cargo.container.spi.deployable.AbstractDeployable which implements org.codehaus.cargo.container.Deployable.
          Hide
          Savas Ali Tokmen added a comment -

          Hmm that's true... Meanwhile, do you think splitting this ticket into two makes sense?

          Show
          Savas Ali Tokmen added a comment - Hmm that's true... Meanwhile, do you think splitting this ticket into two makes sense?
          Hide
          Anders Hammar added a comment -

          This ticket will only focus on adding support for (some) JBoss specific archives. I'm currently thinking I'll limit this work to add .har and .aop, but we'll see.

          Everything else I've run into kind of shows that a bigger overhaul of the code would be nice.

          Show
          Anders Hammar added a comment - This ticket will only focus on adding support for (some) JBoss specific archives. I'm currently thinking I'll limit this work to add .har and .aop, but we'll see. Everything else I've run into kind of shows that a bigger overhaul of the code would be nice.
          Hide
          Savas Ali Tokmen added a comment -

          Let's then rename the ticket to something less ... ambitious

          Show
          Savas Ali Tokmen added a comment - Let's then rename the ticket to something less ... ambitious
          Hide
          Anders Hammar added a comment -

          Support for HAR added in r2830 on trunk. One small deficiency though: Cargo reports support for HAR in JBoss 3, while JBoss 3 doesn't support it. There was some strange double handling in the JBoss part regarding Capability which I didn't find worth the effort to refactor just to get this fully accurate.

          Show
          Anders Hammar added a comment - Support for HAR added in r2830 on trunk. One small deficiency though: Cargo reports support for HAR in JBoss 3, while JBoss 3 doesn't support it. There was some strange double handling in the JBoss part regarding Capability which I didn't find worth the effort to refactor just to get this fully accurate.
          Hide
          Anders Hammar added a comment -

          Support for JBoss AOP deployables added in r2835 on trunk. Tests only added for JBoss AS 5+ as no simple solution found to verify on earlier JBoss containers. Documentation generator updated as well.

          Show
          Anders Hammar added a comment - Support for JBoss AOP deployables added in r2835 on trunk. Tests only added for JBoss AS 5+ as no simple solution found to verify on earlier JBoss containers. Documentation generator updated as well.
          Hide
          Anders Hammar added a comment -

          Support for HAR and AOP added. Closing.
          If anyone want support for the JBoss specific deployables still missing, they may open a new ticket (with patch). The tricky part is the integration tests though.

          Show
          Anders Hammar added a comment - Support for HAR and AOP added. Closing. If anyone want support for the JBoss specific deployables still missing, they may open a new ticket (with patch). The tricky part is the integration tests though.
          Hide
          Anders Hammar added a comment -

          I investigated what versions of JBoss support which deployables and here's the findings:

          .sar deployables supporter since at least 3.0.x (verified with 3.0.8).
          .har deployables supported since 4.0 (verified with 4.0.0).
          .aop deployables supported since 4.2 (verified with 4.2.0).

          Show
          Anders Hammar added a comment - I investigated what versions of JBoss support which deployables and here's the findings: .sar deployables supporter since at least 3.0.x (verified with 3.0.8). .har deployables supported since 4.0 (verified with 4.0.0). .aop deployables supported since 4.2 (verified with 4.2.0).
          Hide
          Savas Ali Tokmen added a comment -

          Great, thank you

          Show
          Savas Ali Tokmen added a comment - Great, thank you
          Hide
          Savas Ali Tokmen added a comment -

          Tests don't pass on JBoss 4.x and 4.2.x anymore since:

          java.lang.UnsupportedOperationException: Method not supported for the current container: jboss42x
          	at org.codehaus.cargo.sample.java.jboss.AbstractJBossCapabilityTestCase.getServiceUrl(AbstractJBossCapabilityTestCase.java:114)
          

          That's because getServiceUrl hasn't been implemented for JBoss 4.x; as these versions of JBoss don't expose an RMI JMX connector but an HTTP one (making the code more complex).

          We should read http://community.jboss.org/message/167161 to add support for MBeanServerConnection on these servers. The code reads like:

          Properties env = new Properties();
           env.setProperty(Context.INITIAL_CONTEXT_FACTORY,
           "org.jboss.naming.HttpNamingContextFactory");
           
           env.setProperty(Context.PROVIDER_URL,
           "http://[hostname]:8080/invoker/JNDIFactory");
           
           Context ctx = new InitialContext(env);
           
           MBeanServerConnection server = (MBeanServerConnection) ctx.lookup("jmx/invoker/RMIAdaptor");
          

          ... and for it to work we need to add many JBoss libraries in the TCCL.

          Show
          Savas Ali Tokmen added a comment - Tests don't pass on JBoss 4.x and 4.2.x anymore since: java.lang.UnsupportedOperationException: Method not supported for the current container: jboss42x at org.codehaus.cargo.sample.java.jboss.AbstractJBossCapabilityTestCase.getServiceUrl(AbstractJBossCapabilityTestCase.java:114) That's because getServiceUrl hasn't been implemented for JBoss 4.x; as these versions of JBoss don't expose an RMI JMX connector but an HTTP one (making the code more complex). We should read http://community.jboss.org/message/167161 to add support for MBeanServerConnection on these servers. The code reads like: Properties env = new Properties(); env.setProperty(Context.INITIAL_CONTEXT_FACTORY, "org.jboss.naming.HttpNamingContextFactory"); env.setProperty(Context.PROVIDER_URL, "http://[hostname]:8080/invoker/JNDIFactory"); Context ctx = new InitialContext(env); MBeanServerConnection server = (MBeanServerConnection) ctx.lookup("jmx/invoker/RMIAdaptor"); ... and for it to work we need to add many JBoss libraries in the TCCL.
          Hide
          Anders Hammar added a comment -

          I saw this problem as well. The reason was that you changed the tests so they don't check the id of the container but rather performs the tests on all JBoss conatiners that support har. The tests can only be run on JBoss 5+ as that's the only containers that the jmx check works on.
          I've been trying for the last hour to get the validator back (ContainerIdRegExValidator) that you removed, but me and svn don't really cooperate here...

          Show
          Anders Hammar added a comment - I saw this problem as well. The reason was that you changed the tests so they don't check the id of the container but rather performs the tests on all JBoss conatiners that support har. The tests can only be run on JBoss 5+ as that's the only containers that the jmx check works on. I've been trying for the last hour to get the validator back (ContainerIdRegExValidator) that you removed, but me and svn don't really cooperate here...
          Hide
          Savas Ali Tokmen added a comment -

          Reading the tips from the JBoss forums, I do not think implementing the MBeanServerConnection with the JBoss HTTP naming factory should not be hard; and will work with JBoss 4.x.

          I'll take a look

          Show
          Savas Ali Tokmen added a comment - Reading the tips from the JBoss forums, I do not think implementing the MBeanServerConnection with the JBoss HTTP naming factory should not be hard; and will work with JBoss 4.x. I'll take a look
          Hide
          Anders Hammar added a comment -

          OK, I finally managed re-adding that validator in r2859.
          We'll close this ticket when we see the tests pass.

          Show
          Anders Hammar added a comment - OK, I finally managed re-adding that validator in r2859. We'll close this ticket when we see the tests pass.
          Hide
          Anders Hammar added a comment -

          Ali, JBoss is working on syncing their artifacts to central so unless you have too much spare time I suggest we wait for that instead of implementing this. If har/aop deployment works on JBoss 5+, it should work on 4.x as well. And if it for any reason doesn't, I would suspect that the user base being affect is very limited as that's a very old version.

          Show
          Anders Hammar added a comment - Ali, JBoss is working on syncing their artifacts to central so unless you have too much spare time I suggest we wait for that instead of implementing this. If har/aop deployment works on JBoss 5+, it should work on 4.x as well. And if it for any reason doesn't, I would suspect that the user base being affect is very limited as that's a very old version.
          Hide
          Savas Ali Tokmen added a comment -

          Committed revision 2860: Added JMX connection method for JBoss 4.x in order to test AOP and HAR on these container as well. This is now fully OK

          Show
          Savas Ali Tokmen added a comment - Committed revision 2860: Added JMX connection method for JBoss 4.x in order to test AOP and HAR on these container as well. This is now fully OK

            People

            • Assignee:
              Anders Hammar
              Reporter:
              Anders Hammar
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: