-
Notifications
You must be signed in to change notification settings - Fork 105
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.
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 support for dynamic message types #640
Conversation
New field Schema of type any on Spec objects. For proto based schemas the type will be of protoreflect.MethodDescriptor. This allows for easy introspection to interceptors.
An Initializer helps to construct dynamic messages on Receive. This lets clients and servers use dynamic messages. A default initializer for dynamicpb.Message is provided. Other IDLs can provide custom Initializers using the WithInitializer option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of small suggestions. Otherwise LGTM
client_stream.go
Outdated
if s.initializer != nil { | ||
if err := s.initializer(s.conn.Spec(), s.msg); err != nil { | ||
s.receiveErr = err | ||
return false | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This nil checks is made in 8 separate places. What if instead you added an unexported initialize
function to config and then pass config.initialize
instead of config.Initializer
? That way, that one method could do the nil check (and just return nil error if the func is nil).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having the function branch off the two configs I've added a maybeInitializer
that wraps the func and does the nil check. Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exported APIs LGTM.
Adds support for dynamic clients and servers with options to initialize messages on receives. See #523 for more details. Two new options
WithRequestInitializer
andWithResponseInitializer
provide initializer functions that can construct messages before unmarshalling. Proto based schemas can use the newSchema
field onSpec
to introspect the method types. Other IDLs can set their own schema using theWithSchema
option.Dynamic Message Initializer
As an example to support
dynamicpb.Message
we will inspect the schema and set the message descriptor. An intializer func can be run on the client or handller. This func is provided to the examples below as an option:Dynamic Clients
Construct a client using
dynamicpb.Message
. Ensure the URL includes the suffix of the handler method to invoke and to includeWithSchema
andWithResponseInitializer
options:Dynamic Handlers
Create the server func with a signature that includes the
dynamicpb.Message
. Then construct the handler specifying the procedure URL and ensure to includeWithSchema
andWithRequestInitializer
options:Fixes #523