dev@javaserverfaces.java.net

Re: Ryan: Seeking Review: Add more generics to Application

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Thu, 18 Sep 2008 10:24:06 -0700

Ed Burns wrote:
> Simon Lessard from the EG suggested these changes:
>
>
>>>>>> On Wed, 10 Sep 2008 19:14:50 -0400, Simon Lessard said:
>>>>>>
>
> SL> Hi,
> SL> I think we should change the following in Application class:
> SL> ____________________________________________________________________________public void subscribeToEvent(Class<? extends SystemEvent> systemEventClass,
> SL> Class sourceClass,
> SL> SystemEventListener listener)
>
> SL> to
>
> SL> public void subscribeToEvent(Class<? extends SystemEvent> systemEventClass,
> SL> Class<?> sourceClass,
> SL> SystemEventListener listener)
> SL> ____________________________________________________________________________
> SL> public abstract void addConverter(Class targetClass,
> SL> String converterClass)
>
> SL> to
>
> SL> public abstract void addConverter(Class<?> targetClass,
> SL> String converterClass)
> SL> ____________________________________________________________________________
> SL> public abstract Converter createConverter(Class targetClass)
>
> SL> to
>
> SL> public abstract Converter createConverter(Class<?> targetClass)
> SL> ____________________________________________________________________________
> SL> public Object evaluateExpressionGet(FacesContext context,
> SL> String expression,
> SL> Class expectedType)
> SL> throws javax.el.ELException
>
> SL> to
>
> SL> public <T> T evaluateExpressionGet(FacesContext context,
> SL> String expression,
> SL> Class<? extends T> expectedType)
> SL> throws javax.el.ELException
> SL> ____________________________________________________________________________
> SL> public abstract Iterator<Class> getConverterTypes()
>
> SL> to
>
> SL> public abstract Iterator<Class<?>> getConverterTypes()
> SL> ____________________________________________________________________________public void subscribeToEvent(Class<? extends SystemEvent> systemEventClass,
> SL> Class sourceClass,
> SL> SystemEventListener listener)
>
> SL> to
>
> SL> public void subscribeToEvent(Class<? extends SystemEvent> systemEventClass,
> SL> Class<?> sourceClass,
> SL> SystemEventListener listener)
> SL> ____________________________________________________________________________
> SL> public void unsubscribeFromEvent(Class<? extends SystemEvent> systemEventClass,
> SL> Class sourceClass,
> SL> SystemEventListener listener)
>
> SL> to
>
> SL> public void unsubscribeFromEvent(Class<? extends SystemEvent> systemEventClass,
> SL> Class<?> sourceClass,
> SL> SystemEventListener listener)
> SL> ____________________________________________________________________________
>
> Ryan, first, I want to ask the backwards compatibility question. Isn't
> this change just as disruptive with respect to backwards compatibility
> as adding resetValue() to EditableValueHolder? If so, I think we either
> need to take both changes or neither change.
>
With the generics, I don't think so. Don't forget about erasure.

So I'm fine with the generic changes. I'd be less fine with the
interface change. EG call of course.

