1 2 Previous Next 21 Replies Latest reply on Aug 15, 2014 8:53 AM by jesper.pedersen Go to original post
      • 15. Re: Proposed SPI change to XAResourceWrapper
        ataylor

        mines the pink one

        • 16. Re: Proposed SPI change to XAResourceWrapper
          tomjenkinson

          I think the issue is that the method name isn't a good match for the semantics of what the transaction manager does with the value of it. XAResourceWrapper::getJndiName()  is used (jboss-transaction-spi/XAResourceWrapper.java at master · jbosstm/jboss-transaction-spi · GitHub) to get a name representing the resource manager to assist with manual clean up of branches. Traditionally the JNDI name was unique enough to suit that purpose. Basically someone fires up the TX CLI looks at the JNDI name and says "oh that resource definitely committed, mark this branch as completed."  (For background you can see the Jira it was added for here: https://issues.jboss.org/browse/JBTM-859)

           

          If we add another method there will be no references to XAResourceWrapper::getJndiName any more and they will link to the new method which has the same intention from the transaction managers point of view but now has a different name.

          • 17. Re: Proposed SPI change to XAResourceWrapper
            ataylor

            I think the issue is that the method name isn't a good match for the semantics of what the transaction manager does with the value of it. XAResourceWrapper::getJndiName()

            +1, which was my initial point. when we initially were going to implement this we were just going to return the jndi name without understanding the true semantics of what was needed, we basically returned what was implied by the method name, which is also what was done for the outgoing connection factory created by the JCA layer.  It was only after further discussions that we understood what was required. If you arent expecting more implementations of this to be provided then i would just leave it, if you are then i would change it to avoid confusion in the future, or at least document it well in the API.

            • 18. Re: Proposed SPI change to XAResourceWrapper
              tomjenkinson

              Andy Taylor wrote:

               

              I think the issue is that the method name isn't a good match for the semantics of what the transaction manager does with the value of it. XAResourceWrapper::getJndiName()

              +1, which was my initial point. when we initially were going to implement this we were just going to return the jndi name without understanding the true semantics of what was needed, we basically returned what was implied by the method name, which is also what was done for the outgoing connection factory created by the JCA layer.  It was only after further discussions that we understood what was required. If you arent expecting more implementations of this to be provided then i would just leave it, if you are then i would change it to avoid confusion in the future, or at least document it well in the API.

              +1 to API docs being updated to reflect the confusion, personally I think we just leave the wonky name and just doc it, ideally alert existing users of the API to the true meaning.

              • 19. Re: Proposed SPI change to XAResourceWrapper
                marklittle

                Or change it but deprecate the old name.

                • 20. Re: Proposed SPI change to XAResourceWrapper
                  martyn-taylor

                  This would mean that any Wildfly component implementing this interface would need to be upgraded to implement this method.  Unless we create a new interface and deprecate the old one?  You could keep the same XAResourceWrapper and name just change the package name.  Won't the package name be getting changed to to io.narayana anyway: [JBTM-1699] Changes to the transaction SPI - JBoss Issue Tracker

                   

                  Updated.  The JCA XAResourceWrapper does not inherit from tx as I origijnally thought.  It just deifines the exact same methods.

                  • 21. Re: Proposed SPI change to XAResourceWrapper
                    jesper.pedersen

                    Just add a new mix-in interface with the methods needed to org.jboss.tm. Then let the XAResource implementation suck in what is needed.

                     

                    IronJacamar doesn't need it, so completely up to you guys.

                     

                    Martyn, no, the IronJacamar TX SPI doesn't inherit from the Narayana one, as IronJacamar can work with any transaction manager implementation. But, as you say, is similar.

                    1 2 Previous Next