You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "David Jencks (JIRA)" <de...@myfaces.apache.org> on 2010/11/18 01:27:16 UTC
[jira] Created: (MYFACES-2976) Support hiding myfaces impl classes
in osgi, and provide a single osgi bundle for api + impl
Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
--------------------------------------------------------------------------------------------
Key: MYFACES-2976
URL: https://issues.apache.org/jira/browse/MYFACES-2976
Project: MyFaces Core
Issue Type: Improvement
Components: Extension Feature
Affects Versions: 2.0.3-SNAPSHOT
Reporter: David Jencks
The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
Also this does not easily support hiding the impl classes.
Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (MYFACES-2976) Support hiding myfaces impl classes
in osgi, and provide a single osgi bundle for api + impl
Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jakob Korherr resolved MYFACES-2976.
------------------------------------
Resolution: Fixed
Fix Version/s: 2.0.3-SNAPSHOT
Thanks again, David! Resolving this one.
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Jakob Korherr
> Fix For: 2.0.3-SNAPSHOT
>
> Attachments: MYFACES-2976-osgi-bundle.patch, MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2976) Support hiding myfaces impl
classes in osgi, and provide a single osgi bundle for api + impl
Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12934897#action_12934897 ]
Jakob Korherr commented on MYFACES-2976:
----------------------------------------
I committed the FactoryFinder classloading part of David's patch and provided a new patch for the OSGi bundle: MYFACES-2976-osgi-bundle.patch
The patch includes exports of org.apache.myfaces.config.impl.digester.element and org.apache.myfaces.config.element as discussed.
If no objections, I will commit this patch soon!
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Jakob Korherr
> Attachments: MYFACES-2976-osgi-bundle.patch, MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2976) Support hiding myfaces impl
classes in osgi, and provide a single osgi bundle for api + impl
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12933468#action_12933468 ]
Leonardo Uribe commented on MYFACES-2976:
-----------------------------------------
Just add some links related to this stuff:
http://wikis.sun.com/display/GlassFish/Maven+Versioning+Rules
And the issue in Mojarra:
https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=924
I'll put a review of the patch proposed soon.
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Jakob Korherr
> Attachments: MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2976) Support hiding myfaces impl
classes in osgi, and provide a single osgi bundle for api + impl
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12933657#action_12933657 ]
Leonardo Uribe commented on MYFACES-2976:
-----------------------------------------
I have reviewed the patch and it looks good. It is a very clever solution the one used on FactoryFinder. Since all classes are in a single bundle, just fallback to FactoryFinder classLoader is enough. In MyFaces javax.faces.FactoryFinder class we have two maps:
/**
* used as a monitor for itself and _factories. Maps in this map are used as monitors for themselves and the
* corresponding maps in _factories.
*/
private static Map<ClassLoader, Map<String, List<String>>> _registeredFactoryNames = new HashMap<ClassLoader, Map<String, List<String>>>();
/**
* Maps from classLoader to another map, the container (i.e. Tomcat) will create a class loader for each web app
* that it controls (typically anyway) and that class loader is used as the key.
*
* The secondary map maps the factory name (i.e. FactoryFinder.APPLICATION_FACTORY) to actual instances that are
* created via getFactory. The instances will be of the class specified in the setFactory method for the factory
* name, i.e. FactoryFinder.setFactory(FactoryFinder.APPLICATION_FACTORY, MyFactory.class).
*/
private static Map<ClassLoader, Map<String, Object>> _factories = new HashMap<ClassLoader, Map<String, Object>>();
In theory we use the webapp classloader as a "key" to diferentiate between Factories created by different webapp living on a same server. This solution keeps using webapp classloader as a key, but since not all packages are exported from the bundle, the fallback just try to find them inside myfaces bundle and do the magic in a better way.
I tried long time ago on MYFACES-2290 different alternatives, but those ones involves have an alternate javax.faces.FactoryFinder class (because we was thinking on keep myfaces-api and myfaces-impl jars in different bundles). But this solution has the advantage that we will only have one javax.faces.FactoryFinder implementation, and we just need to rewrite the MANIFEST.MF file for the specific bundle.
The only thing to get better is instead add this package as Export-Package in MANIFEST.MF:
org.apache.myfaces.config.impl.digester.elements
use this one:
org.apache.myfaces.config.element
In MYFACES-2945, I'll add some code to make these interfaces public, so please use this package instead.
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Jakob Korherr
> Attachments: MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2976) Support hiding myfaces impl
classes in osgi, and provide a single osgi bundle for api + impl
Posted by "David Jencks (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12936113#action_12936113 ]
David Jencks commented on MYFACES-2976:
---------------------------------------
I think the downloaded snapshot is working fine with geronimo. Thanks!
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Jakob Korherr
> Attachments: MYFACES-2976-osgi-bundle.patch, MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2976) Support hiding myfaces impl
classes in osgi, and provide a single osgi bundle for api + impl
Posted by "David Jencks (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12933662#action_12933662 ]
David Jencks commented on MYFACES-2976:
---------------------------------------
I added the export for the org.apache.myfaces.config.impl.digester.elements because digester wasn't working and I didn't want to investigate why at that moment :-). We might need to do some adjustment of the classloading environment for digester if we don't export these classes. I hope that can be made to work, I might have time to investigate more in the next couple days.
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Jakob Korherr
> Attachments: MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2976) Support hiding myfaces impl
classes in osgi, and provide a single osgi bundle for api + impl
Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12933393#action_12933393 ]
Jakob Korherr commented on MYFACES-2976:
----------------------------------------
The patch looks very good. Thanks!
Two comments: I'd prefer "bundle" as module name over "org.apache.myfaces.bundle" and also you don't need to include myfaces-impl-ee6 as dependency, because it already ships with myfaces-impl.
Besides that, I am in favour of committing this patch.
If there are no objections, I will commit (a slightly modified version of) this patch soon!
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Attachments: MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2976) Support hiding myfaces impl
classes in osgi, and provide a single osgi bundle for api + impl
Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12935967#action_12935967 ]
Jakob Korherr commented on MYFACES-2976:
----------------------------------------
I just committed the patch to create the OSGi bundle.
David, can you please test this again? If everything works well with Geronimo, I will resolve this issue! Thanks!
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Jakob Korherr
> Attachments: MYFACES-2976-osgi-bundle.patch, MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2976) Support hiding myfaces impl
classes in osgi, and provide a single osgi bundle for api + impl
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12933687#action_12933687 ]
Leonardo Uribe commented on MYFACES-2976:
-----------------------------------------
I remember that one. The problem is digester requires to set a classloader, and it is set to the TCCL, so if the package is not exported, the TCCL will not see the required classes. The better in this case is add both packages.
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Jakob Korherr
> Attachments: MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2976) Support hiding myfaces impl
classes in osgi, and provide a single osgi bundle for api + impl
Posted by "David Jencks (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12935946#action_12935946 ]
David Jencks commented on MYFACES-2976:
---------------------------------------
Jakob's revised patch seems to work fine with geronimo. It would be great to get this committed at least as a starting point so we can do more extensive testing.
> Support hiding myfaces impl classes in osgi, and provide a single osgi bundle for api + impl
> --------------------------------------------------------------------------------------------
>
> Key: MYFACES-2976
> URL: https://issues.apache.org/jira/browse/MYFACES-2976
> Project: MyFaces Core
> Issue Type: Improvement
> Components: Extension Feature
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Jakob Korherr
> Attachments: MYFACES-2976-osgi-bundle.patch, MYFACES-2976.diff
>
>
> The current osgi solution of using require-bundle between the api and impl jars is generally thought of as osgi worst-practice and just doesn't work with snapshot versions (presumably a bug in the bundle plugin).
> Also this does not easily support hiding the impl classes.
> Geronimo would like to use a single bundle containing both api and impl and hide the impl classes as much as possible. This requires some slight modifications to the FactoryFinder so that impl classes can be loaded from the same classloader as FactoryFinder if they are not visible to the context class loader.
> I think myfaces would be the best place for this combined bundle but geronimo can build it if necessary. The FactoryFinder classloader changes are therefore more important.
> I've suggested a module name which maps to the bundle symbolic name well, as this seems to be the preferred maven/osgi strategy at apache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.