From cbe89a01d9ea6ea88dd8011bc322432278342b5f Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Wed, 20 May 2026 17:21:09 +0200 Subject: [PATCH 1/6] fix(rpc): action with file --- .../Utils/rpc_client.rb | 20 +++--- .../forest_admin_datasource_rpc/collection.rb | 25 +++++++- .../collection_spec.rb | 64 ++++++++++++++++++- .../utils/rpc_client_spec.rb | 18 ++++++ .../routes/action_execute.rb | 27 +++++++- 5 files changed, 138 insertions(+), 16 deletions(-) diff --git a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb index be755d5a0..49bdef36c 100644 --- a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb +++ b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb @@ -31,17 +31,20 @@ def initialize(api_url, auth_secret) end # rubocop:disable Metrics/ParameterLists - def call_rpc(endpoint, caller: nil, method: :get, payload: nil, symbolize_keys: false, if_none_match: nil) + def call_rpc(endpoint, caller: nil, method: :get, payload: nil, symbolize_keys: false, + if_none_match: nil, with_response: false) response = make_request(endpoint, caller: caller, method: method, payload: payload, symbolize_keys: symbolize_keys, if_none_match: if_none_match) - handle_response(response) - end + return NotModified if response.status == HTTP_NOT_MODIFIED + raise_appropriate_error(response) unless response.success? + + with_response ? response : response.body + end # rubocop:enable Metrics/ParameterLists def fetch_schema(endpoint, if_none_match: nil) - response = make_request(endpoint, method: :get, symbolize_keys: true, if_none_match: if_none_match) - handle_response(response) + call_rpc(endpoint, method: :get, symbolize_keys: true, if_none_match: if_none_match) end private @@ -111,13 +114,6 @@ def generate_signature(timestamp) OpenSSL::HMAC.hexdigest('SHA256', @auth_secret, timestamp) end - def handle_response(response) - return response.body if response.success? - return NotModified if response.status == HTTP_NOT_MODIFIED - - raise_appropriate_error(response) - end - def raise_appropriate_error(response) error_body = parse_error_body(response) status = response.status diff --git a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb index 46a17c196..b662e597f 100644 --- a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb +++ b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb @@ -1,4 +1,7 @@ require 'base64' +require 'cgi' +require 'json' +require 'stringio' module ForestAdminDatasourceRpc class Collection < ForestAdminDatasourceToolkit::Collection @@ -127,7 +130,12 @@ def execute(caller, name, data, filter = nil) "Forwarding '#{@name}' action #{name} call to the Rpc agent on #{url}." ) - @client.call_rpc(url, caller: caller, method: :post, payload: params, symbolize_keys: true) + response = @client.call_rpc(url, caller: caller, method: :post, payload: params, + symbolize_keys: true, with_response: true) + + return build_file_result(response) if response.headers['x-forest-action-type'] == 'File' + + response.body end def get_form(caller, name, data = nil, filter = nil, metas = nil) @@ -176,5 +184,20 @@ def encode_form_data(data) end end end + + def build_file_result(response) + response_headers_header = response.headers['x-forest-action-response-headers'] + file_name_header = response.headers['x-forest-action-file-name'] + + result = { + type: 'File', + mime_type: response.headers['content-type'], + name: CGI.unescape(file_name_header.to_s), + stream: response.body.to_s + } + result[:response_headers] = JSON.parse(response_headers_header) if response_headers_header + + result + end end end diff --git a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb index 0d307ec67..97ac034be 100644 --- a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb +++ b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb @@ -13,7 +13,14 @@ module ForestAdminDatasourceRpc allow(Utils::RpcClient).to receive(:new).and_return(rpc_client) end + let(:raw_response) do + instance_double(Faraday::Response, body: {}, headers: {}, status: 200, success?: true) + end let(:rpc_client) { instance_double(Utils::RpcClient, call_rpc: {}) } + + before do + allow(rpc_client).to receive(:call_rpc).with(any_args, hash_including(with_response: true)).and_return(raw_response) + end let(:datasource) { Datasource.new({ uri: 'http://localhost' }, introspection) } let(:collection) { datasource.get_collection('Product') } let(:caller) { build_caller } @@ -275,10 +282,11 @@ module ForestAdminDatasourceRpc expect(rpc_client).to have_received(:call_rpc) do |_url, options| expect(options[:symbolize_keys]).to be(true) + expect(options[:with_response]).to be(true) end end - it 'returns the action result as-is so :type and other keys reach ActionResult.parse' do + it 'returns the parsed body so :type and other keys reach ActionResult.parse' do success_result = { type: 'Success', message: 'ok', @@ -286,13 +294,65 @@ module ForestAdminDatasourceRpc html: nil, response_headers: {} } - allow(rpc_client).to receive(:call_rpc).and_return(success_result) + allow(raw_response).to receive(:body).and_return(success_result) result = collection.execute(caller, 'my_action', {}) expect(result).to eq(success_result) expect(result[:type]).to eq('Success') end + + context 'when the server replies with X-Forest-Action-Type=File' do + let(:file_body) { 'binary-payload' } + let(:raw_response) do + instance_double( + Faraday::Response, + body: file_body, + headers: { + 'content-type' => 'application/pdf', + 'x-forest-action-type' => 'File', + 'x-forest-action-file-name' => CGI.escape('report final.pdf'), + 'x-forest-action-response-headers' => { 'set-cookie' => 'token=xyz' }.to_json + }, + status: 200, + success?: true + ) + end + + it 'rebuilds a File action result from the response headers and body' do + result = collection.execute(caller, 'download', {}) + + expect(result[:type]).to eq('File') + expect(result[:mime_type]).to eq('application/pdf') + expect(result[:name]).to eq('report final.pdf') + expect(result[:response_headers]).to eq({ 'set-cookie' => 'token=xyz' }) + expect(result[:stream]).to eq(file_body) + end + + context 'when response_headers header is absent' do + let(:raw_response) do + instance_double( + Faraday::Response, + body: 'hi', + headers: { + 'content-type' => 'text/plain', + 'x-forest-action-type' => 'File', + 'x-forest-action-file-name' => 'note.txt' + }, + status: 200, + success?: true + ) + end + + it 'omits response_headers from the rebuilt result' do + result = collection.execute(caller, 'download', {}) + + expect(result[:type]).to eq('File') + expect(result[:name]).to eq('note.txt') + expect(result).not_to have_key(:response_headers) + end + end + end end context 'when call get_form' do diff --git a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb index 5ff24a78d..59bae1edf 100644 --- a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb +++ b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb @@ -97,6 +97,24 @@ module Utils expect(result).to eq({}) end + + context 'when called with with_response: true' do + it 'returns the Faraday::Response object instead of the body' do + result = rpc_client.call_rpc('/rpc/test', method: :post, with_response: true) + + expect(result).to be(response) + expect(result.body).to eq({}) + expect(result.headers).to eq(response_headers) + end + end + + context 'when called without with_response (default)' do + it 'returns the response body for backward compatibility' do + result = rpc_client.call_rpc('/rpc/test', method: :post) + + expect(result).to eq({}) + end + end end describe '#fetch_schema' do 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 30d9235e1..8657bbabd 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 @@ -1,3 +1,5 @@ +require 'cgi' +require 'json' require 'jsonapi-serializers' module ForestAdminRpcAgent @@ -18,7 +20,30 @@ def handle_request(args) data = args[:params]['data'] action = args[:params]['action'] - collection.execute(args[:caller], action, data, filter) + result = collection.execute(args[:caller], action, data, filter) + + return build_file_response(result) if file_result?(result) + + result + end + + private + + def file_result?(result) + result.is_a?(Hash) && result[:type] == 'File' + end + + def build_file_response(result) + encoded_name = CGI.escape(result[:name].to_s) + headers = { + 'Content-Type' => result[:mime_type], + 'Content-Disposition' => %(attachment; filename="#{encoded_name}"), + 'X-Forest-Action-Type' => 'File', + 'X-Forest-Action-File-Name' => encoded_name + } + headers['X-Forest-Action-Response-Headers'] = result[:response_headers].to_json if result[:response_headers] + + { status: 200, headers: headers, content: result[:stream] } end end end From 37ae6bdc204e3b987c494deb672279dac47c6dc1 Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Thu, 21 May 2026 16:13:41 +0200 Subject: [PATCH 2/6] ci: lint --- .../collection_spec.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb index 97ac034be..e6430eced 100644 --- a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb +++ b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb @@ -7,24 +7,22 @@ module ForestAdminDatasourceRpc include ForestAdminDatasourceToolkit::Components::Query::ConditionTree describe Collection do - before do - logger = instance_double(Logger, log: nil) - allow(ForestAdminAgent::Facades::Container).to receive_messages(logger: logger, cache: 'secret') - allow(Utils::RpcClient).to receive(:new).and_return(rpc_client) - end - let(:raw_response) do instance_double(Faraday::Response, body: {}, headers: {}, status: 200, success?: true) end let(:rpc_client) { instance_double(Utils::RpcClient, call_rpc: {}) } - - before do - allow(rpc_client).to receive(:call_rpc).with(any_args, hash_including(with_response: true)).and_return(raw_response) - end let(:datasource) { Datasource.new({ uri: 'http://localhost' }, introspection) } let(:collection) { datasource.get_collection('Product') } let(:caller) { build_caller } + before do + logger = instance_double(Logger, log: nil) + allow(ForestAdminAgent::Facades::Container).to receive_messages(logger: logger, cache: 'secret') + allow(Utils::RpcClient).to receive(:new).and_return(rpc_client) + allow(rpc_client).to receive(:call_rpc) + .with(any_args, hash_including(with_response: true)).and_return(raw_response) + end + include_context 'with introspection' context 'when initialized' do From 1b6e6b546c3e08956bda8959afcee26994a58b31 Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 26 May 2026 16:45:25 +0200 Subject: [PATCH 3/6] fix(rpc): Skip JSON encoding for raw file action payloads File action results carry binary bytes (PDF, images, ...) which raise JSON::GeneratorError when serialize_response forces them through .to_json. ActionExecute now tags the file response with raw: true so BaseRoute passes the body to Rack/Sinatra untouched. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../routes/action_execute.rb | 3 +- .../routes/base_route.rb | 16 ++++++- .../routes/action_execute_spec.rb | 48 +++++++++++++++++++ .../routes/base_route_spec.rb | 17 +++++++ 4 files changed, 81 insertions(+), 3 deletions(-) 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 8657bbabd..6c4fd7b7f 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 @@ -43,7 +43,8 @@ def build_file_response(result) } headers['X-Forest-Action-Response-Headers'] = result[:response_headers].to_json if result[:response_headers] - { status: 200, headers: headers, content: result[:stream] } + # raw: true skips JSON encoding — file streams are arbitrary binary bytes. + { status: 200, headers: headers, content: result[:stream], raw: true } end end end 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 c0ab2c20a..01da82071 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 @@ -26,7 +26,13 @@ def register_sinatra(app) status result[:status] # Set custom headers if provided result[:headers]&.each { |key, value| headers[key] = value } - result[:content] ? serialize_response(result[:content]) : '' + if result[:content].nil? + '' + elsif result[:raw] + result[:content].to_s + else + serialize_response(result[:content]) + end else serialize_response(result) end @@ -68,7 +74,13 @@ def build_rails_response(result) if result.is_a?(Hash) && result.key?(:status) response_headers = { 'Content-Type' => 'application/json' } response_headers.merge!(result[:headers]) if result[:headers] - body = result[:content] ? serialize_response(result[:content]) : '' + body = if result[:content].nil? + '' + elsif result[:raw] + result[:content].to_s + else + serialize_response(result[:content]) + end [result[:status], response_headers, [body]] else [200, { 'Content-Type' => 'application/json' }, [serialize_response(result)]] 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 16d87e941..4f0305540 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 @@ -94,6 +94,54 @@ module Routes .to raise_error(ForestAdminAgent::Http::Exceptions::NotFoundError) end end + + context 'when the action returns a File result' do + let(:binary_payload) { String.new("%PDF-1.4\n%\xE2\xE3\xCF\xD3\n", encoding: 'ASCII-8BIT') } + let(:file_result) do + { + type: 'File', + name: 'report final.pdf', + mime_type: 'application/pdf', + stream: binary_payload, + response_headers: { 'set-cookie' => 'token=xyz' } + } + end + + before do + allow(@datasource.get_collection('users')) + .to receive(:execute).and_return(file_result) + end + + it 'returns a raw HTTP response so the binary content is not JSON-encoded' do + response = route.handle_request(args) + + expect(response).to include(status: 200, raw: true) + expect(response[:content]).to eq(binary_payload) + expect(response[:content].encoding.name).to eq('ASCII-8BIT') + end + + it 'sets File-specific headers including the encoded filename' do + response = route.handle_request(args) + headers = response[:headers] + + expect(headers['Content-Type']).to eq('application/pdf') + expect(headers['X-Forest-Action-Type']).to eq('File') + expect(headers['X-Forest-Action-File-Name']).to eq(CGI.escape('report final.pdf')) + expect(headers['Content-Disposition']).to include(CGI.escape('report final.pdf')) + expect(headers['X-Forest-Action-Response-Headers']).to eq({ 'set-cookie' => 'token=xyz' }.to_json) + end + + context 'when the action does not provide response_headers' do + let(:file_result) do + { type: 'File', name: 'note.txt', mime_type: 'text/plain', stream: 'hi' } + end + + it 'omits the X-Forest-Action-Response-Headers header' do + response = route.handle_request(args) + expect(response[:headers]).not_to have_key('X-Forest-Action-Response-Headers') + end + end + end 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 01a2f4cc4..6c8116da1 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 @@ -166,6 +166,23 @@ def handle_request(_params) expect(response[1]).to eq({ 'Content-Type' => 'application/json' }) expect(response[2]).to eq(['']) end + + it 'passes raw binary content through without JSON-encoding it (e.g. File action result)' do + binary = String.new("%PDF-1.4\n%\xE2\xE3\xCF\xD3\n", encoding: 'ASCII-8BIT') + result = { + status: 200, + headers: { 'Content-Type' => 'application/pdf' }, + content: binary, + raw: true + } + + response = route.send(:build_rails_response, result) + + expect(response[0]).to eq(200) + expect(response[1]['Content-Type']).to eq('application/pdf') + expect(response[2]).to eq([binary]) + expect(response[2].first.encoding.name).to eq('ASCII-8BIT') + end end context 'when result is not a Hash with :status key' do From 5ddcd8dce6b83ff8c4726997e1cceddb22b307d3 Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 26 May 2026 17:03:36 +0200 Subject: [PATCH 4/6] test(rpc): Cover binary body integration through Rack Adds a BinaryTestRoute and a register_rails context that captures the handler proc and invokes it with a minimal Rack env. Asserts the body bytes survive the BaseRoute pipeline byte-identical, with ASCII-8BIT encoding preserved. Existing unit tests stubbed collection.execute and never exercised the Rack serialization path that produced the original JSON::GeneratorError. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../routes/base_route_spec.rb | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) 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 6c8116da1..c4e8bca3d 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 @@ -19,6 +19,20 @@ def handle_request(_params) end end + # Test route that returns a binary payload — emulates an action File result. + # The "%\xE2\xE3\xCF\xD3" prefix forces non-UTF8 bytes (the PDF binary marker), + # which is exactly what used to crash serialize_response. + class BinaryTestRoute < BaseRoute + def handle_request(_args) + { + status: 200, + headers: { 'Content-Type' => 'application/pdf' }, + content: String.new("%PDF-1.4\n%\xE2\xE3\xCF\xD3\nbinary-body-bytes", encoding: 'ASCII-8BIT'), + raw: true + } + end + end + describe BaseRoute do subject(:route) { TestRoute.new('/test', 'get', 'test_route') } @@ -136,6 +150,43 @@ def handle_request(_params) ) end end + + # Integration: exercises the handler proc that register_rails passes + # to router.match. This is the only place where binary action results + # actually hit Rack — unit tests that stub collection.execute return a + # Ruby Hash that never goes through serialize_response. + context 'when the handler returns a raw binary response (File action)' do + subject(:binary_route) { BinaryTestRoute.new('/binary', 'post', 'binary_test') } + let(:expected_bytes) do + String.new("%PDF-1.4\n%\xE2\xE3\xCF\xD3\nbinary-body-bytes", encoding: 'ASCII-8BIT') + end + let(:request) do + instance_double( + ActionDispatch::Request, + path_parameters: {}, query_parameters: {}, request_parameters: {}, env: {} + ) + end + + it 'passes the binary body to Rack untouched (no JSON encoding)' do + captured_handler = nil + allow(rails_router).to receive(:match) { |_, opts| captured_handler = opts[:to] } + binary_route.send(:register_rails, rails_router) + + env = { + 'REQUEST_METHOD' => 'POST', + 'PATH_INFO' => '/binary', + 'QUERY_STRING' => '', + 'rack.input' => StringIO.new + } + status, response_headers, body = captured_handler.call(env) + + expect(status).to eq(200) + expect(response_headers['Content-Type']).to eq('application/pdf') + expect(body).to be_an(Array) + expect(body.first).to eq(expected_bytes) + expect(body.first.encoding.name).to eq('ASCII-8BIT') + end + end end describe '#build_rails_response' do From 3b5a51f57746ee967872d334672acf360ccafd51 Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 26 May 2026 17:09:37 +0200 Subject: [PATCH 5/6] refactor(rpc): Split call_rpc into body and raw variants The with_response flag toggled call_rpc between returning the parsed body and the raw Faraday::Response, which forces the call site to know about the flag to know the return type. A dedicated call_rpc_raw makes the intent explicit and lets Collection#execute fetch headers without the implicit contract change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Utils/rpc_client.rb | 13 ++++++++--- .../forest_admin_datasource_rpc/collection.rb | 4 ++-- .../collection_spec.rb | 9 +++----- .../utils/rpc_client_spec.rb | 22 ++++++------------- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb index 49bdef36c..963ed336f 100644 --- a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb +++ b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/Utils/rpc_client.rb @@ -31,15 +31,22 @@ def initialize(api_url, auth_secret) end # rubocop:disable Metrics/ParameterLists - def call_rpc(endpoint, caller: nil, method: :get, payload: nil, symbolize_keys: false, - if_none_match: nil, with_response: false) + def call_rpc(endpoint, caller: nil, method: :get, payload: nil, symbolize_keys: false, if_none_match: nil) + resp = call_rpc_raw(endpoint, caller: caller, method: method, payload: payload, + symbolize_keys: symbolize_keys, if_none_match: if_none_match) + return resp if resp == NotModified + + resp.body + end + + def call_rpc_raw(endpoint, caller: nil, method: :get, payload: nil, symbolize_keys: false, if_none_match: nil) response = make_request(endpoint, caller: caller, method: method, payload: payload, symbolize_keys: symbolize_keys, if_none_match: if_none_match) return NotModified if response.status == HTTP_NOT_MODIFIED raise_appropriate_error(response) unless response.success? - with_response ? response : response.body + response end # rubocop:enable Metrics/ParameterLists diff --git a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb index b662e597f..acf0bbb86 100644 --- a/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb +++ b/packages/forest_admin_datasource_rpc/lib/forest_admin_datasource_rpc/collection.rb @@ -130,8 +130,8 @@ def execute(caller, name, data, filter = nil) "Forwarding '#{@name}' action #{name} call to the Rpc agent on #{url}." ) - response = @client.call_rpc(url, caller: caller, method: :post, payload: params, - symbolize_keys: true, with_response: true) + response = @client.call_rpc_raw(url, caller: caller, method: :post, payload: params, + symbolize_keys: true) return build_file_result(response) if response.headers['x-forest-action-type'] == 'File' diff --git a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb index e6430eced..322e5b9a5 100644 --- a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb +++ b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb @@ -10,7 +10,7 @@ module ForestAdminDatasourceRpc let(:raw_response) do instance_double(Faraday::Response, body: {}, headers: {}, status: 200, success?: true) end - let(:rpc_client) { instance_double(Utils::RpcClient, call_rpc: {}) } + let(:rpc_client) { instance_double(Utils::RpcClient, call_rpc: {}, call_rpc_raw: raw_response) } let(:datasource) { Datasource.new({ uri: 'http://localhost' }, introspection) } let(:collection) { datasource.get_collection('Product') } let(:caller) { build_caller } @@ -19,8 +19,6 @@ module ForestAdminDatasourceRpc logger = instance_double(Logger, log: nil) allow(ForestAdminAgent::Facades::Container).to receive_messages(logger: logger, cache: 'secret') allow(Utils::RpcClient).to receive(:new).and_return(rpc_client) - allow(rpc_client).to receive(:call_rpc) - .with(any_args, hash_including(with_response: true)).and_return(raw_response) end include_context 'with introspection' @@ -275,12 +273,11 @@ module ForestAdminDatasourceRpc end end - it 'asks the RPC client to symbolize response keys so ActionResult.parse sees :type' do + it 'calls call_rpc_raw with symbolized keys so the response headers stay reachable' do collection.execute(caller, 'my_action', {}) - expect(rpc_client).to have_received(:call_rpc) do |_url, options| + expect(rpc_client).to have_received(:call_rpc_raw) do |_url, options| expect(options[:symbolize_keys]).to be(true) - expect(options[:with_response]).to be(true) end end diff --git a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb index 59bae1edf..2d0d6b34c 100644 --- a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb +++ b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb @@ -97,23 +97,15 @@ module Utils expect(result).to eq({}) end + end - context 'when called with with_response: true' do - it 'returns the Faraday::Response object instead of the body' do - result = rpc_client.call_rpc('/rpc/test', method: :post, with_response: true) - - expect(result).to be(response) - expect(result.body).to eq({}) - expect(result.headers).to eq(response_headers) - end - end - - context 'when called without with_response (default)' do - it 'returns the response body for backward compatibility' do - result = rpc_client.call_rpc('/rpc/test', method: :post) + describe '#call_rpc_raw' do + it 'returns the Faraday::Response object so callers can read headers' do + result = rpc_client.call_rpc_raw('/rpc/test', method: :post) - expect(result).to eq({}) - end + expect(result).to be(response) + expect(result.body).to eq({}) + expect(result.headers).to eq(response_headers) end end From 822b9bb6da0d73261ec5e57f124dfa6804ac0064 Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 26 May 2026 17:17:46 +0200 Subject: [PATCH 6/6] fix(rpc): Update specs after call_rpc/call_rpc_raw split CI revealed two specs broken by the call_rpc_raw split: the execute test in collection_spec still expected :call_rpc, and the #call_rpc_raw describe in rpc_client_spec lacked the Faraday and Time stubs that the surrounding context provided. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../forest_admin_datasource_rpc/collection_spec.rb | 2 +- .../utils/rpc_client_spec.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb index 322e5b9a5..fff27c825 100644 --- a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb +++ b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/collection_spec.rb @@ -254,7 +254,7 @@ module ForestAdminDatasourceRpc collection.execute(caller, 'my_action', data) - expect(rpc_client).to have_received(:call_rpc) do |url, options| + expect(rpc_client).to have_received(:call_rpc_raw) do |url, options| expect(url).to eq('/forest/rpc/Product/action-execute') expect(options[:caller]).to eq(caller) expect(options[:method]).to eq(:post) diff --git a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb index 2d0d6b34c..1cf32e4b6 100644 --- a/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb +++ b/packages/forest_admin_datasource_rpc/spec/lib/forest_admin_datasource_rpc/utils/rpc_client_spec.rb @@ -100,6 +100,18 @@ module Utils end describe '#call_rpc_raw' do + let(:response_headers) { { 'x-forest-action-type' => 'File' } } + let(:response) do + instance_double(Faraday::Response, status: 200, body: {}, success?: true, headers: response_headers) + end + let(:faraday_connection) { instance_double(Faraday::Connection, send: response) } + let(:timestamp) { '2025-04-01T14:07:02Z' } + + before do + allow(Faraday::Connection).to receive(:new).and_return(faraday_connection) + allow(Time).to receive(:now).and_return(instance_double(Time, utc: instance_double(Time, iso8601: timestamp))) + end + it 'returns the Faraday::Response object so callers can read headers' do result = rpc_client.call_rpc_raw('/rpc/test', method: :post)