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

add party's done handlers is strange #901

Closed
likakuli opened this issue Feb 8, 2018 · 2 comments
Closed

add party's done handlers is strange #901

likakuli opened this issue Feb 8, 2018 · 2 comments
Labels
good first issue A user wrote a good first issue with clear instructions 🤘 status:resolved

Comments

@likakuli
Copy link

likakuli commented Feb 8, 2018

I am new to use iris. I am not sure what i said is right, but just feel the party's done method is strange.
For example:

a:=app.Party("/a")
a.Done(ADone)

b:=app.Party("/b")
b.Get("/c",BCHandler)
b.Done(BDone)

when i call /b/c, i want to just call the BDone method finnaly, but it called both ADone and BDone method. After searching in the source code i found there are two methods strange:

// Party groups routes which may have the same prefix and share same handlers,
// returns that new rich subrouter.
//
// You can even declare a subdomain with relativePath as "mysub." or see `Subdomain`.
func (api *APIBuilder) Party(relativePath string, handlers ...context.Handler) Party {
	parentPath := api.relativePath
	dot := string(SubdomainPrefix[0])
	if len(parentPath) > 0 && parentPath[0] == '/' && strings.HasSuffix(relativePath, dot) {
		// if ends with . , i.e admin., it's subdomain->
		parentPath = parentPath[1:] // remove first slash
	}

	// this is checked later on but for easier debug is better to do it here:
	if api.relativePath[len(api.relativePath)-1] == '/' && relativePath[0] == '/' {
		relativePath = relativePath[1:] // remove first slash if parent ended with / and new one started with /.
	}

	// if it's subdomain then it has priority, i.e:
	// api.relativePath == "admin."
	// relativePath == "panel."
	// then it should be panel.admin.
	// instead of admin.panel.
	if hasSubdomain(parentPath) && hasSubdomain(relativePath) {
		relativePath = relativePath + parentPath
		parentPath = ""
	}

	fullpath := parentPath + relativePath
	// append the parent's + child's handlers
	middleware := joinHandlers(api.middleware, handlers)

	return &APIBuilder{
		// global/api builder
		macros:              api.macros,
		routes:              api.routes,
		errorCodeHandlers:   api.errorCodeHandlers,
		beginGlobalHandlers: api.beginGlobalHandlers,
		doneGlobalHandlers:  api.doneGlobalHandlers,
		reporter:            api.reporter,
		// per-party/children
		middleware:   middleware,
		relativePath: fullpath,
	}
}



// Done appends to the very end, Handler(s) to the current Party's routes and child routes
// The difference from .Use is that this/or these Handler(s) are being always running last.
func (api *APIBuilder) Done(handlers ...context.Handler) {
	for _, r := range api.routes.routes {
		r.done(handlers) // append the handlers to the existing routes
	}
	// set as done handlers for the next routes as well.
	api.doneGlobalHandlers = append(api.doneGlobalHandlers, handlers...)
}

The Party method set the global routes api.routes to the APIBuilder, and then return it, the Done method range the api.routes.routes to append the handlers to the existing routes, lead to the above result.

Why the Done method use the api.routes.routes? When use api.routes.routes, the actual execute result will rely on the order that the routes registed and it may lead to unexpected result when work together. I think it should use api.apiRoutes instead of api.routes.routes.

@kataras
Copy link
Owner

kataras commented Feb 8, 2018

You've almost right, the apiRoutes are not used, we don't need them because we have different variables for begin handlers and done handlers and they are managed at the Handle method, the apiRoutes are there but are not used anywhere, they will be removed. You have right, that the Done registers done handlers to all routes instead of its party's only, that's the problem but that was not by accident, that was my itention, it was created before the UseGlobal, so now I will fix the Done and add the DoneGlobal as well. I will deal with it right now, let me prepare a coffee first :)

@kataras
Copy link
Owner

kataras commented Feb 8, 2018

Fixed with a totally new minor version family, read https://github.com/kataras/iris/blob/master/HISTORY.md#th-08-february-2018--v1020 for more, thank you @likakuli, it was time to change it.

@kataras kataras added good first issue A user wrote a good first issue with clear instructions 🤘 status:resolved labels Feb 8, 2018
@kataras kataras closed this as completed Feb 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A user wrote a good first issue with clear instructions 🤘 status:resolved
Projects
None yet
Development

No branches or pull requests

2 participants