You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@qpid.apache.org by Andrew Stitcher <as...@apache.org> on 2011/02/22 17:30:24 UTC

Re: Review Request: Recreation of JCA Resource Adapter review

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/
-----------------------------------------------------------

(Updated 2011-02-22 08:30:24.350312)


Review request for qpid.


Changes
-------

Update from Kevin Conner:

QpidRAManagedConnection.java
  work around but in qpid, I believe Gordon has fixed it
QpidRAProperties.java
  Force configuration of transaction manager lookup rather than default
QpidResourceAdapter.java
  Add missing RA properties for TM etc.

inflow/QpidActivation.java
  Move to single connection for inflow, handle failures
  Relax destination detection per review comments
inflow/QpidMessageHandler.java
  Move connection for inflow

resources/META-INF/ra.xml
  Add comments for additional properties


Summary (updated)
-------

Review for a qpid JCA resource adapter.

So far no build infrastructure is included.

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).

Any and all comments welcome.


This addresses bug QPID-3044.
    https://issues.apache.org/jira/browse/QPID-3044


Diffs (updated)
-----

  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
  /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
  /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 

Diff: https://reviews.apache.org/r/441/diff


Testing
-------


Thanks,

Andrew

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Andrew Stitcher <as...@apache.org>.


> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java, line 48
> > <https://reviews.apache.org/r/441/diff/3/?file=13133#file13133line48>
> >
> >     "QPID-CF" should be external constant 
> >     r.get() result not checked for null

Agreed - fixed in newer version


> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java, line 127
> > <https://reviews.apache.org/r/441/diff/3/?file=13137#file13137line127>
> >
> >     "QPID-CF" should be external constant

Agreed - fixed in newer version


> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java, line 324
> > <https://reviews.apache.org/r/441/diff/3/?file=13140#file13140line324>
> >
> >     null check is redundant, since instanceof will return false for a null value of obj

Agreed - fixed in newer version


> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java, line 349
> > <https://reviews.apache.org/r/441/diff/3/?file=13140#file13140line349>
> >
> >     hashCode does not meet Object contract with equals and should include clientId in calculation

Agreed - fixed in newer version


> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java, line 355
> > <https://reviews.apache.org/r/441/diff/3/?file=13140#file13140line355>
> >
> >     can just use type here?

Agreed - fixed in newer version


> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java, line 357
> > <https://reviews.apache.org/r/441/diff/3/?file=13140#file13140line357>
> >
> >     can just use acknowledgeMode here?

Agreed - fixed in newer version


> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java, line 304
> > <https://reviews.apache.org/r/441/diff/3/?file=13146#file13146line304>
> >
> >     null check is redundant, due to use of instanceof

Agreed - fixed in newer version


> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java, line 321
> > <https://reviews.apache.org/r/441/diff/3/?file=13146#file13146line321>
> >
> >     null check redundant, due to instanceof

Agreed - fixed in newer version


> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java, line 176
> > <https://reviews.apache.org/r/441/diff/3/?file=13141#file13141line176>
> >
> >     do we really want to use Object's toString here?

On balance we've decided to leave it as is for now, it does no real harm.


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/#review298
-----------------------------------------------------------


On 2011-02-24 15:12:05, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
> 
> (Updated 2011-02-24 15:12:05)
> 
> 
> Review request for qpid.
> 
> 
> Summary
> -------
> 
> Review for a qpid JCA resource adapter.
> 
> So far no build infrastructure is included.
> 
> 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).
> 
> Any and all comments welcome.
> 
> 
> This addresses bug QPID-3044.
>     https://issues.apache.org/jira/browse/QPID-3044
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
>   /trunk/qpid/java/build.deps 1070497 
>   /trunk/qpid/java/build.xml 1070497 
>   /trunk/qpid/java/jca/build.xml PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
>   /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
>   /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
> 
> Diff: https://reviews.apache.org/r/441/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Andrew Kennedy <gr...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/#review298
-----------------------------------------------------------



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java
<https://reviews.apache.org/r/441/#comment560>

    "QPID-CF" should be external constant 
    r.get() result not checked for null



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java
<https://reviews.apache.org/r/441/#comment561>

    "QPID-CF" should be external constant



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java
<https://reviews.apache.org/r/441/#comment564>

    null check is redundant, since instanceof will return false for a null value of obj



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java
<https://reviews.apache.org/r/441/#comment565>

    hashCode does not meet Object contract with equals and should include clientId in calculation



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java
<https://reviews.apache.org/r/441/#comment566>

    can just use type here?



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java
<https://reviews.apache.org/r/441/#comment567>

    can just use acknowledgeMode here?



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java
<https://reviews.apache.org/r/441/#comment568>

    do we really want to use Object's toString here?



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java
<https://reviews.apache.org/r/441/#comment569>

    null check is redundant, due to use of instanceof



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java
<https://reviews.apache.org/r/441/#comment570>

    null check redundant, due to instanceof