> Second, here's the change-bundle for the record. Depending on your
> answer to the backwards compatibility question, I'll check it in. All
> automated tests continue to run, and I've got a change-bundle ready for
> jsf-extensions-test-time as well, as updating the jar. I'll check that
> in atomically, of course.
>
Comments inline
> SECTION: Modified Files
> ----------------------------
> M common/ant/dependencies.xml
> M jsf-api/src/javax/faces/application/Application.java
> M jsf-api/src/overview.html
> M jsf-api/build.xml
> M jsf-ri/src/com/sun/faces/application/ApplicationImpl.java
> M jsf-ri/systest-per-webapp/replace-statemanager/src/java/com/sun/faces/systest/NewApplication.java
> M jsf-ri/systest-per-webapp/replace-application/src/java/com/sun/faces/systest/NewApplication.java
> M build.xml
> M jsf-demo/ezcomp00/src/main/webapp/index.xhtml
> M lib/jsf-extensions-test-time.jar
>
>
> SECTION: Diffs
> ----------------------------
> Index: common/ant/dependencies.xml
> ===================================================================
> --- common/ant/dependencies.xml (revision 5449)
> +++ common/ant/dependencies.xml (working copy)
> @@ -180,9 +180,12 @@
> <available file="${jsdoc.home}" property="ignored"/>
> </not>
> <then>
> + <echo message="edburns ${jsdoc.jar}" />
> +<!--
> <get src="http://jsdoc-toolkit.googlecode.com/files/jsdoc_toolkit-${jsdoc.version}.zip"
> dest="${dependency.base.dir}/downloads/jsdoc_toolkit-${jsdoc.version}.zip"
> usetimestamp="true"/>
> +-->
> <unzip
> src="${dependency.base.dir}/downloads/jsdoc_toolkit-${jsdoc.version}.zip"
> dest="${dependency.base.dir}"/>
>
What is the purpose of the changes above? I personally don't want to
see 'edburns'
dumped to the console every time I build. Is there a problem with the
downloading
of jsdoc.jar?

