Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-beta
    • Fix Version/s: 2.2.0
    • Component/s: osgi
    • Labels:
      None

      Description

      Follow-on to OPENJPA-1524 to create a generic OSGiManagedRuntime class that would work for vendors/frameworks other than Apache Aries.

      Tim's original suggestion was - One implementation suggestion would be to use a ServiceTracker to keep track of the JTA services. This would allow for lazy lookup and also provide a notification if the service is unregistered.
      This should then be possible to implement as a relatively simple subclass of RegistryManagedRuntime that overrides getTransactionManager(). I don't know if you have a nice mechanism to pass a BundleContext yet, but I don't think that should pose a significant problem.

      Now that we have a BundleUtils.java to require OSGi classes in our runtime, we should be able to lookup "javax.transaction.TransactionSynchronizationRegistry" from the ServiceRegistry to use, instead of relying on the current JNDI lookup which is Aries specific.

      1. OPENJPA-1593-1.patch
        9 kB
        Wolfgang Glas
      2. OPENJPA-1593-2.patch
        9 kB
        Wolfgang Glas
      3. OPENJPA-1593-3.patch
        9 kB
        Wolfgang Glas
      4. OPENJPA-1593-4.patch
        26 kB
        Wolfgang Glas

        Issue Links

          Activity

          Hide
          Wolfgang Glas added a comment -

          The way to go here is to register a service listener for a javax.transaction.TransactionManager inside

          org.apache.openjpa.persistence.osgi.PersistenceActivator.start(BundleContext)

          with a filter expression of

          "(objectClass=javax.transaction.TransactionManager)"

          The best solution would be to let OSGiManagedRuntime implement OSGi's ServiceListener and openjpa's ManagedRuntime interfaces and maintain
          a synchronized copy of the TransactionManager inside OSGiManagedRuntime.

          And yes, I would really like to have this feature in openjpa-2.1.0

          Best regards,

          Wolfgang

          Show
          Wolfgang Glas added a comment - The way to go here is to register a service listener for a javax.transaction.TransactionManager inside org.apache.openjpa.persistence.osgi.PersistenceActivator.start(BundleContext) with a filter expression of "(objectClass=javax.transaction.TransactionManager)" The best solution would be to let OSGiManagedRuntime implement OSGi's ServiceListener and openjpa's ManagedRuntime interfaces and maintain a synchronized copy of the TransactionManager inside OSGiManagedRuntime. And yes, I would really like to have this feature in openjpa-2.1.0 Best regards, Wolfgang
          Hide
          Wolfgang Glas added a comment -

          I've attached patch, which adds an OSGiManagedRuntime. I've carefully debugged this stuff an tracked all possible start/stop combinations of the transaction and the openjpa bundle carefully.

          Please review and possibly apply my patch, which has been generated for the 2.1.x branch.

          Best regards, Wolfgang

          Show
          Wolfgang Glas added a comment - I've attached patch, which adds an OSGiManagedRuntime. I've carefully debugged this stuff an tracked all possible start/stop combinations of the transaction and the openjpa bundle carefully. Please review and possibly apply my patch, which has been generated for the 2.1.x branch. Best regards, Wolfgang
          Hide
          Michael Dick added a comment -

          I took a quick look through the patch, and nothing major jumped out at me.

          My main concern is that the new managed runtime gets tested. Can you elaborate on what you've done to exercise the new class?

          One other nit - I noticed a lot of one line if statements that don't have curly braces, could you add those in?

          Show
          Michael Dick added a comment - I took a quick look through the patch, and nothing major jumped out at me. My main concern is that the new managed runtime gets tested. Can you elaborate on what you've done to exercise the new class? One other nit - I noticed a lot of one line if statements that don't have curly braces, could you add those in?
          Hide
          Wolfgang Glas added a comment - - edited

          Michael,

          Thanks for reviewing my stuff, I've attached a version with slight javadoc improvements and with curly braces add to all if-statements.

          I don't know how to test OSGi service discovery inside a JUnit test.

          What I've done is to attach a remote debugger to my apache karaf an add breakpoints in side the ServiceListener and the bunde startup and shutdown code. I then started and stopped the openjpa bundle with the transaction bundle started or stopped. Then I started and stopped the transaction bundle while the openjpa bundle was up in order to verify that the cached transaction manager instance is reliably removed in all possible situations.

          Ideas for automating these test are very much welcome

          Best regards, Wolfgang

          Show
          Wolfgang Glas added a comment - - edited Michael, Thanks for reviewing my stuff, I've attached a version with slight javadoc improvements and with curly braces add to all if-statements. I don't know how to test OSGi service discovery inside a JUnit test. What I've done is to attach a remote debugger to my apache karaf an add breakpoints in side the ServiceListener and the bunde startup and shutdown code. I then started and stopped the openjpa bundle with the transaction bundle started or stopped. Then I started and stopped the transaction bundle while the openjpa bundle was up in order to verify that the cached transaction manager instance is reliably removed in all possible situations. Ideas for automating these test are very much welcome Best regards, Wolfgang
          Hide
          Wolfgang Glas added a comment -

          Well, another cup of sleep brought to me the idea of testing the servicediscovery using a fake BundleContext implementation.

          Attached you will find the leatest an greatest version of my patch with a testcase and a cleanup of the code, which now calls BundelContext.ungetReference() as specified by the OSGi specs.

          Best regards, Wolfgang

          Show
          Wolfgang Glas added a comment - Well, another cup of sleep brought to me the idea of testing the servicediscovery using a fake BundleContext implementation. Attached you will find the leatest an greatest version of my patch with a testcase and a cleanup of the code, which now calls BundelContext.ungetReference() as specified by the OSGi specs. Best regards, Wolfgang
          Hide
          Wolfgang Glas added a comment -

          I've just browsed through org.apache.openjpa.ee.AutomaticManagedRuntime and got the impression, that OSGiManageRuntime should be integrated in the service dicovery mode, if running under OSGi.

          Is there someone eligible for undertaking this task?

          Show
          Wolfgang Glas added a comment - I've just browsed through org.apache.openjpa.ee.AutomaticManagedRuntime and got the impression, that OSGiManageRuntime should be integrated in the service dicovery mode, if running under OSGi. Is there someone eligible for undertaking this task?
          Hide
          Michael Dick added a comment -

          I don't see a testcase in the attached patches, did it get missed when you ran svn add?

          Adding it to AutomaticManagedRuntime should probably wait for 2.2.0 - not sure we'll find any takers in the next couple of days that can take it on. If you have a unit test that manually specifies it we might be able to get that into 2.1.0 though..

          Show
          Michael Dick added a comment - I don't see a testcase in the attached patches, did it get missed when you ran svn add? Adding it to AutomaticManagedRuntime should probably wait for 2.2.0 - not sure we'll find any takers in the next couple of days that can take it on. If you have a unit test that manually specifies it we might be able to get that into 2.1.0 though..
          Hide
          Wolfgang Glas added a comment -

          OK, I've forgot to 'svn add' my testcase, so it was missed by 'svn diff'.

          This new version should include everything you need

          Wolfgang

          Show
          Wolfgang Glas added a comment - OK, I've forgot to 'svn add' my testcase, so it was missed by 'svn diff'. This new version should include everything you need Wolfgang
          Hide
          Michael Dick added a comment -

          Mailing list post jogged my memory to look at the patch, which looks good.

          The only question I have is with the service registration triggered in PersistenceActivator. It looks benign but it'd be nice to know whether there's an effect when running with Aries before we commit.

          Show
          Michael Dick added a comment - Mailing list post jogged my memory to look at the patch, which looks good. The only question I have is with the service registration triggered in PersistenceActivator. It looks benign but it'd be nice to know whether there's an effect when running with Aries before we commit.
          Hide
          Michael Dick added a comment -

          Fixed in r1068588.

          Show
          Michael Dick added a comment - Fixed in r1068588.
          Hide
          Wolfgang Glas added a comment -

          Michael,

          Thanks for applying the patch,

          Best regards, Wolfgang

          Show
          Wolfgang Glas added a comment - Michael, Thanks for applying the patch, Best regards, Wolfgang

            People

            • Assignee:
              Michael Dick
              Reporter:
              Donald Woods
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development