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

Subgroups with middlewares are overriding parent groups middlewares #17

Closed
es-lab opened this issue Jun 25, 2017 · 5 comments
Closed

Subgroups with middlewares are overriding parent groups middlewares #17

es-lab opened this issue Jun 25, 2017 · 5 comments
Assignees
Labels

Comments

@es-lab
Copy link

es-lab commented Jun 25, 2017

Hi @joeybloggs ,

I'm doing some testing with this router, and I found an issue.
Here is an example:

a := server.Group("/a", aM)
a.Get("/test", func(ctx *internal.Ctx) {
	ctx.JSON(http.StatusOK, "a-ok")
})

b := a.Group("/b", bM)
b.Get("/test", func(ctx *internal.Ctx) {
	ctx.JSON(http.StatusOK, "b-ok")
})

c := b.Group("/c", cM)
c.Get("/test", func(ctx *internal.Ctx) {
	ctx.JSON(http.StatusOK, "c-ok")
})
func aM(ctx *internal.Ctx) {
	ctx.Next()
}

func bM(ctx *internal.Ctx) {
	ctx.Next()
}

func cM(ctx *internal.Ctx) {
	ctx.Next()
}

And this is the behavior:

  1. /a/test executes the aM. Everthing OK.
  2. /a/b/test executes only bM, it does not execute aM. But it should.
  3. /a/b/c/test executes only cM, it does not execute aM and bM. But it should execute them all.

If this is a bug, I hope it will be fixed ASAP. From a lot of routers out there this one seems pretty good to me, and this issue is blocking me to migrate to it.

@deankarn
Copy link
Contributor

Thanks @es-lab I'll check it out ASAP this afternoon.

Can you confirm you have pulled the latest changes, this sounds like the issue that was solved in the last pull request, just to confirm #16

P.S. if you like this router you may want to also check out this routers pure implementation https://github.com/go-playground/pure I found I was passing the custom context object everywhere which got messy after a while.

@es-lab
Copy link
Author

es-lab commented Jun 25, 2017

@joeybloggs Yes I'm using the last version as you indicated.

I saw the other router too, but I already have started a project that uses the Context. Now my project uses the routing system from the kataras/iris framework. As you my already know there will be no more support for it, and I'd like to migrate to another router.

From the list of routers on awesome-go.com this one is pretty good and will require less changes to migrate.

@deankarn
Copy link
Contributor

Cool I'll look into it when I'm back at a computer.. I'm out and about right now.

@deankarn deankarn self-assigned this Jun 25, 2017
@deankarn deankarn added the bug label Jun 25, 2017
@deankarn
Copy link
Contributor

Hey @es-lab I've corrected the issue and taken the liberty of splitting up the group function into:

  • Group
  • GroupWithMore
  • GroupWithNone

which I've been meaning to do for a while for clarity and ease of use.

Please let me know if you discovery anything else.

@es-lab
Copy link
Author

es-lab commented Jun 26, 2017

Hey @joeybloggs I've tested with the last changes, and it works perfectly. Thanks for the support!

P.S.: Splitting the group function was a really good idea!

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

No branches or pull requests

2 participants