- Andrew


On 2011-02-24 15:12:05, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
> 
> (Updated 2011-02-24 15:12:05)
> 
> 
> Review request for qpid.
> 
> 
> Summary
> -------
> 
> Review for a qpid JCA resource adapter.
> 
> So far no build infrastructure is included.
> 
> 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).
> 
> Any and all comments welcome.
> 
> 
> This addresses bug QPID-3044.
>     https://issues.apache.org/jira/browse/QPID-3044
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
>   /trunk/qpid/java/build.deps 1070497 
>   /trunk/qpid/java/build.xml 1070497 
>   /trunk/qpid/java/jca/build.xml PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
>   /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
>   /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
> 
> Diff: https://reviews.apache.org/r/441/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Andrew Stitcher <as...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/
-----------------------------------------------------------

(Updated 2011-12-06 22:55:17.757580)


Review request for qpid, Gordon Sim, Andrew Kennedy, Robbie Gemmell, rajith attapattu, and Weston Price.


Changes
-------

Candidate JCA code for putback to trunk:

Changes include:
* Now based on recent trunk (please ignore the spurious inter change diffs for code outside the jca tree).
* Lots of work on the examples.
* Changed code conventions to match qpid.
* Ships with configuration that works in Geronimo 2.2.1 and JBoss 5 and JBoss 6.
* All Bugs found in Red Hats testing to date fixed.

You can find the development tree for this work on github:
    git@github.com:astitcher/qpid-jca.git
This review is squashed into the branch jca-candidate.


Summary (updated)
-------

Review for qpid JCA resource adapter.

All comments welcome.


This addresses bug QPID-3044.
    https://issues.apache.org/jira/browse/QPID-3044


Diffs (updated)
-----

  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionFactory.java 1208733 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQQueueBrowser.java 1208733 
  /trunk/qpid/java/build.xml 1208733 
  /trunk/qpid/java/build.deps 1208733 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/TopicPublisherAdapter.java 1208733 
  /trunk/qpid/java/jca/README-GERONIMO.txt PRE-CREATION 
  /trunk/qpid/java/jca/README-JBOSS.txt PRE-CREATION 
  /trunk/qpid/java/jca/README.txt PRE-CREATION 
  /trunk/qpid/java/jca/build.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/.gitignore PRE-CREATION 
  /trunk/qpid/java/jca/example/README.txt PRE-CREATION 
  /trunk/qpid/java/jca/example/build-geronimo-properties.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/build-jboss-properties.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/build-properties.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/build-properties.xml.temp PRE-CREATION 
  /trunk/qpid/java/jca/example/build.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/application.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/ejb-jar.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/geronimo-application.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/geronimo-ra.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/jboss-web.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/jboss.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/log4j.properties PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/qpid-jca-ds.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/web.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/qpid-jca-example-properties.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/client/QpidRequestResponseClient.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/client/QpidTestClient.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/client/QpidTestUtil.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidGoodByeListenerBean.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidGoodByeSubscriberBean.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidHelloListenerBean.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidHelloSubscriberBean.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidJMSResponderBean.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidTest.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidTestBean.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidTestLocal.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidTestRemote.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/ejb/QpidUtil.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/main/java/org/apache/qpid/jca/example/web/QpidTestServlet.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueBrowser.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionImpl.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/AdminObjectFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidBindingURL.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidConnectionFactoryProxy.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidDestinationProxy.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidQueue.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidTopic.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/GeronimoTransactionManagerLocator.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBoss7TransactionManagerLocator.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/resources/META-INF/jboss-ra.xml PRE-CREATION 
  /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
  /trunk/qpid/java/lib/geronimo-ejb_3.0_spec-1.0.1.jar UNKNOWN 
  /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
  /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
  /trunk/qpid/java/lib/geronimo-kernel-2.2.1.jar UNKNOWN 

Diff: https://reviews.apache.org/r/441/diff


Testing
-------

This code is now substantially the same as tested by Red Hat, passes the TCK.


Thanks,

Andrew

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Robbie Gemmell <ro...@gmail.com>.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > 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.
> > 
> > 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.
> > 
> > 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.
> > 
> > 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.
> 
> Andrew Stitcher wrote:
>     A quick answer to a couple of things -
>     
>     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).
>     
>     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.
>     
>     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.
> 
> Weston Price wrote:
>     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.
>     
>     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.
>     
>     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.
>

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.

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@.

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).

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.


- Robbie


-----------------------------------------------------------
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:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
> 
> (Updated 2011-07-14 22:39:37)
> 
> 
> Review request for qpid, Gordon Sim, Andrew Kennedy, Robbie Gemmell, rajith attapattu, and Weston Price.
> 
> 
> Summary
> -------
> 
> Review for a qpid JCA resource adapter.
> 
> So far no build infrastructure is included.
> 
> 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).
> 
> Any and all comments welcome.
> 
> 
> This addresses bug QPID-3044.
>     https://issues.apache.org/jira/browse/QPID-3044
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
>   /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidConnectionFactoryProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidDestinationProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidQueue.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidTopic.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/AdminObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidBindingURL.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueBrowser.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java PRE-CREATION 
>   /trunk/qpid/java/jca/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/application.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/web.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidStandaloneClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTest.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestBean.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestLocal.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestRemote.java PRE-CREATION 
>   /trunk/qpid/java/build.deps 1070497 
>   /trunk/qpid/java/build.xml 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQQueueBrowser.java 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/TopicPublisherAdapter.java 1070497 
>   /trunk/qpid/java/eclipse-projects/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/README-JBOSS.txt PRE-CREATION 
>   /trunk/qpid/java/jca/README.txt PRE-CREATION 
>   /trunk/qpid/java/jca/build.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/example/README PRE-CREATION 
>   /trunk/qpid/java/jca/example/build.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/441/diff
> 
> 
> Testing
> -------
> 
> This code is now substantially the same as tested by Red Hat, passes the TCK.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Weston Price <wp...@redhat.com>.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > 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.
> > 
> > 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.
> > 
> > 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.
> > 
> > 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.
> 
> Andrew Stitcher wrote:
>     A quick answer to a couple of things -
>     
>     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).
>     
>     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.
>     
>     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.

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.

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.

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.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > /trunk/qpid/java/eclipse-projects/.gitignore, lines 1-2
> > <https://reviews.apache.org/r/441/diff/4/?file=25573#file25573line1>
> >
> >     Do we need an 'empty' directory in the repository for this?
> >     
> >     We have plenty of directories in the Java tree as it is without adding redundant (for me at least) ones. Could this instead be added to the existing global .gitignore, or a new one be added in the Java directory to ignore the subdirectory and its contents for those that actually wish to create it?
> 
> Andrew Stitcher wrote:
>     Oops, that was meant to be removed, I'll get rid of it.

I agree, these should have never been checked in.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > /trunk/qpid/java/jca/example/build.xml, line 41
> > <https://reviews.apache.org/r/441/diff/4/?file=25579#file25579line41>
> >
> >     Out of date hard coded value. Can we use the existing variables to keep this up to date?
> 
> Andrew Stitcher wrote:
>     Good idea - I'll look at that

Agreed, a better strategy should be determined for this value.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > /trunk/qpid/java/jca/example/build.xml, lines 208-211
> > <https://reviews.apache.org/r/441/diff/4/?file=25579#file25579line208>
> >
> >     Out of date hard coded values. (project version as before, and we also just upgraded to mina 1.1.7)
> 
> Andrew Stitcher wrote:
>     Yes I noticed the mina upgrade - that's good as it removes the backport dependency

See above.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > /trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java, lines 42-43
> > <https://reviews.apache.org/r/441/diff/4/?file=25590#file25590line42>
> >
> >     There are tab characters instead of spaces in several places within this file.
> 
> Andrew Stitcher wrote:
>     Yeah I noticed there are some remaining space problems in the code - I'll try to get rid of them.

Agreed. Will fix.


