When using a @GroupSequenceProvider, why is it called 3 times? And why the first time with a null validation instance?

When using a @GroupSequenceProvider, why is it called 3 times?
And why the first time with a null validation instance?

Consider this class:

@Data
@AllArgsConstructor
@GroupSequenceProvider(CarGroupSequenceProvider.class)
public class Car {

  @NotBlank
  private String make;

  @NotBlank
  private String model;

  private boolean registered;

  @NotBlank(groups = RegistrationValidationGroup.class)
  private String licensePlate;

}

With this GroupSequenceProvider:


public class CarGroupSequenceProvider implements DefaultGroupSequenceProvider<Car> {

  @Override
  public List<Class<?>> getValidationGroups(Car car) {

    System.out.println("CarGroupSequenceProvider::getValidationGroups called with car=" + car);

    List<Class<?>> groupSequence = new ArrayList<>();

    groupSequence.add(Car.class);

    if (car != null) {

      if (car.isRegistered()) {
        groupSequence.add(RegistrationValidationGroup.class);
      }

    }

    return groupSequence;

  }

}

When you run this test:

public class CarValidationTest {

  private static Validator validator;

  @BeforeClass
  public static void setUpValidator() {
    ValidatorFactory factory = Validation.buildDefaultValidatorFactory().getValidator();
  }

  @Test
  public void testEmptyMakeModel() {

    Car car = new Car(null, null, false, null);

    Set<ConstraintViolation<Car>> constraintViolations = validator.validate(car);

    assertEquals(2, constraintViolations.size());

  }

}

Will result in this output:

CarGroupSequenceProvider::getValidationGroups called with car=null
CarGroupSequenceProvider::getValidationGroups called with car=Car(make=null, model=null, registered=false, licensePlate=null)
CarGroupSequenceProvider::getValidationGroups called with car=Car(make=null, model=null, registered=false, licensePlate=null)

Why is CarGroupSequenceProvider::getValidationGroups called 3 times?
Why is the first time it’s call, the test instance provided as null?

Interesting finding.

So, the first one is used when building the metadata descriptors exposed to the Bean Validation metadata API. As the example in the documentation shows (https://docs.jboss.org/hibernate/stable/validator/reference/en-US/html_single/#_code_groupsequenceprovider_code), you might want to add groups even if the object is null and these ones are exposed in the metadata API. Note that this call is executed only once when bootstrapping the metadata.

As for the other two, it’s just because of how the code is organized. I admit it’s not ideal. I opened https://hibernate.atlassian.net/browse/HHH-12648 .

Thanks for raising this issue.

@gsmet Thanks for getting back to me.

Just trying to wrap my head around this… What’s the use case for adding groups if the instance is null?

Can you even do validations on a null instance? This test:

@Test
public void testNull(){
  Car car = null;
  validator.validate(car);
}

Fails with:

java.lang.IllegalArgumentException: HV000116: The object to be validated must not be null.
	at org.hibernate.validator.internal.util.Contracts.assertNotNull(Contracts.java:44)
	at org.hibernate.validator.internal.engine.ValidatorImpl.validate(ValidatorImpl.java:152)

It’s not about validating a null bean, it’s about providing the best metadata possible i.e. it tries to gather the group sequence provider result in case it adds elements even if the passed object is null.

Frankly, I think it was a bad decision but I don’t feel like changing it as people might rely on it.

The other thing is that it allows to detect some errors earlier (e.g. when bootstrapping the bean metadata rather than when executing the validation).

I also stumbled across this issue while trying to implement a generic group sequence provider, i.e. one that is not tied to a specific validation target type.

it’s about providing the best metadata possible i.e. it tries to gather the group sequence provider result in case it adds elements even if the passed object is null

That’s fine so far, but why don’t you accept an empty list if the passed object was null? In that case you could just add the bean class as default yourself. This would allow for more flexibility without breaking backward compatibility.

@tamm0r maybe you can draft what you have in mind as a PR and we can discuss it?