Add generation of back-compat methods for optional query parameters #21
Conversation
| } | ||
| }; | ||
|
|
||
| private interface DefaultTypeVisitor<T> extends Type.Visitor<T> { |
There was a problem hiding this comment.
for ergonomics, I'd love derive4j style visitors, so I could write:
foo.visitor()
.optional(x -> blargh())
.elseThrow()
.visit()There was a problem hiding this comment.
#22 reduces a lot of the union code/might make it simpler to add friendlier visit patterns
| @@ -117,4 +121,42 @@ public interface TestService { | |||
| @GET | |||
| @Path("catalog/integer") | |||
| int testInteger(@HeaderParam("Authorization") AuthHeader authHeader); | |||
|
|
|||
| @Deprecated | |||
| default int testQueryParams( | |||
There was a problem hiding this comment.
note to self: works fine in feign as described in https://github.com/OpenFeign/feign#static-and-default-methods
| } | ||
| }; | ||
|
|
||
| private static final Type.Visitor<Boolean> TYPE_BACKFILL_PREDICATE = new DefaultTypeVisitor<Boolean>() { |
There was a problem hiding this comment.
might be worth (minimal) javadoc here too, given this is a bit more opaque than the above two visitors - I have to scan up to understand what this is
There was a problem hiding this comment.
This visitor and the one above are kinda coupled (ie we'd never want them to every be out of sync). Instead of a whole visitor here with DefaultTypeVisitor inheritance, can we just use an if statement with a fallback?
private static boolean canBeCoercedFromAbsent(Type type) {
return type.accept(TypeVisitor.IS_OPTIONAL) || type.accept(TypeVisitor.IS_LIST) || type.accept(TypeVisitor.IS_SET) || type.accept(TypeVisitor.IS_MAP);
}
private static int typeSortOrder(Type type) {
return canBeCoercedFromAbsent(type) ? 1 : 0;
}We might then be able to delete the DefaultTypeVisitor entirely (as I think we've got another of these in the TypeVisitor class already)
|
Got some context behind this PR from slack: Every time an API author adds something to their API endpoints (even if it's backwards compatible), all their consumers suffer a compilation break as they now have to supply This PR tries to reduce these compilation failures by generating |
| CreateDatasetRequest request, | ||
| @HeaderParam("Test-Header") String testHeaderArg); | ||
| @HeaderParam("Test-Header") String testHeaderArg, | ||
| CreateDatasetRequest request); |
There was a problem hiding this comment.
This will be a one-off compilation break, but I like that it'll be stable from now onwards :)
| @@ -236,4 +343,188 @@ private static ClassName httpMethodToClassName(String method) { | |||
| throw new IllegalArgumentException("Unrecognized HTTP method: " + method); | |||
| } | |||
| } | |||
|
|
|||
| private static final ParameterType.Visitor<Boolean> QUERY_PARAM_PREDICATE = new ParameterType.Visitor<Boolean>() { | |||
There was a problem hiding this comment.
Use ParameterTypeVisitor.IS_QUERY instead of this
|
One tricky thing about the sort order stuff is that a parameter that used to be required but is now optional will move in position. So this doesn’t fix all kinds of compile breaks due to Conjure changes, but it should fix a large class of practical changes we observe. |
* update conjure to 4.0.0-rc* * use cli * checkstyle * update test * remove unneeded test resources * fix test * rebase and fix conflicts * address comments * bump to rc3
No description provided.