javaserverfaces
  1. javaserverfaces
  2. JAVASERVERFACES-2537

ApplicationImpl.newConverter confusing behaviour

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: 2.1.8, 2.1.14
    • Fix Version/s: None
    • Component/s: conversion
    • Labels:
      None

      Description

      Hi guys,

      I'm unclear how com.sun.faces.application.ApplicationImpl.newConverter is supposed to work. It appears to try two ways of constructing a Converter: a parameterized Constructor and a parameter-less Constructor. I couldn't find where this behavour is covered in the JavaDoc or JSF spec?

      I'm assuming the idea behind the parameterized Constructor is to pass the targetClass of the Object to be converted? This would be enormously useful for writing generic Converters.

      However, this behaviour appears to be scuppered by ApplicationImpl.createConverterBasedOnClass. This method calls itself recursively, but manages to lose the original targetClass because the parameter names get switched around...

      protected Converter createConverterBasedOnClass(Class<?> targetClass, Class<?> baseClass) {
      ...
      returnVal = createConverterBasedOnClass(superclass, targetClass);
      }

      Note the targetClass goes from being the first parameter to the second parameter, and subsequently is mangled by the time it ends up on newConverter's parameterized Constructor. Is this intentional?

        Activity

        Hide
        Ed Burns added a comment -

        This parameter transposition does not seem intentional. Thanks for your detail analysis, which will make this a little easier to fix.

        Show
        Ed Burns added a comment - This parameter transposition does not seem intentional. Thanks for your detail analysis, which will make this a little easier to fix.
        Hide
        Manfred Riem added a comment -

        Can you please set the affected version?

        Show
        Manfred Riem added a comment - Can you please set the affected version?
        Hide
        kennardconsulting added a comment -

        Sorry - the code I referred to is there in 2.1.8 as well as 2.1.14:

        returnVal = createConverterBasedOnClass(superclass, targetClass);

        Show
        kennardconsulting added a comment - Sorry - the code I referred to is there in 2.1.8 as well as 2.1.14: returnVal = createConverterBasedOnClass(superclass, targetClass);
        Hide
        Manfred Riem added a comment -

        From the Application.createConverter API docs, see http://docs.oracle.com/javaee/6/api/javax/faces/application/Application.html#createConverter(java.lang.Class)

        The method createConverter is supposed to recursively try to find a converter that applies to this type. The utlity method referred to uses the name targetClass and baseClass, more accurate would have been to call it candidateClass and targetClass.

        Closing this as "Invalid" as it is not incorrect, just a bit of naming galore.

        Show
        Manfred Riem added a comment - From the Application.createConverter API docs, see http://docs.oracle.com/javaee/6/api/javax/faces/application/Application.html#createConverter(java.lang.Class ) The method createConverter is supposed to recursively try to find a converter that applies to this type. The utlity method referred to uses the name targetClass and baseClass, more accurate would have been to call it candidateClass and targetClass. Closing this as "Invalid" as it is not incorrect, just a bit of naming galore.
        Hide
        kennardconsulting added a comment -

        Manfred,

        Thanks for your analysis. Can you please comment on how this affects "creating generic Converters"? For example, let's say I create a Converter:

        public class GenericConverter {

        public GenericConverter( Class<?> toConvertTo )

        { this.toConvertTo = toConvertTo; }

        public Object getAsObject( ... )

        { // convert value into class toConvertTo }

        The idea is I have a single Converter which can handle many subtypes. Clearly I need to know which subtype to convert to. It'd be really useful to pass that in via my constructor.

        The code in ApplicationImpl.newConverter appears to be trying to support this:

        Constructor ctor = ReflectionUtils.lookupConstructor(clazz, Class.class);

        But because of the way createConverterBasedOnClass swaps the targetClass/baseClass parameters while traversing the superclass chain, this approach does not work.

        If the targetClass/baseClass swap really is intentional, as you say, could you please comment on how one would write a GenericConverter?

        Show
        kennardconsulting added a comment - Manfred, Thanks for your analysis. Can you please comment on how this affects "creating generic Converters"? For example, let's say I create a Converter: public class GenericConverter { public GenericConverter( Class<?> toConvertTo ) { this.toConvertTo = toConvertTo; } public Object getAsObject( ... ) { // convert value into class toConvertTo } The idea is I have a single Converter which can handle many subtypes. Clearly I need to know which subtype to convert to. It'd be really useful to pass that in via my constructor. The code in ApplicationImpl.newConverter appears to be trying to support this: Constructor ctor = ReflectionUtils.lookupConstructor(clazz, Class.class); But because of the way createConverterBasedOnClass swaps the targetClass/baseClass parameters while traversing the superclass chain, this approach does not work. If the targetClass/baseClass swap really is intentional, as you say, could you please comment on how one would write a GenericConverter?

          People

          • Assignee:
            Manfred Riem
            Reporter:
            kennardconsulting
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: