Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-M2
    • Component/s: validation
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      Bean Validation APIs should be an optional runtime dependency by using reflection to determine if the APIs are available before accessing our ValidatorImpl class or any javax.validation.* API classes.
      openjpa-persistence will set scope=provided on geronimo-validation_1.0_spec
      openjpa-all will include the spec in the aggregated all jar
      openjpa-persistence-jdbc will not add a test dependency on the spec
      openjpa-integration will be used to test the validation support via OPENJPA-1106

      1. openjpa-1114.2.txt
        12 kB
        Pinaki Poddar
      2. OPENJPA-1114.patch
        10 kB
        Donald Woods
      3. OPENJPA-1114.patch
        9 kB
        Donald Woods

        Issue Links

          Activity

          Hide
          Donald Woods added a comment -

          OK, thanks for clearing up the plugin concept, but I think we still have the issue of when to set the alias to "jpa2", since we need all of the product derivations applied to the config before we try to initialize the plugin. I'll look at this some more, as part of OPENJPA-1111. Thanks.

          Show
          Donald Woods added a comment - OK, thanks for clearing up the plugin concept, but I think we still have the issue of when to set the alias to "jpa2", since we need all of the product derivations applied to the config before we try to initialize the plugin. I'll look at this some more, as part of OPENJPA-1111 . Thanks.
          Hide
          Pinaki Poddar added a comment -

          The attached patch shows how a ValidatingLifecycleManager can be configured without bringing in compile-time dependency to Validation API jar.
          This conventional plug-in route also helps you to configure VLEM as a bean. And hence this patch can be improved.
          For example, making ValidationMode and Factory setting as part of VLEM itself. This will help removing separate API method on OpenJPAConfiguration for ValidationMode/factory which are semantically interdependent but that fact is neither obvious nor perhaps enforceable within the API.

          Show
          Pinaki Poddar added a comment - The attached patch shows how a ValidatingLifecycleManager can be configured without bringing in compile-time dependency to Validation API jar. This conventional plug-in route also helps you to configure VLEM as a bean. And hence this patch can be improved. For example, making ValidationMode and Factory setting as part of VLEM itself. This will help removing separate API method on OpenJPAConfiguration for ValidationMode/factory which are semantically interdependent but that fact is neither obvious nor perhaps enforceable within the API.
          Hide
          Donald Woods added a comment -

          It is created at the EMF level, as a ValidatorFactory can only be passed in on a createEMF() or createContainerEMF() call as a Map property, or by trying to create a default validator off the classpath if none were supplied.

          A little background - The ValidatingLifecycleEventManager (VLEM) was created as part of OPENJPA-1068 as a way to extend the existing LifecycleEventManager (LEM) with Bean Validation (BV) specific behavior.
          Previously, the LEM was created by BrokerImpl.initialize(), but we couldn't put the VLEM vs. LEM logic there, as BrokerImpl is in openjpa-kernel and doesn't have access to any of the JPA Spec APIs. We also couldn't place the logic in OpenJPAConfigurationImpl, as that class is also in openjpa-kernel and can't access the JPA Spec APIs. So, we followed Albert's coding for LockMgr and used the PersistenceProductDerivation to setup the configuration objects.
          Now, there is still one scenario that doesn't work and will be addressed by OPENJPA-1111, which may lead us to moving the VLEM vs LEM creation logic to some other location.

          Any thoughts on where this logic (which requires access to the javax.persistence.validation spec classes) should reside would be greatly appreciated.

          Show
          Donald Woods added a comment - It is created at the EMF level, as a ValidatorFactory can only be passed in on a createEMF() or createContainerEMF() call as a Map property, or by trying to create a default validator off the classpath if none were supplied. A little background - The ValidatingLifecycleEventManager (VLEM) was created as part of OPENJPA-1068 as a way to extend the existing LifecycleEventManager (LEM) with Bean Validation (BV) specific behavior. Previously, the LEM was created by BrokerImpl.initialize(), but we couldn't put the VLEM vs. LEM logic there, as BrokerImpl is in openjpa-kernel and doesn't have access to any of the JPA Spec APIs. We also couldn't place the logic in OpenJPAConfigurationImpl, as that class is also in openjpa-kernel and can't access the JPA Spec APIs. So, we followed Albert's coding for LockMgr and used the PersistenceProductDerivation to setup the configuration objects. Now, there is still one scenario that doesn't work and will be addressed by OPENJPA-1111 , which may lead us to moving the VLEM vs LEM creation logic to some other location. Any thoughts on where this logic (which requires access to the javax.persistence.validation spec classes) should reside would be greatly appreciated.
          Hide
          Pinaki Poddar added a comment -

          Can this vaalidator be configured per-EM basis or only per_EMF basis?
          If it can only be done per-EMF basis then there should not be warning per EM.

          PersistenceProductDerivation constructs ValidatingLifecycleEventManager.
          This is an unusual usage of plug-in. Can you elaborate why ValidatingLifecycleEventManager can not be plugged-in in the usual way other plug-ins work?
          There are many plug-ins for which compile-time dependency to a third-party library is avoided.

          Show
          Pinaki Poddar added a comment - Can this vaalidator be configured per-EM basis or only per_EMF basis? If it can only be done per-EMF basis then there should not be warning per EM. PersistenceProductDerivation constructs ValidatingLifecycleEventManager. This is an unusual usage of plug-in. Can you elaborate why ValidatingLifecycleEventManager can not be plugged-in in the usual way other plug-ins work? There are many plug-ins for which compile-time dependency to a third-party library is avoided.
          Hide
          Donald Woods added a comment -

          The warning happens everytime you create a new EM/BrokerImpl, as a new LifecycleEventManager gets created for each Broker created.
          If we decide later on to include the BV API from Geronimo as a runtime dependency in the POMs (currently it is not, but we include it in the release assembly and openjpa-all.jar) then we can change that log msg to a Trace, but for now, we should include the warning since it is an optional depend (which means a user should supply their own API + Impl for now.)

          Show
          Donald Woods added a comment - The warning happens everytime you create a new EM/BrokerImpl, as a new LifecycleEventManager gets created for each Broker created. If we decide later on to include the BV API from Geronimo as a runtime dependency in the POMs (currently it is not, but we include it in the release assembly and openjpa-all.jar) then we can change that log msg to a Trace, but for now, we should include the warning since it is an optional depend (which means a user should supply their own API + Impl for now.)
          Hide
          Pinaki Poddar added a comment -
          • If you have the BV APIs but not a provider, then you'll get a warning in the log.

          Can we warn only once? I noticed the warning messages simply because there were too many of them.

          Show
          Pinaki Poddar added a comment - If you have the BV APIs but not a provider, then you'll get a warning in the log. Can we warn only once? I noticed the warning messages simply because there were too many of them.
          Hide
          Donald Woods added a comment -

          Some details about the included logging/tracing:

          • If the BV APIs are not on the classpath or ValidationMode=NONE, then nothing will get logged unless you have TRACE enabled.
          • If you have the BV APIs but not a provider, then you'll get a warning in the log.
          • If you have validation mode = CALLBACK but either the BV API or Impl is missing, then you'll get a error in the log and eventually a fatal exception once OPENJPA-1111 is resolved.
          Show
          Donald Woods added a comment - Some details about the included logging/tracing: If the BV APIs are not on the classpath or ValidationMode=NONE, then nothing will get logged unless you have TRACE enabled. If you have the BV APIs but not a provider, then you'll get a warning in the log. If you have validation mode = CALLBACK but either the BV API or Impl is missing, then you'll get a error in the log and eventually a fatal exception once OPENJPA-1111 is resolved.
          Hide
          Donald Woods added a comment -

          updated patch

          Show
          Donald Woods added a comment - updated patch
          Hide
          Donald Woods added a comment -

          work in-progress. will test some more tomorrow before committing....

          Show
          Donald Woods added a comment - work in-progress. will test some more tomorrow before committing....

            People

            • Assignee:
              Donald Woods
              Reporter:
              Donald Woods
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development