From bb971bb134fc1719fa4b5568cb16a436bd8dbd1f Mon Sep 17 00:00:00 2001 From: akadusei Date: Sat, 9 May 2026 11:04:52 +0000 Subject: [PATCH 1/3] Allow readding skipped pipes This also allows reskipping a readded pipe, and so on. Ultimately, the last rule wins. --- spec/lucky/action_pipes_spec.cr | 18 +++++ src/lucky/action_pipes.cr | 117 +++++++++++++++----------------- 2 files changed, 74 insertions(+), 61 deletions(-) diff --git a/spec/lucky/action_pipes_spec.cr b/spec/lucky/action_pipes_spec.cr index bfeaed992..f95237b04 100644 --- a/spec/lucky/action_pipes_spec.cr +++ b/spec/lucky/action_pipes_spec.cr @@ -38,6 +38,16 @@ class Pipes::Skipped < InheritablePipes end end +class Pipes::Readded < Pipes::Skipped + before set_before_cookie + after overwrite_after_cookie + + get "/readded_pipes" do + cookies.set("after", "This should be overwritten by the after pipe") + plain_text "Body" + end +end + class Pipes::Index < InheritablePipes before set_second_before_cookie after set_second_after_cookie @@ -140,6 +150,14 @@ describe Lucky::Action do response.context.cookies.get?("after").should be_nil end + it "readds skipped pipes" do + Lucky::ContinuedPipeLog.dexter.temp_config do |log_io| + response = Pipes::Readded.new(build_context, params).call + response.context.cookies.get?("before").should eq "before" + response.context.cookies.get?("after").should eq "after" + end + end + describe "handles before pipes" do it "runs through all the pipes if no Lucky::Response is returned" do Lucky::ContinuedPipeLog.dexter.temp_config do |log_io| diff --git a/src/lucky/action_pipes.cr b/src/lucky/action_pipes.cr index 4285c862c..a5f065252 100644 --- a/src/lucky/action_pipes.cr +++ b/src/lucky/action_pipes.cr @@ -10,8 +10,9 @@ module Lucky::ActionPipes # ``` macro skip(*pipes) {% for pipe in pipes %} - {% if BEFORE_PIPES.includes?(pipe.id) || AFTER_PIPES.includes?(pipe.id) %} - {% SKIPPED_PIPES << pipe.id %} + {% if BEFORE_PIPES[pipe.id] || AFTER_PIPES[pipe.id] %} + {% BEFORE_PIPES[pipe.id] = false %} + {% AFTER_PIPES[pipe.id] = false %} {% else %} {% pipe.raise <<-ERROR.lines.join(" ") Can't skip '#{pipe}' because the pipe is not used. @@ -24,14 +25,12 @@ module Lucky::ActionPipes # :nodoc: macro included - AFTER_PIPES = [] of Symbol - BEFORE_PIPES = [] of Symbol - SKIPPED_PIPES = [] of Symbol + AFTER_PIPES = {} of Symbol => Bool + BEFORE_PIPES = {} of Symbol => Bool macro inherited - AFTER_PIPES = [] of Symbol - BEFORE_PIPES = [] of Symbol - SKIPPED_PIPES = [] of Symbol + AFTER_PIPES = {} of Symbol => Bool + BEFORE_PIPES = {} of Symbol => Bool inherit_pipes end @@ -39,16 +38,12 @@ module Lucky::ActionPipes # :nodoc: macro inherit_pipes - \{% for v in @type.ancestors.first.constant :BEFORE_PIPES %} - \{% BEFORE_PIPES << v %} + \{% for k, v in @type.ancestors.first.constant :BEFORE_PIPES %} + \{% BEFORE_PIPES[k] = v %} \{% end %} - \{% for v in @type.ancestors.first.constant :AFTER_PIPES %} - \{% AFTER_PIPES << v %} - \{% end %} - - \{% for v in @type.ancestors.first.constant :SKIPPED_PIPES %} - \{% SKIPPED_PIPES << v %} + \{% for k, v in @type.ancestors.first.constant :AFTER_PIPES %} + \{% AFTER_PIPES[k] = v %} \{% end %} end @@ -82,7 +77,7 @@ module Lucky::ActionPipes # end # ``` macro before(method_name) - {% BEFORE_PIPES << method_name.id %} + {% BEFORE_PIPES[method_name.id] = true %} end # Run a method after an action ends @@ -108,59 +103,59 @@ module Lucky::ActionPipes # end # ``` macro after(method_name) - {% AFTER_PIPES << method_name.id %} + {% AFTER_PIPES[method_name.id] = true %} end # :nodoc: macro run_before_pipes - {% pipes = BEFORE_PIPES.reject { |pipe| SKIPPED_PIPES.includes?(pipe) } %} - - {% for pipe_method in pipes %} - pipe_result = {{ pipe_method }} - ensure_pipe_return_response_or_continue(pipe_result) - # Pipe {{ pipe_method }} should return a Lucky::Response or Lucky::ActionPipes::Continue - # Do this by using `continue` or one of rendering methods like `html` or `redirect` - # - # def {{ pipe_method }} - # cookies["name"] = "John" - # continue # or redirect, render - # end - - if pipe_result.is_a?(Lucky::Response) - publish_before_event("{{ pipe_method.id }}", continued: false) - Lucky::ActionPipes.log_halted_pipe("{{ pipe_method.id }}") - return pipe_result - else - publish_before_event("{{ pipe_method.id }}", continued: true) - Lucky::ActionPipes.log_continued_pipe("{{ pipe_method.id }}") - end + {% for pipe_method, is_enabled in BEFORE_PIPES %} + {% if is_enabled %} + pipe_result = {{ pipe_method }} + ensure_pipe_return_response_or_continue(pipe_result) + # Pipe {{ pipe_method }} should return a Lucky::Response or Lucky::ActionPipes::Continue + # Do this by using `continue` or one of rendering methods like `html` or `redirect` + # + # def {{ pipe_method }} + # cookies["name"] = "John" + # continue # or redirect, render + # end + + if pipe_result.is_a?(Lucky::Response) + publish_before_event("{{ pipe_method.id }}", continued: false) + Lucky::ActionPipes.log_halted_pipe("{{ pipe_method.id }}") + return pipe_result + else + publish_before_event("{{ pipe_method.id }}", continued: true) + Lucky::ActionPipes.log_continued_pipe("{{ pipe_method.id }}") + end + {% end %} {% end %} end # :nodoc: macro run_after_pipes - {% pipes = AFTER_PIPES.reject { |pipe| SKIPPED_PIPES.includes?(pipe) } %} - - {% for pipe_method in pipes %} - pipe_result = {{ pipe_method }} - - ensure_pipe_return_response_or_continue(pipe_result) - # Pipe {{ pipe_method }} should return a Lucky::Response or Lucky::ActionPipes::Continue - # Do this by using `continue` or one of rendering methods like `html` or `redirect` - # - # def {{ pipe_method }} - # cookies["name"] = "John" - # continue # or redirect, render - # end - - if pipe_result.is_a?(Lucky::Response) - publish_after_event("{{ pipe_method.id }}", continued: false) - Lucky::ActionPipes.log_halted_pipe("{{ pipe_method.id }}") - return pipe_result - else - publish_after_event("{{ pipe_method.id }}", continued: true) - Lucky::ActionPipes.log_continued_pipe("{{ pipe_method.id }}") - end + {% for pipe_method, is_enabled in AFTER_PIPES %} + {% if is_enabled %} + pipe_result = {{ pipe_method }} + + ensure_pipe_return_response_or_continue(pipe_result) + # Pipe {{ pipe_method }} should return a Lucky::Response or Lucky::ActionPipes::Continue + # Do this by using `continue` or one of rendering methods like `html` or `redirect` + # + # def {{ pipe_method }} + # cookies["name"] = "John" + # continue # or redirect, render + # end + + if pipe_result.is_a?(Lucky::Response) + publish_after_event("{{ pipe_method.id }}", continued: false) + Lucky::ActionPipes.log_halted_pipe("{{ pipe_method.id }}") + return pipe_result + else + publish_after_event("{{ pipe_method.id }}", continued: true) + Lucky::ActionPipes.log_continued_pipe("{{ pipe_method.id }}") + end + {% end %} {% end %} end From 14f734bb3cbbd2d0ba1cf870e14a2167b9b0987e Mon Sep 17 00:00:00 2001 From: akadusei Date: Sat, 9 May 2026 11:08:03 +0000 Subject: [PATCH 2/3] Fix format errors --- src/lucky/action_pipes.cr | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lucky/action_pipes.cr b/src/lucky/action_pipes.cr index a5f065252..889853749 100644 --- a/src/lucky/action_pipes.cr +++ b/src/lucky/action_pipes.cr @@ -25,12 +25,12 @@ module Lucky::ActionPipes # :nodoc: macro included - AFTER_PIPES = {} of Symbol => Bool - BEFORE_PIPES = {} of Symbol => Bool + AFTER_PIPES = {} of Symbol => Bool + BEFORE_PIPES = {} of Symbol => Bool macro inherited - AFTER_PIPES = {} of Symbol => Bool - BEFORE_PIPES = {} of Symbol => Bool + AFTER_PIPES = {} of Symbol => Bool + BEFORE_PIPES = {} of Symbol => Bool inherit_pipes end From 4a049247dc2f3bcea52979fc844389dacf07cbcf Mon Sep 17 00:00:00 2001 From: akadusei Date: Sat, 9 May 2026 11:20:47 +0000 Subject: [PATCH 3/3] Fix lint warning ``` [W] Lint/UnusedArgument: Unused argument `log_io`. If it's necessary, use `_` as an argument name to indicate that it won't be used. > Lucky::ContinuedPipeLog.dexter.temp_config do |log_io| ``` See . --- spec/lucky/action_pipes_spec.cr | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/spec/lucky/action_pipes_spec.cr b/spec/lucky/action_pipes_spec.cr index f95237b04..3dd927370 100644 --- a/spec/lucky/action_pipes_spec.cr +++ b/spec/lucky/action_pipes_spec.cr @@ -151,11 +151,9 @@ describe Lucky::Action do end it "readds skipped pipes" do - Lucky::ContinuedPipeLog.dexter.temp_config do |log_io| - response = Pipes::Readded.new(build_context, params).call - response.context.cookies.get?("before").should eq "before" - response.context.cookies.get?("after").should eq "after" - end + response = Pipes::Readded.new(build_context, params).call + response.context.cookies.get?("before").should eq "before" + response.context.cookies.get?("after").should eq "after" end describe "handles before pipes" do