Overloaded methods for optional query params may silently allow compilation in some unsafe cases
With #21, we generate @Deprecated methods with the intention of avoiding compile breaks in cases where new query params are added. However, in some cases we might silently let compilation go through when it shouldn't. Specifically, if we add another optional query params with the same type as an existing one and in the api.yml declare it right before the existing one.
As an example, lets say we have the following API:
getTransactionsPage:
http: POST /get-page
args:
query: string
optionalEnd:
type: optional<ResourceIdentifier>
param-type: query
returns: set<ResourceIdentifier>
-----
@POST
@Path("catalog/get-page")
Set<ResourceIdentifier> getTransactionsPage(
@HeaderParam("Authorization") AuthHeader authHeader,
@QueryParam("optionalEnd") Optional<ResourceIdentifier> optionalEnd,
String query);
@Deprecated
default Set<ResourceIdentifier> getTransactionsPage(AuthHeader authHeader, String query) {
return getTransactionsPage(authHeader, Optional.empty(), query);
}
and this is being used as follows in the client code:
Set<ResourceIdentifier> transactions = getTransactionsPage(authHeader, endTransactionRid, query);
Lets say the API was included to also have an optionalStart query param:
getTransactionsPage:
http: POST /get-page
args:
query: string
optionalStart:
type: optional<ResourceIdentifier>
param-type: query
optionalEnd:
type: optional<ResourceIdentifier>
param-type: query
returns: set<ResourceIdentifier>
-----
@POST
@Path("catalog/get-page")
Set<ResourceIdentifier> getTransactionsPage(
@HeaderParam("Authorization") AuthHeader authHeader,
@QueryParam("optionalStart") Optional<ResourceIdentifier> optionalStart,
@QueryParam("optionalEnd") Optional<ResourceIdentifier> optionalEnd,
String query);
@Deprecated
default Set<ResourceIdentifier> getTransactionsPage(AuthHeader authHeader, String query) {
return getTransactionsPage(authHeader, Optional.empty(), Optional.empty(), query);
}
@Deprecated
default Set<ResourceIdentifier> getTransactionsPage(
AuthHeader authHeader, Optional<ResourceIdentifier> optionalStart, String query) {
return getTransactionsPage(authHeader, optionalStart, Optional.empty(), query);
}
Compilation of the client code wouldn't break but it is now doing something semantically different.
To prevent, we'd have to ensure that whenever devs add new optional query params they always declare it last. However, I don't think thats something we do at the moment.
I discussed this @iamdanfox and he had a proposal around using builders instead to pass optional query and header params.