Shiro
  1. Shiro
  2. SHIRO-389

Fix OSGI Exports for shiro-ehcache

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0, 1.2.1
    • Fix Version/s: 1.2.2, 1.3.0
    • Component/s: Caching
    • Labels:
      None

      Description

      Currently the osgi-export in the pom file is org.apache.shiro.ehcache which isn't a valid package. It should be changed to org.apache.shiro.cache.ehcache

      1. SHIRO-389.patch
        0.8 kB
        Chris Geer
      2. SHIRO_389_core.patch
        0.6 kB
        Mladen Marev

        Issue Links

          Activity

          Chris Geer created issue -
          Chris Geer made changes -
          Field Original Value New Value
          Link This issue is related to SHIRO-214 [ SHIRO-214 ]
          Hide
          Chris Geer added a comment -

          Patch to fix the OSGI exports.

          Show
          Chris Geer added a comment - Patch to fix the OSGI exports.
          Chris Geer made changes -
          Attachment SHIRO-389.patch [ 12550090 ]
          Hide
          Mladen Marev added a comment - - edited

          Actually this is not enough.
          If you have this in shiro.ini in OSGi environment:
          cacheManager = org.apache.shiro.cache.ehcache.EhCacheManager
          securityManager.cacheManager = $cacheManager

          shiro does not see the class.
          It needs optional import on the exported org.apache.shiro.cache.ehcache package, so that if it is available as bundle to be able to use it.
          Other option is to make shiro-ehcache a fragment to shiro-core.

          Show
          Mladen Marev added a comment - - edited Actually this is not enough. If you have this in shiro.ini in OSGi environment: cacheManager = org.apache.shiro.cache.ehcache.EhCacheManager securityManager.cacheManager = $cacheManager shiro does not see the class. It needs optional import on the exported org.apache.shiro.cache.ehcache package, so that if it is available as bundle to be able to use it. Other option is to make shiro-ehcache a fragment to shiro-core.
          Hide
          Mladen Marev added a comment -

          Patch for optional core import of org.apache.shiro.cache.ehcache package

          Show
          Mladen Marev added a comment - Patch for optional core import of org.apache.shiro.cache.ehcache package
          Mladen Marev made changes -
          Attachment SHIRO_389_core.patch [ 12551696 ]
          Hide
          Les Hazlewood added a comment - - edited

          Thanks for the patches!

          Show
          Les Hazlewood added a comment - - edited Thanks for the patches!
          Hide
          Chris Geer added a comment -

          Les, any update on getting this applied? We are currently having to run off custom snapshot versions so we can use Shiro in an OSGI container and it would be nice to have this in the 1.2.2 patch release so we can get back on a non-snapshot version.

          Show
          Chris Geer added a comment - Les, any update on getting this applied? We are currently having to run off custom snapshot versions so we can use Shiro in an OSGI container and it would be nice to have this in the 1.2.2 patch release so we can get back on a non-snapshot version.
          Hide
          Les Hazlewood added a comment -

          Hi Chris,

          I'll definitely apply it to the 1.2.x branch so we can see about getting a release out.

          Thanks!

          Les

          Show
          Les Hazlewood added a comment - Hi Chris, I'll definitely apply it to the 1.2.x branch so we can see about getting a release out. Thanks! Les
          Hide
          Les Hazlewood added a comment - - edited

          I have a question:

          It feels odd to me to have the following line in core's pom.xml OSGi statements:

          org.apache.shiro.cache.ehcache;version="$

          {shiro.osgi.importRange}

          ";resolution:=optional,

          shiro-core has no dependencies whatsoever on ehcache - why then would this be included in the core's pom file?

          Show
          Les Hazlewood added a comment - - edited I have a question: It feels odd to me to have the following line in core's pom.xml OSGi statements: org.apache.shiro.cache.ehcache;version="$ {shiro.osgi.importRange} ";resolution:=optional, shiro-core has no dependencies whatsoever on ehcache - why then would this be included in the core's pom file?
          Hide
          Les Hazlewood added a comment -

          P.S. I've committed the first patch (SHIRO-389.patch) to both the trunk and the 1.2.x branch. Still awaiting feedback on the 2nd patch modifying core's pom.xml...

          Show
          Les Hazlewood added a comment - P.S. I've committed the first patch ( SHIRO-389 .patch) to both the trunk and the 1.2.x branch. Still awaiting feedback on the 2nd patch modifying core's pom.xml...
          Les Hazlewood made changes -
          Assignee Les Hazlewood [ lhazlewood ]
          Les Hazlewood made changes -
          Affects Version/s 1.2.0 [ 12315478 ]
          Affects Version/s 1.3.0 [ 12317961 ]
          Fix Version/s 1.2.2 [ 12323469 ]
          Fix Version/s 1.3.0 [ 12317961 ]
          Hide
          Mladen Marev added a comment - - edited

          We need that, because we will be able to run with ehcache configured manager from shiro.ini and without.
          If application needs it, it has to add it to OSGi runtime and modify configuration file. Because it is optional it will be loaded only if available.
          If application does not need shiro-ehcache it just does not add shiro-ehcache bundle to OSGi environment and optional import does not break shiro-core from starting up and initialize successfully.
          If you use shiro-ehcache and you do not have the import, then initialization fails even if the bundle is available in the OSGi runtime, because classloader does not see the class located in shiro-ehcache bundle.
          So to summarize it is needed as optional, because shiro-core bundle class loader does not know at startup if ehcache is needed or not. It knows it later (after startup) when it reads configuration file.
          You say shiro-core has no dependency on shiro-ehcache.
          You can try this configuration in OSGi environment (example is taken from shiro documentation on session management):
          [main]
          cacheManager = org.apache.shiro.cache.ehcache.EhCacheManager
          securityManager.cacheManager = $cacheManager

          You will see, how ClassNotFoundException is thrown on initialization even if shiro-ehcache bundle is available and loaded in the OSGi runtime.
          I hope this info is enough.

          P.S. You might also consider other optional import packages also if they can be configured to be loaded on initialization from shiro.ini

          Show
          Mladen Marev added a comment - - edited We need that, because we will be able to run with ehcache configured manager from shiro.ini and without. If application needs it, it has to add it to OSGi runtime and modify configuration file. Because it is optional it will be loaded only if available. If application does not need shiro-ehcache it just does not add shiro-ehcache bundle to OSGi environment and optional import does not break shiro-core from starting up and initialize successfully. If you use shiro-ehcache and you do not have the import, then initialization fails even if the bundle is available in the OSGi runtime, because classloader does not see the class located in shiro-ehcache bundle. So to summarize it is needed as optional, because shiro-core bundle class loader does not know at startup if ehcache is needed or not. It knows it later (after startup) when it reads configuration file. You say shiro-core has no dependency on shiro-ehcache. You can try this configuration in OSGi environment (example is taken from shiro documentation on session management): [main] cacheManager = org.apache.shiro.cache.ehcache.EhCacheManager securityManager.cacheManager = $cacheManager You will see, how ClassNotFoundException is thrown on initialization even if shiro-ehcache bundle is available and loaded in the OSGi runtime. I hope this info is enough. P.S. You might also consider other optional import packages also if they can be configured to be loaded on initialization from shiro.ini
          Hide
          Jared Bunting added a comment -

          The say you describe this makes sense to me. However, I think there's a deeper issue here. shiro-core cannot possibly know about every bundle that may be referenced in shiro.ini - the very nature of shiro.ini is that you can specify ANY class. In addition, I think that Les's holdup is that adding shiro-ehcache as an osgi dependency of shiro-core essentially introduces a circular dependency issue - shiro-ehcache has to know about shiro-core (for compile time) and shiro-core has to know about shiro-ehcache (explicitly in osgi configuration).

          My knowledge of OSGI is limited primarily to what I've read, and my practical experience is limited to a few tutorials. However, this seems like a situation where using fragments would make sense. My understanding of fragments is that shiro-ehcache would declare it's "Fragment-Host" as shiro-core, and then shiro-ehcache's classes would be available on shiro-core's classpath without having to modify shiro-core. The upside of this is that it would work for any thirdparty extension as well - say a jcache implementation, or an internal corporate Realm implementation.

          Thoughts?

          Show
          Jared Bunting added a comment - The say you describe this makes sense to me. However, I think there's a deeper issue here. shiro-core cannot possibly know about every bundle that may be referenced in shiro.ini - the very nature of shiro.ini is that you can specify ANY class. In addition, I think that Les's holdup is that adding shiro-ehcache as an osgi dependency of shiro-core essentially introduces a circular dependency issue - shiro-ehcache has to know about shiro-core (for compile time) and shiro-core has to know about shiro-ehcache (explicitly in osgi configuration). My knowledge of OSGI is limited primarily to what I've read, and my practical experience is limited to a few tutorials. However, this seems like a situation where using fragments would make sense. My understanding of fragments is that shiro-ehcache would declare it's "Fragment-Host" as shiro-core, and then shiro-ehcache's classes would be available on shiro-core's classpath without having to modify shiro-core. The upside of this is that it would work for any thirdparty extension as well - say a jcache implementation, or an internal corporate Realm implementation. Thoughts?
          Hide
          Jared Bunting added a comment -

          I'd also be curious if there's another way to handle this plugin-style relationship in OSGI - where bundle A needs access to bundle B but may not know anything about its existence until runtime.

          Show
          Jared Bunting added a comment - I'd also be curious if there's another way to handle this plugin-style relationship in OSGI - where bundle A needs access to bundle B but may not know anything about its existence until runtime.
          Hide
          Chris Geer added a comment -

          Jared, I tend to agree that a fragment would be a better way to handle it.

          On a side note, I'd also argue that in OSGI you don't need the INI configurations for wiring assuming you are using a non-INI based username/password store. Using something like Blueprint allows you to wire together all the components in an OSGI standard way. That's what I'm doing which is why I didn't have the problem Mladen saw.

          As an example:

          <bean id="sessionMgr" class="org.apache.shiro.session.mgt.eis.EnterpriseCacheSessionDAO">
          <property name="activeSessionsCacheName" value="shiro-activeSessionCache"/>
          </bean>

          <bean id="ehCacheMgr" class="org.apache.shiro.cache.ehcache.EhCacheManager" destroy-method="destroy">
          <property name="cacheManagerConfigFile" value="file:./etc/ehcache.xml"/>
          </bean>

          <bean id="dbRealm" class="...JdbcRealm">
          <property name="authenticationCachingEnabled" value="true"/>
          </bean>

          <bean id="secMgr" class="org.apache.shiro.mgt.DefaultSecurityManager">
          <argument ref="dbRealm"/>
          <property name="sessionManager.sessionDAO" ref="sessionMgr"/>
          <property name="subjectDAO.sessionStorageEvaluator.sessionStorageEnabled" value="false"/>
          <property name="sessionManager.sessionValidationSchedulerEnabled" value="false"/>
          <property name="cacheManager" ref="ehCacheMgr"/>
          </bean>

          Show
          Chris Geer added a comment - Jared, I tend to agree that a fragment would be a better way to handle it. On a side note, I'd also argue that in OSGI you don't need the INI configurations for wiring assuming you are using a non-INI based username/password store. Using something like Blueprint allows you to wire together all the components in an OSGI standard way. That's what I'm doing which is why I didn't have the problem Mladen saw. As an example: <bean id="sessionMgr" class="org.apache.shiro.session.mgt.eis.EnterpriseCacheSessionDAO"> <property name="activeSessionsCacheName" value="shiro-activeSessionCache"/> </bean> <bean id="ehCacheMgr" class="org.apache.shiro.cache.ehcache.EhCacheManager" destroy-method="destroy"> <property name="cacheManagerConfigFile" value="file:./etc/ehcache.xml"/> </bean> <bean id="dbRealm" class="...JdbcRealm"> <property name="authenticationCachingEnabled" value="true"/> </bean> <bean id="secMgr" class="org.apache.shiro.mgt.DefaultSecurityManager"> <argument ref="dbRealm"/> <property name="sessionManager.sessionDAO" ref="sessionMgr"/> <property name="subjectDAO.sessionStorageEvaluator.sessionStorageEnabled" value="false"/> <property name="sessionManager.sessionValidationSchedulerEnabled" value="false"/> <property name="cacheManager" ref="ehCacheMgr"/> </bean>
          Hide
          Jared Bunting added a comment -

          Even if you're using an INI-based username/password store, you can use the Blueprint stuff for setting up your object graph.

          Show
          Jared Bunting added a comment - Even if you're using an INI-based username/password store, you can use the Blueprint stuff for setting up your object graph.
          Hide
          Mladen Marev added a comment -

          Normally I'd agree with Chris, but...
          Blueprint would work when you know your configuration upfront. How the user changes "authenticationCachingEnabled" flag? Opens the bundle and modifies the flag in the blueprint.xml? Or you would use another file to point if bean should be instantiated or not.
          What if you deliver a shiro.ini to be modified?
          I do not agree with circular dependencies note also, because optional imported packages has nothing to do with compilation. It is a conscious decision what to be imported optionally.
          Anyway. I think someone should decide how the situation will be handled (optional imports or fragments) for all shiro delivered bundles. I personally vote for optional imports. Fragments can not be used by other bundles.

          Show
          Mladen Marev added a comment - Normally I'd agree with Chris, but... Blueprint would work when you know your configuration upfront. How the user changes "authenticationCachingEnabled" flag? Opens the bundle and modifies the flag in the blueprint.xml? Or you would use another file to point if bean should be instantiated or not. What if you deliver a shiro.ini to be modified? I do not agree with circular dependencies note also, because optional imported packages has nothing to do with compilation. It is a conscious decision what to be imported optionally. Anyway. I think someone should decide how the situation will be handled (optional imports or fragments) for all shiro delivered bundles. I personally vote for optional imports. Fragments can not be used by other bundles.
          Hide
          Les Hazlewood added a comment -

          Hi Mladen,

          If someone were to create a custom bundle (e.g. maybe a new Hazelcast-based Shiro Cache implementation) - and the Shiro project has no knowledge of it - how would they use their Shiro cache implementation (bundle) in their OSGi-based project?

          Les

          Show
          Les Hazlewood added a comment - Hi Mladen, If someone were to create a custom bundle (e.g. maybe a new Hazelcast-based Shiro Cache implementation) - and the Shiro project has no knowledge of it - how would they use their Shiro cache implementation (bundle) in their OSGi-based project? Les
          Hide
          Mladen Marev added a comment - - edited

          Hi Les,

          Of course not. That someone will create a fragment to shiro core in order to load properly. This new bundle will be under the control of the implementer and has no role here.
          Here we are talking about out-of-the-box behavior of the released shiro bundles, right? Imagine we are placing shiro bundles in OSGi and want out-of-the-box to start a caching authentication. We have to be able to do it without modifying original bundles. We have to be able to just edit shiro.ini. We have several options for succeeding:
          1. shiro-ehcache to be a fragment to shiro-core, or
          2. shiro-core to have optional import of shiro-ehcache package.
          3. use blueprint to initialize defaultsecuritymanager before actually calling shiro functions. (This is too static and not working for my case)
          4. add ClassLoader parameter to IniSecurityManagerFactory constructor (maybe to other factories too) - as additional class loader for classes. This way user bundle can import everything it needs and supply the classloader to the factory. This is the best OSGi approach in my opinion, but I doubt it will be implemented.
          As I said earlier, I would prefer optional import as fastest and least intrusive approach.

          Show
          Mladen Marev added a comment - - edited Hi Les, Of course not. That someone will create a fragment to shiro core in order to load properly. This new bundle will be under the control of the implementer and has no role here. Here we are talking about out-of-the-box behavior of the released shiro bundles, right? Imagine we are placing shiro bundles in OSGi and want out-of-the-box to start a caching authentication. We have to be able to do it without modifying original bundles. We have to be able to just edit shiro.ini. We have several options for succeeding: 1. shiro-ehcache to be a fragment to shiro-core, or 2. shiro-core to have optional import of shiro-ehcache package. 3. use blueprint to initialize defaultsecuritymanager before actually calling shiro functions. (This is too static and not working for my case) 4. add ClassLoader parameter to IniSecurityManagerFactory constructor (maybe to other factories too) - as additional class loader for classes. This way user bundle can import everything it needs and supply the classloader to the factory. This is the best OSGi approach in my opinion, but I doubt it will be implemented. As I said earlier, I would prefer optional import as fastest and least intrusive approach.
          Hide
          Chris Geer added a comment -

          Option 5 would be to add a dynamicimport instruction for shiro-core which means it would pick up additional stuff as needed but that might have side effects. I'm going to forward this to the Karaf user list and see if anyone can provide any additional thoughts.

          Show
          Chris Geer added a comment - Option 5 would be to add a dynamicimport instruction for shiro-core which means it would pick up additional stuff as needed but that might have side effects. I'm going to forward this to the Karaf user list and see if anyone can provide any additional thoughts.
          Hide
          Christian Schneider added a comment -

          How about exporting shiro extensions/plugins as OSGi services from the bundle that offers them. Then you could declare the filter statement or just the interface name in the ini and resolve the service when evaluating the config. This would avoid that shiro core needs to know about the impl packages.

          Show
          Christian Schneider added a comment - How about exporting shiro extensions/plugins as OSGi services from the bundle that offers them. Then you could declare the filter statement or just the interface name in the ini and resolve the service when evaluating the config. This would avoid that shiro core needs to know about the impl packages.
          Hide
          Achim Nierbeck added a comment -

          I have to second Christian on this.

          The proper solution for this would be what's called the White-Board-Pattern.

          Shiro needs to provide an interface for "Cache Managers", and opens a Service Tracker listening for this interface.
          Now anybody is able to add additional Cache Mangers by implementing this interface and registering it as a service.
          Shiro will pick those up and will be able to use them, since they all use the interface the core bundle doesn't need to know of any other optional packages.

          If this kind of solution isn't possible, you have to fallback to the fragment solution. It's the cleanest solution at hand for this kind of issue.

          Show
          Achim Nierbeck added a comment - I have to second Christian on this. The proper solution for this would be what's called the White-Board-Pattern. Shiro needs to provide an interface for "Cache Managers", and opens a Service Tracker listening for this interface. Now anybody is able to add additional Cache Mangers by implementing this interface and registering it as a service. Shiro will pick those up and will be able to use them, since they all use the interface the core bundle doesn't need to know of any other optional packages. If this kind of solution isn't possible, you have to fallback to the fragment solution. It's the cleanest solution at hand for this kind of issue.
          Hide
          Chris Geer added a comment -

          While I tend to like the suggestions from Christian/Achimn, that's rather large change, but one that would make Shiro really nice in an OSGI environment.

          I would propose that we split this ticket at this point. The original intent was to simply fix a bug where the export statement was wrong on the shiro-ehcache bundle so it could be used in an OSGI environment. That problem has been patched and can be released.

          Can we move the follow-up conversation to another ticket and resolve this one?

          Show
          Chris Geer added a comment - While I tend to like the suggestions from Christian/Achimn, that's rather large change, but one that would make Shiro really nice in an OSGI environment. I would propose that we split this ticket at this point. The original intent was to simply fix a bug where the export statement was wrong on the shiro-ehcache bundle so it could be used in an OSGI environment. That problem has been patched and can be released. Can we move the follow-up conversation to another ticket and resolve this one?
          Hide
          Christian Schneider added a comment -

          Sure

          Show
          Christian Schneider added a comment - Sure
          Chris Geer made changes -
          Link This issue is related too SHIRO-419 [ SHIRO-419 ]
          Hide
          Chris Geer added a comment -

          This is the followup ticket to this one.

          Show
          Chris Geer added a comment - This is the followup ticket to this one.
          Hide
          Chris Geer added a comment -

          I created SHIRO-419 which we can continue this conversation in on how shiro can better support 3rd party bundles through ini files.

          Les, since you committed the first patch my opinion is this ticket can be closed.

          Show
          Chris Geer added a comment - I created SHIRO-419 which we can continue this conversation in on how shiro can better support 3rd party bundles through ini files. Les, since you committed the first patch my opinion is this ticket can be closed.
          Hide
          Les Hazlewood added a comment -

          Resolving per the first patch being committed. We'll use SHIRO-419 for further discussion of 3rd party bundles.

          Show
          Les Hazlewood added a comment - Resolving per the first patch being committed. We'll use SHIRO-419 for further discussion of 3rd party bundles.
          Les Hazlewood made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Gavin made changes -
          Link This issue is related to SHIRO-419 [ SHIRO-419 ]
          Gavin made changes -
          Link This issue is related to SHIRO-419 [ SHIRO-419 ]

            People

            • Assignee:
              Les Hazlewood
              Reporter:
              Chris Geer
            • Votes:
              2 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development