ExpressionInspector recordCall method throws exception for dynamic property formed with string concatenation #225

Closed
vijpan opened this Issue Mar 29, 2016 · 8 comments

Projects

None yet

3 participants

@vijpan
vijpan commented Mar 29, 2016

We are making use of validateBean tag handler and have a scenario where base is a "Map" and property is formed with EL string concatenation e.g.

<p:inputText value="#{mapObject[var1 += var2]}" />

Both var1 and var2 are coming from an outer ui:repeat - to form the property for the mapObject.

In ValidateBean - it uses ExpressionInspector.getValueReference - which internally execute's "recordCall" method and for the above scenario it fails here

if (passTwoCallCount == passOneCallCount && (base != lastBase || property != lastProperty)) {
                        // We're at the same call count as the first phase ended with.
                        // If the chain has resolved the same, we should be dealing with the same base and property now
                        // If that is not the case, then throw ISE.
                        throw new IllegalStateException(
                            "First and second pass of resolver at call #" + passTwoCallCount +
                            " resolved to different base or property.");
                    }

In the above check even when property and lastProperty contents are equals they won't match since the string concatenation will lead to a different object - obviously property.equals(lastProperty) will return true but (property != lastProperty) will return false.

@BalusC
Member
BalusC commented Mar 29, 2016

@arjantijms Was there any technical reason reference equality is tested here instead of value equality?

@arjantijms
Member

Probably no technical reason. Since the first two comparisons do needed the ==/!= operator I likely accidentally used it for the last comparison as well. Logically it indeed wouldn't make sense, as it's the name of the property that counts, not its identity (always wondered why the property is an Object and not a String tbh)

@vijpan
vijpan commented Mar 29, 2016

Is there a chance that the fix for this can go as part of omnifaces 2.3

On Tuesday, March 29, 2016, Arjan Tijms notifications@github.com wrote:

Probably no technical reason. Since the first two comparisons do needed
the ==/!= operator I likely accidentally used it for the last comparison as
well. Logically it indeed wouldn't make sense, as it's the name of the
property that counts, not its identity (always wondered why the property is
an Object and not a String tbh)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#225 (comment)

@arjantijms
Member

Absolutely! We only need to run some tests locally and then we'll commit the change tomorrow if everything passes. Thanks a lot for the report.

@vijpan
vijpan commented Mar 29, 2016

Great! Thanks for the quick fix.

On Tuesday, March 29, 2016, Arjan Tijms notifications@github.com wrote:

Absolutely! We only need to run some tests locally and then we'll commit
the change tomorrow if everything passes. Thanks a lot for the report.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#225 (comment)

@BalusC BalusC closed this in 53edffd Mar 30, 2016
@BalusC
Member
BalusC commented Mar 30, 2016

Fix is available in today's 2.3 snapshot. Can you confirm it's working for you?

@vijpan
vijpan commented Mar 30, 2016

Yes, tested with 2.3 snapshot and its working fine now. Thanks

@vijpan
vijpan commented Mar 30, 2016

Also i might have found another issue related to similar use case of dynamic property in ValidateBean tag handler - will create a different issue.

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