-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
Trie Tree matching problem #92
Comments
I think this would need just a bit more justification as to why Vapor should support it. The examples given so far have relatively simple workarounds: import RoutingKit
let router = Router<Int>()
- router.register(0, at: [.constant("b"), .anything, .constant("c")])
+ router.register(0, at: [.constant("b"), .parameter("a"), .constant("c")])
router.register(1, at: [.constant("b"), .parameter("a")])
var params = Parameters()
router.route(path: ["b", "c", "c"], parameters: ¶ms) - orders/:id
+ orders/:orderID
orders/:orderID/work/:workID Perhaps the simplicity and good performance characteristics make these limitations worth it. If we do decide this is a necessary feature, I think I'd prefer to try adding support for searching the entire tree to But before writing any code, I think we should take a look at how routers in other web frameworks work and what their performance characteristics are. |
Forcing the second example is painful though. Something besides me might have added part of that route. JWT3PA and PassKit are both examples of packages that automatically add to your routes for you. Now I have to use the exact same names they do for the parameters. What happens if those two items both add something like this: foo/:someID/passkit Now I'm stuck because one implementer used the obvious ':id' while another took a more specific ':someID'. |
In .NET, for example, I can call it whatever I want. I'd just give a routing attribute: [Route("{id}/contact")]
public async Task<IHttpActionResult> DeleteContact(int id) and that'll map the id paramater to the ID string part. Doesn't require they all be the same.. |
Stumbled upon this Issue when searching why my routes aren't working. My example is:
It makes a lot of sense to me to have such structure, for example, use one route for all products, except "computers" |
A follow up. As I think it would be beneficial to change that so any Router implementation can be used. What does community think? |
Allowing changing the router should be allowed. Note that you can only have one dynamic parameter for a path component - your example of |
@0xTim , hi! Do you have an example of that? Only thing that comes into mind, is to replace |
@ULazdins we could expose another initialiser on |
I could try to do a PR for that, but I'm not sure about some details. Noticed that a custom responder can be configured via |
Yeah I think that's probably the best way. I have no problem opening up |
Closing this issue due to age, inactivity, and the fact that it'd take some investigation to figure out whether the problem still exists and is still relevant. Please feel free to open a new issue or ask for this one to be reopened if it seems apropos. |
Steps to reproduce
Expected behavior
Actual behavior
Environment
Reason
Different from lexicographic matching strategy, routing should follow a strategy like BFS to match the longest defined route. This issue is directly related to the following ones:
#48 Actually the same case. As for developers that really have such needs,
vapor
ought to provide an alternative, despite performance losses compared to the existing one.#90 #91 Following the current model,
RoutingKit
is treating nodes with the same matching cases differently, which directly leads to unexpectednotFound
s.#74 This bug is also caused by the matching stratergy, where
.parameter
is always prior to.catchall
. The current solution is far from elegant as it’s using an awkward way to patch the matching solution. Neither extensible, nor clear; hardly Swifty.Suggestion
If
RoutingKit
itself would not change its matching strategy,vapor
does need to provide an alternative which has a more reliable and accurate model.For
RoutingKit
, it means abstracting its APIs withProtocol
s.For
Vapor
, it should allow developers to use their own routing backends inconfigure.swift
.Once it becomes necessary or performance-qualified, such module may become a
vapor-community
project.The text was updated successfully, but these errors were encountered: