Skip to content
This repository has been archived by the owner on Sep 29, 2024. It is now read-only.

[Feature] Add wildcard handlers for events #612

Closed

Conversation

grahamjenson
Copy link
Contributor

@grahamjenson grahamjenson commented Aug 23, 2023

This feature lets a server or client handle events with wildcards, e.g.

server.OnEvent("/", "*", func(s socketio.Conn, e socketio.EventRequest, msg interface{}) interface{} {

would handle all events in the namespace if no other handler was specified.

It uses a radix structure to find the closest wildcard match, so if an event called "clients" came in then "clie*" will handle it over "c*"

This will always return an exact match before looking for wildcard matches, so performance should not be impacted if you use exact matches. This would also be a workaround if an event ended with a wildcard, e.g. an event called "star*".

This PR also:

  1. Expands the possible receiving functions for an event handler, because we need to pass along the event name to the handler. (I am not happy with this solution, though not sure how to make it better, suggestions?)
  2. splits out the Ack and Event Call functions as they are now dissimilar enough
  3. Only looks up (locks) the event once per call by refactoring to the method GetEventHandler
  4. Adds an example for a proxy (my ultimate goal) using the client and wildcard server to intercept and pass along messages. This is not a perfect proxy, but does the basic work.

@grahamjenson
Copy link
Contributor Author

@erkie @sshaplygin I think this is ready to merge. Tests are running locally for me, not sure why there are failures (flaky?).

@sshaplygin
Copy link
Collaborator

Hey, @grahamjenson. Thanks for you great job! Is this feature comparable with source socket.io into ts implementation? From which version is the functionality supported in the original version?

go.mod Show resolved Hide resolved
return
}

const FN_TYPE_ERROR string = "\nHandler Function must be in form, func(...), func(socketio.Conn, ...) or func(socketio.Conn, EventRequest ...)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use camel case instead of snake case for const FN_TYPE_ERROR


switch ft.NumIn() {
case 0:
hasConn = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

we don't need overwrite vars hasConn and hasEventRequest , because by default it is false too

)

func main() {
remote := os.Args[1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

it is required params for start your app. We must check with args

if err != nil {
c.onError(header.Namespace, err)
logger.Info("Error decoding the message type", "namespace", header.Namespace, "event", event, "eventType", handler.getEventTypes(event), "err", err.Error())
logger.Info("Error decoding the message type", "namespace", header.Namespace, "event", event, "argTypes", eventHandler.argTypes, "err", err.Error())
Copy link
Collaborator

Choose a reason for hiding this comment

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

pls, separate logs rows code lines with backspaces

nh.eventsLock.Lock()
defer nh.eventsLock.Unlock()
ef := newEventFunc(f)
Copy link
Collaborator

Choose a reason for hiding this comment

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

pls, separate code worked with mutex with backspace.

i suggest next style:

nh.eventsLock.Lock()
defer nh.eventsLock.Unlock()

ef := newEventFunc(f)

func (nh *namespaceHandler) GetEventHandler(event string) *funcHandler {
nh.eventsLock.Lock()
defer nh.eventsLock.Unlock()
eventHandler, ok := nh.events[event]
Copy link
Collaborator

Choose a reason for hiding this comment

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

pls, separate code worked with mutex with backspace


if namespaceHandler != nil {
return namespaceHandler.argTypes
if !ok {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls, rewrite this part to if ok we will return eventHandler, else we make magic :)

hasConn := false
hasEventRequest := false

switch ft.NumIn() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

i suggest to rewrite this part by method. for example:

// getArgsStart finding the number of remaining arguments
func getArgsStart(ft reflect.Type) int {
	hasConn := false
	hasEventRequest := false

	if ft.NumIn() == 1 {
		if implementsEventRequest(ft.In(0)) {
			panic("Cannot have EventRequest as first argument" + FN_TYPE_ERROR)
		}

		hasConn = implementsConn(ft.In(0))
	}

	if ft.NumIn() > 1 {
		if implementsEventRequest(ft.In(0)) {
			panic("Cannot have EventRequest as first argument" + FN_TYPE_ERROR)
		}

		if implementsConn(ft.In(1)) {
			panic("Cannot have Conn as second argument" + FN_TYPE_ERROR)
		}

		hasConn = implementsConn(ft.In(0))
		hasEventRequest = implementsEventRequest(ft.In(1))
	}

	if hasEventRequest && !hasConn {
		panic("Must have Conn if has EventRequest" + FN_TYPE_ERROR)
	}

	// Finding the number of remaining arguments
	argsStart := 0
	if hasConn {
		argsStart += 1
	}
	if hasEventRequest {
		argsStart += 1
	}
	
	return argsStart
}


var idToClient map[string]*socketio.Client = map[string]*socketio.Client{}

func findOrCreateClient(remote string, id string, serverSock socketio.Conn) *socketio.Client {
Copy link
Collaborator

Choose a reason for hiding this comment

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

findOrCreateClient it is bad method, because witch made double thinks on once method: create or find. Please, create client on server obviously

@googollee
Copy link
Owner

archived.

@googollee googollee closed this Sep 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants