jcelliott / turnpike

Go implementation of a WAMP (Web Application Messaging Protocol) client and router
MIT License
258 stars 88 forks source link

Service registration like in net/rpc #87

Open marshauf opened 9 years ago

marshauf commented 9 years ago

I would like to implement something similar to the net/rpc package service registration. It would remove the reflection/type checking in the user code and would allow for WAMP reflection later.

For example:

type Echo struct {}

func (e *Echo) Echo(message string) (string, error) {
  return message, nil
}

func main() {
e := &Echo{}
err := client.Register(e)
err = client.RegisterName("echo2", e)

var res string
err = client.Call("echo2.Echo", "hello world", &res)
}
jcelliott commented 9 years ago

This is an excellent idea. I've wanted to do the same thing, but haven't had the time yet. It looks like the net/rpc client implementation would be a great starting place.

I've added you as a collaborator. If you want to work on this, you can use the dev branch until it's ready to merge into v2.

marshauf commented 9 years ago

I got a prototype wrapper around the current client.Register/Call function working. It needs a lot more work and testing. I will push to the dev branch once I moved the code to turnpike and wrote a test suite.

I could reduce a lot of argument/type checking in my code. It definitely helps.

marshauf commented 9 years ago

I need to add a dependency https://github.com/mitchellh/mapstructure or write my own map[string]interface{} to decoder. What would you prefer?

jcelliott commented 9 years ago

I hadn't seen that library before--I could have used it a few times in the past. It looks fairly well-tested, so I wouldn't mind depending on it.

I would like to look into vendoring dependencies at some point in the future. But the Go 1.5 vendor feature is only experimental and you have to manually enable it with an environment variable. Also, go get works fine for now.

marshauf commented 9 years ago

I got it working with a test-suit and without the mapstructure dependency. I don't know how well it will work with a none Go caller. I need to test it with my JavaScript code.

marshauf commented 9 years ago

The tests work because no serialization happens between the two test clients. This means I need the mapstructure package to convert a map to a struct like I wrote before.

marshauf commented 9 years ago

I would like to do something similar to Publish and Subscribe.

Add a method to Client:

func (c *Client) Broadcast(topic string, arguments ...interface{}) error

And extend Client.RegisterService to use methods which start with "On" as event listeners for publish messages, instead of a registration as a procedure.

Or add a separate method to subscribe to topics with a service, like:

func (c *Client) Listen(service interface{}) error

Which uses all exported methods like RegisterService does and instead of registering it as a procedure, it subscribes to a topic.

What do you prefer? Any thoughts?

marshauf commented 9 years ago

I implemented the first idea, minus the Client.Broadcast function. It would just be a one line short form of Client.Publish.

@jcelliott could you check the code, please. I would like to merge it into v2 and start on the 33 data races, which are currently in v2. In dev there are only two.

jcelliott commented 9 years ago

I like the idea of a "service" interface for events.

I was also thinking it would be nice to use the reflection code you added for a regular registration as well. Then maybe the current client.Register could be moved to client.RegisterRaw or something. I think most people using the client would like that functionality, since it simplifies implementations so much. What do you think?

I will take a look at it this evening and hopefully merge the dev branch in.

marshauf commented 9 years ago

If we merge Client.Register and Client.RegisterService, I wouldn't export the current Client.Register after the merge as Client.RegisterRaw. We would have the same amount of exported methods but one does more complicated stuff. I am not sure how I would handle the options argument in Client.Register. I guess we could just send it with each service method registration/subscription.

We could also merge Client.Subscribe which has the same WAMP arguments as register. Register,RegisterService and Subscribe can be merged into one method.

The problem I see is that we no longer follow the "naming convention" of the WAMP spec. Also we would have one method for registration but three each for invoking and deleting.

jcelliott commented 9 years ago

I was thinking we would keep RegisterService separate, where a service is a struct with methods. Then add a reflection layer to Register, still registering individual functions like we do now, then move the current Register to RegisterRaw for anyone that doesn't want to use the reflection layer (probably a bit slower?). But maybe keeping RegisterRaw isn't necessary.

marshauf commented 9 years ago

The service methods are being registered as "Type.Method" at the moment. In the WAMP spec the example names for procedures have "type.method" all lowercase naming. Do you think I should change it to all lowercase?

marshauf commented 9 years ago

I got an idea on how to do integrate event publishing into the service feature. Given the following struct:

type Service struct {
 PublishX func(arg1 string) error
}

Client.RegisterService would set PublishX and calls to PublishX would send an PUBLISH wamp event to topic Service.OnX. What do you think?