fix: Drop verb client interfaces #3110
Conversation
tomdaffurn
requested review from
stuartwdouglas
and removed request for
a team
github-actions
bot
changed the title
tomdaffurn
changed the title
| @@ -280,3 +280,13 @@ func TypeEnumVerb(ctx context.Context, val AnimalWrapper) (AnimalWrapper, error) | |||
| //func MixedEnumVerb(ctx context.Context, val Mixed) (Mixed, error) { | |||
| // return val, nil | |||
| //} | |||
|
|
|||
| //ftl:verb export | |||
| func PrimitiveResponseVerb(ctx context.Context, val string) (int, error) { | |||
There was a problem hiding this comment.
There is already IntVerb above that has an int response type.
There was a problem hiding this comment.
So looking into this I don't know if we need this change:
Currently we generate:
@VerbClientDefinition(
name = "intVerb",
module = "gomodule"
)
public interface IntVerbClient extends VerbClient<Long, Long> {
long call(long value);
}
Which is valid, even though the primitive method does not override the interface type, because we actually generate both methods:
Basically in the int -> int case we generate three different methods:
Object -> Object
Long -> Long
long -> long
There was a problem hiding this comment.
The compile error doesn't show up for IntVerb because the generated signature of call(long) is different to call(Long) in VerbClient. Need to have an Object param with primitive response value to make the signature the same but different response type.
I guess we can restrict boxing to cases where the param is an Object and the response isn't.
There was a problem hiding this comment.
Ah, I see what you mean.
I wonder how much implementing VerbClient actually matters. For the generated classes I don't think it actually adds anything, and not being able to use primitives is just kinda annoying. I am not 100% happy with the way verb clients currently work anyway, as invocation within the same module is clunky. Let me think about this a bit.
There was a problem hiding this comment.
Possibly something like: https://github.com/TBD54566975/ftl/compare/stuartwdouglas/jvm-api-refactor?expand=1
tomdaffurn
changed the title
| } | ||
| } | ||
| return new VerbClientBuildItem(clients); | ||
| } | ||
|
|
||
| private MethodInfo getCallMethod(ClassInfo verbClient) { | ||
| for (var call : verbClient.methods()) { |
There was a problem hiding this comment.
I think we need to throw an exception if the interface has additional non-static non-default methods, as they won't be implemented by the generated client.
There was a problem hiding this comment.
That would only happen if the user has mistakenly created their own @VerbClient interface right?
Drop the
VerbClient,VerbClientSink,VerbClientSource,VerbClientEmptyinterfaces. They conflict with verbs that use primitive params and responses.Fixes #2757