Freefair lombok 6.4.3+lombok 1.8.24 introduces javax.validation inconsistencies

[quotidian-ennui]

This is caused by the recently released projectlombok 1.18.24 release which has the release notes :

IMPROBABLE BREAKING CHANGE: Lombok now understands a few more annotations that imply "this field should not ever contain a null reference". Lombok will thus copy some of these new annotations e.g. to generated getters and the like. Pull Request #2904

1.18.24 is introduced because of freefair lombok upgrade from 6.4.2 to 6.4.3. The symptom can be seen in this PR : adaptris/interlok-aws#900

You have a class that implements ConnectedService and you use 'lombok' so you annotated it like this

  @Getter
  @Setter
  @NotNull
  private AdaptrisConnection connection;

This leads to the generated code

public void setConnection(@NotNull AdaptrisConnection  connection) {}
@NotNull
public AdaptrisConnection getConnection() {}

Which now differs in signature from what's defined in the ConnectedService. Which, when you run an XMLRoundTrip validation test gives you a a HV000151 hibernator validator exception.

javax.validation.ConstraintDeclarationException:
   HV000151: A method overriding another method must not redefine the parameter constraint configuration,
   but method AWSKMSServiceImpl#setConnection(AdaptrisConnection) redefines the configuration of
ConnectedService#setConnection(AdaptrisConnection).
	at org.hibernate.validator.internal.metadata.aggregated.rule.OverridingMethodMustNotAlterParameterConstraints.apply(OverridingMethodMustNotAlterParameterConstraints.java:24)

This is because javax.validation.ConstraintDeclarationException is raised if you add parameter constraints to a method which overrides or implements a super-type method. This behavior is mandated by the Bean Validation spec (see http://beanvalidation.org/1.1/spec/#constraintdeclarationvalidationprocess-methodlevelconstraints-inheritance)

Solution 1 (changes to the "most files").

Modify the ConnectedService interface to add the @NotNull annotations, since pretty much all services that have connections will have those set to be not null. If the default is 'null-connection' then we should just default it and accept the XML clutter; we'd also need to review all the implementations of ConnectedService and provide sensible defaults if it really could be null.

Solution 2 (least change, but breaks future upgradeability)

Modify all the build.gradle files such that, and keep monitoring the change log, they may end up adding a config item to disable the generation (raise a PR on lombok itself)

lombok {
    version = "1.18.22"
}

Solution 3 (safest)?

Modify the projects that are currently failing with 6.4.3 to remove the @Getter/@Setter annotation from the connection, and maintain the boilerplate

All 3 solutions have their issues

1. Even if we do this, there will be other inheritable methods that will cause violations
2. We can't upgrade lombok until a config item is available
3. Boilerplate sucks, and we can't control 3rd party devs.

[quotidian-ennui]

ConnectedService as an interface is fine; it's just declaring method availability not behaviour, some implementations care that the connection is not null, other implmentations don't so we can't always decide at the interface level what to enforce.

Probably downgrade to 1.8.22 is the "least work since there's only 5 affected projects" but that leaves us with the situation where those projects can't use features post 1.8.24

[quotidian-ennui]

projectlombok/lombok#3180 is the umbrella issue for this.