dev@javaserverfaces.java.net

Part 2: Re: Retroactive review: rename FacesAnnotationHandler* to DiscoveryHandler*

From: Ed Burns <Ed.Burns_at_Sun.COM>
Date: Fri, 31 Oct 2008 11:15:56 -0400

>>>>> On Fri, 31 Oct 2008 10:05:14 -0400, Ed Burns <ed.burns_at_sun.com> said:

>>>>> On Fri, 31 Oct 2008 09:59:36 -0400, Ed Burns <Ed.Burns_at_Sun.COM> said:
EB> Issue: [121-JarOrdering]
EB> I discussed this with Pete Muir from RedHat and we agreed this would be ok.

EB> Considering that [121-JarOrdering] needs a pluggable classpath scanner,
EB> I think this would make sense to add-on to the feature set of the
EB> existing annotation-handler. However, doing so requires a rename, so I
EB> chose discovery-handler.

EB> Oops, this isn't done yet.

EB> I need to also change the class from being on the application instance
EB> to being a FactoryFinder type thing. Working on that now.

EB> The change bundle to which this message is a reply is just a name-change
EB> anyway, so don't bother to review it. Just review the next one.

Here it is.

Issue: 121-JarOrdering

In preparation for 121-JarOrdering, we need to make DiscoveryHandler be
vended from DiscoveryHandlerFactory, instead of from the Application.
It should have been this way from the beginning.


SECTION: Modified Files
----------------------------
M jsf-api/doc/web-facesconfig_2_0.xsd

- remove the discovery-handler element, in favor of adding a
  discovery-handler-factory to the <factory> element.

M jsf-api/src/javax/faces/FactoryFinder.java

- add a new constant DISCOVERY_HANDLER_FACTORY

M jsf-api/src/javax/faces/application/Application.java
M jsf-api/src/javax/faces/application/ApplicationWrapper.java
M jsf-ri/src/com/sun/faces/application/ApplicationImpl.java

- remove the annotationHandler property

M jsf-ri/src/com/sun/faces/jsf-ri-config.xml

- remove discovery-handler

- add discovery-handler-factory

M jsf-ri/src/com/sun/faces/config/processor/ApplicationConfigProcessor.java

- Use FactoryFinder to get DiscoveryHandler, not Application.

- remove setAnnotationHandler method.

M jsf-ri/src/com/sun/faces/config/processor/FactoryConfigProcessor.java

- process discovery-handler-factory

A jsf-api/src/javax/faces/application/DiscoveryHandlerFactory.java
A jsf-ri/src/com/sun/faces/application/DiscoveryHandlerFactoryImpl.java

- new files


SECTION: Diffs
----------------------------
Index: jsf-api/doc/web-facesconfig_2_0.xsd
===================================================================
--- jsf-api/doc/web-facesconfig_2_0.xsd (revision 5650)
+++ jsf-api/doc/web-facesconfig_2_0.xsd (working copy)
@@ -327,20 +327,6 @@
                     </xsd:documentation>
                 </xsd:annotation>
             </xsd:element>
- <xsd:element name="discovery-handler"
- type="javaee:fully-qualified-classType">
- <xsd:annotation>
- <xsd:documentation>
-
- The "annotation-handler" element contains the fully
- qualified class name of the concrete
- FacesAnnotationHandler implementation that will
- be used to discover and scan for the annotations
- supported by JavaServer Faces.
-
- </xsd:documentation>
- </xsd:annotation>
- </xsd:element>
             <xsd:element name="default-render-kit-id"
                          type="javaee:string">
                 <xsd:annotation>
@@ -593,6 +579,21 @@
                     </xsd:documentation>
                 </xsd:annotation>
             </xsd:element>
+ <xsd:element name="discovery-handler-factory"
+ type="javaee:fully-qualified-classType">
+ <xsd:annotation>
+ <xsd:documentation>
+
+ The "discovery-handler-factory" element contains the
+ fully qualified class name of the concrete
+ DiscoveryHandler implementation class that will
+ be called when
+ FactoryFinder.getFactory(DISOVERY_HANDLER_FACTORY) is
+ called.
+
+ </xsd:documentation>
+ </xsd:annotation>
+ </xsd:element>
             <xsd:element name="faces-context-factory"
                          type="javaee:fully-qualified-classType">
                 <xsd:annotation>
Index: jsf-api/src/javax/faces/FactoryFinder.java
===================================================================
--- jsf-api/src/javax/faces/FactoryFinder.java (revision 5651)
+++ jsf-api/src/javax/faces/FactoryFinder.java (working copy)
@@ -157,6 +157,12 @@
     public final static String APPLICATION_FACTORY =
          "javax.faces.application.ApplicationFactory";
 
+ /**
+ * <p>The property name for the {_at_link
+ * javax.faces.application.DiscoveryHandlerFactory} class name.</p>
+ */
+ public final static String DISCOVERY_HANDLER_FACTORY =
+ "javax.faces.application.DiscoveryHandlerFactory";
 
     /**
      * <p>The property name for the
@@ -193,6 +199,7 @@
      */
     private static final String[] FACTORY_NAMES = {
          APPLICATION_FACTORY,
+ DISCOVERY_HANDLER_FACTORY,
          FACES_CONTEXT_FACTORY,
          LIFECYCLE_FACTORY,
          RENDER_KIT_FACTORY
Index: jsf-api/src/javax/faces/application/Application.java
===================================================================
--- jsf-api/src/javax/faces/application/Application.java (revision 5650)
+++ jsf-api/src/javax/faces/application/Application.java (working copy)
@@ -165,40 +165,6 @@
     public abstract void setActionListener(ActionListener listener);
 
     /**
- * <p class="changed_added_2_0">Return the default
- * <code>FacesAnnotation</code> handler used to scan for and process
- * startup time annotations.</p>
- *
- * @since 2.0
- */
-
- public DiscoveryHandler getFacesAnnotationHandler() {
-
- if (defaultApplication != null) {
- return defaultApplication.getFacesAnnotationHandler();
- }
- throw new UnsupportedOperationException();
-
- }
-
- /**
- * <p class="changed_added_2_0">Set the default
- * <code>FacesAnnotation</code> handler used to scan for and process
- * startup time annotations.</p>
- *
- * @since 2.0
- */
- public void setFacesAnnotationHandler(DiscoveryHandler newHandler) {
- if (defaultApplication != null) {
- defaultApplication.setFacesAnnotationHandler(newHandler);
- } else {
- throw new UnsupportedOperationException();
- }
-
- }
-
-
- /**
      * <p>Return the default <code>Locale</code> for this application. If
      * not explicitly set, <code>null</code> is returned.</p>
      */
Index: jsf-api/src/javax/faces/application/ApplicationWrapper.java
===================================================================
--- jsf-api/src/javax/faces/application/ApplicationWrapper.java (revision 5650)
+++ jsf-api/src/javax/faces/application/ApplicationWrapper.java (working copy)
@@ -164,11 +164,6 @@
     }
 
     @Override
- public DiscoveryHandler getFacesAnnotationHandler() {
- return getWrapped().getFacesAnnotationHandler();
- }
-
- @Override
     public PageDeclarationLanguage getPageDeclarationLanguage() {
         return getWrapped().getPageDeclarationLanguage();
     }
@@ -204,11 +199,6 @@
     }
 
     @Override
- public void setFacesAnnotationHandler(DiscoveryHandler newHandler) {
- getWrapped().setFacesAnnotationHandler(newHandler);
- }
-
- @Override
     public void setPageDeclarationLanguage(PageDeclarationLanguage pdl) {
         getWrapped().setPageDeclarationLanguage(pdl);
     }
