Merge pull request #402 from ahx/fix-match-trailing-slash

Ignore trailing slashes when routing requests

Author: ahx

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Breaking: Trailing slashes are no longer ignored in dynamic paths. See [#403](https://github.com/ahx/openapi_first/issues/403).
 - Remove superflous `Test::Coverage.current_run, .plans, .install, .uninstall`
 - Update dependency `openapi_paramters` to >= 0.7.0, because that version fixes unpacking exploded deepObject paramters
 - Add `OpenapiFirst::Test::Configuration#ignore_unknown_response_status=`. When this is set to true, it won't complain about undefined response statuses it sees during a test run. Use with caution.

lib/openapi_first/router/path_template.rb

@@ -41,7 +41,7 @@ def build_pattern(template)
           part.start_with?('{') ? ALLOWED_PARAMETER_CHARACTERS : Regexp.escape(part)
         end
 
-        %r{^#{parts.join}/?$}
+        /^#{parts.join}$/
       end
     end
   end

spec/router/path_template_spec.rb

@@ -23,8 +23,8 @@
       expect(described_class.new('/{a}/{b}').match('/1/2')).to eq({ 'a' => '1', 'b' => '2' })
     end
 
-    it 'ignores trailing slashes in paths' do
-      expect(described_class.new('/{a}/{b}').match('/1/2/')).to eq({ 'a' => '1', 'b' => '2' })
+    it 'respect trailing slashes in paths' do
+      expect(described_class.new('/{a}/{b}').match('/1/2/')).to be_nil
     end
 
     it 'returns params with kebab-case names' do

spec/router_spec.rb

@@ -41,6 +41,16 @@
       expect(router.match('DELETE', '/b').error).to have_attributes(type: :method_not_allowed)
     end
 
+    it 'does not match a dynamic path with a trailing slash' do
+      match = router.match('GET', '/42/')
+      expect(match.request_definition).to be_nil
+    end
+
+    it 'does not match a static path with a trailing slash' do
+      match = router.match('GET', '/a/')
+      expect(match.request_definition).to be_nil
+    end
+
     context 'with ambiguous paths' do
       let(:requests) do
         [
@@ -55,6 +65,35 @@
       end
     end
 
+    context 'with trailing slashes in path definitions' do
+      let(:requests) do
+        [
+          double(path: '/{id}/', request_method: 'get'),
+          double(path: '/a/', request_method: 'get')
+        ]
+      end
+
+      it 'matches a dynamic path' do
+        match = router.match('GET', '/42/')
+        expect(match.request_definition.path).to eq('/{id}/')
+      end
+
+      it 'does not match a dynamic path with missing trailing slash' do
+        match = router.match('GET', '/42')
+        expect(match.error.type).to eq(:not_found)
+      end
+
+      it 'matches a static path' do
+        match = router.match('GET', '/a/')
+        expect(match.request_definition.path).to eq('/a/')
+      end
+
+      it 'does not match a static path with missing trailing slash' do
+        match = router.match('GET', '/a')
+        expect(match.error.type).to eq(:not_found)
+      end
+    end
+
     context 'with ambiguous paths reversed' do
       let(:requests) do
         [