From 2c98325b36876ccd1e84be23c00319472b837cbd Mon Sep 17 00:00:00 2001
From: Marcin Lulek <info@webreactor.eu>
Date: Tue, 13 Dec 2016 02:18:23 +0100
Subject: [PATCH] fixes #156 - do not append unexpected trailing slashes to
 routes with query params

---
 dev/src/pattern_lexer.js           |  9 ++++++++-
 dev/tests/spec/interpolate.spec.js | 27 ++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/dev/src/pattern_lexer.js b/dev/src/pattern_lexer.js
index f6ad89c..0607978 100644
--- a/dev/src/pattern_lexer.js
+++ b/dev/src/pattern_lexer.js
@@ -211,11 +211,18 @@
             if (! TOKENS.OS.trail) {
                 TOKENS.OS.trail = new RegExp('(?:'+ TOKENS.OS.id +')+$');
             }
-
+            // we need to replace the query separator with something else
+            // because .replace(TOKENS.OS.rRestore, '/') would place incorrect
+            // trailing slash and trailing slashes have meaning
+            if (! TOKENS.OS.fixQueryOptional) {
+                TOKENS.OS.fixQueryOptional = new RegExp('(:\\?|\\{\\?)');
+            }
             return pattern
+                        .replace(TOKENS.OS.fixQueryOptional, '__TRAILING_QUERY__$1')
                         .replace(TOKENS.OS.rgx, TOKENS.OS.save)
                         .replace(PARAMS_REGEXP, replaceFn)
                         .replace(TOKENS.OS.trail, '') // remove trailing
+                        .replace('__TRAILING_QUERY__', '') // remove saving token
                         .replace(TOKENS.OS.rRestore, '/'); // add slash between segments
         }
 
diff --git a/dev/tests/spec/interpolate.spec.js b/dev/tests/spec/interpolate.spec.js
index c89e6b3..08cc7fd 100644
--- a/dev/tests/spec/interpolate.spec.js
+++ b/dev/tests/spec/interpolate.spec.js
@@ -78,11 +78,36 @@ describe('Route.interpolate()', function(){
         }).toThrow( 'Generated string doesn\'t validate against `Route.rules`.' );
     });
 
-    it('should replace query segments', function(){
+    it('should replace required query segments', function(){
+        var a = crossroads.addRoute('/{foo}/{?query}');
+        expect( a.interpolate({foo: 'lorem', query: {some: 'test'}}) ).toEqual( '/lorem/?some=test' );
+        expect( a.interpolate({foo: 'dolor-sit', query: {multiple: 'params', works: 'fine'}}) ).toEqual( '/dolor-sit/?multiple=params&works=fine' );
+        expect( a.interpolate({foo: 'amet', query: {multiple: ['paramsWith', 'sameName'], works: 'fine2'}}) ).toEqual( '/amet/?multiple=paramsWith&multiple=sameName&works=fine2' );
+        expect( a.interpolate({foo: 'amet2', query: {"multiple[]": ['paramsWith', 'sameName'], works: 'fine2'}}) ).toEqual( '/amet2/?multiple[]=paramsWith&multiple[]=sameName&works=fine2' );
+    });
+
+    it('should replace optional query segments', function(){
         var a = crossroads.addRoute('/{foo}/:?query:');
         expect( a.interpolate({foo: 'lorem', query: {some: 'test'}}) ).toEqual( '/lorem/?some=test' );
         expect( a.interpolate({foo: 'dolor-sit', query: {multiple: 'params', works: 'fine'}}) ).toEqual( '/dolor-sit/?multiple=params&works=fine' );
         expect( a.interpolate({foo: 'amet', query: {multiple: ['paramsWith', 'sameName'], works: 'fine2'}}) ).toEqual( '/amet/?multiple=paramsWith&multiple=sameName&works=fine2' );
         expect( a.interpolate({foo: 'amet2', query: {"multiple[]": ['paramsWith', 'sameName'], works: 'fine2'}}) ).toEqual( '/amet2/?multiple[]=paramsWith&multiple[]=sameName&works=fine2' );
     });
+
+    it('should replace optional query segments without last slash', function(){
+        var a = crossroads.addRoute('/{foo}:?query:');
+        expect( a.interpolate({foo: 'lorem', query: {some: 'test'}, x:'x', y:'y', z:'z'}) ).toEqual( '/lorem?some=test' );
+        expect( a.interpolate({foo: 'dolor-sit', query: {multiple: 'params', works: 'fine'}}) ).toEqual( '/dolor-sit?multiple=params&works=fine' );
+        expect( a.interpolate({foo: 'amet', query: {multiple: ['paramsWith', 'sameName'], works: 'fine2'}}) ).toEqual( '/amet?multiple=paramsWith&multiple=sameName&works=fine2' );
+        expect( a.interpolate({foo: 'amet2', query: {"multiple[]": ['paramsWith', 'sameName'], works: 'fine2'}}) ).toEqual( '/amet2?multiple[]=paramsWith&multiple[]=sameName&works=fine2' );
+    });
+
+    it('should replace required query segments without last slash', function(){
+        var a = crossroads.addRoute('/{foo}{?query}');
+        expect( a.interpolate({foo: 'lorem', query: {some: 'test'}, x:'x', y:'y', z:'z'}) ).toEqual( '/lorem?some=test' );
+        expect( a.interpolate({foo: 'dolor-sit', query: {multiple: 'params', works: 'fine'}}) ).toEqual( '/dolor-sit?multiple=params&works=fine' );
+        expect( a.interpolate({foo: 'amet', query: {multiple: ['paramsWith', 'sameName'], works: 'fine2'}}) ).toEqual( '/amet?multiple=paramsWith&multiple=sameName&works=fine2' );
+        expect( a.interpolate({foo: 'amet2', query: {"multiple[]": ['paramsWith', 'sameName'], works: 'fine2'}}) ).toEqual( '/amet2?multiple[]=paramsWith&multiple[]=sameName&works=fine2' );
+    });
+
 });