Index: jsf-api/src/javax/faces/application/DiscoveryHandlerFactory.java
===================================================================
--- jsf-api/src/javax/faces/application/DiscoveryHandlerFactory.java (revision 0)
+++ jsf-api/src/javax/faces/application/DiscoveryHandlerFactory.java (revision 0)
@@ -0,0 +1,96 @@
+/*
+ * $Id: DiscoveryHandlerFactory.java,v 1.19 2007/04/27 22:00:06 ofung Exp $
+ */
+
+/*
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
+ *
+ * Copyright 1997-2007 Sun Microsystems, Inc. All rights reserved.
+ *
+ * The contents of this file are subject to the terms of either the GNU
+ * General Public License Version 2 only ("GPL") or the Common Development
+ * and Distribution License("CDDL") (collectively, the "License"). You
+ * may not use this file except in compliance with the License. You can obtain
+ * a copy of the License at https://glassfish.dev.java.net/public/CDDL+GPL.html
+ * or glassfish/bootstrap/legal/LICENSE.txt. See the License for the specific
+ * language governing permissions and limitations under the License.
+ *
+ * When distributing the software, include this License Header Notice in each
+ * file and include the License file at glassfish/bootstrap/legal/LICENSE.txt.
+ * Sun designates this particular file as subject to the "Classpath" exception
+ * as provided by Sun in the GPL Version 2 section of the License file that
+ * accompanied this code. If applicable, add the following below the License
+ * Header, with the fields enclosed by brackets [] replaced by your own
+ * identifying information: "Portions Copyrighted [year]
+ * [name of copyright owner]"
+ *
+ * Contributor(s):
+ *
+ * If you wish your version of this file to be governed by only the CDDL or
+ * only the GPL Version 2, indicate your decision by adding "[Contributor]
+ * elects to include this software in this distribution under the [CDDL or GPL
+ * Version 2] license." If you don't indicate a single choice of license, a
+ * recipient has the option to distribute your version of this file under
+ * either the CDDL, the GPL Version 2 or to extend the choice of license to
+ * its licensees as provided above. However, if you add GPL Version 2 code
+ * and therefore, elected the GPL Version 2 license, then the option applies
+ * only if the new code is made subject to such option by the copyright
+ * holder.
+ */
+
+package javax.faces.application;
+
+import javax.faces.context.*;
+import javax.faces.FacesException;
+import javax.faces.FacesWrapper;
+import javax.faces.lifecycle.Lifecycle;
+
+
+/**
+ * <p><strong class="changed_added_2_0">DiscoveryHandlerFactory</strong>
+ * is a factory object that creates (if needed) and returns a {_at_link
+ * DiscoveryHandler} instance.</p>
+
+ * <div class="changed_added_2_0">
+ *
+ * <p>There must be one <code>DiscoveryHandlerFactory</code> instance per web
+ * application that is utilizing JavaServer Faces. This instance can be
+ * acquired, in a portable manner, by calling:</p>
+ *
+ * <pre><code>
+ * DiscoveryHandlerFactory factory = (DiscoveryHandlerFactory)
+ * FactoryFinder.getFactory(FactoryFinder.DISCOVERY_HANDLER_FACTORY);
+ * </code></pre>
+ *
+ * </div>
+ */
+
+public abstract class DiscoveryHandlerFactory implements FacesWrapper<DiscoveryHandlerFactory> {
+
+ /**
+ * <p class="changed_added_2_0">If this factory has been decorated, the
+ * implementation doing the decorating may override this method to provide
+ * access to the implementation being wrapped. A default implementation
+ * is provided that returns <code>null</code>.</p>
+ *
+ * @since 2.0
+ */
+
+ public DiscoveryHandlerFactory getWrapped() {
+ return null;
+ }
+
+ /**
+ * <p>Create (if needed) and return a {_at_link DiscoveryHandler}
+ * instance.</p>
+
+ * @throws FacesException if a {_at_link DiscoveryHandler} cannot be
+ * constructed for the specified parameters
+ * @throws NullPointerException if any of the parameters
+ * are <code>null</code>
+ */
+ public abstract DiscoveryHandler getDiscoveryHandler()
+ throws FacesException;
+
+
+}
Index: jsf-ri/src/com/sun/faces/jsf-ri-config.xml
===================================================================
--- jsf-ri/src/com/sun/faces/jsf-ri-config.xml (revision 5650)
+++ jsf-ri/src/com/sun/faces/jsf-ri-config.xml (working copy)
@@ -50,6 +50,9 @@
     <application-factory>
       com.sun.faces.application.ApplicationFactoryImpl
     </application-factory>
+ <discovery-handler-factory>
+ com.sun.faces.application.DiscoveryHandlerFactoryImpl
+ </discovery-handler-factory>
     <faces-context-factory>
       com.sun.faces.context.FacesContextFactoryImpl
     </faces-context-factory>
@@ -80,9 +83,6 @@
     <resource-handler>
       com.sun.faces.application.resource.ResourceHandlerImpl
     </resource-handler>
- <discovery-handler>
- com.sun.faces.application.annotation.AnnotationHandler
- </discovery-handler>
     <system-event-listener>
         <system-event-listener-class>
             com.sun.faces.config.listeners.ProjectStagePhaseListenerInstallationListener
Index: jsf-ri/src/com/sun/faces/application/DiscoveryHandlerFactoryImpl.java
===================================================================
--- jsf-ri/src/com/sun/faces/application/DiscoveryHandlerFactoryImpl.java (revision 0)
+++ jsf-ri/src/com/sun/faces/application/DiscoveryHandlerFactoryImpl.java (revision 0)
@@ -0,0 +1,63 @@
+/*
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
+ *
+ * Copyright 1997-2007 Sun Microsystems, Inc. All rights reserved.
+ *
+ * The contents of this file are subject to the terms of either the GNU
+ * General Public License Version 2 only ("GPL") or the Common Development
+ * and Distribution License("CDDL") (collectively, the "License"). You
+ * may not use this file except in compliance with the License. You can obtain
+ * a copy of the License at https://glassfish.dev.java.net/public/CDDL+GPL.html
+ * or glassfish/bootstrap/legal/LICENSE.txt. See the License for the specific
+ * language governing permissions and limitations under the License.
+ *
+ * When distributing the software, include this License Header Notice in each
+ * file and include the License file at glassfish/bootstrap/legal/LICENSE.txt.
+ * Sun designates this particular file as subject to the "Classpath" exception
+ * as provided by Sun in the GPL Version 2 section of the License file that
+ * accompanied this code. If applicable, add the following below the License
+ * Header, with the fields enclosed by brackets [] replaced by your own
+ * identifying information: "Portions Copyrighted [year]
+ * [name of copyright owner]"
+ *
+ * Contributor(s):
+ *
+ * If you wish your version of this file to be governed by only the CDDL or
+ * only the GPL Version 2, indicate your decision by adding "[Contributor]
+ * elects to include this software in this distribution under the [CDDL or GPL
+ * Version 2] license." If you don't indicate a single choice of license, a
+ * recipient has the option to distribute your version of this file under
+ * either the CDDL, the GPL Version 2 or to extend the choice of license to
+ * its licensees as provided above. However, if you add GPL Version 2 code
+ * and therefore, elected the GPL Version 2 license, then the option applies
+ * only if the new code is made subject to such option by the copyright
+ * holder.
+ */
+
+package com.sun.faces.application;
+
+import com.sun.faces.application.annotation.AnnotationHandler;
+import javax.faces.FacesException;
+import javax.faces.application.DiscoveryHandler;
+import javax.faces.application.DiscoveryHandlerFactory;
+
+/**
+ *
+ * @author edburns
+ */
+public class DiscoveryHandlerFactoryImpl extends DiscoveryHandlerFactory {
+
+ private DiscoveryHandler discoveryHandler = null;
+
+ @Override
+ public DiscoveryHandler getDiscoveryHandler() throws FacesException {
+
+ if (null == discoveryHandler) {
+ discoveryHandler = new AnnotationHandler();
+ }
+ return discoveryHandler;
+ }
+
+
+
+}
Index: jsf-ri/src/com/sun/faces/application/ApplicationImpl.java
===================================================================
--- jsf-ri/src/com/sun/faces/application/ApplicationImpl.java (revision 5650)
+++ jsf-ri/src/com/sun/faces/application/ApplicationImpl.java (working copy)
@@ -540,29 +540,6 @@
 
 
     /**
- * @see javax.faces.application.Application#getFacesAnnotationHandler()
- * @return
- */
- @Override
- public DiscoveryHandler getFacesAnnotationHandler() {
-
- return annotationHandler;
-
- }
-
-
- /**
- * @see javax.faces.application.Application#setFacesAnnotationHandler(javax.faces.application.DiscoveryHandler)
- */
- @Override
- public synchronized void setFacesAnnotationHandler(DiscoveryHandler newHandler) {
-
- annotationHandler = newHandler;
-
- }
-
-
- /**
      * @see javax.faces.application.Application#getViewHandler()
      */
     public ViewHandler getViewHandler() {
Index: jsf-ri/src/com/sun/faces/config/processor/ApplicationConfigProcessor.java
===================================================================
--- jsf-ri/src/com/sun/faces/config/processor/ApplicationConfigProcessor.java (revision 5650)
+++ jsf-ri/src/com/sun/faces/config/processor/ApplicationConfigProcessor.java (working copy)
@@ -74,6 +74,8 @@
 import com.sun.faces.config.ConfigurationException;
 import com.sun.faces.config.WebConfiguration;
 import com.sun.faces.config.WebConfiguration.BooleanWebContextInitParameter;
+import javax.faces.FactoryFinder;
+import javax.faces.application.DiscoveryHandlerFactory;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
@@ -295,9 +297,7 @@
                                 setResourceHandler(app, n);
                             } else if (SYSTEM_EVENT_LISTENER.equals(n.getLocalName())) {
                                 addSystemEventListener(app, n);
- } else if (DISCOVERY_HANDLER.equals(n.getLocalName())) {
- setAnnotationHandler(app, n);
- }
+ }
                         }
                     }
                 }
