From 23512bea1412744fe66033abb4dab8babe8e36a5 Mon Sep 17 00:00:00 2001
From: tenma <brvnocasteleiro@gmail.com>
Date: Mon, 19 Aug 2019 22:43:50 +0100
Subject: [PATCH] [CORE][ROUTES] Update urlmapper to search dynamic routes
 before static ones when generating URLs.

This solves the problem of routes that differ only in having
or not $_GET params. The ones not having params (static) were
being matched first during URL generation.

The way this problem was solved was by separating the $reverse
array in both $reverse_statics and $reverse_dynamics and explicitly
traversing this last one first in the generation function. Note that
maintaining the $reverse array and unshifting dynamic routes to its
head ( and therefore to the front of the static ones ) doesn't work
since even among dynamic routes the order of arrival should be kept.
---
 lib/urlmapper.php | 94 ++++++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 42 deletions(-)

diff --git a/lib/urlmapper.php b/lib/urlmapper.php
index 01e4e6fa6c..f2a93b2fbe 100644
--- a/lib/urlmapper.php
+++ b/lib/urlmapper.php
@@ -57,7 +57,8 @@ class URLMapper
 
     protected $statics = [];
     protected $variables = [];
-    protected $reverse = [];
+    protected $reverse_dynamics = [];
+    protected $reverse_statics = [];
     protected $allpaths = [];
 
     /**
@@ -97,10 +98,10 @@ class URLMapper
 
         if (empty($paramNames)) {
             $this->statics[$path] = $args;
-            if (array_key_exists($action, $this->reverse)) {
-                $this->reverse[$action][] = [$args, $path];
+            if (array_key_exists($action, $this->reverse_statics)) {
+                $this->reverse_statics[$action][] = [$args, $path];
             } else {
-                $this->reverse[$action] = [[$args, $path]];
+                $this->reverse_statics[$action] = [[$args, $path]];
             }
         } else {
             // fix for the code that still make improper use of this function's params
@@ -115,7 +116,6 @@ class URLMapper
             // $variables is used for path matching, so we can't store invalid routes
             if ($should) {
                 $regex = self::makeRegex($path, $paramPatterns);
-
                 if (isset($this->variables[$regex]) || !$acceptHeaders) {
                     $this->variables[$regex] = [$args, $paramNames];
                 } else {
@@ -128,10 +128,10 @@ class URLMapper
 
             $format = $this->makeFormat($path);
 
-            if (array_key_exists($action, $this->reverse)) {
-                $this->reverse[$action][] = [$args, $format, $paramNames];
+            if (array_key_exists($action, $this->reverse_dynamics)) {
+                $this->reverse_dynamics[$action][] = [$args, $format, $paramNames];
             } else {
-                $this->reverse[$action] = [[$args, $format, $paramNames]];
+                $this->reverse_dynamics[$action] = [[$args, $format, $paramNames]];
             }
         }
     }
@@ -164,48 +164,58 @@ class URLMapper
 
         $action = $args[self::ACTION];
 
-        if (!array_key_exists($action, $this->reverse)) {
+        if (!array_key_exists($action, $this->reverse_dynamics) && !array_key_exists($action, $this->reverse_statics)) {
             throw new Exception(sprintf('No candidate paths for action "%s"', $action));
         }
 
-        $candidates = $this->reverse[$action];
+        $candidates = $this->reverse_dynamics[$action];
 
         foreach ($candidates as $candidate) {
-            if (count($candidate) == 2) { // static
-                list($tryArgs, $tryPath) = $candidate;
-                foreach ($tryArgs as $key => $value) {
-                    if (!array_key_exists($key, $args) || $args[$key] != $value) {
-                        // next candidate
-                        continue 2;
-                    }
+            list($tryArgs, $format, $paramNames) = $candidate;
+
+            foreach ($tryArgs as $key => $value) {
+                if (!array_key_exists($key, $args) || $args[$key] != $value) {
+                    // next candidate
+                    continue 2;
                 }
-                // success
-                $path = $tryPath;
-            } else {
-                list($tryArgs, $format, $paramNames) = $candidate;
-
-                foreach ($tryArgs as $key => $value) {
-                    if (!array_key_exists($key, $args) || $args[$key] != $value) {
-                        // next candidate
-                        continue 2;
-                    }
-                }
-
-                // success
-
-                $toFormat = [];
-
-                foreach ($paramNames as $name) {
-                    if (!array_key_exists($name, $args)) {
-                        // next candidate
-                        continue 2;
-                    }
-                    $toFormat[] = $args[$name];
-                }
-
-                $path = vsprintf($format, $toFormat);
             }
 
+            // success
+            $toFormat = [];
+
+            foreach ($paramNames as $name) {
+                if (!array_key_exists($name, $args)) {
+                    // next candidate
+                    continue 2;
+                }
+                $toFormat[] = $args[$name];
+            }
+
+            $path = vsprintf($format, $toFormat);
+
+            if (!empty($qstring)) {
+                $formatted = http_build_query($qstring);
+                $path .= '?' . $formatted;
+            }
+
+            return $path;
+        }
+
+        $candidates = $this->reverse_statics[$action];
+
+        foreach ($candidates as $candidate) {
+            list($tryArgs, $tryPath) = $candidate;
+
+            foreach ($tryArgs as $key => $value) {
+                if (!array_key_exists($key, $args) || $args[$key] != $value) {
+                    // next candidate
+                    continue 2;
+                }
+            }
+
+            // success
+            $path = $tryPath;
+
             if (!empty($qstring)) {
                 $formatted = http_build_query($qstring);
                 $path .= '?' . $formatted;