From 78b38912e84b99f16d818ba5d6e6e25f0ca7f4da Mon Sep 17 00:00:00 2001 From: Cristian Butiri Date: Wed, 28 Jun 2023 16:22:17 +0300 Subject: [PATCH 1/9] Improved support for multipart/form-data example recording --- lib/apipie/extractor/collector.rb | 3 +- lib/apipie/extractor/recorder.rb | 30 ++++++++++++- spec/lib/apipie/extractor/recorder_spec.rb | 49 +++++++++++++++++++--- 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/lib/apipie/extractor/collector.rb b/lib/apipie/extractor/collector.rb index e25f862d7..ed841d99d 100644 --- a/lib/apipie/extractor/collector.rb +++ b/lib/apipie/extractor/collector.rb @@ -24,10 +24,10 @@ def ignore_call?(record) end def handle_record(record) - add_to_records(record) if ignore_call?(record) Extractor.logger.info("REST_API: skipping #{record_to_s(record)}") else + add_to_records(record) refine_description(record) end end @@ -114,4 +114,3 @@ def record_to_s(record) end end end - diff --git a/lib/apipie/extractor/recorder.rb b/lib/apipie/extractor/recorder.rb index 5e2b857e9..a4da98337 100644 --- a/lib/apipie/extractor/recorder.rb +++ b/lib/apipie/extractor/recorder.rb @@ -44,7 +44,11 @@ def analyze_functional_test(test_context) @path = request.path @params = request.request_parameters if [:POST, :PUT, :PATCH, :DELETE].include?(@verb) - @request_data = @params + if request.content_type == "multipart/form-data" + @request_data = reformat_multipart_data(@params) + else + @request_data = @params + end else @query = request.query_string end @@ -68,6 +72,12 @@ def reformat_multipart_data(form) form.each do |key, attrs| if attrs.is_a?(String) lines << boundary << content_disposition(key) << "Content-Length: #{attrs.size}" << '' << attrs + elsif attrs.is_a?(Rack::Test::UploadedFile) || attrs.is_a?(ActionDispatch::Http::UploadedFile) + reformat_uploaded_file(boundary, attrs, key, lines) + elsif attrs.is_a?(Array) + reformat_array(boundary, attrs, key, lines) + elsif attrs.is_a?(TrueClass) || attrs.is_a?(FalseClass) + reformat_boolean(boundary, attrs, key, lines) else reformat_hash(boundary, attrs, lines) end @@ -88,6 +98,24 @@ def reformat_hash(boundary, attrs, lines) end end + def reformat_boolean(boundary, attrs, key, lines) + lines << boundary << content_disposition(key) + lines << '' << attrs.to_s + end + + def reformat_array(boundary, attrs, key, lines) + attrs.each do |item| + lines << boundary << content_disposition("#{key}[]") + lines << '' << item + end + end + + def reformat_uploaded_file(boundary, file, key, lines) + lines << boundary << %{#{content_disposition(key)}; filename="#{file.original_filename}"} + lines << "Content-Length: #{file.size}" << "Content-Type: #{file.content_type}" << "Content-Transfer-Encoding: binary" + lines << '' << %{... contents of "#{key}" ...} + end + def content_disposition(name) %{Content-Disposition: form-data; name="#{name}"} end diff --git a/spec/lib/apipie/extractor/recorder_spec.rb b/spec/lib/apipie/extractor/recorder_spec.rb index b3e028954..d2daa34d7 100644 --- a/spec/lib/apipie/extractor/recorder_spec.rb +++ b/spec/lib/apipie/extractor/recorder_spec.rb @@ -4,6 +4,11 @@ describe 'Apipie::Extractor::Recorder' do let(:recorder) { Apipie::Extractor::Recorder.new } + let(:controller) do + controller = ActionController::Metal.new + controller.set_request!(request) + controller + end describe '#analyse_controller' do subject do @@ -19,12 +24,6 @@ request end - let(:controller) do - controller = ActionController::Metal.new - controller.set_request!(request) - controller - end - it { is_expected.to eq(action) } context 'when a api_action_matcher is configured' do @@ -37,4 +36,42 @@ it { is_expected.to eq(matcher_action) } end end + + describe '#analyse_functional_test' do + context 'with multipart-form data' do + let(:test_context) do + double(controller:, request:, response: ActionDispatch::Response.new(200)) + end + + let(:file) do + instance_double( + ActionDispatch::Http::UploadedFile, + original_filename: 'file.txt', + content_type: 'text/plain', + size: '1MB' + ) + end + + let(:request) do + request = ActionDispatch::Request.new({}) + request.request_method = 'POST' + request.headers['Content-Type'] = "multipart/form-data" + request.request_parameters = { file: } + request + end + + subject do + recorder.analyse_controller(controller) + recorder.analyze_functional_test(test_context) + recorder.record[:request_data] + end + + before do + allow(file).to receive(:is_a?).and_return(false) + allow(file).to receive(:is_a?).with(ActionDispatch::Http::UploadedFile).and_return(true) + end + + it { is_expected.to include("filename=\"#{file.original_filename}\"") } + end + end end From dc6eb8a6506b8347d919f5ea89b067cd7c2aec5c Mon Sep 17 00:00:00 2001 From: Mathieu Jobin <99191+mathieujobin@users.noreply.github.com> Date: Fri, 14 Jul 2023 08:45:11 +0900 Subject: [PATCH 2/9] Update recorder_spec.rb --- spec/lib/apipie/extractor/recorder_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/apipie/extractor/recorder_spec.rb b/spec/lib/apipie/extractor/recorder_spec.rb index d2daa34d7..27bef0bc6 100644 --- a/spec/lib/apipie/extractor/recorder_spec.rb +++ b/spec/lib/apipie/extractor/recorder_spec.rb @@ -56,7 +56,7 @@ request = ActionDispatch::Request.new({}) request.request_method = 'POST' request.headers['Content-Type'] = "multipart/form-data" - request.request_parameters = { file: } + request.request_parameters = { file: file } request end From 5ee006c461f53745eb00ef80bbb13af9f52ec3fd Mon Sep 17 00:00:00 2001 From: Mathieu Jobin <99191+mathieujobin@users.noreply.github.com> Date: Fri, 14 Jul 2023 08:46:51 +0900 Subject: [PATCH 3/9] Update recorder_spec.rb --- spec/lib/apipie/extractor/recorder_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/apipie/extractor/recorder_spec.rb b/spec/lib/apipie/extractor/recorder_spec.rb index 27bef0bc6..aa1a6b204 100644 --- a/spec/lib/apipie/extractor/recorder_spec.rb +++ b/spec/lib/apipie/extractor/recorder_spec.rb @@ -40,7 +40,7 @@ describe '#analyse_functional_test' do context 'with multipart-form data' do let(:test_context) do - double(controller:, request:, response: ActionDispatch::Response.new(200)) + double(controller: controller, request: request, response: ActionDispatch::Response.new(200)) end let(:file) do From 8eac2aa7947578b0b95d4de290611af7507c1f3d Mon Sep 17 00:00:00 2001 From: Mathieu Jobin <99191+mathieujobin@users.noreply.github.com> Date: Fri, 14 Jul 2023 08:54:44 +0900 Subject: [PATCH 4/9] Rubocop Update recorder.rb --- lib/apipie/extractor/recorder.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/apipie/extractor/recorder.rb b/lib/apipie/extractor/recorder.rb index a4da98337..99180ed68 100644 --- a/lib/apipie/extractor/recorder.rb +++ b/lib/apipie/extractor/recorder.rb @@ -44,11 +44,7 @@ def analyze_functional_test(test_context) @path = request.path @params = request.request_parameters if [:POST, :PUT, :PATCH, :DELETE].include?(@verb) - if request.content_type == "multipart/form-data" - @request_data = reformat_multipart_data(@params) - else - @request_data = @params - end + @request_data = request.content_type == "multipart/form-data" ? reformat_multipart_data(@params) : @params else @query = request.query_string end @@ -70,13 +66,14 @@ def reformat_multipart_data(form) lines = ["Content-Type: multipart/form-data; boundary=#{MULTIPART_BOUNDARY}",''] boundary = "--#{MULTIPART_BOUNDARY}" form.each do |key, attrs| - if attrs.is_a?(String) + case attrs + when String lines << boundary << content_disposition(key) << "Content-Length: #{attrs.size}" << '' << attrs - elsif attrs.is_a?(Rack::Test::UploadedFile) || attrs.is_a?(ActionDispatch::Http::UploadedFile) + when Rack::Test::UploadedFile, ActionDispatch::Http::UploadedFile reformat_uploaded_file(boundary, attrs, key, lines) - elsif attrs.is_a?(Array) + when Array reformat_array(boundary, attrs, key, lines) - elsif attrs.is_a?(TrueClass) || attrs.is_a?(FalseClass) + when TrueClass, FalseClass reformat_boolean(boundary, attrs, key, lines) else reformat_hash(boundary, attrs, lines) From b40060f5d41b4bd1c429dcad6d2255023640c939 Mon Sep 17 00:00:00 2001 From: Mathieu Jobin <99191+mathieujobin@users.noreply.github.com> Date: Fri, 14 Jul 2023 08:56:55 +0900 Subject: [PATCH 5/9] Rubocop Update recorder_spec.rb --- spec/lib/apipie/extractor/recorder_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/lib/apipie/extractor/recorder_spec.rb b/spec/lib/apipie/extractor/recorder_spec.rb index aa1a6b204..b47b31dc8 100644 --- a/spec/lib/apipie/extractor/recorder_spec.rb +++ b/spec/lib/apipie/extractor/recorder_spec.rb @@ -39,8 +39,14 @@ describe '#analyse_functional_test' do context 'with multipart-form data' do + subject do + recorder.analyse_controller(controller) + recorder.analyze_functional_test(test_context) + recorder.record[:request_data] + end + let(:test_context) do - double(controller: controller, request: request, response: ActionDispatch::Response.new(200)) + instance_double(controller: controller, request: request, response: ActionDispatch::Response.new(200)) end let(:file) do @@ -55,17 +61,11 @@ let(:request) do request = ActionDispatch::Request.new({}) request.request_method = 'POST' - request.headers['Content-Type'] = "multipart/form-data" + request.headers['Content-Type'] = 'multipart/form-data' request.request_parameters = { file: file } request end - subject do - recorder.analyse_controller(controller) - recorder.analyze_functional_test(test_context) - recorder.record[:request_data] - end - before do allow(file).to receive(:is_a?).and_return(false) allow(file).to receive(:is_a?).with(ActionDispatch::Http::UploadedFile).and_return(true) From 82d99a3a2bb148204cdfa7d80a6a6c81ba05ebbb Mon Sep 17 00:00:00 2001 From: Mathieu Jobin <99191+mathieujobin@users.noreply.github.com> Date: Fri, 14 Jul 2023 09:56:25 +0900 Subject: [PATCH 6/9] switch back because instance_double does not return equivalent object --- lib/apipie/extractor/recorder.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/apipie/extractor/recorder.rb b/lib/apipie/extractor/recorder.rb index 99180ed68..afa48e5af 100644 --- a/lib/apipie/extractor/recorder.rb +++ b/lib/apipie/extractor/recorder.rb @@ -61,19 +61,18 @@ def parse_data(data) data end - def reformat_multipart_data(form) + def reformat_multipart_data(form) # rubocop:disable Style/CaseLikeIf form.empty? and return '' lines = ["Content-Type: multipart/form-data; boundary=#{MULTIPART_BOUNDARY}",''] boundary = "--#{MULTIPART_BOUNDARY}" form.each do |key, attrs| - case attrs - when String + if attrs.is_a?(String) lines << boundary << content_disposition(key) << "Content-Length: #{attrs.size}" << '' << attrs - when Rack::Test::UploadedFile, ActionDispatch::Http::UploadedFile + elsif attrs.is_a?(Rack::Test::UploadedFile) || attrs.is_a?(ActionDispatch::Http::UploadedFile) reformat_uploaded_file(boundary, attrs, key, lines) - when Array + elsif attrs.is_a?(Array) reformat_array(boundary, attrs, key, lines) - when TrueClass, FalseClass + elsif attrs.is_a?(TrueClass) || attrs.is_a?(FalseClass) reformat_boolean(boundary, attrs, key, lines) else reformat_hash(boundary, attrs, lines) From 4e402d4a32f1cbe4a41db1bba20868fc4345a356 Mon Sep 17 00:00:00 2001 From: Mathieu Jobin <99191+mathieujobin@users.noreply.github.com> Date: Fri, 14 Jul 2023 10:01:58 +0900 Subject: [PATCH 7/9] move down comment --- lib/apipie/extractor/recorder.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/apipie/extractor/recorder.rb b/lib/apipie/extractor/recorder.rb index afa48e5af..500182d5f 100644 --- a/lib/apipie/extractor/recorder.rb +++ b/lib/apipie/extractor/recorder.rb @@ -61,12 +61,12 @@ def parse_data(data) data end - def reformat_multipart_data(form) # rubocop:disable Style/CaseLikeIf + def reformat_multipart_data(form) form.empty? and return '' lines = ["Content-Type: multipart/form-data; boundary=#{MULTIPART_BOUNDARY}",''] boundary = "--#{MULTIPART_BOUNDARY}" form.each do |key, attrs| - if attrs.is_a?(String) + if attrs.is_a?(String) # rubocop:disable Style/CaseLikeIf lines << boundary << content_disposition(key) << "Content-Length: #{attrs.size}" << '' << attrs elsif attrs.is_a?(Rack::Test::UploadedFile) || attrs.is_a?(ActionDispatch::Http::UploadedFile) reformat_uploaded_file(boundary, attrs, key, lines) From 4212225ffa83464cefc291d223c83117e2b43605 Mon Sep 17 00:00:00 2001 From: Mathieu Jobin <99191+mathieujobin@users.noreply.github.com> Date: Fri, 14 Jul 2023 10:14:48 +0900 Subject: [PATCH 8/9] switch back to simple double --- spec/lib/apipie/extractor/recorder_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/apipie/extractor/recorder_spec.rb b/spec/lib/apipie/extractor/recorder_spec.rb index b47b31dc8..f5bfb5b8d 100644 --- a/spec/lib/apipie/extractor/recorder_spec.rb +++ b/spec/lib/apipie/extractor/recorder_spec.rb @@ -46,7 +46,7 @@ end let(:test_context) do - instance_double(controller: controller, request: request, response: ActionDispatch::Response.new(200)) + double(controller: controller, request: request, response: ActionDispatch::Response.new(200)) end let(:file) do From 9e8cac6d0ea76194b1b8a6d55e791e26d7dcc731 Mon Sep 17 00:00:00 2001 From: Mathieu Jobin <99191+mathieujobin@users.noreply.github.com> Date: Fri, 14 Jul 2023 10:16:04 +0900 Subject: [PATCH 9/9] ignore --- .rubocop_todo.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c8e9c2d66..98cd81a6a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1242,6 +1242,7 @@ RSpec/VerifiedDoubles: - 'spec/lib/apipie/apipies_controller_spec.rb' - 'spec/lib/apipie/extractor/writer_spec.rb' - 'spec/lib/validators/array_validator_spec.rb' + - 'spec/lib/apipie/extractor/recorder_spec.rb' # Offense count: 1 RSpec/VoidExpect: