Saturday, July 12, 2014

ConstraintViolationImpl.equals seems incorrect. Please verify / confirm.

Hi,

While trying to understand a weird issue I'm facing (http://stackoverflow.com/questions/24599410/gwt-bean-validation-does-not-return-all-constraint-violations-in-compiled-mode ) I seem to have found an issue with the equals method of ConstraintViolationImpl that could be at the root of the issue.

As I'm not 100% sure could someone verify my observatiom.

the source (I'm using 2.6.0) of the ConstraintViolationImpl.equals method is:

 @Override
  public boolean equals(Object o) {
    if (this == o) {
      return true;
    }
    if (!(o instanceof ConstraintViolationImpl)) {
      return false;
    }
    ConstraintViolationImpl<?> other = (ConstraintViolationImpl<?>) o;
    return (message == null ? other.message == null : message.equals(other.message)
        && propertyPath == null ? other.propertyPath == null :
          propertyPath.equals(other.propertyPath)
        && rootBean == null ? other.rootBean == null : rootBean.equals(other.rootBean)
        && leafBean == null ? other.leafBean == null : leafBean.equals(other.leafBean)
        && elementType == null ? other.elementType == null : elementType.equals(other.elementType)
        && invalidValue == null ? other.invalidValue == null :
          invalidValue.equals(other.invalidValue));
  }

I think the comparisons at the return statement are incorrect as ?: has a lower precedence than &&. (see also http://stackoverflow.com/questions/18788063/why-does-false-falsefalsetrue-return-true).

A simple unit test showing the issue is:

@Test
public void equalsReturnsTrueWhileItShouldnt() {

Path propertyPath = new PathImpl();
String message1 = "[message1]";
String message2 = "[message2]";
Builder<?> builder = ConstraintViolationImpl.builder();
builder.setPropertyPath(propertyPath);
// ... normally other fields should be set but not needed to show issue
builder.setMessage(message1);
ConstraintViolationImpl<?> constraintViolation1 = builder.build();
builder.setMessage(message2);
ConstraintViolationImpl<?> constraintViolation2 = builder.build(); 

boolean equalsGotten = constraintViolation1
.equals(constraintViolation2);

assertFalse(equalsGotten);
}

where the assertion will fail.


Am I overlooking something or is this indeed a bug?


best regards,

Rutger

--
You received this message because you are subscribed to the Google Groups "Google Web Toolkit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit+unsubscribe@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit.
For more options, visit https://groups.google.com/d/optout.

No comments:

Post a Comment