Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Operation InputType and OutputType helper methods #21

Merged
merged 2 commits into from
Oct 4, 2024
Merged

Add Operation InputType and OutputType helper methods #21

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb8f902
Add Operation.IsInputAssignable helper method
bergundy Oct 4, 2024
038b25c
Address review comments
bergundy Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions nexus/operation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ type NoValue *struct{}
// available.
type OperationReference[I, O any] interface {
Name() string
// IsInputCompatible returns a boolean indicating whether the given input is assignable to the generic I type.
IsInputAssignable(any) bool
// A type inference helper for implementations of this interface.
inferType(I, O)
}
Expand All @@ -36,6 +38,13 @@ func (r operationReference[I, O]) Name() string {
return string(r)
}

func (r operationReference[I, O]) IsInputAssignable(i any) bool {
Copy link

@cretz cretz Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this needs to be on every operation reference (nor does IsOutputAssignable). If you had a InputType() reflect.Type that could maybe make a bit more sense, but anyone can obtain the generic of the operation reference via reflection. IMO having a helper for Go reflection is unnecessary for this public API.

Can just update the Temporal side to use reflection if it needs to check whether something is assignable, same as it would for any generic API it would need to do a similar check for. I wouldn't expect types with generics to provide this helper.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's possible to get the generic input type in Go. I tried this before and ended up blocked on golang/go#54393.

Maybe there's another way you're aware of that I'm not.

Copy link

@cretz cretz Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, my mistake. I think InputType() reflect.Type and OutputType() reflect.Type would be better general-purpose methods to add (or Types() (input reflect.Type, output reflect.Type)).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works for me.

var zero [0]I
tt := reflect.TypeOf(zero).Elem()
it := reflect.TypeOf(i)
return it.AssignableTo(tt)
}

func (operationReference[I, O]) inferType(I, O) {} //nolint:unused

// A RegisterableOperation is accepted in [OperationRegistry.Register].
Expand Down
5 changes: 5 additions & 0 deletions nexus/operation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,8 @@ func TestHandlerError(t *testing.T) {
require.Equal(t, HandlerErrorTypeUnauthorized, handlerError.Type)
require.Equal(t, "unauthorized in test", handlerError.Failure.Message)
}

func TestIsInputAssignable(t *testing.T) {
require.True(t, numberValidatorOperation.IsInputAssignable(3))
require.False(t, numberValidatorOperation.IsInputAssignable("s"))
}
8 changes: 8 additions & 0 deletions nexus/unimplemented_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package nexus

import (
"context"
"reflect"
)

// UnimplementedHandler must be embedded into any [Handler] implementation for future compatibility.
Expand Down Expand Up @@ -40,6 +41,13 @@ func (*UnimplementedOperation[I, O]) inferType(I, O) {} //nolint:unused

func (*UnimplementedOperation[I, O]) mustEmbedUnimplementedOperation() {}

func (*UnimplementedOperation[I, O]) IsInputAssignable(i any) bool {
var zero [0]I
tt := reflect.TypeOf(zero).Elem()
it := reflect.TypeOf(i)
return it.AssignableTo(tt)
}

// Cancel implements Operation.
func (*UnimplementedOperation[I, O]) Cancel(context.Context, string, CancelOperationOptions) error {
return HandlerErrorf(HandlerErrorTypeNotImplemented, "not implemented")
Expand Down
Loading