I think this could be simlified by adding a new exception type:
UpdateModelException extends FacesException { }
Then, the JSF 1.2 ExceptionHandler can look for instances of this
exception type and handle appropriately (i.e. log, and queue a message).
The 2.0 exception handler would instead throw the exception if encountered.
We can simplify the testing process by setting the ExceptionHandler we
want to verify by setting it directly into the current FacesContext
instance,
so no need to use the services mechanism.
Thoughts?
Ed Burns wrote:
>>>>>> On Fri, 12 Dec 2008 08:13:24 -0800, Ed Burns <Ed.Burns_at_Sun.COM> said:
>>>>>>
>
>
>>>>>> On Fri, 12 Dec 2008 12:38:12 +0100, Martin Marinschek <mmarinschek_at_APACHE.ORG> said:
>>>>>>
> MM> I have (while preparing for Devoxx) read through the spec once
> MM> more. I think we are on a good track, however:
>
> MM> - exception handling states 3 exceptions for the exception handler
> MM> _not_ to be called:
>
> MM> - convert --> ok
> MM> - validate --> ok
>
> MM> - model update --> IMHO, not ok
>
> MM> why?
>
> EB> I assert that this it is consistent with current behavior.
>
> Therefore, I have come up with the following changebundle.
>
> https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=248
>
> Issue: 248-UIInput.updateModel
>
> - Modify the contract for UIInput.updateModel to not queue a message if
> the updateModel causes an exception to be thrown. Rather, use the
> ExceptionHandler. The exception is if the ExceptionHandlerFactory is
> the PreJsf2ExceptionHandlerFactory.
>
> All automated tests still run successfully.
>
> SECTION: Modified Files
> ----------------------------
> M jsf-api/src/javax/faces/component/UIInput.java
>
> - take the above action
>
> M jsf-api/build.xml
> M jsf-ri/build-tests.xml
>
> - Leverage the PreJsf2ExceptionHandlerFactory, because these tests
> assume the old behavior.
>
> SECTION: Diffs
> ----------------------------
> Index: jsf-api/src/javax/faces/component/UIInput.java
> ===================================================================
> --- jsf-api/src/javax/faces/component/UIInput.java (revision 6211)
> +++ jsf-api/src/javax/faces/component/UIInput.java (working copy)
> @@ -43,23 +43,31 @@
> import java.util.ArrayList;
> import java.util.Iterator;
> import java.util.List;
> +import java.util.Map;
> import java.util.logging.Level;
> import java.util.logging.Logger;
>
> import javax.el.ELException;
> import javax.el.ValueExpression;
> import javax.faces.FacesException;
> +import javax.faces.FactoryFinder;
> import javax.faces.application.Application;
> import javax.faces.application.FacesMessage;
> +import javax.faces.context.ExceptionHandler;
> +import javax.faces.context.ExceptionHandlerFactory;
> import javax.faces.context.FacesContext;
> import javax.faces.convert.Converter;
> import javax.faces.convert.ConverterException;
> import javax.faces.el.MethodBinding;
> +import javax.faces.event.ExceptionEvent;
> +import javax.faces.event.ExceptionEventContext;
> +import javax.faces.event.PhaseId;
> import javax.faces.event.ValueChangeEvent;
> import javax.faces.event.ValueChangeListener;
> import javax.faces.render.Renderer;
> import javax.faces.validator.Validator;
> import javax.faces.validator.ValidatorException;
> +import javax.faces.webapp.PreJsf2ExceptionHandlerFactory;
>
> /**
> * <p><span class="changed_modified_2_0"><strong>UIInput</strong></span>
> @@ -767,11 +775,14 @@
> }
> ValueExpression ve = getValueExpression("value");
> if (ve != null) {
> + Throwable caught = null;
> + FacesMessage message = null;
> try {
> ve.setValue(context.getELContext(), getLocalValue());
> setValue(null);
> setLocalValueSet(false);
> } catch (ELException e) {
> + caught = e;
> String messageStr = e.getMessage();
> Throwable result = e.getCause();
> while (null != result &&
> @@ -779,7 +790,6 @@
> messageStr = result.getMessage();
> result = result.getCause();
> }
> - FacesMessage message;
> if (null == messageStr) {
> message =
> MessageFactory.getMessage(context, UPDATE_MESSAGE_ID,
> @@ -790,28 +800,54 @@
> messageStr,
> messageStr);
> }
> - LOGGER.log(Level.SEVERE, message.getSummary(), result);
> - context.addMessage(getClientId(context), message);
> setValid(false);
> } catch (IllegalArgumentException e) {
> - FacesMessage message =
> + caught = e;
> + message =
> MessageFactory.getMessage(context, UPDATE_MESSAGE_ID,
> MessageFactory.getLabel(
> context, this));
> - LOGGER.log(Level.SEVERE, message.getSummary(), e);
> - context.addMessage(getClientId(context), message);
> setValid(false);
> } catch (Exception e) {
> - FacesMessage message =
> + caught = e;
> + message =
> MessageFactory.getMessage(context, UPDATE_MESSAGE_ID,
> MessageFactory.getLabel(
> context, this));
> - LOGGER.log(Level.SEVERE, message.getSummary(), e);
> - context.addMessage(getClientId(context), message);
> setValid(false);
> }
> + if (null != caught) {
> + assert(null != message);
> + if (isPreJsf2ExceptionHandler(context)) {
> + LOGGER.log(Level.SEVERE, message.getSummary(), caught);
> + context.addMessage(getClientId(context), message);
> + } else {
> + ExceptionHandler exHandler = context.getExceptionHandler();
> + ExceptionEventContext eventContext = new ExceptionEventContext(context, caught, this,
> + PhaseId.UPDATE_MODEL_VALUES);
> + exHandler.processEvent(new ExceptionEvent(eventContext));
> + }
> + }
> +
> }
> }
> +
> + private static final String PRE_JSF2_EXCEPTION_HANDLER_ATTR_NAME =
> + "javax.faces.component.PRE_JSF2_EXCEPTION_HANDLER";
> +
> + private boolean isPreJsf2ExceptionHandler(FacesContext context) {
> + boolean result = false;
> + Map<String, Object> appMap = context.getExternalContext().getApplicationMap();
> + if (appMap.containsKey(PRE_JSF2_EXCEPTION_HANDLER_ATTR_NAME)) {
> + result = (Boolean) appMap.get(PRE_JSF2_EXCEPTION_HANDLER_ATTR_NAME);
> + } else {
> + ExceptionHandlerFactory exHandlerFactory = (ExceptionHandlerFactory)
> + FactoryFinder.getFactory(FactoryFinder.EXCEPTION_HANDLER_FACTORY);
> + result = exHandlerFactory.getClass().isAssignableFrom(PreJsf2ExceptionHandlerFactory.class);
> + appMap.put(PRE_JSF2_EXCEPTION_HANDLER_ATTR_NAME, (Boolean)result);
> + }
> + return result;
> + }
>
> // ------------------------------------------------------ Validation Methods
>
> Index: jsf-api/build.xml
> ===================================================================
> --- jsf-api/build.xml (revision 6211)
> +++ jsf-api/build.xml (working copy)
> @@ -396,6 +396,12 @@
> </fileset>
> </copy>
>
> + <delete dir="${build.test.dir}/META-INF/services" />
> + <mkdir dir="${build.test.dir}/META-INF/services"/>
> + <echo
> + file="${build.test.dir}/META-INF/services/javax.faces.context.ExceptionHandlerFactory">javax.faces.webapp.PreJsf2ExceptionHandlerFactory</echo>
> +
> +
> </target>
>
>
> Index: jsf-ri/build-tests.xml
> ===================================================================
> --- jsf-ri/build-tests.xml (revision 6211)
> +++ jsf-ri/build-tests.xml (working copy)
> @@ -193,6 +193,12 @@
> <filter token="test.root.dir" value="${out.test.dir}"/>
> <copy file="${conf.test.dir}/web.xml" todir="${out.test.dir}"
> filtering="on"/>
> +
> + <delete dir="${out.test.dir}/classes/META-INF/services" />
> + <mkdir dir="${out.test.dir}/classes/META-INF/services"/>
> + <echo
> + file="${out.test.dir}/classes/META-INF/services/javax.faces.context.ExceptionHandlerFactory">javax.faces.webapp.PreJsf2ExceptionHandlerFactory</echo>
> +
> <jsf.war basedir="${basedir}/build"
> archive-name="test"
> webxml="${out.test.dir}/web.xml">
>
>
>
>
>
>
>