New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SkipValidators does not restore ValueExpression (regression) #408

Closed
mmariotti opened this Issue Oct 14, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@mmariotti
Copy link
Contributor

mmariotti commented Oct 14, 2017

Hello,
I've met this issue before, but I stumbled on this again.

Here is a sample page that shows the wrong behavior:

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml" xmlns:ui="http://xmlns.jcp.org/jsf/facelets"
    xmlns:h="http://xmlns.jcp.org/jsf/html" xmlns:f="http://xmlns.jcp.org/jsf/core"
    xmlns:o="http://omnifaces.org/ui" xmlns:of="http://omnifaces.org/functions">

<f:metadata>
    <f:viewAction>
        <f:setPropertyActionListener value="required" target="#{viewScope.mode}" />
    </f:viewAction>
</f:metadata>

<h:head>
</h:head>

<h:body>
    <h:form id="form">
        <h:panelGrid columns="3">
            <h:selectOneMenu id="mode" value="#{viewScope.mode}">
                <f:selectItem itemLabel="optional" itemValue="optional" />
                <f:selectItem itemLabel="required" itemValue="required" />
                <f:ajax execute=":form" render=":form" />
                <o:skipValidators />
            </h:selectOneMenu>

            <h:inputText id="value" value="#{viewScope.value}" required="#{mode == 'required'}" />

            <h:message for="value" />
            

            <h:commandButton value="submit">
                <f:ajax execute=":form" render=":form" />
            </h:commandButton>

            <h:outputText value="mode: #{mode}" />
        </h:panelGrid>
    </h:form>
</h:body>
</html>

To trigger the issue:

  1. load the page
  2. hit submit button (the required message is shown)
  3. select "optional" from dropdown
  4. select "required" from dropdown
  5. hit submit button (the required message is NOT shown this time)

The cause is:

private void processEventForUIInput(SystemEvent event, UIInput input) {
	String clientId = input.getClientId();

	if (event instanceof PreValidateEvent) {
		ValueExpression requiredExpression = input.getValueExpression("required");
		attributes.put(clientId, (requiredExpression != null) ? requiredExpression : input.isRequired());
		input.setRequired(false);	// <-------------- THIS

		Validator[] validators = input.getValidators();
		allValidators.put(clientId, validators);

		for (Validator validator : validators) {
			input.removeValidator(validator);
		}

Setting a "literal" value on component attribute (or even a "literal" ValueExpression via UIComponent.setValueExpression) makes the value reach the ComponentStateHelper.defaultMap.
On evaluation side, UIInput.isRequired calls ComponentStateHelper.eval, which accesses ComponentStateHelper.defaultMap at first, by checking if the return value ofComponentStateHelper.get is null, thus returning an obsolete value and ignoring the underlying ValueExpression.

To handle such case, when "required" is provided by a ValueExpression, the code should be:

private void processEventForUIInput(SystemEvent event, UIInput input) {
	String clientId = input.getClientId();

	if (event instanceof PreValidateEvent) {
		ValueExpression requiredExpression = input.getValueExpression("required");
		if(requiredExpression != null) {
			attributes.put(clientId, requiredExpression);
			// cannot call 'input.setRequired(false)' because the 'false' cannot be removed - see javax.faces.component.ComponentStateHelper.put(Serializable, Object)
			// the ValueExpression MUST NOT be "literal" - see javax.faces.component.UIComponent.setValueExpression(String, ValueExpression)
			input.setValueExpression(Components.createValueExpression("#{false}", Boolean.class)) 
		} else {
			attributes.put(clientId, input.isRequired());
			input.setRequired(false);
		}

		Validator[] validators = input.getValidators();
		allValidators.put(clientId, validators);

		for (Validator validator : validators) {
			input.removeValidator(validator);
		}

Thanks

@BalusC

This comment has been minimized.

Copy link
Member

BalusC commented Oct 14, 2017

Makes fully sense.

BalusC added a commit that referenced this issue Oct 14, 2017

BalusC added a commit that referenced this issue Oct 14, 2017

@BalusC

This comment has been minimized.

Copy link
Member

BalusC commented Oct 14, 2017

I added a new SkipValidatorsIT and it has passed.

Thank you! It's in OmniFaces 2.6.5-SNAPSHOT.

@BalusC BalusC closed this Oct 14, 2017

@mmariotti

This comment has been minimized.

Copy link
Contributor

mmariotti commented Oct 15, 2017

Wow, that was quite fast!
Thank you, as always ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment