[
https://issues.apache.org/jira/browse/QPID-3044?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13066991#comment-13066991
]
jirapos...@reviews.apache.org commented on QPID-3044:
-----------------------------------------------------
bq. On 2011-07-15 15:40:16, Robbie Gemmell wrote:
bq. > The apparent JBoss 'dependency' for this seems less than ideal. I don't
think we should/can accept something into the Qpid code base when it hasn't
even been attempted to use it with a product that is actually Apache Licence
compatible. I think this really needs to be able to work fully out the box with
something like Geronimo, with additional rather than sole configuration
supplied for JBoss AS.
bq. >
bq. > It probably gets round the letter of the licence, since although the
example actually imports some JBoss annotations it probably gets away with it
because it doesn't look to be built by default (correct?), whereas the adapter
itself either has the bits turned off that are JBoss dependent and/or uses
reflection to avoid the imports. That doesn't really seem to change the fact
you don't appear to be able to use all of the 'optional' features without JBoss
though, which is probably still in violation of the spirit of the licence.
bq. >
bq. > It still appears that there is absolutely zero test coverage for this?
We keep talking about trying to improve our test situation as a project, but
particularly in the Java tree, and additions like this are not helpful.
bq. >
bq. > The code continues to violate the field naming convention we use on the
Java components ( https://cwiki.apache.org/qpid/java-coding-standards.html ),
although in contrast to earlier versions this diff actually has a mixture of
field naming conventions, making it inconsistent as well.
bq.
bq. Andrew Stitcher wrote:
bq. A quick answer to a couple of things -
bq.
bq. The adapter has actually been used with multiple application servers
including JBoss and Websphere. Also I believe that the TCK testing actually
uses Sun/Oracles reference app server (I didn't do the TCK testing myself).
bq.
bq. It's not at all clear to me why the licensing of the App server
matters for this code as the resource adapter should work for all. So you're
only talking about the default configuration, why shouldn't the default
configuration be JBoss? Its license is LGPL which is compatible with the Apache
License, but I'm not even sure that this is relevant.
bq.
bq. I thought we hashed out the "testing" issue before on the dev list -
in short there is little coherent/useful that you could add to unit tests for
the JCA as it's nearly all straight delegation to the JMS client. And including
an appserver into our java tree just to do unit testing seems fantastic
overhead. There appears to be little in between. The example code provides a
reasonable smoke test (given that you've installed an app server) which I
believe is as good as any unit tests would be.
bq.
bq. Weston Price wrote:
bq. There is no real 'dependency' on JBoss, the sample configuration file
is there to provide a convenient starting point for that particular application
server but does necessitate it's use. The adapter has been tested in Weblogic
as well as in Websphere. Since both these application servers do not require an
XML configuration file, none has been provided. In sum the *-ds.xml file, is
provided as a convenience, not a requirement.
bq.
bq. There is one specific JBoss annotation on QpidListener that is there
simply to which RAR file is in use. This is a requirement of JBoss but can be
removed in favor of an ejb-jar.xml file if you think this would be more
acceptable.
bq.
bq. The adapter itself has no dependency on JBoss, it is a standard
resource adapter that conforms to the 1.5 JCA specification and as such is
application server agnostic.
bq.
bq.
bq. Robbie Gemmell wrote:
bq. OK, so my confusion over whether multiple application servers had been
used in its development may have been linked to there being 3 README files
included in the diff that outright note (or at least suggest) otherwise.
bq.
bq. LGPL 2.1 is a Category X Excluded licence according to the ASF legal
pages (http://www.apache.org/legal/3party.html), i.e. it is not Apache Licence
compatible, which is why the licencing is of interest particularly when there
are imports present or features that only seem to be implemented for one
product. The example at least would seem to fall into requiring consideration
of the 'options for excluded works' sections as a result
(http://www.apache.org/legal/3party.html#options), and for the adapter itself
possibly the 'optional' XA features around transactions that appear to only
have JBoss specific implementation. IANAL though, so it might be best if we
cleared things with legal@.
bq.
bq. Regardless of any actual legal requirements though, whilst I think we
all can guess why JBoss AS is of interest here I still think that if the
adapter is considered truly generic then it deserves some configuration/example
and the ability to function fully with something something else, such as
Geronimo (that obviously is Apache Licence compatible).
bq.
bq. On the testing, I'd say there looked to be quite a lot of stuff that
isn't simply delegating to the client, and with something like 14000 lines in
the patch adding new classes there's certainly a lot of new code going untested
even if it is just delegating; the licence header is big but its not that big.
Unit tests wouldn't require an Application Server within the codebase, however
the ability to put one in optionally for system testing doesn't seem overly OTT
to me and would be sufficient to e.g. run tests on the Jenkins instances if
nothing else. I'll hold my hand up now and say I don't plan to manually use/run
the included example often/ever, so that means I probably wont have any idea if
I accidentally break this which is obviously far less than ideal.
In regards to XA, the adapter requires access to a transaction manager for XA
support. There is no 'standard' way to acquire a TM reference. As a result,
both a class and a particular method can be listed in the configuration file
that allows the adapter to accomplish this. By default, we are configured to
use JBoss, but this again, does not have to be the case. At the very least, we
can include in comments about other choices. Again, just because JBoss is
listed in the config file does not mean the use of JBoss is required to use/run
the adapter.
In terms of the example, I am working on a Geronimo
configuration/deployment-plan that should alleviate some of the concerns around
alternate app servers, as well as the usefulness of the example.
I agree, 'in theory', with the testing comment however, as has already been
pointed out, the TCK was/is used as the primary testing mechanism as this is
the most comprehensive testing and required no app server to be introduced into
the code base which imo is not really feasible. Without an app server, testing
would be fairly limited, or at the very least would require significant mock
objects and other mechanisms that emulate the EE environment. As QPID currently
does not have many features as far as it's testing framework, a significant
amount of new material would have to be introduced into the code that again
would most likely not come close to the expansiveness of the TCK in testing the
adapter.
- Weston
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/#review1077
-----------------------------------------------------------
On 2011-07-14 22:39:37, Andrew Stitcher wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/441/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-07-14 22:39:37)
bq.
bq.
bq. Review request for qpid, Gordon Sim, Andrew Kennedy, Robbie Gemmell,
rajith attapattu, and Weston Price.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Review for a qpid JCA resource adapter.
bq.
bq. So far no build infrastructure is included.
bq.
bq. I'd also like an opinion as to whether java/jca is the appropriate name
for this (I'm thinking perhaps java/ra would be more usual).
bq.
bq. Any and all comments welcome.
bq.
bq.
bq. This addresses bug QPID-3044.
bq. https://issues.apache.org/jira/browse/QPID-3044
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN
bq. /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java
PRE-CREATION
bq. /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidConnectionFactoryProxy.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidDestinationProxy.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidQueue.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidTopic.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/AdminObjectFactory.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidBindingURL.java
PRE-CREATION
bq. /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueBrowser.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java
PRE-CREATION
bq. /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java
PRE-CREATION
bq. /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java
PRE-CREATION
bq. /trunk/qpid/java/jca/qpid-jca-ds.xml PRE-CREATION
bq. /trunk/qpid/java/jca/example/conf/application.xml PRE-CREATION
bq. /trunk/qpid/java/jca/example/conf/qpid-jca-ds.xml PRE-CREATION
bq. /trunk/qpid/java/jca/example/conf/web.xml PRE-CREATION
bq. /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidClient.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/example/src/qpid/jca/example/QpidStandaloneClient.java
PRE-CREATION
bq. /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidListener.java
PRE-CREATION
bq. /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTest.java
PRE-CREATION
bq. /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestBean.java
PRE-CREATION
bq. /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestLocal.java
PRE-CREATION
bq.
/trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestRemote.java
PRE-CREATION
bq. /trunk/qpid/java/build.deps 1070497
bq. /trunk/qpid/java/build.xml 1070497
bq.
/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQQueueBrowser.java
1070497
bq.
/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/TopicPublisherAdapter.java
1070497
bq. /trunk/qpid/java/eclipse-projects/.gitignore PRE-CREATION
bq. /trunk/qpid/java/jca/README-JBOSS.txt PRE-CREATION
bq. /trunk/qpid/java/jca/README.txt PRE-CREATION
bq. /trunk/qpid/java/jca/build.xml PRE-CREATION
bq. /trunk/qpid/java/jca/example/.gitignore PRE-CREATION
bq. /trunk/qpid/java/jca/example/README PRE-CREATION
bq. /trunk/qpid/java/jca/example/build.xml PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/441/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. This code is now substantially the same as tested by Red Hat, passes the
TCK.
bq.
bq.
bq. Thanks,
bq.
bq. Andrew
bq.
bq.
> Implement JCA Adapter for Java JMS client
> -----------------------------------------
>
> Key: QPID-3044
> URL: https://issues.apache.org/jira/browse/QPID-3044
> Project: Qpid
> Issue Type: New Feature
> Components: Java Client
> Affects Versions: 0.8, 0.9, 0.10
> Reporter: Andrew Stitcher
> Assignee: Andrew Stitcher
> Labels: JCA, JMS, Java, qpid
> Fix For: 0.11
>
>
> Currently there is no way to integrate the use of Qpid messaging into a Java
> Application Server.
> The solution is to create a JCA (J2EE Connector Architecture) adapter to allow
> the Qpid JMS client to correctly work with the J2EE container.
> This is an entirely new Java component, but may require small amounts of
> change to the
> JMS client code.
--
This message is automatically generated by JIRA.
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:dev-subscr...@qpid.apache.org
