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

Route overlapping issue #379

Merged
merged 5 commits into from
Apr 2, 2019
Merged

Route overlapping issue #379

merged 5 commits into from
Apr 2, 2019

Conversation

Vkt0r
Copy link
Member

@Vkt0r Vkt0r commented Mar 30, 2019

This PR should fix #355 can be resumed in the following steps:

  • Fix an issue causing the HttpRouter was not resolving correctly the overlapping of routes.
  • Add the new property isEndOfRoute to the Node class to know when a node is the end of the route or not.
  • Update the full project to Xcode 10.2 and the recommended settings to remove the warnings.
  • Include a unit test to verify the bug.
  • Update the Xcode version in the CircleCI config file.
  • Update the Linux manifest tests
  • Remove the unnecessary construction of the UnsafePointer as the sterror returns a UnsafeMutablePointer<Int8>! causing the build fails in CircleCI with Xcode 10.2. (Disambiguate swift method call #367)

The Trie formed during the registration of the routes was not handling when a node was the end of a route or not causing it was impossible to detect or match routes ending in this node correctly.

The good examples provided in #355 summarize the unit test added:

  1. a/:id
  2. a/:id/c

The first case should be recognized as a final of a router when it's added to Trie to be able to match possible prefix or routes in the tree.

The function inflate(_ node: inout Node, generator: inout IndexingIterator<[String]>) -> Node in the HttpRouter was refactored to set when a node is the end of the route or not during the insertion of a new route and to remove the recursion.

* Fix an issue causing the HttpRouter was not resolving correclty the overlapping of routes.
* Add the new property `isEndOfRoute` to the `Node` class to know when a node is the end of route or not.
Update the full project to Xcode 10.2 and the recommended settings to remove the warnings
Remove the unnecessary construction of the UnsafePointer as the `sterror` returns a `UnsafeMutablePointer<Int8>!`
Copy link
Collaborator

@pvzig pvzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

@Vkt0r Vkt0r merged commit bdfd5f5 into httpswift:stable Apr 2, 2019
@Vkt0r Vkt0r deleted the route-overlapping branch April 2, 2019 14:26
tomieq pushed a commit to tomieq/swifterfork that referenced this pull request Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HttpRouter takes incorrect route
2 participants