diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_execute.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_execute.rb index 18b2f5c8c..30d9235e1 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_execute.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_execute.rb @@ -12,8 +12,6 @@ def initialize end def handle_request(args) - return {} unless args[:params]['collection_name'] - datasource = ForestAdminRpcAgent::Facades::Container.datasource collection = get_collection_safe(datasource, args[:params]['collection_name']) filter = FilterFactory.from_plain_object(args[:params]['filter']) diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_form.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_form.rb index dc4f0b91c..2ccac87ad 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_form.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/action_form.rb @@ -12,8 +12,6 @@ def initialize end def handle_request(args) - return {} unless args[:params]['collection_name'] - datasource = ForestAdminRpcAgent::Facades::Container.datasource collection = get_collection_safe(datasource, args[:params]['collection_name']) filter = FilterFactory.from_plain_object(args[:params]['filter']) diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/aggregate.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/aggregate.rb index 3b9605390..6cfd03ad7 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/aggregate.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/aggregate.rb @@ -12,15 +12,13 @@ def initialize end def handle_request(args) - return {} unless args[:params]['collection_name'] - datasource = ForestAdminRpcAgent::Facades::Container.datasource collection = get_collection_safe(datasource, args[:params]['collection_name']) aggregation = Aggregation.new( operation: args[:params]['aggregation']['operation'], field: args[:params]['aggregation']['field'], - groups: args[:params]['aggregation']['groups'] + groups: args[:params]['aggregation']['groups'] || [] ) filter = FilterFactory.from_plain_object(args[:params]['filter']) diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/base_route.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/base_route.rb index 8e07c0651..c0ab2c20a 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/base_route.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/base_route.rb @@ -39,16 +39,16 @@ def register_rails(router) # Skip authentication for health check (root path) if @url == '/' - params = deep_symbolize_keys(request.query_parameters.merge(request.request_parameters)) - result = handle_request({ params: params.with_indifferent_access, caller: nil, request: request }) + params = extract_request_params(request) + result = handle_request({ params: params, caller: nil, request: request }) build_rails_response(result) else auth_middleware = ForestAdminRpcAgent::Middleware::Authentication.new(->(_env) { [200, {}, ['OK']] }) status, headers, response = auth_middleware.call(request.env) if status == 200 - params = deep_symbolize_keys(request.query_parameters.merge(request.request_parameters)) - result = handle_request({ params: params.with_indifferent_access, caller: headers[:caller], request: request }) + params = extract_request_params(request) + result = handle_request({ params: params, caller: headers[:caller], request: request }) build_rails_response(result) else [status, headers, response] @@ -87,15 +87,15 @@ def get_collection_safe(datasource, collection_name) private - def deep_symbolize_keys(obj) - case obj - when Hash - obj.transform_keys(&:to_sym).transform_values { |v| deep_symbolize_keys(v) } - when Array - obj.map { |v| deep_symbolize_keys(v) } - else - obj - end + # Merge path params (e.g. :collection_name from the URL) with query and body params so + # consumers that don't duplicate `collection_name` in the body (the Node datasource-rpc) + # still resolve the route correctly. + def extract_request_params(request) + request.path_parameters + .except(:controller, :action, :format) + .merge(request.query_parameters) + .merge(request.request_parameters) + .with_indifferent_access end def serialize_response(result) diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/chart.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/chart.rb index 25efeb113..bd948e76b 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/chart.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/chart.rb @@ -10,8 +10,6 @@ def initialize end def handle_request(args) - return {} unless args[:params]['collection_name'] - chart_name = args[:params]['chart'] parameters = args[:params]['parameters'] datasource = ForestAdminRpcAgent::Facades::Container.datasource diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/create.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/create.rb index 1e5f3be93..1620f42ca 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/create.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/create.rb @@ -8,8 +8,6 @@ def initialize end def handle_request(args) - return {} unless args[:params]['collection_name'] - datasource = ForestAdminRpcAgent::Facades::Container.datasource collection = get_collection_safe(datasource, args[:params]['collection_name']) diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/delete.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/delete.rb index 4bee7a43a..f4ee61d3f 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/delete.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/delete.rb @@ -12,8 +12,6 @@ def initialize end def handle_request(args) - return {} unless args[:params]['collection_name'] - datasource = ForestAdminRpcAgent::Facades::Container.datasource collection = get_collection_safe(datasource, args[:params]['collection_name']) filter = FilterFactory.from_plain_object(args[:params]['filter']) diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/list.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/list.rb index 6f1789ec5..09c5ba7f6 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/list.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/list.rb @@ -12,8 +12,6 @@ def initialize end def handle_request(args) - return {} unless args[:params]['collection_name'] - datasource = ForestAdminRpcAgent::Facades::Container.datasource collection = get_collection_safe(datasource, args[:params]['collection_name']) projection = Projection.new(args[:params]['projection']) diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/update.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/update.rb index 56c1d450d..729f74d34 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/update.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/update.rb @@ -10,8 +10,6 @@ def initialize end def handle_request(args) - return {} unless args[:params]['collection_name'] - datasource = ForestAdminRpcAgent::Facades::Container.datasource collection = get_collection_safe(datasource, args[:params]['collection_name']) filter = FilterFactory.from_plain_object(args[:params]['filter']) diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_execute_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_execute_spec.rb index c878aea81..16d87e941 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_execute_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_execute_spec.rb @@ -86,12 +86,12 @@ module Routes end end - context 'when collection_name is missing' do - let(:params) { {} } + context 'when the collection is unknown' do + let(:params) { { 'collection_name' => 'unknown', 'action' => 'noop', 'data' => {} } } - it 'returns an empty hash' do - response = route.handle_request(args) - expect(response).to eq({}) + it 'raises NotFoundError so the controller maps it to a 404 response' do + expect { route.handle_request(args) } + .to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) end end end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_form_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_form_spec.rb index 91c349fa4..314a464a4 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_form_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/action_form_spec.rb @@ -125,12 +125,12 @@ module Routes end end - context 'when collection_name is missing' do - let(:params) { {} } + context 'when the collection is unknown' do + let(:params) { { 'collection_name' => 'unknown', 'action' => 'noop' } } - it 'returns an empty hash' do - response = route.handle_request(args) - expect(response).to eq({}) + it 'raises NotFoundError so the controller maps it to a 404 response' do + expect { route.handle_request(args) } + .to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) end end end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/aggregate_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/aggregate_spec.rb index 47dde3391..ae4394557 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/aggregate_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/aggregate_spec.rb @@ -112,10 +112,24 @@ module Routes end end - context 'when collection_name is missing' do - it 'returns an empty hash' do - result = route.handle_request(params: {}) - expect(result).to eq({}) + context 'when the collection is unknown' do + it 'raises NotFoundError so the controller maps it to a 404 response' do + expect do + route.handle_request( + params: { 'collection_name' => 'unknown', 'aggregation' => { 'operation' => 'Count' } } + ) + end.to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) + end + end + + context 'when the aggregation payload omits :groups' do + it 'defaults groups to [] so Aggregation.new keeps its default' do + route.handle_request( + params: params.merge('aggregation' => { 'operation' => 'Count' }) + ) + + expect(ForestAdminDatasourceToolkit::Components::Query::Aggregation) + .to have_received(:new).with(operation: 'Count', field: nil, groups: []) end end end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/base_route_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/base_route_spec.rb index f414f6ac6..01a2f4cc4 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/base_route_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/base_route_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' require 'rails' require 'action_dispatch' +require 'active_support/core_ext/hash/indifferent_access' module ForestAdminRpcAgent module Routes @@ -178,6 +179,45 @@ def handle_request(_params) end end end + + describe '#extract_request_params' do + let(:request) do + instance_double( + ActionDispatch::Request, + path_parameters: { controller: 'forest_admin_rails/forest', action: 'index', + format: 'json', collection_name: 'books' }, + query_parameters: { 'page' => '2' }, + request_parameters: { 'filter' => { 'search' => 'hello' } } + ) + end + + it 'merges path, query and body params with indifferent access' do + params = route.send(:extract_request_params, request) + + expect(params['collection_name']).to eq('books') + expect(params[:collection_name]).to eq('books') + expect(params['page']).to eq('2') + expect(params['filter']['search']).to eq('hello') + end + + it 'strips Rails-internal path keys (controller, action, format)' do + params = route.send(:extract_request_params, request) + + expect(params).not_to have_key('controller') + expect(params).not_to have_key('action') + expect(params).not_to have_key('format') + end + + it 'lets body params override path params on key collision' do + allow(request).to receive_messages( + path_parameters: { collection_name: 'from_path' }, + query_parameters: {}, + request_parameters: { 'collection_name' => 'from_body' } + ) + + expect(route.send(:extract_request_params, request)['collection_name']).to eq('from_body') + end + end end end end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/chart_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/chart_spec.rb index 0760a5b28..9f7671363 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/chart_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/chart_spec.rb @@ -94,10 +94,11 @@ module Routes end end - context 'when collection_name is missing' do - it 'returns an empty hash' do - result = route.handle_request(params: {}) - expect(result).to eq({}) + context 'when the collection is unknown' do + it 'raises NotFoundError so the controller maps it to a 404 response' do + expect do + route.handle_request(params: { 'collection_name' => 'unknown', 'chart' => 'foo' }) + end.to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) end end end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/create_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/create_spec.rb index 5a8cb4387..28edb8042 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/create_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/create_spec.rb @@ -92,10 +92,11 @@ module Routes end end - context 'when collection_name is missing' do - it 'returns an empty hash' do - result = route.handle_request(params: {}) - expect(result).to eq({}) + context 'when the collection is unknown' do + it 'raises NotFoundError so the controller maps it to a 404 response' do + expect do + route.handle_request(params: { 'collection_name' => 'unknown', 'data' => [{}] }) + end.to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) end end end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/delete_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/delete_spec.rb index bc6c57fcf..71e5687cf 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/delete_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/delete_spec.rb @@ -56,10 +56,11 @@ module Routes end end - context 'when collection_name is missing' do - it 'returns an empty hash' do - result = route.handle_request(params: {}) - expect(result).to eq({}) + context 'when the collection is unknown' do + it 'raises NotFoundError so the controller maps it to a 404 response' do + expect do + route.handle_request(params: { 'collection_name' => 'unknown', 'filter' => {} }) + end.to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) end end end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/list_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/list_spec.rb index 2bef9b9e4..09c8a177b 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/list_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/list_spec.rb @@ -76,10 +76,11 @@ module Routes end end - context 'when collection_name is missing' do - it 'returns an empty hash' do - result = route.handle_request(params: {}) - expect(result).to eq({}) + context 'when the collection is unknown' do + it 'raises NotFoundError so the controller maps it to a 404 response' do + expect do + route.handle_request(params: { 'collection_name' => 'unknown' }) + end.to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) end end end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/update_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/update_spec.rb index 71cd1373e..f7a978216 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/update_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/routes/update_spec.rb @@ -83,10 +83,11 @@ module Routes end end - context 'when collection_name is missing' do - it 'returns an empty hash' do - result = route.handle_request(params: {}) - expect(result).to eq({}) + context 'when the collection is unknown' do + it 'raises NotFoundError so the controller maps it to a 404 response' do + expect do + route.handle_request(params: { 'collection_name' => 'unknown', 'filter' => {}, 'patch' => {} }) + end.to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) end end end