issue #584: do not mutate route.path

- adds changelog entry

Author: LeFnord

CHANGELOG.md

@@ -9,6 +9,7 @@
 #### Fixes
 
 * [#580](https://github.com/ruby-grape/grape-swagger/pull/580): Issue #578: fixes duplicated path params - [@LeFnord](https://github.com/LeFnord).
+* [#585](https://github.com/ruby-grape/grape-swagger/pull/585): Issue #584: do not mutate route.path - [@LeFnord](https://github.com/LeFnord).
 
 * Your contribution here.
 

lib/grape-swagger/doc_methods/move_params.rb

@@ -8,15 +8,15 @@ def can_be_moved?(params, http_verb)
           move_methods.include?(http_verb) && includes_body_param?(params)
         end
 
-        def to_definition(params, route, definitions)
+        def to_definition(path, params, route, definitions)
           @definitions = definitions
           unify!(params)
 
           params_to_move = movable_params(params)
 
           return (params + correct_array_param(params_to_move)) if should_correct_array?(params_to_move)
 
-          params << parent_definition_of_params(params_to_move, route)
+          params << parent_definition_of_params(params_to_move, path, route)
 
           params
         end
@@ -33,8 +33,8 @@ def correct_array_param(param)
           param
         end
 
-        def parent_definition_of_params(params, route)
-          definition_name = OperationId.manipulate(parse_model(route.path))
+        def parent_definition_of_params(params, path, route)
+          definition_name = OperationId.manipulate(parse_model(path))
           referenced_definition = build_definition(definition_name, params, route.request_method.downcase)
           definition = @definitions[referenced_definition]
 

lib/grape-swagger/doc_methods/operation_id.rb

@@ -4,16 +4,12 @@ class OperationId
       class << self
         def build(route, path = nil)
           if route.options[:nickname]
-            operation_id = route.options[:nickname]
+            route.options[:nickname]
           else
             verb = route.request_method.to_s.downcase
-
             operation = manipulate(path) unless path.nil?
-
-            operation_id = "#{verb}#{operation}"
+            "#{verb}#{operation}"
           end
-
-          operation_id
         end
 
         def manipulate(path)

lib/grape-swagger/doc_methods/parse_params.rb

@@ -2,10 +2,8 @@ module GrapeSwagger
   module DocMethods
     class ParseParams
       class << self
-        def call(param, settings, route, definitions)
-          path = route.path
+        def call(param, settings, path, route, definitions)
           method = route.request_method
-
           additional_documentation = settings.fetch(:documentation, {})
           settings.merge!(additional_documentation)
           data_type = DataType.call(settings)

lib/grape-swagger/doc_methods/path_string.rb

@@ -3,7 +3,7 @@ module DocMethods
     class PathString
       class << self
         def build(route, options = {})
-          path = route.path
+          path = route.path.dup
           # always removing format
           path.sub!(/\(\.\w+?\)$/, '')
           path.sub!('(.:format)', '')

lib/grape-swagger/endpoint.rb

@@ -108,10 +108,10 @@ def method_object(route, options, path)
       method[:description] = description_object(route)
       method[:produces]    = produces_object(route, options[:produces] || options[:format])
       method[:consumes]    = consumes_object(route, options[:format])
-      method[:parameters]  = params_object(route)
+      method[:parameters]  = params_object(route, path)
       method[:security]    = security_object(route)
       method[:responses]   = response_object(route)
-      method[:tags]        = route.options.fetch(:tags, tag_object(route))
+      method[:tags]        = route.options.fetch(:tags, tag_object(route, path))
       method[:operationId] = GrapeSwagger::DocMethods::OperationId.build(route, path)
       method.delete_if { |_, value| value.blank? }
 
@@ -161,7 +161,7 @@ def consumes_object(route, format)
       mime_types
     end
 
-    def params_object(route)
+    def params_object(route, path)
       parameters = partition_params(route).map do |param, value|
         value = { required: false }.merge(value) if value.is_a?(Hash)
         _, value = default_type([[param, value]]).first if value == ''
@@ -170,11 +170,11 @@ def params_object(route)
         elsif value[:documentation]
           expose_params(value[:documentation][:type])
         end
-        GrapeSwagger::DocMethods::ParseParams.call(param, value, route, @definitions)
+        GrapeSwagger::DocMethods::ParseParams.call(param, value, path, route, @definitions)
       end
 
       if GrapeSwagger::DocMethods::MoveParams.can_be_moved?(parameters, route.request_method)
-        parameters = GrapeSwagger::DocMethods::MoveParams.to_definition(parameters, route, @definitions)
+        parameters = GrapeSwagger::DocMethods::MoveParams.to_definition(path, parameters, route, @definitions)
       end
 
       parameters
@@ -227,11 +227,11 @@ def apply_success_codes(route)
       [default_code]
     end
 
-    def tag_object(route)
+    def tag_object(route, path)
       version = GrapeSwagger::DocMethods::Version.get(route)
       version = [version] unless version.is_a?(Array)
       Array(
-        route.path.split('{')[0].split('/').reject(&:empty?).delete_if do |i|
+        path.split('{')[0].split('/').reject(&:empty?).delete_if do |i|
           i == route.prefix.to_s || version.map(&:to_s).include?(i)
         end.first
       )

spec/lib/move_params_spec.rb

@@ -91,18 +91,19 @@
   end
 
   describe 'parent_definition_of_params' do
+    let(:path) { '/in_body' }
     describe 'POST' do
-      let(:params) { paths['/in_body'][:post][:parameters] }
+      let(:params) { paths[path][:post][:parameters] }
       let(:options)  do
         {
           method: 'POST'
         }
       end
-      let(:env) { Rack::MockRequest.env_for('/in_body', options) }
+      let(:env) { Rack::MockRequest.env_for(path, options) }
       let(:request) { Grape::Request.new(env) }
 
       specify do
-        subject.to_definition(params, request, definitions)
+        subject.to_definition(path, params, request, definitions)
         expect(params).to eql(
           [
             { name: 'InBody', in: 'body', required: true, schema: { '$ref' => '#/definitions/postInBody' } }
@@ -120,11 +121,11 @@
           method: 'PUT'
         }
       end
-      let(:env) { Rack::MockRequest.env_for('/in_body', options) }
+      let(:env) { Rack::MockRequest.env_for(path, options) }
       let(:request) { Grape::Request.new(env) }
 
       specify do
-        subject.to_definition(params, request, definitions)
+        subject.to_definition(path, params, request, definitions)
         expect(params).to eql(
           [
             { in: 'path', name: 'key', description: nil, type: 'integer', format: 'int32', required: true },

spec/lib/path_string_spec.rb

@@ -6,7 +6,14 @@
   specify { expect(subject).to eql GrapeSwagger::DocMethods::PathString }
   specify { expect(subject).to respond_to :build }
 
-  describe 'operation_id_object' do
+  describe 'path_string_object' do
+    specify 'The original route path is not mutated' do
+      route = Struct.new(:version, :path).new
+      route.path = '/foo/:dynamic/bar'
+      subject.build(route, add_version: true)
+      expect(route.path).to eq '/foo/:dynamic/bar'
+    end
+
     describe 'version' do
       describe 'defaults: given, true' do
         let(:options) { { add_version: true } }

spec/lib/tag_name_description_spec.rb

@@ -15,11 +15,12 @@
   end
 
   describe '#build' do
-    subject { described_class.build(paths) }
+    let(:object) { described_class.build(paths) }
+
     describe 'empty paths' do
       let(:paths) { {} }
       specify do
-        expect(subject).to eql([])
+        expect(object).to eql([])
       end
     end
 
@@ -30,7 +31,7 @@
         end
 
         specify do
-          expect(subject).to eql [{ name: 'tags_given', description: 'Operations about tags_givens' }]
+          expect(object).to eql [{ name: 'tags_given', description: 'Operations about tags_givens' }]
         end
       end
 
@@ -40,7 +41,7 @@
         end
 
         specify do
-          expect(subject).to eql [{ name: 'tags_given', description: 'Operations about tags_givens' }]
+          expect(object).to eql [{ name: 'tags_given', description: 'Operations about tags_givens' }]
         end
       end
 
@@ -53,7 +54,7 @@
           end
 
           specify do
-            expect(subject).to eql [
+            expect(object).to eql [
               { name: 'tags_given', description: 'Operations about tags_givens' },
               { name: 'another_tag_given', description: 'Operations about another_tag_givens' }
             ]
@@ -68,7 +69,7 @@
           end
 
           specify do
-            expect(subject).to eql [{ name: 'tags_given', description: 'Operations about tags_givens' }]
+            expect(object).to eql [{ name: 'tags_given', description: 'Operations about tags_givens' }]
           end
         end
       end