From 2f4c6c81edb7145fecaa8d9210803d49b3124fe5 Mon Sep 17 00:00:00 2001 From: Oleksandr Rohachev Date: Fri, 5 Jul 2024 16:49:45 +0300 Subject: [PATCH 1/5] Implemented params wrapping through __register_action --- lib/rage/controller/api.rb | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/lib/rage/controller/api.rb b/lib/rage/controller/api.rb index e5c72223..85de7948 100644 --- a/lib/rage/controller/api.rb +++ b/lib/rage/controller/api.rb @@ -77,6 +77,22 @@ def __register_action(action) activerecord_loaded = defined?(::ActiveRecord) + wrap_parameters_chunk = if __wrap_parameters_key + <<~RUBY + options = self.class.__wrap_parameters_options + + wrapped_params = if options[:include] + @__params.slice(*[options[:include]].flatten) + elsif options[:exclude] + @__params.except(*[options[:exclude]].flatten) + else + @__params + end + + @__params = {self.class.__wrap_parameters_key => wrapped_params} + RUBY + end + class_eval <<~RUBY, __FILE__, __LINE__ + 1 def __run_#{action} #{if activerecord_loaded @@ -85,6 +101,7 @@ def __run_#{action} RUBY end} + #{wrap_parameters_chunk} #{before_actions_chunk} #{action} @@ -122,6 +139,7 @@ def __run_#{action} # @private attr_writer :__before_actions, :__after_actions, :__rescue_handlers + attr_accessor :__wrap_parameters_key, :__wrap_parameters_options # @private # pass the variable down to the child; the child will continue to use it until changes need to be made; @@ -130,6 +148,8 @@ def inherited(klass) klass.__before_actions = @__before_actions.freeze klass.__after_actions = @__after_actions.freeze klass.__rescue_handlers = @__rescue_handlers.freeze + klass.__wrap_parameters_key = __wrap_parameters_key + klass.__wrap_parameters_options = __wrap_parameters_options end # @private @@ -277,6 +297,20 @@ def skip_before_action(action_name, only: nil, except: nil) @__before_actions[i] = action end + # Initialize controller params wrapping into a nested hash. + # If initialized, params wrapping logic will be added to the controller. + # + # @param key [Symbol] key that the params hash will nested under + # @param options [Hash] wrapping options + # @example + # wrap_parameters :user, include: [:name, :age] + # @example + # wrap_parameters :user, exclude: :address + def wrap_parameters(key, options = {}) + @__wrap_parameters_key = key + @__wrap_parameters_options = options + end + private # used by `before_action` and `after_action` From 6035ae358a70c0808781d86a6218fd8695d85f57 Mon Sep 17 00:00:00 2001 From: Oleksandr Rohachev Date: Sun, 7 Jul 2024 11:37:28 +0300 Subject: [PATCH 2/5] Applied wrapped parameters merging into initial parameters hash; Added tests for parameters wrapping --- lib/rage/controller/api.rb | 6 +- spec/controller/api/wrap_parameters_spec.rb | 279 ++++++++++++++++++++ 2 files changed, 282 insertions(+), 3 deletions(-) create mode 100644 spec/controller/api/wrap_parameters_spec.rb diff --git a/lib/rage/controller/api.rb b/lib/rage/controller/api.rb index 85de7948..92454158 100644 --- a/lib/rage/controller/api.rb +++ b/lib/rage/controller/api.rb @@ -80,7 +80,7 @@ def __register_action(action) wrap_parameters_chunk = if __wrap_parameters_key <<~RUBY options = self.class.__wrap_parameters_options - + wrapped_params = if options[:include] @__params.slice(*[options[:include]].flatten) elsif options[:exclude] @@ -88,8 +88,8 @@ def __register_action(action) else @__params end - - @__params = {self.class.__wrap_parameters_key => wrapped_params} + + @__params = @__params.merge({self.class.__wrap_parameters_key => wrapped_params}) RUBY end diff --git a/spec/controller/api/wrap_parameters_spec.rb b/spec/controller/api/wrap_parameters_spec.rb new file mode 100644 index 00000000..e72397ec --- /dev/null +++ b/spec/controller/api/wrap_parameters_spec.rb @@ -0,0 +1,279 @@ +RSpec.describe RageController::API do + describe 'parameters wrapping logic' do + context 'when parameters wrapper is not declared' do + let(:controller) do + Class.new(RageController::API) do + def index + render json: params + end + end + end + + it "doesn't wrap the parameters" do + initial_params = {param: :value} + expected_result = {param: :value} + + expect(run_action(controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + + context 'when parameters wrapper is declared without options' do + let(:controller) do + Class.new(RageController::API) do + wrap_parameters :root + + def index + render json: params + end + end + end + + context "and wrapping root doesn't conflict with parameter key" do + it 'wraps the parameters into a nested hash' do + initial_params = {param: :value} + expected_result = {param: :value, root: {param: :value}} + + expect(run_action(controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + + context 'and wrapping root conflicts with parameter key' do + it 'wraps the parameters into a nested hash and overrides the conflicting key' do + initial_params = {root: :value, param: :value} + expected_result = {root: {root: :value, param: :value}, param: :value} + + expect(run_action(controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + end + + context 'when parameters wrapper is declared with :include option' do + context 'and :include option is set as Symbol' do + let(:controller) do + Class.new(RageController::API) do + wrap_parameters :root, include: :param_a + + def index + render json: params + end + end + end + + it 'wraps the param that is set to be included' do + initial_params = {param_a: :value, param_b: :value} + expected_result = {param_a: :value, param_b: :value, root: {param_a: :value}} + + expect(run_action(controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + + context 'and :include option is set as Array' do + let(:controller) do + Class.new(RageController::API) do + wrap_parameters :root, include: [:param_a, :param_b] + + def index + render json: params + end + end + end + + it 'wraps the params that are set to be included' do + initial_params = {param_a: :value, param_b: :value, param_c: :value} + expected_result = { + param_a: :value, + param_b: :value, + param_c: :value, + root: {param_a: :value, param_b: :value} + } + + expect(run_action(controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + end + + context 'when parameters wrapper is declared with :exclude option' do + context 'and :exclude option is set as Symbol' do + let(:controller) do + Class.new(RageController::API) do + wrap_parameters :root, exclude: :param_a + + def index + render json: params + end + end + end + + it 'wraps the params except the param that is set to be excluded' do + initial_params = {param_a: :value, param_b: :value} + expected_result = {param_a: :value, param_b: :value, root: {param_b: :value}} + + expect(run_action(controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + + context 'and :exclude option is set as Array' do + let(:controller) do + Class.new(RageController::API) do + wrap_parameters :root, exclude: [:param_a, :param_b] + + def index + render json: params + end + end + end + + it 'wraps the params except the params that are set to be excluded' do + initial_params = {param_a: :value, param_b: :value, param_c: :value} + expected_result = {param_a: :value, param_b: :value, param_c: :value, root: {param_c: :value}} + + expect(run_action(controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + end + + context 'when parameters wrapper is declared with both :exclude and :include options' do + let(:controller) do + Class.new(RageController::API) do + wrap_parameters :root, exclude: :param_a, include: :param_a + + def index + render json: params + end + end + end + + it 'wraps the params using the :include option' do + initial_params = {param_a: :value, param_b: :value} + expected_result = {param_a: :value, param_b: :value, root: {param_a: :value}} + + expect(run_action(controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + end + + describe 'controller inheritance' do + let(:grandchild_controller) do + Class.new(child_controller) do + def index + render json: params + end + end + end + + context 'when parameters wrapper is declared in parent controller' do + let(:parent_controller) do + Class.new(RageController::API) do + wrap_parameters :parent_root, include: [:parent_param] + + def index + render json: params + end + end + end + + context 'and parameters wrapper is declared in child controller' do + context 'and child wrapper is declared without options' do + let(:child_controller) do + Class.new(parent_controller) do + wrap_parameters :child_root + + def index + render json: params + end + end + end + + let(:initial_params) { {parent_param: :value, child_param: :value} } + let(:expected_result) do + { + parent_param: :value, + child_param: :value, + child_root: {parent_param: :value, child_param: :value} + } + end + + it 'wraps params of child controller using wrapping key of child controller without options' do + expect(run_action(child_controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + + it 'wraps params of grandchild controller using wrapping key of child controller without options' do + expect(run_action(grandchild_controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + + context 'and child wrapper is defined with options' do + let(:child_controller) do + Class.new(parent_controller) do + wrap_parameters :child_root, include: [:child_param] + + def index + render json: params + end + end + end + + let(:initial_params) { {parent_param: :value, child_param: :value} } + let(:expected_result) { {parent_param: :value, child_param: :value, child_root: {child_param: :value}} } + + it 'wraps params of child controller using wrapping key and options of child controller' do + expect(run_action(child_controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + + it 'wraps params of grandchild controller using wrapping key and options of child controller' do + expect(run_action(grandchild_controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + end + + context 'and parameters wrapper is not declared in child controller' do + let(:child_controller) do + Class.new(parent_controller) do + def index + render json: params + end + end + end + + let(:initial_params) { {parent_param: :value, child_param: :value} } + let(:expected_result) { {parent_param: :value, child_param: :value, parent_root: {parent_param: :value}} } + + it 'wraps params of child controller using wrapping key and options of parent controller' do + expect(run_action(child_controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + + it 'wraps params of grandchild controller using wrapping key and options of parent controller' do + expect(run_action(child_controller, :index, params: initial_params)).to match( + [200, instance_of(Hash), [expected_result.to_json]] + ) + end + end + end + end +end From cc2162f8419c8241588a0782ebe6455b368b8166 Mon Sep 17 00:00:00 2001 From: Oleksandr Rohachev Date: Fri, 19 Jul 2024 15:17:55 +0300 Subject: [PATCH 3/5] Added check for CONTENT_TYPE header that has to be present for wrapping to work --- lib/rage/controller/api.rb | 23 ++-- spec/controller/api/wrap_parameters_spec.rb | 124 +++++++++++++------- 2 files changed, 95 insertions(+), 52 deletions(-) diff --git a/lib/rage/controller/api.rb b/lib/rage/controller/api.rb index 92454158..8e0fb33b 100644 --- a/lib/rage/controller/api.rb +++ b/lib/rage/controller/api.rb @@ -79,17 +79,20 @@ def __register_action(action) wrap_parameters_chunk = if __wrap_parameters_key <<~RUBY - options = self.class.__wrap_parameters_options - - wrapped_params = if options[:include] - @__params.slice(*[options[:include]].flatten) - elsif options[:exclude] - @__params.except(*[options[:exclude]].flatten) - else - @__params + wrap_key = self.class.__wrap_parameters_key + if !@__params.key?(wrap_key) && @__env['CONTENT_TYPE'] + wrap_options = self.class.__wrap_parameters_options + + wrapped_params = if wrap_options[:include] + @__params.slice(*[wrap_options[:include]].flatten) + elsif wrap_options[:exclude] + @__params.except(*[wrap_options[:exclude]].flatten) + else + @__params + end + + @__params = @__params.merge({wrap_key => wrapped_params}) end - - @__params = @__params.merge({self.class.__wrap_parameters_key => wrapped_params}) RUBY end diff --git a/spec/controller/api/wrap_parameters_spec.rb b/spec/controller/api/wrap_parameters_spec.rb index e72397ec..f3a79104 100644 --- a/spec/controller/api/wrap_parameters_spec.rb +++ b/spec/controller/api/wrap_parameters_spec.rb @@ -13,9 +13,8 @@ def index initial_params = {param: :value} expected_result = {param: :value} - expect(run_action(controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] - ) + response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end @@ -31,24 +30,40 @@ def index end context "and wrapping root doesn't conflict with parameter key" do - it 'wraps the parameters into a nested hash' do - initial_params = {param: :value} - expected_result = {param: :value, root: {param: :value}} + context 'and CONTENT_TYPE header is blank' do + it "doesn't wrap the parameters into a nested hash" do + initial_params = {param: :value} + expected_result = {param: :value} - expect(run_action(controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] - ) + response = run_action(controller, :index, params: initial_params) + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) + end + end + + context 'and CONTENT_TYPE header is present' do + it 'wraps the parameters into a nested hash' do + initial_params = {param: :value} + expected_result = {param: :value, root: {param: :value}} + + response = run_action( + controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} + ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) + end end end context 'and wrapping root conflicts with parameter key' do - it 'wraps the parameters into a nested hash and overrides the conflicting key' do + it "doesn't wrap the parameters into a nested hash" do initial_params = {root: :value, param: :value} - expected_result = {root: {root: :value, param: :value}, param: :value} + expected_result = {root: :value, param: :value} - expect(run_action(controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] - ) + response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end end @@ -69,9 +84,8 @@ def index initial_params = {param_a: :value, param_b: :value} expected_result = {param_a: :value, param_b: :value, root: {param_a: :value}} - expect(run_action(controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] - ) + response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end @@ -95,9 +109,8 @@ def index root: {param_a: :value, param_b: :value} } - expect(run_action(controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] - ) + response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end end @@ -118,9 +131,8 @@ def index initial_params = {param_a: :value, param_b: :value} expected_result = {param_a: :value, param_b: :value, root: {param_b: :value}} - expect(run_action(controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] - ) + response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end @@ -139,9 +151,8 @@ def index initial_params = {param_a: :value, param_b: :value, param_c: :value} expected_result = {param_a: :value, param_b: :value, param_c: :value, root: {param_c: :value}} - expect(run_action(controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] - ) + response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end end @@ -161,9 +172,8 @@ def index initial_params = {param_a: :value, param_b: :value} expected_result = {param_a: :value, param_b: :value, root: {param_a: :value}} - expect(run_action(controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] - ) + response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end end @@ -210,15 +220,25 @@ def index end it 'wraps params of child controller using wrapping key of child controller without options' do - expect(run_action(child_controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] + response = run_action( + child_controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end it 'wraps params of grandchild controller using wrapping key of child controller without options' do - expect(run_action(grandchild_controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] + response = run_action( + grandchild_controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end @@ -237,15 +257,25 @@ def index let(:expected_result) { {parent_param: :value, child_param: :value, child_root: {child_param: :value}} } it 'wraps params of child controller using wrapping key and options of child controller' do - expect(run_action(child_controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] + response = run_action( + child_controller, + :index, + params: initial_params, + env: { 'CONTENT_TYPE' => "application/json" } ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end it 'wraps params of grandchild controller using wrapping key and options of child controller' do - expect(run_action(grandchild_controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] + response = run_action( + grandchild_controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end end @@ -263,15 +293,25 @@ def index let(:expected_result) { {parent_param: :value, child_param: :value, parent_root: {parent_param: :value}} } it 'wraps params of child controller using wrapping key and options of parent controller' do - expect(run_action(child_controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] - ) + response = run_action( + child_controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} + ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end it 'wraps params of grandchild controller using wrapping key and options of parent controller' do - expect(run_action(child_controller, :index, params: initial_params)).to match( - [200, instance_of(Hash), [expected_result.to_json]] + response = run_action( + child_controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end end From 9995a91bfcd690dcb2cd0a1ec980438af8c6eab6 Mon Sep 17 00:00:00 2001 From: Oleksandr Rohachev Date: Sun, 21 Jul 2024 23:29:35 +0300 Subject: [PATCH 4/5] Don't wrap :action and :controller params by default --- lib/rage/controller/api.rb | 6 ++-- spec/controller/api/wrap_parameters_spec.rb | 33 ++++++++++++++------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lib/rage/controller/api.rb b/lib/rage/controller/api.rb index 8e0fb33b..c63117b2 100644 --- a/lib/rage/controller/api.rb +++ b/lib/rage/controller/api.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class RageController::API + RESERVED_PARAMS = %i[action controller].freeze + class << self # @private # used by the router to register a new action; @@ -86,9 +88,9 @@ def __register_action(action) wrapped_params = if wrap_options[:include] @__params.slice(*[wrap_options[:include]].flatten) elsif wrap_options[:exclude] - @__params.except(*[wrap_options[:exclude]].flatten) + @__params.except(*([wrap_options[:exclude]].flatten + RESERVED_PARAMS)) else - @__params + @__params.except(*RESERVED_PARAMS) end @__params = @__params.merge({wrap_key => wrapped_params}) diff --git a/spec/controller/api/wrap_parameters_spec.rb b/spec/controller/api/wrap_parameters_spec.rb index f3a79104..e7acf2f8 100644 --- a/spec/controller/api/wrap_parameters_spec.rb +++ b/spec/controller/api/wrap_parameters_spec.rb @@ -41,9 +41,9 @@ def index end context 'and CONTENT_TYPE header is present' do - it 'wraps the parameters into a nested hash' do - initial_params = {param: :value} - expected_result = {param: :value, root: {param: :value}} + it 'wraps the parameters into a nested hash without the reserved params' do + initial_params = {param: :value, action: :action, controller: :controller} + expected_result = {param: :value, action: :action, controller: :controller, root: {param: :value}} response = run_action( controller, @@ -59,8 +59,8 @@ def index context 'and wrapping root conflicts with parameter key' do it "doesn't wrap the parameters into a nested hash" do - initial_params = {root: :value, param: :value} - expected_result = {root: :value, param: :value} + initial_params = {root: :value, param: :value, action: :action, controller: :controller} + expected_result = {root: :value, param: :value, action: :action, controller: :controller} response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) @@ -81,8 +81,14 @@ def index end it 'wraps the param that is set to be included' do - initial_params = {param_a: :value, param_b: :value} - expected_result = {param_a: :value, param_b: :value, root: {param_a: :value}} + initial_params = {param_a: :value, param_b: :value, action: :action, controller: :controller} + expected_result = { + param_a: :value, + param_b: :value, + action: :action, + controller: :controller, + root: {param_a: :value} + } response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) @@ -101,11 +107,13 @@ def index end it 'wraps the params that are set to be included' do - initial_params = {param_a: :value, param_b: :value, param_c: :value} + initial_params = {param_a: :value, param_b: :value, param_c: :value, action: :action, controller: :controller} expected_result = { param_a: :value, param_b: :value, param_c: :value, + action: :action, + controller: :controller, root: {param_a: :value, param_b: :value} } @@ -128,8 +136,13 @@ def index end it 'wraps the params except the param that is set to be excluded' do - initial_params = {param_a: :value, param_b: :value} - expected_result = {param_a: :value, param_b: :value, root: {param_b: :value}} + initial_params = {param_a: :value, param_b: :value, action: :action, controller: :controller} + expected_result = { + param_a: :value, + param_b: :value, action: :action, + controller: :controller, + root: {param_b: :value} + } response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) From c427449530ebd0656c30cfaf1ba27dc32ee1a7d7 Mon Sep 17 00:00:00 2001 From: Oleksandr Rohachev Date: Thu, 25 Jul 2024 11:09:59 +0300 Subject: [PATCH 5/5] Refactored wrapper code, added new specs --- lib/rage/controller/api.rb | 35 ++- spec/controller/api/wrap_parameters_spec.rb | 268 +++++++++----------- 2 files changed, 139 insertions(+), 164 deletions(-) diff --git a/lib/rage/controller/api.rb b/lib/rage/controller/api.rb index c63117b2..aa4f937b 100644 --- a/lib/rage/controller/api.rb +++ b/lib/rage/controller/api.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class RageController::API - RESERVED_PARAMS = %i[action controller].freeze - class << self # @private # used by the router to register a new action; @@ -84,16 +82,14 @@ def __register_action(action) wrap_key = self.class.__wrap_parameters_key if !@__params.key?(wrap_key) && @__env['CONTENT_TYPE'] wrap_options = self.class.__wrap_parameters_options - - wrapped_params = if wrap_options[:include] - @__params.slice(*[wrap_options[:include]].flatten) - elsif wrap_options[:exclude] - @__params.except(*([wrap_options[:exclude]].flatten + RESERVED_PARAMS)) - else - @__params.except(*RESERVED_PARAMS) - end - - @__params = @__params.merge({wrap_key => wrapped_params}) + wrapped_params = if wrap_options[:include].any? + @__params.slice(*wrap_options[:include]) + else + params_to_exclude_by_default = %i[action controller] + @__params.except(*(wrap_options[:exclude] + params_to_exclude_by_default)) + end + + @__params[wrap_key] = wrapped_params end RUBY end @@ -304,16 +300,19 @@ def skip_before_action(action_name, only: nil, except: nil) # Initialize controller params wrapping into a nested hash. # If initialized, params wrapping logic will be added to the controller. + # Params get wrapped only if the CONTENT_TYPE header is present and params hash doesn't contain a param that + # has the same name as the wrapper key. # - # @param key [Symbol] key that the params hash will nested under - # @param options [Hash] wrapping options + # @param key [Symbol] key that the wrapped params hash will nested under + # @param include [Array] array of params that should be included to the wrapped params hash + # @param exclude [Array] array of params that should be excluded from the wrapped params hash # @example - # wrap_parameters :user, include: [:name, :age] + # wrap_parameters :user, include: %i[name age] # @example - # wrap_parameters :user, exclude: :address - def wrap_parameters(key, options = {}) + # wrap_parameters :user, exclude: %i[address] + def wrap_parameters(key, include: [], exclude: []) @__wrap_parameters_key = key - @__wrap_parameters_options = options + @__wrap_parameters_options = {include:, exclude:} end private diff --git a/spec/controller/api/wrap_parameters_spec.rb b/spec/controller/api/wrap_parameters_spec.rb index e7acf2f8..cc044a8a 100644 --- a/spec/controller/api/wrap_parameters_spec.rb +++ b/spec/controller/api/wrap_parameters_spec.rb @@ -69,25 +69,26 @@ def index end context 'when parameters wrapper is declared with :include option' do - context 'and :include option is set as Symbol' do - let(:controller) do - Class.new(RageController::API) do - wrap_parameters :root, include: :param_a + let(:controller) do + Class.new(RageController::API) do + wrap_parameters :root, include: %i[param_a param_b] - def index - render json: params - end + def index + render json: params end end + end - it 'wraps the param that is set to be included' do - initial_params = {param_a: :value, param_b: :value, action: :action, controller: :controller} + context 'and params to include are present in request' do + it 'wraps the params that are set to be included' do + initial_params = {param_a: :value, param_b: :value, param_c: :value, action: :action, controller: :controller} expected_result = { param_a: :value, param_b: :value, + param_c: :value, action: :action, controller: :controller, - root: {param_a: :value} + root: {param_a: :value, param_b: :value} } response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) @@ -95,27 +96,10 @@ def index end end - context 'and :include option is set as Array' do - let(:controller) do - Class.new(RageController::API) do - wrap_parameters :root, include: [:param_a, :param_b] - - def index - render json: params - end - end - end - - it 'wraps the params that are set to be included' do - initial_params = {param_a: :value, param_b: :value, param_c: :value, action: :action, controller: :controller} - expected_result = { - param_a: :value, - param_b: :value, - param_c: :value, - action: :action, - controller: :controller, - root: {param_a: :value, param_b: :value} - } + context "and params to include aren't present in request" do + it 'adds empty hash under wrapping key to params' do + initial_params = {param_c: :value, action: :action, controller: :controller} + expected_result = {param_c: :value, action: :action, controller: :controller, root: {}} response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) @@ -124,24 +108,26 @@ def index end context 'when parameters wrapper is declared with :exclude option' do - context 'and :exclude option is set as Symbol' do - let(:controller) do - Class.new(RageController::API) do - wrap_parameters :root, exclude: :param_a + let(:controller) do + Class.new(RageController::API) do + wrap_parameters :root, exclude: %i[param_a param_b] - def index - render json: params - end + def index + render json: params end end + end - it 'wraps the params except the param that is set to be excluded' do - initial_params = {param_a: :value, param_b: :value, action: :action, controller: :controller} + context 'and params to exclude are present in request' do + it 'wraps the params except those that are set to be excluded and those that need to be excluded by default' do + initial_params = {param_a: :value, param_b: :value, param_c: :value, action: :action, controller: :controller} expected_result = { param_a: :value, - param_b: :value, action: :action, + param_b: :value, + param_c: :value, + action: :action, controller: :controller, - root: {param_b: :value} + root: {param_c: :value} } response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) @@ -149,20 +135,10 @@ def index end end - context 'and :exclude option is set as Array' do - let(:controller) do - Class.new(RageController::API) do - wrap_parameters :root, exclude: [:param_a, :param_b] - - def index - render json: params - end - end - end - - it 'wraps the params except the params that are set to be excluded' do - initial_params = {param_a: :value, param_b: :value, param_c: :value} - expected_result = {param_a: :value, param_b: :value, param_c: :value, root: {param_c: :value}} + context "and params to exclude aren't present in request" do + it 'wraps the params except those that need to be excluded by default ' do + initial_params = {param_c: :value, action: :action, controller: :controller} + expected_result = {param_c: :value, action: :action, controller: :controller, root: {param_c: :value}} response = run_action(controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"}) expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) @@ -173,7 +149,7 @@ def index context 'when parameters wrapper is declared with both :exclude and :include options' do let(:controller) do Class.new(RageController::API) do - wrap_parameters :root, exclude: :param_a, include: :param_a + wrap_parameters :root, exclude: %i[param_a], include: %i[param_a] def index render json: params @@ -189,77 +165,112 @@ def index expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end end - end - describe 'controller inheritance' do - let(:grandchild_controller) do - Class.new(child_controller) do - def index - render json: params + context 'controller inheritance' do + let(:grandchild_controller) do + Class.new(child_controller) do + def index + render json: params + end end end - end - context 'when parameters wrapper is declared in parent controller' do - let(:parent_controller) do - Class.new(RageController::API) do - wrap_parameters :parent_root, include: [:parent_param] + context 'when parameters wrapper is declared in parent controller' do + let(:parent_controller) do + Class.new(RageController::API) do + wrap_parameters :parent_root, include: %i[parent_param] - def index - render json: params + def index + render json: params + end end end - end - context 'and parameters wrapper is declared in child controller' do - context 'and child wrapper is declared without options' do - let(:child_controller) do - Class.new(parent_controller) do - wrap_parameters :child_root + context 'and parameters wrapper is declared in child controller' do + context 'and child wrapper is declared without options' do + let(:child_controller) do + Class.new(parent_controller) do + wrap_parameters :child_root - def index - render json: params + def index + render json: params + end end end - end - let(:initial_params) { {parent_param: :value, child_param: :value} } - let(:expected_result) do - { - parent_param: :value, - child_param: :value, - child_root: {parent_param: :value, child_param: :value} - } - end + let(:initial_params) { {parent_param: :value, child_param: :value} } + let(:expected_result) do + { + parent_param: :value, + child_param: :value, + child_root: {parent_param: :value, child_param: :value} + } + end - it 'wraps params of child controller using wrapping key of child controller without options' do - response = run_action( - child_controller, - :index, - params: initial_params, - env: {'CONTENT_TYPE' => "application/json"} - ) + it 'wraps params of child controller using wrapping key of child controller without options' do + response = run_action( + child_controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} + ) - expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) + end + + it 'wraps params of grandchild controller using wrapping key of child controller without options' do + response = run_action( + grandchild_controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} + ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) + end end - it 'wraps params of grandchild controller using wrapping key of child controller without options' do - response = run_action( - grandchild_controller, - :index, - params: initial_params, - env: {'CONTENT_TYPE' => "application/json"} - ) + context 'and child wrapper is defined with options' do + let(:child_controller) do + Class.new(parent_controller) do + wrap_parameters :child_root, include: %i[child_param] - expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) + def index + render json: params + end + end + end + + let(:initial_params) { {parent_param: :value, child_param: :value} } + let(:expected_result) { {parent_param: :value, child_param: :value, child_root: {child_param: :value}} } + + it 'wraps params of child controller using wrapping key and options of child controller' do + response = run_action( + child_controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} + ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) + end + + it 'wraps params of grandchild controller using wrapping key and options of child controller' do + response = run_action( + grandchild_controller, + :index, + params: initial_params, + env: {'CONTENT_TYPE' => "application/json"} + ) + + expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) + end end end - context 'and child wrapper is defined with options' do + context 'and parameters wrapper is not declared in child controller' do let(:child_controller) do Class.new(parent_controller) do - wrap_parameters :child_root, include: [:child_param] - def index render json: params end @@ -267,22 +278,22 @@ def index end let(:initial_params) { {parent_param: :value, child_param: :value} } - let(:expected_result) { {parent_param: :value, child_param: :value, child_root: {child_param: :value}} } + let(:expected_result) { {parent_param: :value, child_param: :value, parent_root: {parent_param: :value}} } - it 'wraps params of child controller using wrapping key and options of child controller' do + it 'wraps params of child controller using wrapping key and options of parent controller' do response = run_action( child_controller, :index, params: initial_params, - env: { 'CONTENT_TYPE' => "application/json" } + env: {'CONTENT_TYPE' => "application/json"} ) expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) end - it 'wraps params of grandchild controller using wrapping key and options of child controller' do + it 'wraps params of grandchild controller using wrapping key and options of parent controller' do response = run_action( - grandchild_controller, + child_controller, :index, params: initial_params, env: {'CONTENT_TYPE' => "application/json"} @@ -292,41 +303,6 @@ def index end end end - - context 'and parameters wrapper is not declared in child controller' do - let(:child_controller) do - Class.new(parent_controller) do - def index - render json: params - end - end - end - - let(:initial_params) { {parent_param: :value, child_param: :value} } - let(:expected_result) { {parent_param: :value, child_param: :value, parent_root: {parent_param: :value}} } - - it 'wraps params of child controller using wrapping key and options of parent controller' do - response = run_action( - child_controller, - :index, - params: initial_params, - env: {'CONTENT_TYPE' => "application/json"} - ) - - expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) - end - - it 'wraps params of grandchild controller using wrapping key and options of parent controller' do - response = run_action( - child_controller, - :index, - params: initial_params, - env: {'CONTENT_TYPE' => "application/json"} - ) - - expect(response).to match([200, instance_of(Hash), [expected_result.to_json]]) - end - end end end end