MyFaces Core
  1. MyFaces Core
  2. MYFACES-2697

BeanValidation class is annotated with @FacesValidator tag

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 2.0.1
    • Component/s: JSR-314
    • Labels:
      None

      Description

      BeanValidation class is annotated with @FacesVallidator annotation, and it should not be.

      We know that by default BeanValidation should be added to the Application object only if the param javax.faces.validator.DISABLE_DEFAULT_BEAN_VALIDATOR is set to false. So, there is no reason to use @FacesValidator, let the scanner find it and then check the property. It is better keep things simple and simplify the code on FacesConfigurator.

        Activity

        Hide
        Jakob Korherr added a comment -

        I'm afraid the last commit (with revision 941039) breaks the functionality committed in MYFACES-2514. However it has to be verified which one is right.

        Show
        Jakob Korherr added a comment - I'm afraid the last commit (with revision 941039) breaks the functionality committed in MYFACES-2514 . However it has to be verified which one is right.
        Hide
        Jakob Korherr added a comment -

        ...and why did you remove the installation of the DebugPhaseListener with the latest commit (revision 941050)?

        Show
        Jakob Korherr added a comment - ...and why did you remove the installation of the DebugPhaseListener with the latest commit (revision 941050)?
        Hide
        Leonardo Uribe added a comment -

        Ok, I reverted the change, but now it is registered from src/main/conf/META-INF/standard-faces-config-base.xml, so I'm happy with that.

        But checking the spec javadoc something is not right. See jsf 2.0 spec section 3.5.3 For example:

        "....Any configuration resource that declares a list of default validators overrides any list provided in a previously processed
        configuration resource. If an empty <default-validators/> element is found in a configuration resource, the list
        of default validators must be cleared...."

        But the code in DigesterFacesConfigDispenserImpl do this:

        defaultValidatorIds.addAll (application.getDefaultValidatorIds());

        There is no check for that condition. We are not saving the difference between no <default-validators/> and empty <default-validators/>.

        Now, this paragraph is not clear:

        "....The runtime must guarantee that the validator id javax.faces.Bean is included in the result from a call to
        Application.getDefaultValidatorInfo() (see Section 7.1.11.1 "Default Validator Ids"), regardless of any
        configuration found in the application configuration resources or via the @FacesValidator annotation..."

        That means, no matter what happen with defaultValidatorIds, javax.faces.Bean should be included. If defaultValidatorIds is empty, but javax.faces.validator.DISABLE_DEFAULT_BEAN_VALIDATOR is false, javax.faces.Bean should be included.

        Jakob, could you take a look at this one?

        Show
        Leonardo Uribe added a comment - Ok, I reverted the change, but now it is registered from src/main/conf/META-INF/standard-faces-config-base.xml, so I'm happy with that. But checking the spec javadoc something is not right. See jsf 2.0 spec section 3.5.3 For example: "....Any configuration resource that declares a list of default validators overrides any list provided in a previously processed configuration resource. If an empty <default-validators/> element is found in a configuration resource, the list of default validators must be cleared...." But the code in DigesterFacesConfigDispenserImpl do this: defaultValidatorIds.addAll (application.getDefaultValidatorIds()); There is no check for that condition. We are not saving the difference between no <default-validators/> and empty <default-validators/>. Now, this paragraph is not clear: "....The runtime must guarantee that the validator id javax.faces.Bean is included in the result from a call to Application.getDefaultValidatorInfo() (see Section 7.1.11.1 "Default Validator Ids"), regardless of any configuration found in the application configuration resources or via the @FacesValidator annotation..." That means, no matter what happen with defaultValidatorIds, javax.faces.Bean should be included. If defaultValidatorIds is empty, but javax.faces.validator.DISABLE_DEFAULT_BEAN_VALIDATOR is false, javax.faces.Bean should be included. Jakob, could you take a look at this one?
        Hide
        Leonardo Uribe added a comment -

        I commented the previous solution and committed what I think it is the correct interpretation of the spec.

        I think the config param just takes precedence over empty <default-validators/>. Why? because do the opposite means in some conditions the param just will not work and it will not be clear for the user. Also, the documentation of the param does not mention that side effect:

        "....If this param is defined, and calling toLowerCase().equals("true") on a String representation of its value returns true, the runtime must not automatically add the validator with validator-id equal to the value of the symbolic constant VALIDATOR_ID to the list of default validators. Setting this parameter to true will have the effect of disabling the automatic installation of Bean Validation to every input component in every view in the application, though manual installation is still possible....."

        Show
        Leonardo Uribe added a comment - I commented the previous solution and committed what I think it is the correct interpretation of the spec. I think the config param just takes precedence over empty <default-validators/>. Why? because do the opposite means in some conditions the param just will not work and it will not be clear for the user. Also, the documentation of the param does not mention that side effect: "....If this param is defined, and calling toLowerCase().equals("true") on a String representation of its value returns true, the runtime must not automatically add the validator with validator-id equal to the value of the symbolic constant VALIDATOR_ID to the list of default validators. Setting this parameter to true will have the effect of disabling the automatic installation of Bean Validation to every input component in every view in the application, though manual installation is still possible....."
        Hide
        Jakob Korherr added a comment -

        I will check this again (also against Mojarra) and add some test cases to verify the behavior!

        Show
        Jakob Korherr added a comment - I will check this again (also against Mojarra) and add some test cases to verify the behavior!
        Hide
        Jakob Korherr added a comment -

        I cleaned up the code on FacesConfigurator, FacesConfigDispenser and DigesterFacesConfigDispenserImpl. Furthermore I removed the installation of the BeanValidator as a default-validator from standard-faces-config-base.xml, because this can only happend programmatically (we have to check if bean validation is available), and added a comment there.

        I also added a check for two special scenarios on FacesConfigurator, which will result in log messages to inform the user, because they may lead to problems:
        1) the BeanValidator was disabled as a default-validator via DISABLE_DEFAULT_BEAN_VALIDATOR, but a faces-config file adds it, which is totally ok by the spec
        2) a faces-config file added the BeanValidator but bean validation is not available in the classpath, thus it will not work

        These two scenarios could be hard to trace, because there can be lots of config files when using some frameworks, thus I thought it would be a good idea to inform the user.

        In addition I added a lot of test cases on FacesConfiguratorDefaultValidatorsTestCase which check nearly every possible configuration scenario.

        So this issue can now be resolved

        Show
        Jakob Korherr added a comment - I cleaned up the code on FacesConfigurator, FacesConfigDispenser and DigesterFacesConfigDispenserImpl. Furthermore I removed the installation of the BeanValidator as a default-validator from standard-faces-config-base.xml, because this can only happend programmatically (we have to check if bean validation is available), and added a comment there. I also added a check for two special scenarios on FacesConfigurator, which will result in log messages to inform the user, because they may lead to problems: 1) the BeanValidator was disabled as a default-validator via DISABLE_DEFAULT_BEAN_VALIDATOR, but a faces-config file adds it, which is totally ok by the spec 2) a faces-config file added the BeanValidator but bean validation is not available in the classpath, thus it will not work These two scenarios could be hard to trace, because there can be lots of config files when using some frameworks, thus I thought it would be a good idea to inform the user. In addition I added a lot of test cases on FacesConfiguratorDefaultValidatorsTestCase which check nearly every possible configuration scenario. So this issue can now be resolved

          People

          • Assignee:
            Jakob Korherr
            Reporter:
            Leonardo Uribe
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development