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

[BUG] : HTTP method helper "Any" functionality has changed in the latest version. #1448

Closed
bharath-b23 opened this issue Feb 10, 2020 · 5 comments
Assignees

Comments

@bharath-b23
Copy link

Greetings!!!

I was making use of http method helpers Get and Any to restrict actions to read-only. And the Get helper had a handler associated to fetch the requested resource and Any to restrict any other HTTP methods.

Below is the code snippet used for the above scenario.
router := iris.New()
action := router.Party("/")
action.Get("/", listAll)
action.Any("/", sendError)

In v11.1.1, the behavior was as expected, but in v12.1.6, Any method is given precedence or handler defined for Get is being overwritten with that of Any, and for all operations on "/" path, am observing sendError handler being invoked.

Please let me know, if the behavior has been modified or my understanding is not inline with the usage.
And could the scenario described above be achieved in any other way, other than defining sendError handler for all HTTP methods except for GET.

@kataras
Copy link
Owner

kataras commented Feb 10, 2020

Hello @BharathB23 this is an expected behavior, iris v11.1.x had a bug which new routes didn't override the old ones (it's not a common thing to write same route again and again though), which was fixed at the upcoming releases months ago.

So, you want to register route on all methods except the registered ones. We can introduce a new method for that purpose or add a configurable section like we have for Party#SetExecutionRules to override the default behavior of app.Any, e.g

// RouteRegisterRule is a type of uint8.
// Defines the register rule for new routes that already exists.
// Available values are: RouteOverride, RouteSkip and RouteError.
//
// See `Party#SetRegisterRule`.
type RouteRegisterRule uint8

const (
	// RouteOverride an existing route with the new one, the default rule.
	RouteOverride RouteRegisterRule = iota
	// RouteSkip registering a new route twice.
	RouteSkip
	// RouteError log when a route already exists, shown after the `Build` state,
	// server never starts.
	RouteError
)

// SetRegisterRule sets a `RouteRegisterRule` for this Party and its children.
// Available values are: RouteOverride (the default one), RouteSkip and RouteError.
func (api *APIBuilder) SetRegisterRule(rule RouteRegisterRule) Party {
	api.routeRegisterRule = rule
	return api
}

Usage:

app:= iris.New()
v1 := app.Party("/v1")
v1.SetRegisterRule(iris.RouteSkip)
v1.Get("/", handler)
v1.Any("/", handler2) // all except GET.

Do you have some suggestions that suit your needs?

UPDATE:

It's almost ready, have to add some tests, I have to go for a meeting, if you (or anybody) reply with alternative until I come back we will discuss it further, otherwise the above API will be pushed today as v12.1.7 sounds fair?

Thanks and welcome to Iris,
Gerasimos Maropoulos

@kataras kataras added good first issue A user wrote a good first issue with clear instructions 🚀 status:implemented 🤘 status:resolved 💡type:idea labels Feb 10, 2020
@kataras
Copy link
Owner

kataras commented Feb 10, 2020

It's done @BharathB23,

You just have to upgrade to v12.1.7. Add: app.SetRegisterRule(iris.RouteSkip) and the previous behavior you are used to will be enabled.

Thanks,
Gerasimos Maropoulos

@kataras kataras closed this as completed Feb 10, 2020
@bharath-b23
Copy link
Author

Thank you so much @kataras, for such a fast resolution.
Solution is apt for my use case and with the new method, expected behavior is observed.

@kataras
Copy link
Owner

kataras commented Feb 11, 2020

Hi @BharathB23,

You are welcome! We are doing our bests.

Thank you for opening this issue, it was a good touch to the core code.

By the way, did you successfully receive the email for the Iris E-book?

@bharath-b23
Copy link
Author

Hi @kataras ,

Yes, received the E-book, Thank you for sharing!!

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