URLNavigator icon indicating copy to clipboard operation
URLNavigator copied to clipboard

When a route matches multiple handlers, make it FIFO.

Open Kalzem opened this issue 7 years ago • 3 comments

Let's imagine the handlers

navigator.handle("app://services/defaultService") // n°1
navigator.handle("app://services/hidden/<string:service_id>") // n°2
navigator.handle("app://services/<string:service_id>") // n°3

If I pass the route app://services/github, then only handler n°3 will be triggered. But if I pass the route app://services/defaultService, technically handlers n°1 and n°3 should trigger.

And it's where the problem starts: Because URLNavigator relies on a dictionary to save all the handlers, there is no order guaranteed.

open func handler(for url: URLConvertible, context: Any?) -> URLOpenHandler? {
    // handlerFactories is a dictionary, the keys are not guaranteed order of declaration.
    let urlPatterns = Array(self.handlerFactories.keys) 
    guard let match = self.matcher.match(url, from: urlPatterns) else { return nil }
    guard let handler = self.handlerFactories[match.pattern] else { return nil }
    return { handler(url, match.values, context) }
}

I suggest to implement an OrderedDictionary to save all the handlers and to follow a FIFO logic for the matchers.

Here is an example of an ordered dictionary: https://gist.github.com/BabyAzerty/2efda8bcef7a4286b5dc62770ea5aab5

It wouldn't change any URLNavigator code beside the declaration of the handlers to:

private var handlerFactories = OrderedDictionary<URLPattern, URLOpenHandlerFactory>()

Kalzem avatar Nov 06 '18 17:11 Kalzem

I met, too. This is very important.

YuanzhengWang avatar Dec 17 '18 02:12 YuanzhengWang

I met, too. This is very important. :)

strangeliu avatar May 16 '19 01:05 strangeliu

I think the matcher should not be dependent on the order but how many paths the url matches. #120 improves matching algorithm. So app://services/github will only match with n°3 and app://services/defaultService will only match with n°1.

devxoul avatar Sep 07 '19 15:09 devxoul