Skip to content
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

Mojarra issue 5171 #5172

Merged
merged 2 commits into from
Nov 16, 2022
Merged

Mojarra issue 5171 #5172

merged 2 commits into from
Nov 16, 2022

Conversation

BalusC
Copy link
Contributor

@BalusC BalusC commented Nov 13, 2022

the ValueExpressionAnalyzer from 3.0
@NicolaIsotta
Copy link
Contributor

Hi @BalusC
Testing locally I found out the fix can be simplified.
Adding this override to ContextualCompositeValueExpression:

@Override
public ValueReference getValueReference(ELContext elContext) {

    FacesContext ctx = (FacesContext) elContext.getContext(FacesContext.class);
    boolean pushed = pushCompositeComponent(ctx);
    try {
        return originalVE.getValueReference(elContext);
    } finally {
        if (pushed) {
            popCompositeComponent(ctx);
        }
    }

}

InterceptingResolver in ValueExpressionAnalyzer is not needed anymore, so getReference can be simplified this way:

public ValueReference getReference(ELContext elContext) {
    ValueReference reference = expression.getValueReference(elContext);
    if (reference != null) {
        Object base = reference.getBase();
        if (base instanceof CompositeComponentExpressionHolder) {
            ValueExpression ve = ((CompositeComponentExpressionHolder) base).getExpression(String.valueOf(reference.getProperty()));
            if (ve != null) {
                expression = ve;
                reference = getReference(elContext);
            }
        }
    }
    return reference;
}

I can make a PR if needed, let me know

@BalusC
Copy link
Contributor Author

BalusC commented Nov 15, 2022

I'll adjust the PR and re-test when time allows me.

Further improved by adjusting ContextualCompositeValueExpression so we
can get rid of the ValueExpressionAnalyzer
@BalusC
Copy link
Contributor Author

BalusC commented Nov 15, 2022

Thank you very much, @NicolaIsotta , I've been able to get rid of the ValueExpressionAnalyzer completely with your insightful suggestion! The PR has been updated.

@arjantijms arjantijms added this to the 4.0.1 milestone Nov 15, 2022
@arjantijms arjantijms added bug Something isn't working 4.0 labels Nov 15, 2022
@BalusC BalusC merged commit bb31991 into master Nov 16, 2022
@BalusC BalusC deleted the mojarra_issue_5171 branch November 16, 2022 18:43
@BalusC BalusC restored the mojarra_issue_5171 branch November 16, 2022 18:46
@BalusC
Copy link
Contributor Author

BalusC commented Nov 16, 2022

4.0 branch was split off from master after this PR was created and then master became 4.1 but the PR wasn't updated to retarget 4.0.

New PR created for 4.0: #5177

@BalusC BalusC deleted the mojarra_issue_5171 branch November 16, 2022 18:48
@BalusC BalusC mentioned this pull request Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.0 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants