Consider tightening `AuditEventRepository` contract

[vpavic]

AuditEventRepository exposes 3 operations for retrieving AuditEvents:

• AuditEventRepository#find(java.util.Date)
• AuditEventRepository#find(java.lang.String, java.util.Date)
• AuditEventRepository#find(java.lang.String, java.util.Date, java.lang.String)

All of these operations allow their parameters to be null which IMO isn't ideal, and is also somewhat confusing as all the retrieval options can actually be expressed using AuditEventRepository#find(java.lang.String, java.util.Date, java.lang.String).

Perhaps AuditEventRepository contract could be tightened so that it wouldn't allow null parameters? This was actually proposed in #5854 as a part of improvements to AuditEventRepository however at the time the change wasn't feasible due to backward compatibility considerations. The upcoming 2.0 could be the chance to revisit this.

It's also worth noting that auditevents Endpoint already behaves inline with what's proposed above as it requires the presence of after parameter.

[snicoll]

@vpavic this request has my vote for the after parameter. I am not sure I find this so useful for the principal or the type. Both are quite interesting when you're looking for any event after a certain date or a given event type after a certain date, but regardless of the principal.

I can see the endpoint requires all three to be set but I find the features above interesting.

What do you think?

[vpavic]

Thanks for the feedback @snicoll.

I agree that after parameter should definitely be required i.e. the AuditEventRepository#find in any form shouldn't allow it to be null.

Regarding the other two, principal and type, they are optional and it's only a question of how to express that the best way in AuditEventRepository API's find operations. My point is, if we for example have both List<AuditEvent> find(String principal, Date after, String type)
and List<AuditEvent> find(String principal, Date after) then the type parameter in the former shouldn't be nullable. OTOH if List<AuditEvent> find(String principal, Date after, String type) allows type to be nullable that makes the List<AuditEvent> find(String principal, Date after) redundant, as API should IMO be straightforward and without overlapping operations.

For the sake of simplicity I think there should only be a single find operation, that requires a non-null after parameter and allows nullable principal and type parameters. This is also how the endpoints have it through AuditEventsEndpointWebExtension and AuditEventsJmxEndpointExtension.

[wilkinsona]

FWIW, I find the null restriction on after to be rather arbitrary and, ultimately, a bit pointless. Rather than passing in null I could pass in something like 1900-01-01T00:00:00 and get the same effect.

[snicoll]

Thanks @wilkinsona. IMO that makes a compelling case for the status quo. I don't see much benefit in us aligning the contract of AuditEventRepository to what the endpoint uses concretely.