> Index: jsf-api/src/javax/faces/application/Application.java
> ===================================================================
> --- jsf-api/src/javax/faces/application/Application.java (revision 5449)
> +++ jsf-api/src/javax/faces/application/Application.java (working copy)
> @@ -1112,7 +1112,7 @@
> * @throws NullPointerException if <code>targetClass</code>
> * or <code>converterClass</code> is <code>null</code>
> */
> - public abstract void addConverter(Class targetClass,
> + public abstract void addConverter(Class<?> targetClass,
> String converterClass);
>
>
> @@ -1189,7 +1189,7 @@
> * @throws NullPointerException if <code>targetClass</code>
> * is <code>null</code>
> */
> - public abstract Converter createConverter(Class targetClass);
> + public abstract Converter createConverter(Class<?> targetClass);
>
>
> /**
> @@ -1204,7 +1204,7 @@
> * instances for which {_at_link Converter} classes have been explicitly
> * registered.</p>
> */
> - public abstract Iterator<Class> getConverterTypes();
> + public abstract Iterator<Class<?>> getConverterTypes();
>
> /**
> * <p>Return the {_at_link ExpressionFactory} instance for this
> @@ -1247,9 +1247,9 @@
> *
> */
>
> - public Object evaluateExpressionGet(FacesContext context,
> - String expression,
> - Class expectedType) throws ELException {
> + public <T> T evaluateExpressionGet(FacesContext context,
> + String expression,
> + Class<? extends T> expectedType) throws ELException {
>
> if (defaultApplication != null) {
> return defaultApplication.evaluateExpressionGet(context,
> @@ -1652,7 +1652,7 @@
> * @since 2.0
> */
> public void subscribeToEvent(Class<? extends SystemEvent> systemEventClass,
> - Class sourceClass,
> + Class<?> sourceClass,
> SystemEventListener listener) {
>
> if (defaultApplication != null) {
> @@ -1730,7 +1730,7 @@
> * @since 2.0
> */
> public void unsubscribeFromEvent(Class<? extends SystemEvent> systemEventClass,
> - Class sourceClass,
> + Class<?> sourceClass,
> SystemEventListener listener) {
>
> if (defaultApplication != null) {
> Index: jsf-api/src/overview.html
> ===================================================================
> --- jsf-api/src/overview.html (revision 5449)
> +++ jsf-api/src/overview.html (working copy)
> @@ -83,6 +83,16 @@
>
> </tr>
>
> +<tr class="changed_added_2_0">
> +
> +<td>The JSDoc for the JavaScript API (including Ajax functionality)</td>
> +
> +<td><a target="_" href="../jsdocs/index.html">generated HTML</a>
> +</td>
> +
> +</tr>
> +
> +
> <tr class="changed_modified_2_0">
>
> <td><a name="prose_document">The prose document</a></td>
> Index: jsf-api/build.xml
> ===================================================================
> --- jsf-api/build.xml (revision 5449)
> +++ jsf-api/build.xml (working copy)
> @@ -773,6 +773,7 @@
> </target>
>
> <target name="push-to-maven-prepare" depends="jars">
> + <echo message="${impl.version}" />
>
> <mkdir dir="${target.dir}/classes" />
> <unjar src="${build.lib.dir}/${name}.jar" overwrite="yes"
> @@ -796,7 +797,7 @@
>
> <target name="edburns">
>
> - <echo message="install:install-file -DgroupId=javax.faces -DartifactId=${artifact-id} -Dversion=${snapshot.version} -Dpackaging=jar -Dfile=${build.dir}/maven-repo/javax.faces/jars/${artifact-id}-${snapshot.version}.jar" />
> + <echo message="${impl.version}" />
>
> </target>
>
If we're dumping a message with a version, I'd rather it be more
complete (i.e. Preparing to push ${impl.version} to the maven repository
${repo})

Also, I'd like to see named targets such as edburns removed from the
build. If there is a specific use
for this target, and it could generally be useful to all of us, then
please add a generic target.
Otherwise, for private stuff, please have a private ant file outside of
the respository that does what
you need.

>
> Index: jsf-ri/src/com/sun/faces/application/ApplicationImpl.java
> ===================================================================
> --- jsf-ri/src/com/sun/faces/application/ApplicationImpl.java (revision 5449)
> +++ jsf-ri/src/com/sun/faces/application/ApplicationImpl.java (working copy)
> @@ -183,7 +183,7 @@
> //
> private Map<String,Object> componentMap = null;
> private Map<String,Object> converterIdMap = null;
> - private Map<Class,Object> converterTypeMap = null;
> + private Map<Class<?>,Object> converterTypeMap = null;
> private Map<String,Object> validatorMap = null;
> private volatile String messageBundle = null;
>
> @@ -203,7 +203,7 @@
> associate = new ApplicationAssociate(this);
> componentMap = new ConcurrentHashMap<String, Object>();
> converterIdMap = new ConcurrentHashMap<String, Object>();
> - converterTypeMap = new ConcurrentHashMap<Class, Object>();
> + converterTypeMap = new ConcurrentHashMap<Class<?>, Object>();
> validatorMap = new ConcurrentHashMap<String, Object>();
> elContextListeners = new CopyOnWriteArrayList<ELContextListener>();
> navigationHandler = new NavigationHandlerImpl(associate);
> @@ -297,7 +297,7 @@
> */
> @Override
> public void subscribeToEvent(Class<? extends SystemEvent> systemEventClass,
> - Class sourceClass,
> + Class<?> sourceClass,
> SystemEventListener listener) {
>
> Util.notNull("systemEventClass", systemEventClass);
> @@ -327,7 +327,7 @@
> */
> @Override
> public void unsubscribeFromEvent(Class<? extends SystemEvent> systemEventClass,
> - Class sourceClass,
> + Class<?> sourceClass,
> SystemEventListener listener) {
>
> Util.notNull("systemEventClass", systemEventClass);
> @@ -402,12 +402,12 @@
> * @see javax.faces.application.Application#evaluateExpressionGet(javax.faces.context.FacesContext, String, Class)
> */
> @Override
> - public Object evaluateExpressionGet(FacesContext context,
> - String expression, Class expectedType) throws ELException {
> + public <T> T evaluateExpressionGet(FacesContext context,
> + String expression, Class<? extends T> expectedType) throws ELException {
> ValueExpression ve =
> getExpressionFactory().createValueExpression(context.getELContext(),
> expression,expectedType);
> - return (ve.getValue(context.getELContext()));
> + return ((T)ve.getValue(context.getELContext()));
> }
>
>
> @@ -1080,7 +1080,7 @@
> /**
> * @see javax.faces.application.Application#addConverter(Class, String)
> */
> - public void addConverter(Class targetClass, String converterClass) {
> + public void addConverter(Class<?> targetClass, String converterClass) {
>
> Util.notNull("targetClass", targetClass);
> Util.notNull("converterClass", converterClass);
> @@ -1181,7 +1181,7 @@
> /**
> * @see javax.faces.application.Application#createConverter(Class)
> */
> - public Converter createConverter(Class targetClass) {
> + public Converter createConverter(Class<?> targetClass) {
>
> Util.notNull("targetClass", targetClass);
> Converter returnVal = (Converter) newConverter(targetClass,
> @@ -1300,7 +1300,7 @@
> /**
> * @see javax.faces.application.Application#getConverterTypes()
> */
> - public Iterator<Class> getConverterTypes() {
> + public Iterator<Class<?>> getConverterTypes() {
>
> return converterTypeMap.keySet().iterator();
>
> @@ -1534,7 +1534,7 @@
> * @param targetClass the target class for the single argument ctor
> * @return The new object instance.
> */
> - protected Object newConverter(Class key, Map<Class,Object> map, Class targetClass) {
> + protected Object newConverter(Class key, Map<Class<?>,Object> map, Class targetClass) {
> assert (key != null && map != null);
>
> Object result = null;
> Index: jsf-ri/systest-per-webapp/replace-statemanager/src/java/com/sun/faces/systest/NewApplication.java
> ===================================================================
> --- jsf-ri/systest-per-webapp/replace-statemanager/src/java/com/sun/faces/systest/NewApplication.java (revision 5449)
> +++ jsf-ri/systest-per-webapp/replace-statemanager/src/java/com/sun/faces/systest/NewApplication.java (working copy)
> @@ -135,7 +135,7 @@
>
> @Override
> public void subscribeToEvent(Class<? extends SystemEvent> systemEventClass,
> - Class sourceClass,
> + Class<?> sourceClass,
> SystemEventListener listener) {
> oldApp.subscribeToEvent(systemEventClass, sourceClass, listener);
> }
> @@ -148,7 +148,7 @@
>
> @Override
> public void unsubscribeFromEvent(Class<? extends SystemEvent> systemEventClass,
> - Class sourceClass,
> + Class<?> sourceClass,
> SystemEventListener listener) {
> oldApp.unsubscribeFromEvent(systemEventClass,
> sourceClass,
> @@ -304,9 +304,9 @@
> return oldApp.getELContextListeners();
> }
>
> - public Object evaluateExpressionGet(FacesContext context,
> + public <T> T evaluateExpressionGet(FacesContext context,
> String expression,
> - Class expectedType) throws ELException {
> + Class<? extends T> expectedType) throws ELException {
> return oldApp.evaluateExpressionGet(context, expression, expectedType);
> }
>
> @@ -450,7 +450,7 @@
>
>
>
> - public void addConverter(Class targetClass,
> + public void addConverter(Class<?> targetClass,
>
> String converterClass) {
>
> @@ -472,7 +472,7 @@
>
>
>
> - public Converter createConverter(Class targetClass) {
> + public Converter createConverter(Class<?> targetClass) {
>
> return oldApp.createConverter(targetClass);
>
> @@ -492,9 +492,9 @@
>
>
>
> - public Iterator getConverterTypes() {
> + public Iterator<Class<?>> getConverterTypes() {
>
> - return oldApp.getConverterTypes();
> + return (Iterator<Class<?>>) oldApp.getConverterTypes();
>
> }
>
> Index: jsf-ri/systest-per-webapp/replace-application/src/java/com/sun/faces/systest/NewApplication.java
> ===================================================================
> --- jsf-ri/systest-per-webapp/replace-application/src/java/com/sun/faces/systest/NewApplication.java (revision 5449)
> +++ jsf-ri/systest-per-webapp/replace-application/src/java/com/sun/faces/systest/NewApplication.java (working copy)
> @@ -97,7 +97,7 @@
>
> @Override
> public void subscribeToEvent(Class<? extends SystemEvent> systemEventClass,
> - Class sourceClass,
> + Class<?> sourceClass,
> SystemEventListener listener) {
> oldApp.subscribeToEvent(systemEventClass, sourceClass, listener);
> }
> @@ -110,7 +110,7 @@
>
> @Override
> public void unsubscribeFromEvent(Class<? extends SystemEvent> systemEventClass,
> - Class sourceClass,
> + Class<?> sourceClass,
> SystemEventListener listener) {
> oldApp.unsubscribeFromEvent(systemEventClass,
> sourceClass,
> @@ -254,7 +254,7 @@
> }
>
>
> - public void addConverter(Class targetClass,
> + public void addConverter(Class<?> targetClass,
> String converterClass) {
> oldApp.addConverter(targetClass, converterClass);
> }
> @@ -265,7 +265,7 @@
> }
>
>
> - public Converter createConverter(Class targetClass) {
> + public Converter createConverter(Class<?> targetClass) {
> return oldApp.createConverter(targetClass);
> }
>
> @@ -275,7 +275,7 @@
> }
>
>
> - public Iterator getConverterTypes() {
> + public Iterator<Class<?>> getConverterTypes() {
> return oldApp.getConverterTypes();
> }
>
> @@ -331,8 +331,8 @@
> oldApp.removeELContextListener(listener);
> }
>
> - public Object evaluateExpressionGet(FacesContext context,
> - String expression, Class expectedType) throws ELException {
> + public <T> T evaluateExpressionGet(FacesContext context,
> + String expression, Class<? extends T> expectedType) throws ELException {
> return oldApp.evaluateExpressionGet(context, expression, expectedType);
> }
>
> Index: build.xml
> ===================================================================
> --- build.xml (revision 5449)
> +++ build.xml (working copy)
> @@ -48,7 +48,7 @@
> <import file="${jsf.build.home}/common/ant/common.xml"/>
> <property name="tasks.home" value="${myenv.TASKS_HOME}"/>
> <property name="dist.dir" value="${basedir}/dist"/>
> - <property name="javaserverfaces-spec-eg.dir" value="${basedir}/../../javaserverfaces-spec-eg"/>
> + <property name="javaserverfaces-spec-eg.dir" value="${basedir}/../../workareas/javaserverfaces-spec-eg"/>
>
>
> <property name="LICENSEFILE"
> Index: jsf-demo/ezcomp00/src/main/webapp/index.xhtml
> ===================================================================
> --- jsf-demo/ezcomp00/src/main/webapp/index.xhtml (revision 5449)
> +++ jsf-demo/ezcomp00/src/main/webapp/index.xhtml (working copy)
> @@ -40,7 +40,8 @@
> xmlns:h="http://java.sun.com/jsf/html"
> xmlns:f="http://java.sun.com/jsf/core"
> xmlns:ui="http://java.sun.com/jsf/facelets"
> - xmlns:ez="http://java.sun.com/jsf/composite/ezcomp">
> + xmlns:ez="http://java.sun.com/jsf/composite/ezcomp"
> + xmlns:ja="http://java.sun.com/jsf/composite/nein">
> <h:head>
> <title>The Simplest EZComp Demo That Could Possibly Work</title>
> <style type="text/css">
> @@ -60,7 +61,7 @@
>
> <div id="compositeComponent" class="grayBox" style="border: 1px solid #090;">
>
> - <ez:loginPanel id="loginPanelInConsumingPage" model="#{bean}">
> + <ez:loginPanel id="loginPanelInConsumingPage" model="#{bean}">
> <f:valueChangeListener for="username"
> binding="#{bean.useridValueChangeListener}" />
> <f:actionListener for="loginEvent"
> @@ -83,7 +84,7 @@
> <h:outputText id="childInConsumingPage"
> value="this is a child component in the consuming page" />
>
> - </ez:loginPanel>
> + </ez:loginPanel>
>
> </div>
>
> Index: lib/jsf-extensions-test-time.jar
> ===================================================================
> Cannot display: file marked as a binary type.
> svn:mime-type = application/octet-stream
>
>
>
>
>
>