Add methods to KiwiResources to return non-null objects for method chaining

[sleberknight]

KiwiResources has a collection of overloaded methods named verifyExistence which have different signatures. Half of them accept T resourceEntity and the other half accept Optional<T> resourceEntity.

All of them throw a JaxrsNotFoundException when the object does not exist, either because resourceEntity is null or is an empty Optional.

The methods that accept Optional<T> return a (non-null) T, which allows code like:

Optional<HealthCheckExecution> latest = healthCheckDao.findLatestExecution();

var executionId = KiwiResources.verifyExistence(latest).getId();  // safe to call getId

But, if you have a possibly nullable object, you must write this code:

HealthCheckExecution latest = healthCheckDao.findLatestExecution();

KiwiResources.verifyExistence(latest);  // can't call getId here because return type is void
var executionId = latest.getId();  // this is safe

The above code is safe, but code analysis tools may still report a possible NPE from latest.getId(), because they don't understand that KiwiResources.verifyExistence will either return a (non-null) object or throw an exception. So then you have to either suppress (e.g., using an annotation, or in Sonar's web UI, etc.) the warning, or wrap latest with Objects.requireNonNull even though it can't ever be null if verifyExistence returns without throwing (or just ignore it).

This issue addresses the method-chaining by introducing new methods similar to the void-returning verifyExistence methods, or possibly just modifying the existing methods to return the same object when it is not null. I think this should be binary compatible since no existing code can possibly have a return type and compile.

A second issue will be created to annotate all the verifyExistence methods with an annotation to indicate that they should not ever return null. This will make static analyzers happy, or should anyway.

[sleberknight]

Never mind, changing the return type is not binary compatible regardless.

Change result type (including void) --> Breaks compatibility

source: https://github.com/eclipse-platform/eclipse.platform/blob/master/docs/Evolving-Java-based-APIs-2.md#evolving-api-interfaces---api-methods

Also:

Changing the result type of a method, or replacing a result type with void, or replacing void with a result type, has the combined effect of deleting the old method and adding a new method with the new result type or newly void result

source: https://docs.oracle.com/javase/specs/jls/se17/html/jls-13.html#jls-13.4.15

This means we must add new methods if we don't want to go to a new major version. And since we have many Dropwizard services that definitely use the void KiwiResources.verifyExistence(T obj) methods, we don't want any of these to break.

So, the question becomes: what should the new methods be named? I asked "a friend" (ChatGPT 4o) what "it" thought and got the following:

1. verifyAndReturn
2. checkAndReturn
3. validateAndReturn
4. verifyPresence
5. ensureExistence

I said the names must contain "verify" and "existence" and got:

1. verifyExistenceAndReturn
2. verifyAndReturnExistence
3. returnVerifiedExistence
4. ensureAndReturnExistence
5. validateExistenceAndReturn

I then told it that 3, 4, and 5 did not contain both "verify" existence" and got an "apology" and these suggestions:

1. verifyExistenceAndReturn
2. verifyAndReturnExistence
3. verifyExistenceWithReturn
4. verifyExistenceAndProvide
5. verifyExistenceReturning

The only thing I came up with before asking ChatGPT was verifyExistenceAndReturn, but I also think verifyExistenceWithReturn and verifyExistenceReturning would be somewhat acceptable.

Overall I think verifyExistenceAndReturn is probably the simplest and most descriptive name for the new methods.

[dsingley]

+1: verifyExistenceAndReturn