@@ -308,7 +308,8 @@
 
         // all application level artifacts are in place, process any annotated
         // components that have been found
- DiscoveryHandler handler = getApplication().getFacesAnnotationHandler();
+ DiscoveryHandler handler = ((DiscoveryHandlerFactory)
+ FactoryFinder.getFactory(FactoryFinder.DISCOVERY_HANDLER_FACTORY)).getDiscoveryHandler();
         FacesContext ctx = FacesContext.getCurrentInstance();
         handler.processAnnotatedClasses(ctx, handler.getClassNamesWithFacesAnnotations(ctx));
 
@@ -766,27 +767,4 @@
         }
     }
 
-
- private void setAnnotationHandler(Application application, Node annotationHandler) {
-
- if (annotationHandler != null) {
- String handler = getNodeText(annotationHandler);
- if (handler != null) {
- Object instance = createInstance(handler,
- DiscoveryHandler.class,
- application.getResourceHandler(),
- annotationHandler);
- if (instance != null) {
- if (LOGGER.isLoggable(Level.FINE)) {
- LOGGER.log(Level.FINE,
- MessageFormat.format(
- "Calling Application.setAnnotationHandler({0})",
- handler));
- }
- application.setFacesAnnotationHandler((DiscoveryHandler) instance);
- }
- }
- }
- }
-
 }
Index: jsf-ri/src/com/sun/faces/config/processor/FactoryConfigProcessor.java
===================================================================
--- jsf-ri/src/com/sun/faces/config/processor/FactoryConfigProcessor.java (revision 5649)
+++ jsf-ri/src/com/sun/faces/config/processor/FactoryConfigProcessor.java (working copy)
@@ -76,6 +76,11 @@
     private static final String APPLICATION_FACTORY = "application-factory";
 
     /**
+ * <code>/faces-config/factory/discovery-handler-factory</code>
+ */
+ private static final String DISCOVERY_HANDLER_FACTORY = "discovery-handler-factory";
+
+ /**
      * <code>/faces-config/factory/faces-context-factory</code>
      */
     private static final String FACES_CONTEXT_FACTORY = "faces-context-factory";
@@ -96,6 +101,7 @@
      */
     private static final String[] FACTORY_NAMES = {
           FactoryFinder.APPLICATION_FACTORY,
+ FactoryFinder.DISCOVERY_HANDLER_FACTORY,
           FactoryFinder.FACES_CONTEXT_FACTORY,
           FactoryFinder.LIFECYCLE_FACTORY,
           FactoryFinder.RENDER_KIT_FACTORY
@@ -148,6 +154,9 @@
         // chain.
         wrapFactories(applicationFactoryCount.get(),
                       facesContextFactoryCount.get());
+
+ // PENDING(rlubke): Not sure if DiscoveryHandlerFactory should get the
+ // wrapFactories treatment. Currently it does not.
 
         // validate that we actually have factories at this point.
         verifyFactoriesExist();
@@ -175,7 +184,10 @@
                     appCount.incrementAndGet();
                     setFactory(FactoryFinder.APPLICATION_FACTORY,
                                getNodeText(n));
- } else if (LIFECYCLE_FACTORY.equals(n.getLocalName())) {
+ } else if (DISCOVERY_HANDLER_FACTORY.equals(n.getLocalName())) {
+ setFactory(FactoryFinder.DISCOVERY_HANDLER_FACTORY,
+ getNodeText(n));
+ }else if (LIFECYCLE_FACTORY.equals(n.getLocalName())) {
                     setFactory(FactoryFinder.LIFECYCLE_FACTORY,
                                getNodeText(n));
                 } else if (FACES_CONTEXT_FACTORY.equals(n.getLocalName())) {



--