From d5d06c122411bb8ce913e0052c79f9f9e095ee98 Mon Sep 17 00:00:00 2001 From: Stephen von Takach Date: Tue, 13 Dec 2022 16:38:48 +1100 Subject: [PATCH] fix: skip filter now works with annotation filter methods fixes #65 --- shard.yml | 2 +- spec/open_api_spec.cr | 2 +- spec/route_builder_spec.cr | 8 ++++++++ spec/spec_helper.cr | 25 ++++++++++++++++++++++++- src/action-controller/base.cr | 23 +++++++++++++++++++++-- src/action-controller/router/builder.cr | 2 +- 6 files changed, 56 insertions(+), 6 deletions(-) diff --git a/shard.yml b/shard.yml index 5e07809..625d034 100644 --- a/shard.yml +++ b/shard.yml @@ -1,5 +1,5 @@ name: action-controller -version: 5.5.1 +version: 5.5.2 crystal: ">= 1.5.0" dependencies: diff --git a/spec/open_api_spec.cr b/spec/open_api_spec.cr index 56c3376..234a2d2 100644 --- a/spec/open_api_spec.cr +++ b/spec/open_api_spec.cr @@ -9,7 +9,7 @@ describe ActionController::OpenAPI do it "generates openapi docs" do result = ActionController::OpenAPI.generate_open_api_docs("title", "version", description: "desc") result[:openapi].should eq "3.0.3" - result[:paths].size.should eq 19 + result[:paths].size.should eq 21 result[:info][:description].should eq "desc" end end diff --git a/spec/route_builder_spec.cr b/spec/route_builder_spec.cr index 10f1e66..8b4497f 100644 --- a/spec/route_builder_spec.cr +++ b/spec/route_builder_spec.cr @@ -148,4 +148,12 @@ describe AC::Route::Builder do result = client.post("/filtering/some_other_entry/", headers: headers, body: body) result.body.should eq("3.14") end + + it "should skip filters as expected" do + result = client.get("/skipping_symbol") + result.status_code.should eq 403 + + result = client.get("/skipping_annotation") + result.status_code.should eq 200 + end end diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index ed5e155..59d4323 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -13,17 +13,40 @@ abstract class FilterOrdering < ActionController::Base @in_around = false before_action :set_trust - before_action :check_trust def set_trust @trusted = true end + @[AC::Route::Filter(:before_action)] def check_trust render :forbidden, text: "Trust check failed" unless @trusted end end +class SkippingAnnotation < FilterOrdering + # `base "/skipping_annotation"` configured automatically + + skip_action :set_trust + skip_action :check_trust + + @[AC::Route::GET("/")] + def index + render text: "ok #{@trusted}" + end +end + +class SkippingSymbol < FilterOrdering + # `base "/skipping_symbol"` configured automatically + + skip_action :set_trust + + @[AC::Route::GET("/")] + def index + render text: "ok #{@trusted}" + end +end + class Filtering < FilterOrdering # `base "/filtering"` configured automatically diff --git a/src/action-controller/base.cr b/src/action-controller/base.cr index 1f817f1..9f55e28 100644 --- a/src/action-controller/base.cr +++ b/src/action-controller/base.cr @@ -415,6 +415,11 @@ abstract class ActionController::Base {% around_actions = around_actions.reject(&.==(method)) %} {% end %} {% end %} + + {% filter_name = options[2] %} + {% if skipping.includes?(filter_name.id) %} + {% around_actions = around_actions.reject(&.==(method)) %} + {% end %} {% end %} {% around_actions = around_actions.reject { |act| skipping.includes?(act) } %} @@ -439,6 +444,11 @@ abstract class ActionController::Base {% before_actions = before_actions.reject(&.==(method)) %} {% end %} {% end %} + + {% filter_name = options[2] %} + {% if skipping.includes?(filter_name.id) %} + {% before_actions = before_actions.reject(&.==(method)) %} + {% end %} {% end %} {% before_actions = before_actions.reject { |act| skipping.includes?(act) } %} @@ -518,6 +528,11 @@ abstract class ActionController::Base {% after_actions = after_actions.reject(&.==(method)) %} {% end %} {% end %} + + {% filter_name = options[2] %} + {% if skipping.includes?(filter_name.id) %} + {% after_actions = after_actions.reject(&.==(method)) %} + {% end %} {% end %} {% after_actions = after_actions.reject { |act| skipping.includes?(act) } %} @@ -664,7 +679,7 @@ abstract class ActionController::Base end macro __define_filter_macro__(name, store, method = nil) - macro {{name.id}}({% if method.nil? %} method, {% end %} only = nil, except = nil) + macro {{name.id}}({% if method.nil? %} method, {% end %} only = nil, except = nil, filter_name = nil) {% if method %} \{% method = {{method}} %} {% else %} @@ -694,7 +709,11 @@ abstract class ActionController::Base \{% else %} \{% except = prev_except %} \{% end %} - \{% {{store}}[method] = {only, except} %} + \{% if filter_name %} + \{% else %} + \{% filter_name = method %} + \{% end %} + \{% {{store}}[method] = {only, except, filter_name} %} end end diff --git a/src/action-controller/router/builder.cr b/src/action-controller/router/builder.cr index 61e5974..5e3c8db 100644 --- a/src/action-controller/router/builder.cr +++ b/src/action-controller/router/builder.cr @@ -173,7 +173,7 @@ module ActionController::Route::Builder {% open_api_route[:params] = open_api_params %} {% OPENAPI_FILTERS[@type.name.stringify + "#" + method_name.stringify] = open_api_route %} - {{filter_type}}({{function_wrapper_name.symbolize}}, only: {{ann[:only]}}, except: {{ann[:except]}}) + {{filter_type}}({{function_wrapper_name.symbolize}}, only: {{ann[:only]}}, except: {{ann[:except]}}, filter_name: {{method_name}}) # :nodoc: def {{function_wrapper_name}}