- 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:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
> 
> (Updated 2011-07-14 22:39:37)
> 
> 
> Review request for qpid, Gordon Sim, Andrew Kennedy, Robbie Gemmell, rajith attapattu, and Weston Price.
> 
> 
> Summary
> -------
> 
> Review for a qpid JCA resource adapter.
> 
> So far no build infrastructure is included.
> 
> 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).
> 
> Any and all comments welcome.
> 
> 
> This addresses bug QPID-3044.
>     https://issues.apache.org/jira/browse/QPID-3044
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
>   /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidConnectionFactoryProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidDestinationProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidQueue.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidTopic.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/AdminObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidBindingURL.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueBrowser.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java PRE-CREATION 
>   /trunk/qpid/java/jca/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/application.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/web.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidStandaloneClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTest.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestBean.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestLocal.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestRemote.java PRE-CREATION 
>   /trunk/qpid/java/build.deps 1070497 
>   /trunk/qpid/java/build.xml 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQQueueBrowser.java 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/TopicPublisherAdapter.java 1070497 
>   /trunk/qpid/java/eclipse-projects/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/README-JBOSS.txt PRE-CREATION 
>   /trunk/qpid/java/jca/README.txt PRE-CREATION 
>   /trunk/qpid/java/jca/build.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/example/README PRE-CREATION 
>   /trunk/qpid/java/jca/example/build.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/441/diff
> 
> 
> Testing
> -------
> 
> This code is now substantially the same as tested by Red Hat, passes the TCK.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Weston Price <wp...@redhat.com>.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > 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.
> > 
> > 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.
> > 
> > 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.
> > 
> > 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.
> 
> Andrew Stitcher wrote:
>     A quick answer to a couple of things -
>     
>     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).
>     
>     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.
>     
>     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.
> 
> Weston Price wrote:
>     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.
>     
>     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.
>     
>     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.
>
> 
> Robbie Gemmell wrote:
>     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.
>     
>     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@.
>     
>     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).
>     
>     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:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
> 
> (Updated 2011-07-14 22:39:37)
> 
> 
> Review request for qpid, Gordon Sim, Andrew Kennedy, Robbie Gemmell, rajith attapattu, and Weston Price.
> 
> 
> Summary
> -------
> 
> Review for a qpid JCA resource adapter.
> 
> So far no build infrastructure is included.
> 
> 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).
> 
> Any and all comments welcome.
> 
> 
> This addresses bug QPID-3044.
>     https://issues.apache.org/jira/browse/QPID-3044
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
>   /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidConnectionFactoryProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidDestinationProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidQueue.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidTopic.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/AdminObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidBindingURL.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueBrowser.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java PRE-CREATION 
>   /trunk/qpid/java/jca/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/application.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/web.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidStandaloneClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTest.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestBean.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestLocal.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestRemote.java PRE-CREATION 
>   /trunk/qpid/java/build.deps 1070497 
>   /trunk/qpid/java/build.xml 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQQueueBrowser.java 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/TopicPublisherAdapter.java 1070497 
>   /trunk/qpid/java/eclipse-projects/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/README-JBOSS.txt PRE-CREATION 
>   /trunk/qpid/java/jca/README.txt PRE-CREATION 
>   /trunk/qpid/java/jca/build.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/example/README PRE-CREATION 
>   /trunk/qpid/java/jca/example/build.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/441/diff
> 
> 
> Testing
> -------
> 
> This code is now substantially the same as tested by Red Hat, passes the TCK.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Andrew Stitcher <as...@apache.org>.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > 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.
> > 
> > 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.
> > 
> > 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.
> > 
> > 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.

A quick answer to a couple of things -

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).

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.

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.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > /trunk/qpid/java/eclipse-projects/.gitignore, lines 1-2
> > <https://reviews.apache.org/r/441/diff/4/?file=25573#file25573line1>
> >
> >     Do we need an 'empty' directory in the repository for this?
> >     
> >     We have plenty of directories in the Java tree as it is without adding redundant (for me at least) ones. Could this instead be added to the existing global .gitignore, or a new one be added in the Java directory to ignore the subdirectory and its contents for those that actually wish to create it?

Oops, that was meant to be removed, I'll get rid of it.


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > /trunk/qpid/java/jca/example/build.xml, line 41
> > <https://reviews.apache.org/r/441/diff/4/?file=25579#file25579line41>
> >
> >     Out of date hard coded value. Can we use the existing variables to keep this up to date?

