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

Router Bugfixes #479

Merged
merged 6 commits into from
Jun 19, 2024
Merged

Router Bugfixes #479

merged 6 commits into from
Jun 19, 2024

Conversation

Joannis
Copy link
Contributor

@Joannis Joannis commented Jun 18, 2024

No description provided.

Joannis and others added 4 commits June 18, 2024 11:00
Co-Authored-By: Tibor Bodecs <mail.tib@gmail.com>
…nd different paths

Move all logic into a ResolveContext. When encountering a dead end, the router goes back up the chain and descends another child. When a catch-all finds a new route to descend, store the nextNodeIndex so that the path can be descended further
@Joannis
Copy link
Contributor Author

Joannis commented Jun 18, 2024

This PR moves a lot of logic into a new struct. I don't know if it has a performance impact yet, so I might need to undo that

Copy link
Member

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

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

In general looks good, frustrating it requires the recursive function.
You will need to do some benchmarks with this to ensure it doesn't cost too much

Sources/Hummingbird/Router/Trie/Trie+resolve.swift Outdated Show resolved Hide resolved
Sources/Hummingbird/Router/Trie/Trie+resolve.swift Outdated Show resolved Hide resolved
@Joannis Joannis requested a review from adam-fowler June 19, 2024 13:50
@Joannis Joannis merged commit c416626 into main Jun 19, 2024
5 checks passed
@Joannis Joannis deleted the jo/router-bugfixes branch June 19, 2024 15:27
@adam-fowler adam-fowler mentioned this pull request Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants