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

WW-5043 Enum comparison failure fix #98

Merged

Conversation

JCgH4164838Gh792C124B5
Copy link
Contributor

WW-5043 Enum comparison failure fix:

  • Improve OgnlOps.compareWithConversion() to handle special case of enumerated types (Enum) with element bodies.
  • Add additional equality and inequality unit tests for enumerated types.

- Improve OgnlOps.compareWithConversion() to handle special case of
  enumerated types (Enum) with element bodies.
- Add additional equality and inequality unit tests for enumerated types.
@JCgH4164838Gh792C124B5
Copy link
Contributor Author

Hello OGNL Team.

This PR is an attempt to address the issue raised in WW-5043. The actual change is small code-wise and I did not see any risk in adding it ... but if anyone sees a risk/flaw please let me know.

For relevant discussion on enumerated type checking see (https://stackoverflow.com/questions/4166488/checking-if-a-class-is-java-lang-enum).

Trying to figure out why the edge case was failing and getting unit tests set up took more time than the actual code changes. 😓

If the solution looks OK for OGNL 3.1.x it can easily be cherry-picked into 3.2.x.

@lukaszlenart
Copy link
Collaborator

Looks good, LGTM 👍

I will prepare new releases as soon as this change will be cherry-picked into 3.2.x, thanks!

@lukaszlenart
Copy link
Collaborator

w00t! This wasn't merged???

@lukaszlenart lukaszlenart merged commit 9874537 into orphan-oss:ognl-3-1-x Feb 10, 2020
@JCgH4164838Gh792C124B5 JCgH4164838Gh792C124B5 deleted the WW-5043_EnumFix branch February 22, 2020 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants