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.

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

Service registration like in net/rpc #87

Open
marshauf opened this issue Aug 23, 2015 · 13 comments
Open

Service registration like in net/rpc #87

marshauf opened this issue Aug 23, 2015 · 13 comments

Comments

@marshauf
Copy link
Collaborator

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
Copy link
Owner

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

marshauf commented Sep 1, 2015

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
Copy link
Owner

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
Copy link
Collaborator Author

marshauf commented Sep 1, 2015

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 pushed a commit that referenced this issue Sep 1, 2015
…s Services to be registered similar to net/rpc package. Add exported functions Client.RegisterService, Client.RegisterServiceName Client.UnregisterService and Client.CallService.
@marshauf
Copy link
Collaborator Author

marshauf commented Sep 3, 2015

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Owner

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
Copy link
Collaborator Author

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
Copy link
Owner

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 pushed a commit that referenced this issue Sep 18, 2015
…s Services to be registered similar to net/rpc package. Add exported functions Client.RegisterService, Client.RegisterServiceName Client.UnregisterService and Client.CallService.
@marshauf
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants