diff --git a/lib/inertia_rails/controller.rb b/lib/inertia_rails/controller.rb index 22b38cd9..56ce8b36 100644 --- a/lib/inertia_rails/controller.rb +++ b/lib/inertia_rails/controller.rb @@ -1,16 +1,26 @@ require_relative "inertia_rails" -require_relative "helper" module InertiaRails module Controller extend ActiveSupport::Concern included do + helper_method :inertia_headers + before_action do - # :inertia_errors are deleted from the session by the middleware - InertiaRails.share(errors: session[:inertia_errors]) if session[:inertia_errors].present? + error_sharing = proc do + # :inertia_errors are deleted from the session by the middleware + if @_request && session[:inertia_errors].present? + { errors: session[:inertia_errors] } + else + {} + end + end + + @_inertia_shared_plain_data ||= {} + @_inertia_shared_blocks ||= [error_sharing] + @_inertia_html_headers ||= [] end - helper ::InertiaRails::Helper after_action do cookies['XSRF-TOKEN'] = form_authenticity_token unless !protect_against_forgery? @@ -18,10 +28,10 @@ module Controller end module ClassMethods - def inertia_share(**args, &block) + def inertia_share(hash = nil, &block) before_action do - InertiaRails.share(**args) if args - InertiaRails.share_block(block) if block + @_inertia_shared_plain_data = @_inertia_shared_plain_data.merge(hash) if hash + @_inertia_shared_blocks = @_inertia_shared_blocks + [block] if block_given? end end @@ -33,6 +43,14 @@ def use_inertia_instance_props end end + def inertia_headers + @_inertia_html_headers.join.html_safe + end + + def inertia_headers=(value) + @_inertia_html_headers = value + end + def default_render if InertiaRails.default_render? render(inertia: true) @@ -41,6 +59,10 @@ def default_render end end + def shared_data + (@_inertia_shared_plain_data || {}).merge(evaluated_blocks) + end + def redirect_to(options = {}, response_options = {}) capture_inertia_errors(response_options) super(options, response_options) @@ -80,5 +102,9 @@ def capture_inertia_errors(options) session[:inertia_errors] = inertia_errors end end + + def evaluated_blocks + (@_inertia_shared_blocks || []).map { |block| instance_exec(&block) }.reduce(&:merge) || {} + end end end diff --git a/lib/inertia_rails/helper.rb b/lib/inertia_rails/helper.rb deleted file mode 100644 index 240e5f9f..00000000 --- a/lib/inertia_rails/helper.rb +++ /dev/null @@ -1,7 +0,0 @@ -require_relative 'inertia_rails' - -module InertiaRails::Helper - def inertia_headers - ::InertiaRails.html_headers.join.html_safe - end -end diff --git a/lib/inertia_rails/inertia_rails.rb b/lib/inertia_rails/inertia_rails.rb index a58fcca9..8a0afa9b 100644 --- a/lib/inertia_rails/inertia_rails.rb +++ b/lib/inertia_rails/inertia_rails.rb @@ -3,20 +3,10 @@ require 'inertia_rails/lazy' module InertiaRails - thread_mattr_accessor :threadsafe_shared_plain_data - thread_mattr_accessor :threadsafe_shared_blocks - thread_mattr_accessor :threadsafe_html_headers - def self.configure yield(Configuration) end - # "Getters" - def self.shared_data(controller) - shared_plain_data. - merge!(evaluated_blocks(controller, shared_blocks)) - end - def self.version Configuration.evaluated_version end @@ -35,35 +25,12 @@ def self.ssr_url def self.default_render? Configuration.default_render - end - - def self.html_headers - self.threadsafe_html_headers || [] end def self.deep_merge_shared_data? Configuration.deep_merge_shared_data end - # "Setters" - def self.share(**args) - self.shared_plain_data = self.shared_plain_data.merge(args) - end - - def self.share_block(block) - self.shared_blocks = self.shared_blocks + [block] - end - - def self.html_headers=(headers) - self.threadsafe_html_headers = headers - end - - def self.reset! - self.shared_plain_data = {} - self.shared_blocks = [] - self.html_headers = [] - end - def self.lazy(value = nil, &block) InertiaRails::Lazy.new(value, &block) end @@ -82,25 +49,4 @@ def self.evaluated_version self.version.respond_to?(:call) ? self.version.call : self.version end end - - # Getters and setters to provide default values for the threadsafe attributes - def self.shared_plain_data - self.threadsafe_shared_plain_data || {} - end - - def self.shared_plain_data=(val) - self.threadsafe_shared_plain_data = val - end - - def self.shared_blocks - self.threadsafe_shared_blocks || [] - end - - def self.shared_blocks=(val) - self.threadsafe_shared_blocks = val - end - - def self.evaluated_blocks(controller, blocks) - blocks.flat_map { |block| controller.instance_exec(&block) }.reduce(&:merge) || {} - end end diff --git a/lib/inertia_rails/middleware.rb b/lib/inertia_rails/middleware.rb index 86644113..db9cd5bf 100644 --- a/lib/inertia_rails/middleware.rb +++ b/lib/inertia_rails/middleware.rb @@ -7,8 +7,6 @@ def initialize(app) def call(env) InertiaRailsRequest.new(@app, env) .response - ensure - ::InertiaRails.reset! end class InertiaRailsRequest @@ -22,7 +20,7 @@ def response status, headers, body = @app.call(@env) request = ActionDispatch::Request.new(@env) - # Inertia errors are added to the session via redirect_to + # Inertia errors are added to the session via redirect_to request.session.delete(:inertia_errors) unless keep_inertia_errors?(status) status = 303 if inertia_non_post_redirect?(status) @@ -97,4 +95,3 @@ def copy_xsrf_to_csrf! end end end - diff --git a/lib/inertia_rails/renderer.rb b/lib/inertia_rails/renderer.rb index b61f2b01..8c96fe2e 100644 --- a/lib/inertia_rails/renderer.rb +++ b/lib/inertia_rails/renderer.rb @@ -1,5 +1,3 @@ -require 'net/http' -require 'json' require_relative "inertia_rails" module InertiaRails @@ -33,8 +31,8 @@ def render def render_ssr uri = URI("#{::InertiaRails.ssr_url}/render") res = JSON.parse(Net::HTTP.post(uri, page.to_json, 'Content-Type' => 'application/json').body) - - ::InertiaRails.html_headers = res['head'] + + @controller.inertia_headers = res['head'] @render_method.call html: res['body'].html_safe, layout: layout, locals: (view_data).merge({page: page}) end @@ -48,7 +46,7 @@ def computed_props # # Functionally, this permits using either string or symbol keys in the controller. Since the results # is cast to json, we should treat string/symbol keys as identical. - _props = ::InertiaRails.shared_data(@controller).deep_symbolize_keys.send(prop_merge_method, @props.deep_symbolize_keys).select do |key, prop| + _props = @controller.shared_data.merge.deep_symbolize_keys.send(prop_merge_method, @props.deep_symbolize_keys).select do |key, prop| if rendering_partial_component? key.in? partial_keys else diff --git a/lib/inertia_rails/rspec.rb b/lib/inertia_rails/rspec.rb index 3af8a781..a2da420f 100644 --- a/lib/inertia_rails/rspec.rb +++ b/lib/inertia_rails/rspec.rb @@ -65,7 +65,6 @@ def inertia_tests_setup? } config.before(:each, inertia: true) do - ::InertiaRails.reset! new_renderer = InertiaRails::Renderer.method(:new) allow(InertiaRails::Renderer).to receive(:new) do |component, controller, request, response, render, named_args| new_renderer.call(component, controller, request, response, inertia_wrap_render(render), **named_args) diff --git a/spec/dummy/app/controllers/inertia_conditional_sharing_controller.rb b/spec/dummy/app/controllers/inertia_conditional_sharing_controller.rb new file mode 100644 index 00000000..3dadb0ad --- /dev/null +++ b/spec/dummy/app/controllers/inertia_conditional_sharing_controller.rb @@ -0,0 +1,22 @@ +class InertiaConditionalSharingController < ApplicationController + before_action :conditionally_share_props, only: [:show] + inertia_share normal_shared_prop: 1 + + def index + render inertia: 'EmptyTestComponent', props: { + index_only_prop: 1, + } + end + + def show + render inertia: 'EmptyTestComponent', props: { + show_only_prop: 1, + } + end + + protected + + def conditionally_share_props + self.class.inertia_share conditionally_shared_show_prop: 1 + end +end diff --git a/spec/dummy/app/controllers/inertia_share_test_controller.rb b/spec/dummy/app/controllers/inertia_share_test_controller.rb index 4157b202..19441db2 100644 --- a/spec/dummy/app/controllers/inertia_share_test_controller.rb +++ b/spec/dummy/app/controllers/inertia_share_test_controller.rb @@ -4,11 +4,17 @@ class InertiaShareTestController < ApplicationController inertia_share do { position: 'center', - number: 29, + number: number, } end - + def share render inertia: 'ShareTestComponent' end + + private + + def number + 29 + end end diff --git a/spec/dummy/app/controllers/inertia_test_controller.rb b/spec/dummy/app/controllers/inertia_test_controller.rb index 241d3b8e..c4e9b0cd 100644 --- a/spec/dummy/app/controllers/inertia_test_controller.rb +++ b/spec/dummy/app/controllers/inertia_test_controller.rb @@ -64,4 +64,8 @@ def content_type_test format.xml { render xml: [ 1, 2, 3 ] } end end + + def redirect_to_share_test + redirect_to share_path + end end diff --git a/spec/dummy/config/application.rb b/spec/dummy/config/application.rb index eefb5911..1ea383c3 100644 --- a/spec/dummy/config/application.rb +++ b/spec/dummy/config/application.rb @@ -34,4 +34,3 @@ class Application < Rails::Application config.secret_key_base = SecureRandom.hex end end - diff --git a/spec/dummy/config/environments/test.rb b/spec/dummy/config/environments/test.rb index adb89080..9efcceb5 100644 --- a/spec/dummy/config/environments/test.rb +++ b/spec/dummy/config/environments/test.rb @@ -29,7 +29,7 @@ config.cache_store = :null_store # Raise exceptions instead of rendering exception templates. - config.action_dispatch.show_exceptions = false + config.action_dispatch.show_exceptions = :none # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index 52fca3b3..90080777 100644 --- a/spec/dummy/config/routes.rb +++ b/spec/dummy/config/routes.rb @@ -33,6 +33,7 @@ get 'default_component_test' => 'inertia_rails_mimic#default_component_test' get 'provided_props_test' => 'inertia_rails_mimic#provided_props_test' + post 'redirect_to_share_test' => 'inertia_test#redirect_to_share_test' inertia 'inertia_route' => 'TestComponent' get 'merge_shared' => 'inertia_merge_shared#merge_shared' @@ -45,4 +46,7 @@ get 'initialize_session' => 'inertia_session_continuity_test#initialize_session' post 'submit_form_to_test_csrf' => 'inertia_session_continuity_test#submit_form_to_test_csrf' delete 'clear_session' => 'inertia_session_continuity_test#clear_session' + + get 'conditional_share_index' => 'inertia_conditional_sharing#index' + get 'conditional_share_show' => 'inertia_conditional_sharing#show' end diff --git a/spec/inertia/conditional_sharing_spec.rb b/spec/inertia/conditional_sharing_spec.rb new file mode 100644 index 00000000..eff2fad6 --- /dev/null +++ b/spec/inertia/conditional_sharing_spec.rb @@ -0,0 +1,29 @@ +RSpec.describe "conditionally shared data in a controller", type: :request do + context "when there is a before_action inside a inertia_share" do + it "does leak data between requests" do + get conditional_share_index_path, headers: {'X-Inertia' => true} + expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq({ + index_only_prop: 1, + normal_shared_prop: 1, + }) + + # NOTE: we actually have to run the show action twice since the new implementation + # sets up a before_action within a before_action to share the data. + # In effect, that means that the shared data isn't rendered until the second time the action is run. + get conditional_share_show_path, headers: {'X-Inertia' => true} + get conditional_share_show_path, headers: {'X-Inertia' => true} + expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq({ + normal_shared_prop: 1, + show_only_prop: 1, + conditionally_shared_show_prop: 1, + }) + + get conditional_share_index_path, headers: {'X-Inertia' => true} + expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq({ + index_only_prop: 1, + normal_shared_prop: 1, + conditionally_shared_show_prop: 1, + }) + end + end +end diff --git a/spec/inertia/rendering_spec.rb b/spec/inertia/rendering_spec.rb index fea6bdec..97459dd2 100644 --- a/spec/inertia/rendering_spec.rb +++ b/spec/inertia/rendering_spec.rb @@ -1,7 +1,7 @@ RSpec.describe 'rendering inertia views', type: :request do subject { response.body } - let(:controller) { double('Controller', inertia_view_assigns: {})} + let(:controller) { ApplicationController.new } context 'first load' do let(:page) { InertiaRails::Renderer.new('TestComponent', controller, request, response, '').send(:page) } diff --git a/spec/inertia/sharing_spec.rb b/spec/inertia/sharing_spec.rb index e16fbf9d..a4516cf3 100644 --- a/spec/inertia/sharing_spec.rb +++ b/spec/inertia/sharing_spec.rb @@ -54,7 +54,7 @@ it { is_expected.to eq props.merge({ errors: errors }) } end - context 'multithreaded intertia share' do + context 'multithreaded inertia share' do let(:props) { { name: 'Michael', has_goat_status: true } } it 'is expected to render props even when another thread shares Inertia data' do start_thread1 = false @@ -70,12 +70,7 @@ thread2 = Thread.new do sleep 0.1 until start_thread2 - # Would prefer to make this a second get request, but RSpec will overwrite - # the @response variable if another request is made in the second thread. - # This simulates the relevant effects of another call to a different - # controller with different values for inertia_share - InertiaRails.reset! - InertiaRails.share(name: 'Brian', has_goat_status: false) + get share_path, headers: {'X-Inertia' => true} end # Thread 1 starts. The controller method runs inertia_share, then sleeps. @@ -88,16 +83,32 @@ thread1.join thread2.join end + end + context 'when raises an exception' do it 'is expected not to leak shared data across requests' do begin get share_multithreaded_error_path, headers: {'X-Inertia' => true} rescue Exception end - expect(InertiaRails.shared_plain_data).to be_empty - expect(InertiaRails.shared_blocks).to be_empty + get share_path, headers: {'X-Inertia' => true} + + is_expected.to eq({name: 'Brandon', sport: 'hockey', position: 'center', number: 29}) + end + end + + context 'using inertia share in redirected requests' do + let(:props) { {name: 'Brandon', sport: 'hockey', position: 'center', number: 29} } + + before do + post redirect_to_share_test_path, headers: {'X-Inertia' => true} + expect(response).to be_redirect + + get response.location, headers: {'X-Inertia' => true} end + + it { is_expected.to eq props } end describe 'deep or shallow merging shared data' do diff --git a/spec/inertia/ssr_spec.rb b/spec/inertia/ssr_spec.rb index d865bb19..e3ea97ea 100644 --- a/spec/inertia/ssr_spec.rb +++ b/spec/inertia/ssr_spec.rb @@ -1,7 +1,8 @@ +require 'net/http' + RSpec.describe 'inertia ssr', type: :request do context 'ssr is enabled' do before do - InertiaRails.reset! InertiaRails.configure do |config| config.ssr_enabled = true config.ssr_url = 'ssr-url'