Ed,
See some comments below. I am reviewing changes to files that I am
associated with.
On Saturday 11 December 2010 07:50 AM, edward.burns_at_oracle.com wrote:
> - Per Sahoo's issue GLASSFISH-14430, use URI instead of URL where
> possible.
>
> SECTION: Changes to osgi-javaee/osgi-web-container to implement API
> changes from issue
> <http://java.net/jira/browse/GLASSFISH-14430>.
>
>
> M
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FacesAnnotationScanner.java
> M
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> WebModuleDecorator.java
> M
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FacesConfigResourceProvider.java
> M
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FaceletConfigResourceProvider.java
>
> - Per Sahoo's issue GLASSFISH-14430, use URI instead of URL where
> possible.
>
>
> SECTION: Diffs
> Index:
> web/weld-integration/src/main/java/org/glassfish/weld/jsf/WeldFacesConf
> igProvider.java
> ===================================================================
> ---
> web/weld-integration/src/main/java/org/glassfish/weld/jsf/WeldFacesConf
> igProvider.java (revision 43680)
> +++
> web/weld-integration/src/main/java/org/glassfish/weld/jsf/WeldFacesConf
> igProvider.java (working copy)
> @@ -40,18 +40,24 @@
>
> package org.glassfish.weld.jsf;
>
> +import java.net.URISyntaxException;
> import java.net.URL;
> import java.util.ArrayList;
> import java.util.Collection;
> import java.util.List;
> +import java.util.logging.Level;
> +import java.util.logging.Logger;
>
> import javax.servlet.ServletContext;
>
> import com.sun.enterprise.deployment.WebBundleDescriptor;
> import com.sun.enterprise.web.WebModule;
> import com.sun.faces.spi.FacesConfigResourceProvider;
> +import com.sun.logging.LogDomains;
> +import java.net.URI;
> import org.glassfish.api.invocation.ComponentInvocation;
> import org.glassfish.api.invocation.InvocationManager;
> +import org.glassfish.weld.WeldApplicationContainer;
> import org.glassfish.weld.WeldDeployer;
> import org.jvnet.hk2.component.Habitat;
>
> @@ -65,9 +71,11 @@
> "org.glassfish.servlet.habitat";
> private InvocationManager invokeMgr;
>
> + private Logger _logger =
> LogDomains.getLogger(WeldApplicationContainer.class,
> LogDomains.CORE_LOGGER);
> +
> private static final String SERVICES_FACES_CONFIG =
> "META-INF/services/faces-config.xml";
>
> - public Collection<URL> getResources(ServletContext context) {
> + public Collection<URI> getResources(ServletContext context) {
>
> Habitat defaultHabitat = (Habitat)context.getAttribute(
> HABITAT_ATTRIBUTE);
> @@ -76,7 +84,7 @@
> WebModule webModule = (WebModule)inv.getContainer();
> WebBundleDescriptor wdesc =
> webModule.getWebBundleDescriptor();
>
> - List<URL> list = new ArrayList<URL>(1);
> + List<URI> list = new ArrayList<URI>(1);
>
> if (!wdesc.hasExtensionProperty(WeldDeployer.WELD_EXTENSION))
> {
> return list;
> @@ -86,7 +94,22 @@
> // be available from the same classloader that loaded this
> instance.
> // Doing so allows us to be more OSGi friendly.
> ClassLoader loader = this.getClass().getClassLoader();
> - list.add(loader.getResource(SERVICES_FACES_CONFIG));
> + URL resource = loader.getResource(SERVICES_FACES_CONFIG);
> + if (null != resource) {
> + URI toAdd = null;
> + try {
> + toAdd = new URI(resource.toExternalForm());
> + } catch (URISyntaxException ex) {
> + if (_logger.isLoggable(Level.SEVERE)) {
> + String message = "Unable to create URI for URL " +
> + resource.toExternalForm();
> + _logger.log(Level.SEVERE, message, ex);
> + }
> + }
> + if (null != toAdd) {
> + list.add(toAdd);
> + }
> + }
> return list;
> }
>
>
Can you change this to use URL.toURI() and adjust it to avoid a null
check so that the code becomes:
URI resource = loader.getResource(SERVICES_FACES_CONFIG);
if (resource != null) {
try {
list.add(resource.toURI());
} catch(URISyntaxException) {
String message = "Unable to create URI for URL " +
resource.toExternalForm();
_logger.log(Level.SEVERE, message, ex);
}
}
>
> Index:
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FacesAnnotationScanner.java
>
Looks good.
> Index:
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> WebModuleDecorator.java
>
> ===================================================================
> ---
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> WebModuleDecorator.java (revision 43680)
> +++
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> WebModuleDecorator.java (working copy)
> @@ -56,7 +56,6 @@
> import java.net.URI;
> import java.net.URISyntaxException;
> import java.net.URL;
> -import java.net.MalformedURLException;
> import java.util.*;
> import java.util.logging.Level;
> import java.util.logging.Logger;
> @@ -111,8 +110,8 @@
> }
>
> private void populateFacesInformation(WebModule module,
> BundleContext bctx, ServletContext sc) {
> - Collection<URL> facesConfigs = new ArrayList<URL>();
> - Collection<URL> faceletConfigs = new ArrayList<URL>();
> + Collection<URI> facesConfigs = new ArrayList<URI>();
> + Collection<URI> faceletConfigs = new ArrayList<URI>();
> discoverJSFConfigs(bctx.getBundle(), facesConfigs,
> faceletConfigs);
> sc.setAttribute(Constants.FACES_CONFIG_ATTR, facesConfigs);
> sc.setAttribute(Constants.FACELET_CONFIG_ATTR,
> faceletConfigs);
> @@ -154,20 +153,16 @@
> * reported in
> https://glassfish.dev.java.net/issues/show_bug.cgi?id=12914, we only
> find faces config resources
> * that ends with .faces-config.xml.
> */
> - private void discoverJSFConfigs(Bundle b, Collection<URL>
> facesConfigs, Collection<URL> faceletConfigs) {
> + private void discoverJSFConfigs(Bundle b, Collection<URI>
> facesConfigs, Collection<URI> faceletConfigs) {
> OSGiBundleArchive archive = new OSGiBundleArchive(b);
> for (BundleResource r : archive) {
> final String path = r.getPath();
> if (path.startsWith("META-INF/")) {
> - try {
> - final URL url = r.getUri().toURL();
> - if (path.endsWith(".taglib.xml")) {
> - faceletConfigs.add(url);
> - } else if (path.endsWith(".faces-config.xml")) {
> // this check automatically excludes META-INF/faces-config.xml
> - facesConfigs.add(url);
> - }
> - } catch (MalformedURLException e) {
> - e.printStackTrace(); // ignore and continue
> + final URI url = r.getUri();
> + if (path.endsWith(".taglib.xml")) {
> + faceletConfigs.add(url);
> + } else if (path.endsWith(".faces-config.xml")) { //
> this check automatically excludes META-INF/faces-config.xml
> + facesConfigs.add(url);
> }
> }
> }
>
Can you change the local variable name to "uri" instead of "url?"
> Index:
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FacesConfigResourceProvider.java
> ===================================================================
> ---
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FacesConfigResourceProvider.java (revision 43680)
> +++
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FacesConfigResourceProvider.java (working copy)
> @@ -41,12 +41,9 @@
> package org.glassfish.osgiweb;
>
> import javax.servlet.ServletContext;
> -import java.net.URL;
> import java.net.URI;
> -import java.net.MalformedURLException;
> import java.util.Collection;
> import java.util.Collections;
> -import java.util.ArrayList;
> import java.util.logging.Logger;
>
> /**
> @@ -61,10 +58,10 @@
> */
> public class OSGiFacesConfigResourceProvider implements
> com.sun.faces.spi.FacesConfigResourceProvider,
> com.sun.faces.spi.ConfigurationResourceProvider {
> private static Logger logger =
> Logger.getLogger(OSGiFacesConfigResourceProvider.class.getPackage().get
> Name());
> - public Collection<URL> getResources(ServletContext context) {
> - Collection<URL> urls = (Collection<URL>)
> context.getAttribute(Constants.FACES_CONFIG_ATTR);
> - if (urls == null) return Collections.EMPTY_LIST;
> - logger.info("Faces Config urls excluding the ones named as
> faces-config.xml = " + urls); // TODO(Sahoo): change to finer
> - return urls;
> + public Collection<URI> getResources(ServletContext context) {
> + Collection<URI> uris = (Collection<URI>)
> context.getAttribute(Constants.FACES_CONFIG_ATTR);
> + if (uris == null) return Collections.EMPTY_LIST;
> + logger.info("Faces Config urls excluding the ones named as
> faces-config.xml = " + uris); // TODO(Sahoo): change to finer
> + return uris;
> }
> }
>
Can you please change the log message to read "uris" instead of "urls?"
> Index:
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FaceletConfigResourceProvider.java
> ===================================================================
> ---
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FaceletConfigResourceProvider.java (revision 43680)
> +++
> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGi
> FaceletConfigResourceProvider.java (working copy)
> @@ -60,10 +60,10 @@
> */
> public class OSGiFaceletConfigResourceProvider implements
> com.sun.faces.spi.FaceletConfigResourceProvider,
> com.sun.faces.spi.ConfigurationResourceProvider {
> private static Logger logger =
> Logger.getLogger(OSGiFaceletConfigResourceProvider.class.getPackage().g
> etName());
> - public Collection<URL> getResources(ServletContext context) {
> - Collection<URL> urls = (Collection<URL>)
> context.getAttribute(Constants.FACELET_CONFIG_ATTR);
> - if (urls == null) return Collections.EMPTY_LIST;
> - logger.info("Facelet Config urls = " + urls); // TODO(Sahoo):
> change to debug level statement
> - return urls;
> + public Collection<URI> getResources(ServletContext context) {
> + Collection<URI> uris = (Collection<URI>)
> context.getAttribute(Constants.FACELET_CONFIG_ATTR);
> + if (uris == null) return Collections.EMPTY_LIST;
> + logger.info("Facelet Config urls = " + uris); // TODO(Sahoo):
> change to debug level statement
> + return uris;
> }
> }
>
Can you please change the log message to read "uris" instead of "urls?"
Can you please also run the OSGi/Faces app containing some Faces
annotation and config files before checking in if you have not already
done so. Last time you had told that you were going to add that test
case to your check-in test suite.
Thanks,
Sahoo