Good idea - I'll look at that


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > /trunk/qpid/java/jca/example/build.xml, lines 208-211
> > <https://reviews.apache.org/r/441/diff/4/?file=25579#file25579line208>
> >
> >     Out of date hard coded values. (project version as before, and we also just upgraded to mina 1.1.7)

Yes I noticed the mina upgrade - that's good as it removes the backport dependency


> On 2011-07-15 15:40:16, Robbie Gemmell wrote:
> > /trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java, lines 42-43
> > <https://reviews.apache.org/r/441/diff/4/?file=25590#file25590line42>
> >
> >     There are tab characters instead of spaces in several places within this file.

Yeah I noticed there are some remaining space problems in the code - I'll try to get rid of them.


- Andrew


-----------------------------------------------------------
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:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
> 
> (Updated 2011-07-14 22:39:37)
> 
> 
> Review request for qpid, Gordon Sim, Andrew Kennedy, Robbie Gemmell, rajith attapattu, and Weston Price.
> 
> 
> Summary
> -------
> 
> Review for a qpid JCA resource adapter.
> 
> So far no build infrastructure is included.
> 
> 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).
> 
> Any and all comments welcome.
> 
> 
> This addresses bug QPID-3044.
>     https://issues.apache.org/jira/browse/QPID-3044
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
>   /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidConnectionFactoryProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidDestinationProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidQueue.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidTopic.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/AdminObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidBindingURL.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueBrowser.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java PRE-CREATION 
>   /trunk/qpid/java/jca/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/application.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/web.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidStandaloneClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTest.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestBean.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestLocal.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestRemote.java PRE-CREATION 
>   /trunk/qpid/java/build.deps 1070497 
>   /trunk/qpid/java/build.xml 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQQueueBrowser.java 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/TopicPublisherAdapter.java 1070497 
>   /trunk/qpid/java/eclipse-projects/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/README-JBOSS.txt PRE-CREATION 
>   /trunk/qpid/java/jca/README.txt PRE-CREATION 
>   /trunk/qpid/java/jca/build.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/example/README PRE-CREATION 
>   /trunk/qpid/java/jca/example/build.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/441/diff
> 
> 
> Testing
> -------
> 
> This code is now substantially the same as tested by Red Hat, passes the TCK.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Robbie Gemmell <ro...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/#review1077
-----------------------------------------------------------


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.

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.

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.

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.


/trunk/qpid/java/eclipse-projects/.gitignore
<https://reviews.apache.org/r/441/#comment2197>

    Do we need an 'empty' directory in the repository for this?
    
    We have plenty of directories in the Java tree as it is without adding redundant (for me at least) ones. Could this instead be added to the existing global .gitignore, or a new one be added in the Java directory to ignore the subdirectory and its contents for those that actually wish to create it? 



/trunk/qpid/java/jca/example/build.xml
<https://reviews.apache.org/r/441/#comment2198>

    Out of date hard coded value. Can we use the existing variables to keep this up to date?



/trunk/qpid/java/jca/example/build.xml
<https://reviews.apache.org/r/441/#comment2199>

    Out of date hard coded values. (project version as before, and we also just upgraded to mina 1.1.7)



/trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java
<https://reviews.apache.org/r/441/#comment2200>

    There are tab characters instead of spaces in several places within this file.


- Robbie


On 2011-07-14 22:39:37, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
> 
> (Updated 2011-07-14 22:39:37)
> 
> 
> Review request for qpid, Gordon Sim, Andrew Kennedy, Robbie Gemmell, rajith attapattu, and Weston Price.
> 
> 
> Summary
> -------
> 
> Review for a qpid JCA resource adapter.
> 
> So far no build infrastructure is included.
> 
> 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).
> 
> Any and all comments welcome.
> 
> 
> This addresses bug QPID-3044.
>     https://issues.apache.org/jira/browse/QPID-3044
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
>   /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidConnectionFactoryProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidDestinationProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidQueue.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidTopic.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/AdminObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidBindingURL.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueBrowser.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java PRE-CREATION 
>   /trunk/qpid/java/jca/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/application.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/web.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidStandaloneClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTest.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestBean.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestLocal.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestRemote.java PRE-CREATION 
>   /trunk/qpid/java/build.deps 1070497 
>   /trunk/qpid/java/build.xml 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQQueueBrowser.java 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/TopicPublisherAdapter.java 1070497 
>   /trunk/qpid/java/eclipse-projects/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/README-JBOSS.txt PRE-CREATION 
>   /trunk/qpid/java/jca/README.txt PRE-CREATION 
>   /trunk/qpid/java/jca/build.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/example/README PRE-CREATION 
>   /trunk/qpid/java/jca/example/build.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/441/diff
> 
> 
> Testing
> -------
> 
> This code is now substantially the same as tested by Red Hat, passes the TCK.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Robbie Gemmell <ro...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/#review1093
-----------------------------------------------------------



