>>>>> On Sat, 11 Dec 2010 09:59:44 +0530, Sahoo <sanjeeb.sahoo_at_oracle.com> said:
S> Ed,
S> See some comments below. I am reviewing changes to files that I am
S> associated with.
[...]
>> SECTION: Diffs
>> Index:
>> web/weld-integration/src/main/java/org/glassfish/weld/jsf/WeldFacesConfigProvider.java
[...]
>> @@ -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;
>> }
>>
>>
S> Can you change this to use URL.toURI() and adjust it to avoid a null
S> check so that the code becomes:
S> URI resource = loader.getResource(SERVICES_FACES_CONFIG);
S> if (resource != null) {
S> try {
S> list.add(resource.toURI());
S> } catch(URISyntaxException) {
S> String message = "Unable to create URI for URL " + resource.toExternalForm();
S> _logger.log(Level.SEVERE, message, ex);
S> }
S> }
Ok, will do.
>> Index:
>> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGiWebModuleDecorator.java
[...]
>> - }
>> - } 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);
S> Can you change the local variable name to "uri" instead of "url?"
Yes, I missed that one. Thanks.
>> Index:
>> osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGiFacesConfigResourceProvider.java
[...]
>> + 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;
>> }
>> }
S> Can you please change the log message to read "uris" instead of "urls?"
Yes.
>> Index:
osgi-javaee/osgi-web-container/src/main/java/org/glassfish/osgiweb/OSGiFaceletConfigResourceProvider.java
[...]
>> 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;
>> }
>> }
>>
S> Can you please change the log message to read "uris" instead of "urls?"
Done.
When the build and QL completes I'll commit these.
S> Can you please also run the OSGi/Faces app containing some Faces
S> annotation and config files before checking in if you have not already
S> done so. Last time you had told that you were going to add that test
S> case to your check-in test suite.
I'll do it manually. Unfortunately, this issue is still not done.
http://java.net/jira/browse/JAVASERVERFACES-1800
The build with your changes is currently proceeding.
http://adc2110030:7070/hudson/job/glassfish-1HEAD/144/
I have to travel to a conference this evening, but I'll get it committed
as soon as I can.
Ed
--
| ed.burns_at_sun.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/