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

Changes in the App middlewares doesn't affect middlewares inside Group #1232

Closed
e-max opened this issue Aug 16, 2018 · 3 comments
Closed

Changes in the App middlewares doesn't affect middlewares inside Group #1232

e-max opened this issue Aug 16, 2018 · 3 comments

Comments

@e-max
Copy link

e-max commented Aug 16, 2018

Hi guys!

I've experienced problem similar to described here #445 (problem on middleware skipping)
But also I am having problem with Middleware.Replace and so on. I've checked the fix but not sure that I understood it.

I noticed that my problems were related to Resources so I continued looking into source code and I found this function in https://github.com/gobuffalo/buffalo/blob/master/router.go#L185

func (a *App) Group(groupPath string) *App {
	g := New(a.Options)
	g.Prefix = path.Join(a.Prefix, groupPath)
	g.Name = g.Prefix

	g.router = a.router
	g.Middleware = a.Middleware.clone()
	g.ErrorHandlers = a.ErrorHandlers
	g.root = a
	if a.root != nil {
		g.root = a.root
	}
	a.children = append(a.children, g)
	return g
}

Wouldn't the line :

g.Middleware = a.Middleware.clone()

cause problems?
Doesn't it mean that all changes you apply to Middleware stack after your create Group (or Resource) won't affect this Resource middlewares?

@markbates
Copy link
Member

markbates commented Aug 16, 2018 via email

@e-max
Copy link
Author

e-max commented Aug 16, 2018

Hi Mark.
I guess I just thought about Middlewares as a totally independent stack of function which applies before (and after) every request. I didn't think about it as a part of hierarchical routing architecture. So when I was deciding how to deal with authentication in tests I decided that the best way is to app.Replace my Authorize middleware with an empty function. Obviously, it didn't work.

It's much clearer now, thanks for the answer.

@markbates
Copy link
Member

I've added a PR that adds more comments to make it clearer: #1233

I did, at one point go down that path you're describing, but it turned out to be very complex as there is no clear definition of "when" to "finalize" the stack. doing on each request was really slow, but caching it meant, again, choosing a "finalize" point that probably wouldn't have made people too happy. Then when you realize that most of how the middleware works is a bunch of magic to get around Go not letting do things like using functions as map keys, etc... you find out it's a big juggle. :)

Doesn't mean if someone came along with an awesome solution we wouldn't be open to it, it just means that I wasn't able to find a good solution, and this seemed like the most sane "compromise" for making it easier to understand.

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

No branches or pull requests

2 participants