/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java
<https://reviews.apache.org/r/441/#comment2226>

    I seem to recall Rajith doing some work around this area. Is 'Bug' fixed?
    
    If so, and this is being left for compatibility purposes with old versions, possibly some comments should be added to say so, preventing later inadvertent cleanup?


- Robbie


On 2011-07-14 22:39:37, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
> 
> (Updated 2011-07-14 22:39:37)
> 
> 
> Review request for qpid, Gordon Sim, Andrew Kennedy, Robbie Gemmell, rajith attapattu, and Weston Price.
> 
> 
> Summary
> -------
> 
> Review for a qpid JCA resource adapter.
> 
> So far no build infrastructure is included.
> 
> 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).
> 
> Any and all comments welcome.
> 
> 
> This addresses bug QPID-3044.
>     https://issues.apache.org/jira/browse/QPID-3044
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
>   /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidConnectionFactoryProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidDestinationProxy.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidQueue.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidTopic.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/AdminObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidBindingURL.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueBrowser.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
>   /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java PRE-CREATION 
>   /trunk/qpid/java/jca/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/application.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/qpid-jca-ds.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/conf/web.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidStandaloneClient.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidListener.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTest.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestBean.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestLocal.java PRE-CREATION 
>   /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestRemote.java PRE-CREATION 
>   /trunk/qpid/java/build.deps 1070497 
>   /trunk/qpid/java/build.xml 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQQueueBrowser.java 1070497 
>   /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/TopicPublisherAdapter.java 1070497 
>   /trunk/qpid/java/eclipse-projects/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/README-JBOSS.txt PRE-CREATION 
>   /trunk/qpid/java/jca/README.txt PRE-CREATION 
>   /trunk/qpid/java/jca/build.xml PRE-CREATION 
>   /trunk/qpid/java/jca/example/.gitignore PRE-CREATION 
>   /trunk/qpid/java/jca/example/README PRE-CREATION 
>   /trunk/qpid/java/jca/example/build.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/441/diff
> 
> 
> Testing
> -------
> 
> This code is now substantially the same as tested by Red Hat, passes the TCK.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Andrew Stitcher <as...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/
-----------------------------------------------------------

(Updated 2011-07-14 22:39:37.941135)


Review request for qpid, Gordon Sim, Andrew Kennedy, Robbie Gemmell, rajith attapattu, and Weston Price.


Changes
-------

- Addresses review comments from Andrew Kennedy
- Adds a bunch of examples and documentation
- Removes author (and some other) comments tags imported with code
- Bug fixes found through testing
- Updated Serialization UIDs (for changes since code import)

I intend this to be the code checked into qpid, so review would be welcome.


Summary
-------

Review for a qpid JCA resource adapter.

So far no build infrastructure is included.

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).

Any and all comments welcome.


This addresses bug QPID-3044.
    https://issues.apache.org/jira/browse/QPID-3044


Diffs (updated)
-----

  /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
  /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidConnectionFactoryProxy.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidDestinationProxy.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidQueue.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidTopic.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/AdminObjectFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/admin/QpidBindingURL.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueBrowser.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/qpid/jca/example/web/QpidTestServlet.java PRE-CREATION 
  /trunk/qpid/java/jca/qpid-jca-ds.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/application.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/qpid-jca-ds.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/conf/web.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidClient.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/qpid/jca/example/QpidStandaloneClient.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidListener.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTest.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestBean.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestLocal.java PRE-CREATION 
  /trunk/qpid/java/jca/example/src/qpid/jca/example/ejb/QpidTestRemote.java PRE-CREATION 
  /trunk/qpid/java/build.deps 1070497 
  /trunk/qpid/java/build.xml 1070497 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQQueueBrowser.java 1070497 
  /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/TopicPublisherAdapter.java 1070497 
  /trunk/qpid/java/eclipse-projects/.gitignore PRE-CREATION 
  /trunk/qpid/java/jca/README-JBOSS.txt PRE-CREATION 
  /trunk/qpid/java/jca/README.txt PRE-CREATION 
  /trunk/qpid/java/jca/build.xml PRE-CREATION 
  /trunk/qpid/java/jca/example/.gitignore PRE-CREATION 
  /trunk/qpid/java/jca/example/README PRE-CREATION 
  /trunk/qpid/java/jca/example/build.xml PRE-CREATION 

Diff: https://reviews.apache.org/r/441/diff


Testing (updated)
-------

This code is now substantially the same as tested by Red Hat, passes the TCK.


Thanks,

Andrew

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Andrew Stitcher <as...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/
-----------------------------------------------------------

(Updated 2011-02-24 15:12:05.717178)


Review request for qpid.


Changes
-------

Updated patch with:

* Changed logging as per review request.
* Some workarounds for problems in Qpid JMS client (bugs to be reported)


Summary
-------

Review for a qpid JCA resource adapter.

So far no build infrastructure is included.

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).

Any and all comments welcome.


This addresses bug QPID-3044.
    https://issues.apache.org/jira/browse/QPID-3044


Diffs (updated)
-----

  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION 
  /trunk/qpid/java/build.deps 1070497 
  /trunk/qpid/java/build.xml 1070497 
  /trunk/qpid/java/jca/build.xml PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION 
  /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION 
  /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN 
  /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN 

Diff: https://reviews.apache.org/r/441/diff


Testing
-------


Thanks,

Andrew

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Andrew Stitcher <as...@redhat.com>.

On Tue, 2011-02-22 at 11:51 -0500, Andrew Stitcher wrote:
> A few notes about the JCA code/review:
> 
> Unfortunately I somehow screwed up and couldn't get the previous
> ReviewBoard review to update with new changes. So I've created a new
> ReviewBoard review - https://reviews.apache.org/r/432 I certainly hope

My heads not screwed on today that is the old review - the new review is
https://reviews.apache.org/r/441

Sorry

Andrew
 


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Robbie Gemmell <ro...@gmail.com>.

On 22 February 2011 16:51, Andrew Stitcher <as...@apache.org> wrote:

> <snip>
> Just to explain the provenance of the code: The original code comes from
> the HornetQ JCA resource adapter. The copyright owner of this is Red Hat
> and it has given permission for it to be submitted to the Qpid project.
> The original code was Apache Licensed anyway so there was no bug deal
> here. Therefore we are using the regular Apache License text.
> <snip>
>

Just to say that the reason I asked about this earlier was because that
wasnt clear originally, it only became visible that it was based on other
work but not what that was (except that it didnt appear to originate from
the ASF). The version of the Apache Licence header we put on our code here
isn't the same as what other people might use for their AL code, but the
fact it is being contributed directly by the copyright holder (/agent of)
changes things :)

Robbie

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Andrew Stitcher <as...@apache.org>.

A few notes about the JCA code/review:

Unfortunately I somehow screwed up and couldn't get the previous
ReviewBoard review to update with new changes. So I've created a new
ReviewBoard review - https://reviews.apache.org/r/432 I certainly hope
that we'll be able to use that going on (assuming there is much more to
say!)

Just to explain the provenance of the code: The original code comes from
the HornetQ JCA resource adapter. The copyright owner of this is Red Hat
and it has given permission for it to be submitted to the Qpid project.
The original code was Apache Licensed anyway so there was no bug deal
here. Therefore we are using the regular Apache License text.

Because of this there may still be some holdovers of code/formatting
standards - please let us know about anything like this and we'll fix
it.

Because the Red Hat Qpid team has very limited experience in J2EE we've
been helped (a lot) by Kevin Conner who also works for Red Hat in the
JBoss group, that's why you will have seen him commenting on the review
changes.

Andrew



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Marnie McCormack <ma...@googlemail.com>.

I don't believe that there's been a response to my comments about testing on
the Review Board or the JIRA.

On this task, I consider the lack of tests, documentation (is there
any?) and possibly intent to maintain the code from the author to preclude
commit.

I'd be glad to hear any response on the issues being raised here, and for
some further discussion. Is this a likely candidate to be set up as a plugin
for the broker and contributed to a sandbox space ?

Thanks,
Marnie

On Wed, Feb 23, 2011 at 4:43 PM, Marnie McCormack <
marnie.mccormack@googlemail.com> wrote:

> Do none of the classes have unit tests ?
>
> I can understand the possible dependencies for system/integration tests but
> 0% unit test coverage is not ideal.
>
> Thanks,
> Marnie
>
>   On Wed, Feb 23, 2011 at 2:53 PM, Andrew Stitcher <as...@redhat.com>wrote:
>
>> On Wed, 2011-02-23 at 13:54 +0000, Marnie McCormack wrote:
>> > Hi Andrew,
>> >
>> > My comment on the new code, from an initial glance is that it doesn't
>> seem
>> > to have any tests - are these attached somewhere else ?
>>
>> Currently the adapter is being tested against the JCA TCK and JBoss EAP
>> as the container. I don't think it makes sense to check these
>> dependencies in (and I expect we can't for the TCK anyway).
>>
>> I will give some thought to what tests we could meaningfully check in -
>> anyone with suggestions please pipe up.
>>
>> Andrew
>>
>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project:      http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Marnie McCormack <ma...@googlemail.com>.

Do none of the classes have unit tests ?

I can understand the possible dependencies for system/integration tests but
0% unit test coverage is not ideal.

Thanks,
Marnie

On Wed, Feb 23, 2011 at 2:53 PM, Andrew Stitcher <as...@redhat.com>wrote:

> On Wed, 2011-02-23 at 13:54 +0000, Marnie McCormack wrote:
> > Hi Andrew,
> >
> > My comment on the new code, from an initial glance is that it doesn't
> seem
> > to have any tests - are these attached somewhere else ?
>
> Currently the adapter is being tested against the JCA TCK and JBoss EAP
> as the container. I don't think it makes sense to check these
> dependencies in (and I expect we can't for the TCK anyway).
>
> I will give some thought to what tests we could meaningfully check in -
> anyone with suggestions please pipe up.
>
> Andrew
>
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project:      http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Andrew Stitcher <as...@redhat.com>.

On Wed, 2011-02-23 at 13:54 +0000, Marnie McCormack wrote:
> Hi Andrew,
> 
> My comment on the new code, from an initial glance is that it doesn't seem
> to have any tests - are these attached somewhere else ?

Currently the adapter is being tested against the JCA TCK and JBoss EAP
as the container. I don't think it makes sense to check these
dependencies in (and I expect we can't for the TCK anyway).

I will give some thought to what tests we could meaningfully check in -
anyone with suggestions please pipe up.

Andrew

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org

Re: Review Request: Recreation of JCA Resource Adapter review

Posted by Marnie McCormack <ma...@googlemail.com>.

Hi Andrew,

My comment on the new code, from an initial glance is that it doesn't seem
to have any tests - are these attached somewhere else ?

Thanks,
Marnie

On Tue, Feb 22, 2011 at 4:30 PM, Andrew Stitcher <as...@apache.org>wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
>
> (Updated 2011-02-22 08:30:24.350312)
>
>
> Review request for qpid.
>
>
> Changes
> -------
>
> Update from Kevin Conner:
>
> QpidRAManagedConnection.java
>  work around but in qpid, I believe Gordon has fixed it
> QpidRAProperties.java
>  Force configuration of transaction manager lookup rather than default
> QpidResourceAdapter.java
>  Add missing RA properties for TM etc.
>
> inflow/QpidActivation.java
>  Move to single connection for inflow, handle failures
>  Relax destination detection per review comments
> inflow/QpidMessageHandler.java
>  Move connection for inflow
>
> resources/META-INF/ra.xml
>  Add comments for additional properties
>
>
> Summary (updated)
> -------
>
> Review for a qpid JCA resource adapter.
>
> So far no build infrastructure is included.
>
> 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).
>
> Any and all comments welcome.
>
>
> This addresses bug QPID-3044.
>    https://issues.apache.org/jira/browse/QPID-3044
>
>
> Diffs (updated)
> -----
>
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java
> PRE-CREATION
>  /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION
>  /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN
>  /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN
>
> Diff: https://reviews.apache.org/r/441/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew
>
>