dev@javaserverfaces.java.net

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

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Fri, 31 Oct 2008 08:43:16 -0700

Per our discussion on the phone, if this entity is responsible for
finding configuration resources,
it can't be defined via a factory element in a configuration resource.
It needs to be exposed
via a META-INF/services entry.



Ed Burns wrote:
>>>>>> 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())) {
>
>
>
>