From 3033e3c9f3769f6857f0a9179b126f3b309f9347 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sun, 20 May 2018 14:05:07 -0400 Subject: [PATCH 1/9] Add support for slashes in feature names to UI refs #357 This is a tricky one. The issue is that I was assuming no one would use slashes. Using a slash completely broke all the routing. I changed the routing to be a block so it is more flexible. Now it is possible to route based on anything in request. The API will likely need the same work. --- lib/flipper/ui/action.rb | 19 +++++---- lib/flipper/ui/action_collection.rb | 2 +- lib/flipper/ui/actions/actors_gate.rb | 18 ++++++--- lib/flipper/ui/actions/add_feature.rb | 4 +- lib/flipper/ui/actions/boolean_gate.rb | 15 +++++-- lib/flipper/ui/actions/feature.rb | 14 +++++-- lib/flipper/ui/actions/features.rb | 4 +- lib/flipper/ui/actions/file.rb | 4 +- lib/flipper/ui/actions/gate.rb | 39 ------------------- lib/flipper/ui/actions/groups_gate.rb | 18 ++++++--- lib/flipper/ui/actions/home.rb | 4 +- .../ui/actions/percentage_of_actors_gate.rb | 15 +++++-- .../ui/actions/percentage_of_time_gate.rb | 15 +++++-- lib/flipper/ui/middleware.rb | 3 +- spec/flipper/ui/actions/actors_gate_spec.rb | 15 +++++++ spec/flipper/ui/actions/feature_spec.rb | 20 ++++++++++ spec/flipper/ui/actions/gate_spec.rb | 37 ------------------ 17 files changed, 133 insertions(+), 113 deletions(-) delete mode 100644 lib/flipper/ui/actions/gate.rb delete mode 100644 spec/flipper/ui/actions/gate_spec.rb diff --git a/lib/flipper/ui/action.rb b/lib/flipper/ui/action.rb index 2f39d41b4..80ed544a1 100644 --- a/lib/flipper/ui/action.rb +++ b/lib/flipper/ui/action.rb @@ -17,11 +17,19 @@ class Action # Public: Call this in subclasses so the action knows its route. # - # regex - The Regexp that this action should run for. + # block - The Block that determines if this can handle a given request. # # Returns nothing. - def self.route(regex) - @regex = regex + def self.match(&block) + if block_given? + @match = block + else + @match || raise("No match block set for #{name}") + end + end + + def self.match?(request) + !!match.call(request) end # Internal: Initializes and runs an action for a given request. @@ -34,11 +42,6 @@ def self.run(flipper, request) new(flipper, request).run end - # Internal: The regex that matches which routes this action will work for. - def self.regex - @regex || raise("#{name}.route is not set") - end - # Private: The path to the views folder. def self.views_path @views_path ||= Flipper::UI.root.join('views') diff --git a/lib/flipper/ui/action_collection.rb b/lib/flipper/ui/action_collection.rb index f2dea61fc..44b551c7e 100644 --- a/lib/flipper/ui/action_collection.rb +++ b/lib/flipper/ui/action_collection.rb @@ -12,7 +12,7 @@ def add(action_class) def action_for_request(request) @action_classes.detect do |action_class| - request.path_info =~ action_class.regex + action_class.match?(request) end end end diff --git a/lib/flipper/ui/actions/actors_gate.rb b/lib/flipper/ui/actions/actors_gate.rb index fb7c98a3d..53f02bf8f 100644 --- a/lib/flipper/ui/actions/actors_gate.rb +++ b/lib/flipper/ui/actions/actors_gate.rb @@ -6,11 +6,11 @@ module Flipper module UI module Actions class ActorsGate < UI::Action - route %r{features/[^/]*/actors/?\Z} + REGEX = %r{\A/features/(.*)/actors/?\Z} + match { |request| request.path_info =~ REGEX } def get - feature_name = Rack::Utils.unescape(request.path.split('/')[-2]) - feature = flipper[feature_name.to_sym] + feature = flipper[feature_name] @feature = Decorators::Feature.new(feature) breadcrumb 'Home', '/' @@ -22,8 +22,7 @@ def get end def post - feature_name = Rack::Utils.unescape(request.path.split('/')[-2]) - feature = flipper[feature_name.to_sym] + feature = flipper[feature_name] value = params['value'].to_s.strip if Util.blank?(value) @@ -42,6 +41,15 @@ def post redirect_to("/features/#{feature.key}") end + + private + + def feature_name + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end + end end end end diff --git a/lib/flipper/ui/actions/add_feature.rb b/lib/flipper/ui/actions/add_feature.rb index 5c1ab75f0..8a81b1258 100644 --- a/lib/flipper/ui/actions/add_feature.rb +++ b/lib/flipper/ui/actions/add_feature.rb @@ -5,7 +5,9 @@ module Flipper module UI module Actions class AddFeature < UI::Action - route %r{features/new/?\Z} + match do |request| + request.path_info =~ %r{\A/features/new/?\Z} + end def get unless Flipper::UI.configuration.feature_creation_enabled diff --git a/lib/flipper/ui/actions/boolean_gate.rb b/lib/flipper/ui/actions/boolean_gate.rb index db455aa5b..038142b02 100644 --- a/lib/flipper/ui/actions/boolean_gate.rb +++ b/lib/flipper/ui/actions/boolean_gate.rb @@ -5,11 +5,11 @@ module Flipper module UI module Actions class BooleanGate < UI::Action - route %r{features/[^/]*/boolean/?\Z} + REGEX = %r{\A/features/(.*)/boolean/?\Z} + match { |request| request.path_info =~ REGEX } def post - feature_name = Rack::Utils.unescape(request.path.split('/')[-2]) - feature = flipper[feature_name.to_sym] + feature = flipper[feature_name] @feature = Decorators::Feature.new(feature) if params['action'] == 'Enable' @@ -20,6 +20,15 @@ def post redirect_to "/features/#{@feature.key}" end + + private + + def feature_name + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end + end end end end diff --git a/lib/flipper/ui/actions/feature.rb b/lib/flipper/ui/actions/feature.rb index bc040d8f1..9e70b256a 100644 --- a/lib/flipper/ui/actions/feature.rb +++ b/lib/flipper/ui/actions/feature.rb @@ -5,10 +5,10 @@ module Flipper module UI module Actions class Feature < UI::Action - route %r{features/[^/]*/?\Z} + REGEX = %r{\A/features/(.*)\Z} + match { |request| request.path_info =~ REGEX } def get - feature_name = Rack::Utils.unescape(request.path.split('/').last) @feature = Decorators::Feature.new(flipper[feature_name]) @page_title = "#{@feature.key} // Features" @percentages = [0, 1, 5, 10, 15, 25, 50, 75, 100] @@ -30,11 +30,19 @@ def delete halt view_response(:feature_removal_disabled) end - feature_name = Rack::Utils.unescape(request.path.split('/').last) feature = flipper[feature_name] feature.remove redirect_to '/features' end + + private + + def feature_name + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end + end end end end diff --git a/lib/flipper/ui/actions/features.rb b/lib/flipper/ui/actions/features.rb index d157e5dce..d31d60267 100644 --- a/lib/flipper/ui/actions/features.rb +++ b/lib/flipper/ui/actions/features.rb @@ -6,7 +6,9 @@ module Flipper module UI module Actions class Features < UI::Action - route %r{features/?\Z} + match do |request| + request.path_info =~ %r{\A/features/?\Z} + end def get @page_title = 'Features' diff --git a/lib/flipper/ui/actions/file.rb b/lib/flipper/ui/actions/file.rb index 173b5b424..cce80f3aa 100644 --- a/lib/flipper/ui/actions/file.rb +++ b/lib/flipper/ui/actions/file.rb @@ -5,7 +5,9 @@ module Flipper module UI module Actions class File < UI::Action - route %r{(images|css|js|octicons|fonts)/.*\Z} + match do |request| + request.path_info =~ %r{(images|css|js|octicons|fonts)/.*\Z} + end def get Rack::File.new(public_path).call(request.env) diff --git a/lib/flipper/ui/actions/gate.rb b/lib/flipper/ui/actions/gate.rb deleted file mode 100644 index 576ced0f5..000000000 --- a/lib/flipper/ui/actions/gate.rb +++ /dev/null @@ -1,39 +0,0 @@ -require 'flipper/ui/action' -require 'flipper/ui/decorators/feature' - -module Flipper - module UI - module Actions - class Gate < UI::Action - route %r{features/[^/]*/[^/]*/?\Z} - - def post - feature_name, gate_name = request.path.split('/').pop(2) - .map(&Rack::Utils.method(:unescape)) - update_gate_method_name = "update_#{gate_name}" - - feature = flipper[feature_name.to_sym] - @feature = Decorators::Feature.new(feature) - - if respond_to?(update_gate_method_name, true) - send(update_gate_method_name, feature) - else - update_gate_method_undefined(gate_name) - end - - redirect_to "/features/#{@feature.key}" - end - - private - - # Private: Returns error response that gate update method is not defined. - def update_gate_method_undefined(gate_name) - error = Rack::Utils.escape( - "#{gate_name.inspect} gate does not exist therefore it cannot be updated." - ) - redirect_to("/features/#{@feature.key}?error=#{error}") - end - end - end - end -end diff --git a/lib/flipper/ui/actions/groups_gate.rb b/lib/flipper/ui/actions/groups_gate.rb index 85484d5be..b5d5d1ab6 100644 --- a/lib/flipper/ui/actions/groups_gate.rb +++ b/lib/flipper/ui/actions/groups_gate.rb @@ -5,11 +5,11 @@ module Flipper module UI module Actions class GroupsGate < UI::Action - route %r{features/[^/]*/groups/?\Z} + REGEX = %r{\A/features/(.*)/groups/?\Z} + match { |request| request.path_info =~ REGEX } def get - feature_name = Rack::Utils.unescape(request.path.split('/')[-2]) - feature = flipper[feature_name.to_sym] + feature = flipper[feature_name] @feature = Decorators::Feature.new(feature) breadcrumb 'Home', '/' @@ -21,8 +21,7 @@ def get end def post - feature_name = Rack::Utils.unescape(request.path.split('/')[-2]) - feature = flipper[feature_name.to_sym] + feature = flipper[feature_name] value = params['value'].to_s.strip if Flipper.group_exists?(value) @@ -39,6 +38,15 @@ def post redirect_to("/features/#{feature.key}/groups?error=#{error}") end end + + private + + def feature_name + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end + end end end end diff --git a/lib/flipper/ui/actions/home.rb b/lib/flipper/ui/actions/home.rb index ea55a01e5..5f2415043 100644 --- a/lib/flipper/ui/actions/home.rb +++ b/lib/flipper/ui/actions/home.rb @@ -5,7 +5,9 @@ module Flipper module UI module Actions class Home < UI::Action - route %r{/?\Z} + match do |request| + request.path_info =~ %r{\A/?\Z} + end def get redirect_to '/features' diff --git a/lib/flipper/ui/actions/percentage_of_actors_gate.rb b/lib/flipper/ui/actions/percentage_of_actors_gate.rb index 5a5a38058..b0cd8b327 100644 --- a/lib/flipper/ui/actions/percentage_of_actors_gate.rb +++ b/lib/flipper/ui/actions/percentage_of_actors_gate.rb @@ -5,11 +5,11 @@ module Flipper module UI module Actions class PercentageOfActorsGate < UI::Action - route %r{features/[^/]*/percentage_of_actors/?\Z} + REGEX = %r{\A/features/(.*)/percentage_of_actors/?\Z} + match { |request| request.path_info =~ REGEX } def post - feature_name = Rack::Utils.unescape(request.path.split('/')[-2]) - feature = flipper[feature_name.to_sym] + feature = flipper[feature_name] @feature = Decorators::Feature.new(feature) begin @@ -21,6 +21,15 @@ def post redirect_to "/features/#{@feature.key}" end + + private + + def feature_name + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end + end end end end diff --git a/lib/flipper/ui/actions/percentage_of_time_gate.rb b/lib/flipper/ui/actions/percentage_of_time_gate.rb index 2e0998457..51f72b97a 100644 --- a/lib/flipper/ui/actions/percentage_of_time_gate.rb +++ b/lib/flipper/ui/actions/percentage_of_time_gate.rb @@ -5,11 +5,11 @@ module Flipper module UI module Actions class PercentageOfTimeGate < UI::Action - route %r{features/[^/]*/percentage_of_time/?\Z} + REGEX = %r{\A/features/(.*)/percentage_of_time/?\Z} + match { |request| request.path_info =~ REGEX } def post - feature_name = Rack::Utils.unescape(request.path.split('/')[-2]) - feature = flipper[feature_name.to_sym] + feature = flipper[feature_name] @feature = Decorators::Feature.new(feature) begin @@ -21,6 +21,15 @@ def post redirect_to "/features/#{@feature.key}" end + + private + + def feature_name + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end + end end end end diff --git a/lib/flipper/ui/middleware.rb b/lib/flipper/ui/middleware.rb index 27a29ff99..431855d76 100644 --- a/lib/flipper/ui/middleware.rb +++ b/lib/flipper/ui/middleware.rb @@ -17,13 +17,12 @@ def initialize(app, options = {}) # UI @action_collection.add UI::Actions::AddFeature - @action_collection.add UI::Actions::Feature @action_collection.add UI::Actions::ActorsGate @action_collection.add UI::Actions::GroupsGate @action_collection.add UI::Actions::BooleanGate @action_collection.add UI::Actions::PercentageOfTimeGate @action_collection.add UI::Actions::PercentageOfActorsGate - @action_collection.add UI::Actions::Gate + @action_collection.add UI::Actions::Feature @action_collection.add UI::Actions::Features # Static Assets/Files diff --git a/spec/flipper/ui/actions/actors_gate_spec.rb b/spec/flipper/ui/actions/actors_gate_spec.rb index 4ff2cc722..eb3ace17c 100644 --- a/spec/flipper/ui/actions/actors_gate_spec.rb +++ b/spec/flipper/ui/actions/actors_gate_spec.rb @@ -27,6 +27,21 @@ end end + describe 'GET /features/:feature/actors with slash in feature name' do + before do + get 'features/a/b/actors' + end + + it 'responds with success' do + expect(last_response.status).to be(200) + end + + it 'renders add new actor form' do + form = '
' + expect(last_response.body).to include(form) + end + end + describe 'POST /features/:feature/actors' do context 'enabling an actor' do let(:value) { 'User:6' } diff --git a/spec/flipper/ui/actions/feature_spec.rb b/spec/flipper/ui/actions/feature_spec.rb index 5d685293c..f7c92b630 100644 --- a/spec/flipper/ui/actions/feature_spec.rb +++ b/spec/flipper/ui/actions/feature_spec.rb @@ -107,4 +107,24 @@ expect(last_response.body).to include('Percentage of Actors') end end + + describe 'GET /features/:feature with slash in feature name' do + before do + get '/features/a/b' + end + + it 'responds with success' do + expect(last_response.status).to be(200) + end + + it 'renders template' do + expect(last_response.body).to include('a/b') + expect(last_response.body).to include('Enable') + expect(last_response.body).to include('Disable') + expect(last_response.body).to include('Actors') + expect(last_response.body).to include('Groups') + expect(last_response.body).to include('Percentage of Time') + expect(last_response.body).to include('Percentage of Actors') + end + end end diff --git a/spec/flipper/ui/actions/gate_spec.rb b/spec/flipper/ui/actions/gate_spec.rb deleted file mode 100644 index c4341c391..000000000 --- a/spec/flipper/ui/actions/gate_spec.rb +++ /dev/null @@ -1,37 +0,0 @@ -require 'helper' - -RSpec.describe Flipper::UI::Actions::Gate do - let(:token) do - if Rack::Protection::AuthenticityToken.respond_to?(:random_token) - Rack::Protection::AuthenticityToken.random_token - else - 'a' - end - end - let(:session) do - { :csrf => token, 'csrf' => token, '_csrf_token' => token } - end - - describe 'POST /features/:feature/non-existent-gate' do - before do - post '/features/search/non-existent-gate', - { 'authenticity_token' => token }, - 'rack.session' => session - end - - it 'responds with redirect' do - expect(last_response.status).to be(302) - end - - # rubocop:disable Metrics/LineLength - it 'escapes error message' do - expect(last_response.headers['Location']).to eq('/features/search?error=%22non-existent-gate%22+gate+does+not+exist+therefore+it+cannot+be+updated.') - end - # rubocop:enable Metrics/LineLength - - it 'renders error in template' do - follow_redirect! - expect(last_response.body).to match(/non-existent-gate.*gate does not exist/) - end - end -end From cccf29f878644029b7defd61851c95f9c81763a5 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sun, 20 May 2018 14:06:06 -0400 Subject: [PATCH 2/9] Make it easy to only log requests but still log feature stuff if you want it --- examples/ui/basic.ru | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/examples/ui/basic.ru b/examples/ui/basic.ru index 3133ffeee..bc41190f1 100644 --- a/examples/ui/basic.ru +++ b/examples/ui/basic.ru @@ -14,6 +14,7 @@ $:.unshift(lib_path) require "flipper-ui" require "flipper/adapters/pstore" +require "active_support/notifications" Flipper.register(:admins) { |actor| actor.respond_to?(:admin?) && actor.admin? @@ -24,10 +25,11 @@ Flipper.register(:early_access) { |actor| } # Setup logging of flipper calls. -$logger = Logger.new(STDOUT) -require "active_support/notifications" -require "flipper/instrumentation/log_subscriber" -Flipper::Instrumentation::LogSubscriber.logger = $logger +if ENV["LOG"] == "1" + $logger = Logger.new(STDOUT) + require "flipper/instrumentation/log_subscriber" + Flipper::Instrumentation::LogSubscriber.logger = $logger +end adapter = Flipper::Adapters::PStore.new flipper = Flipper.new(adapter, instrumenter: ActiveSupport::Notifications) From 569d8c3d2d2d6c1677407519be8e63c1944901c7 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sun, 20 May 2018 14:06:30 -0400 Subject: [PATCH 3/9] Add a slash feature to the example --- examples/ui/basic.ru | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/ui/basic.ru b/examples/ui/basic.ru index bc41190f1..a1af55716 100644 --- a/examples/ui/basic.ru +++ b/examples/ui/basic.ru @@ -44,6 +44,7 @@ flipper = Flipper.new(adapter, instrumenter: ActiveSupport::Notifications) # flipper[:secrets].enable_group :early_access # flipper[:logging].enable_percentage_of_time 5 # flipper[:new_cache].enable_percentage_of_actors 15 +# flipper["a/b"].add run Flipper::UI.app(flipper) { |builder| builder.use Rack::Session::Cookie, secret: "_super_secret" From 1602d2e1488f0cd9b0923ed20693d46c81f1a5fb Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sun, 3 Jun 2018 14:47:13 -0400 Subject: [PATCH 4/9] Add support for slashes in feature names to API refs #357 refs 3033e3c9f3769f6857f0a9179b126f3b309f9347 --- lib/flipper/api/action.rb | 14 ++++-- lib/flipper/api/action_collection.rb | 3 +- lib/flipper/api/v1/actions/actors_gate.rb | 8 +++- lib/flipper/api/v1/actions/boolean_gate.rb | 14 ++++-- lib/flipper/api/v1/actions/clear_feature.rb | 13 +++++- lib/flipper/api/v1/actions/feature.rb | 8 +++- lib/flipper/api/v1/actions/features.rb | 2 +- lib/flipper/api/v1/actions/groups_gate.rb | 8 +++- .../v1/actions/percentage_of_actors_gate.rb | 8 +++- .../api/v1/actions/percentage_of_time_gate.rb | 8 +++- .../api/v1/actions/actors_gate_spec.rb | 18 ++++++++ .../api/v1/actions/boolean_gate_spec.rb | 17 +++++++ .../api/v1/actions/clear_feature_spec.rb | 21 +++++++++ spec/flipper/api/v1/actions/feature_spec.rb | 44 +++++++++++++++++++ .../api/v1/actions/groups_gate_spec.rb | 23 ++++++++++ .../actions/percentage_of_actors_gate_spec.rb | 16 +++++++ .../actions/percentage_of_time_gate_spec.rb | 16 +++++++ 17 files changed, 221 insertions(+), 20 deletions(-) diff --git a/lib/flipper/api/action.rb b/lib/flipper/api/action.rb index df8ab4173..814043010 100644 --- a/lib/flipper/api/action.rb +++ b/lib/flipper/api/action.rb @@ -17,11 +17,19 @@ class Action # Public: Call this in subclasses so the action knows its route. # - # regex - The Regexp that this action should run for. + # block - The Block that determines if this can handle a given request. # # Returns nothing. - def self.route(regex) - @regex = regex + def self.match(&block) + if block_given? + @match = block + else + @match || raise("No match block set for #{name}") + end + end + + def self.match?(request) + !!match.call(request) end # Internal: Initializes and runs an action for a given request. diff --git a/lib/flipper/api/action_collection.rb b/lib/flipper/api/action_collection.rb index 316e33c53..63623e893 100644 --- a/lib/flipper/api/action_collection.rb +++ b/lib/flipper/api/action_collection.rb @@ -1,5 +1,6 @@ module Flipper module Api + # Internal: Used to detect the action that should be used in the middleware. class ActionCollection def initialize @action_classes = [] @@ -11,7 +12,7 @@ def add(action_class) def action_for_request(request) @action_classes.detect do |action_class| - request.path_info =~ action_class.regex + action_class.match?(request) end end end diff --git a/lib/flipper/api/v1/actions/actors_gate.rb b/lib/flipper/api/v1/actions/actors_gate.rb index d38df144c..b340fd058 100644 --- a/lib/flipper/api/v1/actions/actors_gate.rb +++ b/lib/flipper/api/v1/actions/actors_gate.rb @@ -6,7 +6,8 @@ module Api module V1 module Actions class ActorsGate < Api::Action - route %r{features/[^/]*/actors/?\Z} + REGEX = %r{\A/features/(.*)/actors/?\Z} + match { |request| request.path_info =~ REGEX } def post ensure_valid_params @@ -33,7 +34,10 @@ def ensure_valid_params end def feature_name - @feature_name ||= Rack::Utils.unescape(path_parts[-2]) + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end end def flipper_id diff --git a/lib/flipper/api/v1/actions/boolean_gate.rb b/lib/flipper/api/v1/actions/boolean_gate.rb index e51232b63..e85cd021a 100644 --- a/lib/flipper/api/v1/actions/boolean_gate.rb +++ b/lib/flipper/api/v1/actions/boolean_gate.rb @@ -6,10 +6,10 @@ module Api module V1 module Actions class BooleanGate < Api::Action - route %r{features/[^/]*/boolean/?\Z} + REGEX = %r{\A/features/(.*)/boolean/?\Z} + match { |request| request.path_info =~ REGEX } def post - feature_name = Rack::Utils.unescape(path_parts[-2]) feature = flipper[feature_name] feature.enable decorated_feature = Decorators::Feature.new(feature) @@ -17,12 +17,20 @@ def post end def delete - feature_name = Rack::Utils.unescape(path_parts[-2]) feature = flipper[feature_name.to_sym] feature.disable decorated_feature = Decorators::Feature.new(feature) json_response(decorated_feature.as_json, 200) end + + private + + def feature_name + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end + end end end end diff --git a/lib/flipper/api/v1/actions/clear_feature.rb b/lib/flipper/api/v1/actions/clear_feature.rb index edd78f19f..1359cce93 100644 --- a/lib/flipper/api/v1/actions/clear_feature.rb +++ b/lib/flipper/api/v1/actions/clear_feature.rb @@ -6,14 +6,23 @@ module Api module V1 module Actions class ClearFeature < Api::Action - route %r{features/[^/]*/clear/?\Z} + REGEX = %r{\A/features/(.*)/clear/?\Z} + match { |request| request.path_info =~ REGEX } def delete - feature_name = Rack::Utils.unescape(path_parts[-2]) feature = flipper[feature_name] feature.clear json_response({}, 204) end + + private + + def feature_name + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end + end end end end diff --git a/lib/flipper/api/v1/actions/feature.rb b/lib/flipper/api/v1/actions/feature.rb index c5e23306d..873a428af 100644 --- a/lib/flipper/api/v1/actions/feature.rb +++ b/lib/flipper/api/v1/actions/feature.rb @@ -6,7 +6,8 @@ module Api module V1 module Actions class Feature < Api::Action - route %r{features/[^/]*/?\Z} + REGEX = %r{\A/features/(.*)/?\Z} + match { |request| request.path_info =~ REGEX } def get return json_error_response(:feature_not_found) unless feature_exists?(feature_name) @@ -22,7 +23,10 @@ def delete private def feature_name - @feature_name ||= Rack::Utils.unescape(path_parts.last) + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end end def feature_exists?(feature_name) diff --git a/lib/flipper/api/v1/actions/features.rb b/lib/flipper/api/v1/actions/features.rb index e1bde84df..ddced99fe 100644 --- a/lib/flipper/api/v1/actions/features.rb +++ b/lib/flipper/api/v1/actions/features.rb @@ -6,7 +6,7 @@ module Api module V1 module Actions class Features < Api::Action - route(/features\Z/) + match { |request| request.path_info =~ %r{\A/features/?\Z} } def get keys = params['keys'] diff --git a/lib/flipper/api/v1/actions/groups_gate.rb b/lib/flipper/api/v1/actions/groups_gate.rb index 21731db3c..12e6c115e 100644 --- a/lib/flipper/api/v1/actions/groups_gate.rb +++ b/lib/flipper/api/v1/actions/groups_gate.rb @@ -6,7 +6,8 @@ module Api module V1 module Actions class GroupsGate < Api::Action - route %r{features/[^/]*/groups/?\Z} + REGEX = %r{\A/features/(.*)/groups/?\Z} + match { |request| request.path_info =~ REGEX } def post ensure_valid_params @@ -47,7 +48,10 @@ def disallow_unregistered_groups? end def feature_name - @feature_name ||= Rack::Utils.unescape(path_parts[-2]) + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end end def group_name diff --git a/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb b/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb index f57087b56..bc54a701b 100644 --- a/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb +++ b/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb @@ -6,7 +6,8 @@ module Api module V1 module Actions class PercentageOfActorsGate < Api::Action - route %r{features/[^/]*/percentage_of_actors/?\Z} + REGEX = %r{\A/features/(.*)/percentage_of_actors/?\Z} + match { |request| request.path_info =~ REGEX } def post if percentage < 0 || percentage > 100 @@ -29,7 +30,10 @@ def delete private def feature_name - @feature_name ||= Rack::Utils.unescape(path_parts[-2]) + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end end def percentage diff --git a/lib/flipper/api/v1/actions/percentage_of_time_gate.rb b/lib/flipper/api/v1/actions/percentage_of_time_gate.rb index 404e8545b..76398762a 100644 --- a/lib/flipper/api/v1/actions/percentage_of_time_gate.rb +++ b/lib/flipper/api/v1/actions/percentage_of_time_gate.rb @@ -6,7 +6,8 @@ module Api module V1 module Actions class PercentageOfTimeGate < Api::Action - route %r{features/[^/]*/percentage_of_time/?\Z} + REGEX = %r{\A/features/(.*)/percentage_of_time/?\Z} + match { |request| request.path_info =~ REGEX } def post if percentage < 0 || percentage > 100 @@ -30,7 +31,10 @@ def delete private def feature_name - @feature_name ||= Rack::Utils.unescape(path_parts[-2]) + @feature_name ||= begin + match = request.path_info.match(REGEX) + match ? match[1] : nil + end end def percentage diff --git a/spec/flipper/api/v1/actions/actors_gate_spec.rb b/spec/flipper/api/v1/actions/actors_gate_spec.rb index 68a89e1ed..dfa35948e 100644 --- a/spec/flipper/api/v1/actions/actors_gate_spec.rb +++ b/spec/flipper/api/v1/actions/actors_gate_spec.rb @@ -42,6 +42,24 @@ end end + describe 'enable feature with slash in name' do + before do + flipper[:my_feature].disable_actor(actor) + post '/features/my/feature/actors', flipper_id: actor.flipper_id + end + + it 'enables feature for actor' do + expect(last_response.status).to eq(200) + expect(flipper["my/feature"].enabled?(actor)).to be_truthy + expect(flipper["my/feature"].enabled_gate_names).to eq([:actor]) + end + + it 'returns decorated feature with actor enabled' do + gate = json_response['gates'].find { |gate| gate['key'] == 'actors' } + expect(gate['value']).to eq(['1']) + end + end + describe 'enable missing flipper_id parameter' do before do flipper[:my_feature].enable diff --git a/spec/flipper/api/v1/actions/boolean_gate_spec.rb b/spec/flipper/api/v1/actions/boolean_gate_spec.rb index 76d81e05f..71978a87b 100644 --- a/spec/flipper/api/v1/actions/boolean_gate_spec.rb +++ b/spec/flipper/api/v1/actions/boolean_gate_spec.rb @@ -20,6 +20,23 @@ end end + describe 'enable feature with slash in name' do + before do + flipper["my/feature"].disable + post '/features/my/feature/boolean' + end + + it 'enables feature' do + expect(last_response.status).to eq(200) + expect(flipper["my/feature"].on?).to be_truthy + end + + it 'returns decorated feature with boolean gate enabled' do + boolean_gate = json_response['gates'].find { |gate| gate['key'] == 'boolean' } + expect(boolean_gate['value']).to be_truthy + end + end + describe 'disable' do before do flipper[:my_feature].enable diff --git a/spec/flipper/api/v1/actions/clear_feature_spec.rb b/spec/flipper/api/v1/actions/clear_feature_spec.rb index d878cd8bd..bda8f14c4 100644 --- a/spec/flipper/api/v1/actions/clear_feature_spec.rb +++ b/spec/flipper/api/v1/actions/clear_feature_spec.rb @@ -23,4 +23,25 @@ expect(flipper[:my_feature].off?).to be_truthy end end + + describe 'clear feature with slash in name' do + before do + Flipper.register(:admins) {} + actor22 = Flipper::Actor.new('22') + + feature = flipper["my/feature"] + feature.enable flipper.boolean + feature.enable flipper.group(:admins) + feature.enable flipper.actor(actor22) + feature.enable flipper.actors(25) + feature.enable flipper.time(45) + + delete '/features/my/feature/clear' + end + + it 'clears feature' do + expect(last_response.status).to eq(204) + expect(flipper["my/feature"].off?).to be_truthy + end + end end diff --git a/spec/flipper/api/v1/actions/feature_spec.rb b/spec/flipper/api/v1/actions/feature_spec.rb index 728e0085e..1027037be 100644 --- a/spec/flipper/api/v1/actions/feature_spec.rb +++ b/spec/flipper/api/v1/actions/feature_spec.rb @@ -153,6 +153,50 @@ expect(json_response).to eq(response_body) end end + + context 'feature with name that has slash' do + before do + flipper["my/feature"].enable + get '/features/my/feature' + end + + it 'responds with correct attributes' do + response_body = { + 'key' => 'my/feature', + 'state' => 'on', + 'gates' => [ + { + 'key' => 'boolean', + 'name' => 'boolean', + 'value' => 'true', + }, + { + 'key' => 'groups', + 'name' => 'group', + 'value' => [], + }, + { + 'key' => 'actors', + 'name' => 'actor', + 'value' => [], + }, + { + 'key' => 'percentage_of_actors', + 'name' => 'percentage_of_actors', + 'value' => nil, + }, + { + 'key' => 'percentage_of_time', + 'name' => 'percentage_of_time', + 'value' => nil, + }, + ], + } + + expect(last_response.status).to eq(200) + expect(json_response).to eq(response_body) + end + end end describe 'delete' do diff --git a/spec/flipper/api/v1/actions/groups_gate_spec.rb b/spec/flipper/api/v1/actions/groups_gate_spec.rb index c2f69eafa..a330633c7 100644 --- a/spec/flipper/api/v1/actions/groups_gate_spec.rb +++ b/spec/flipper/api/v1/actions/groups_gate_spec.rb @@ -26,6 +26,29 @@ end end + describe 'enable feature with slash in name' do + before do + flipper["my/feature"].disable + Flipper.register(:admins) do |actor| + actor.respond_to?(:admin?) && actor.admin? + end + post '/features/my/feature/groups', name: 'admins' + end + + it 'enables feature for group' do + person = double + allow(person).to receive(:flipper_id).and_return(1) + allow(person).to receive(:admin?).and_return(true) + expect(last_response.status).to eq(200) + expect(flipper["my/feature"].enabled?(person)).to be_truthy + end + + it 'returns decorated feature with group enabled' do + group_gate = json_response['gates'].find { |m| m['name'] == 'group' } + expect(group_gate['value']).to eq(['admins']) + end + end + describe 'enable without name params' do before do flipper[:my_feature].disable diff --git a/spec/flipper/api/v1/actions/percentage_of_actors_gate_spec.rb b/spec/flipper/api/v1/actions/percentage_of_actors_gate_spec.rb index 3eeebf5f8..f568cce0a 100644 --- a/spec/flipper/api/v1/actions/percentage_of_actors_gate_spec.rb +++ b/spec/flipper/api/v1/actions/percentage_of_actors_gate_spec.rb @@ -4,6 +4,22 @@ let(:app) { build_api(flipper) } describe 'enable' do + context 'for feature with slash in name' do + before do + flipper["my/feature"].disable + post '/features/my/feature/percentage_of_actors', percentage: '10' + end + + it 'enables gate for feature' do + expect(flipper["my/feature"].enabled_gate_names).to include(:percentage_of_actors) + end + + it 'returns decorated feature with gate enabled for 10 percent of actors' do + gate = json_response['gates'].find { |gate| gate['name'] == 'percentage_of_actors' } + expect(gate['value']).to eq('10') + end + end + context 'url-encoded request' do before do flipper[:my_feature].disable diff --git a/spec/flipper/api/v1/actions/percentage_of_time_gate_spec.rb b/spec/flipper/api/v1/actions/percentage_of_time_gate_spec.rb index e975fb010..72bb35d5a 100644 --- a/spec/flipper/api/v1/actions/percentage_of_time_gate_spec.rb +++ b/spec/flipper/api/v1/actions/percentage_of_time_gate_spec.rb @@ -19,6 +19,22 @@ end end + describe 'enable for feature with slash in name' do + before do + flipper["my/feature"].disable + post '/features/my/feature/percentage_of_time', percentage: '10' + end + + it 'enables gate for feature' do + expect(flipper["my/feature"].enabled_gate_names).to include(:percentage_of_time) + end + + it 'returns decorated feature with gate enabled for 5% of time' do + gate = json_response['gates'].find { |gate| gate['name'] == 'percentage_of_time' } + expect(gate['value']).to eq('10') + end + end + describe 'disable without percentage' do before do flipper[:my_feature].enable_percentage_of_time(10) From de10683bbbe3026e954cb85620d46e116419bfd0 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sun, 3 Jun 2018 15:05:15 -0400 Subject: [PATCH 5/9] Make rubo happy by removing double negation --- lib/flipper/api/action.rb | 2 +- lib/flipper/ui/action.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/flipper/api/action.rb b/lib/flipper/api/action.rb index 814043010..08d14532d 100644 --- a/lib/flipper/api/action.rb +++ b/lib/flipper/api/action.rb @@ -29,7 +29,7 @@ def self.match(&block) end def self.match?(request) - !!match.call(request) + match.call(request) end # Internal: Initializes and runs an action for a given request. diff --git a/lib/flipper/ui/action.rb b/lib/flipper/ui/action.rb index 80ed544a1..6731127bc 100644 --- a/lib/flipper/ui/action.rb +++ b/lib/flipper/ui/action.rb @@ -29,7 +29,7 @@ def self.match(&block) end def self.match?(request) - !!match.call(request) + match.call(request) end # Internal: Initializes and runs an action for a given request. From f61bbe9feeade1e1a7e5981db45180ee02817e63 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Mon, 30 Jul 2018 14:09:58 -0400 Subject: [PATCH 6/9] Switch to named captures for route regex --- lib/flipper/api/v1/actions/actors_gate.rb | 4 ++-- lib/flipper/api/v1/actions/boolean_gate.rb | 4 ++-- lib/flipper/api/v1/actions/clear_feature.rb | 4 ++-- lib/flipper/api/v1/actions/feature.rb | 4 ++-- lib/flipper/api/v1/actions/groups_gate.rb | 4 ++-- lib/flipper/api/v1/actions/percentage_of_actors_gate.rb | 4 ++-- lib/flipper/api/v1/actions/percentage_of_time_gate.rb | 4 ++-- lib/flipper/ui/actions/actors_gate.rb | 4 ++-- lib/flipper/ui/actions/boolean_gate.rb | 4 ++-- lib/flipper/ui/actions/feature.rb | 4 ++-- lib/flipper/ui/actions/groups_gate.rb | 4 ++-- lib/flipper/ui/actions/percentage_of_actors_gate.rb | 4 ++-- lib/flipper/ui/actions/percentage_of_time_gate.rb | 4 ++-- 13 files changed, 26 insertions(+), 26 deletions(-) diff --git a/lib/flipper/api/v1/actions/actors_gate.rb b/lib/flipper/api/v1/actions/actors_gate.rb index b340fd058..b8e672ee5 100644 --- a/lib/flipper/api/v1/actions/actors_gate.rb +++ b/lib/flipper/api/v1/actions/actors_gate.rb @@ -6,7 +6,7 @@ module Api module V1 module Actions class ActorsGate < Api::Action - REGEX = %r{\A/features/(.*)/actors/?\Z} + REGEX = %r{\A/features/(?.*)/actors/?\Z} match { |request| request.path_info =~ REGEX } def post @@ -36,7 +36,7 @@ def ensure_valid_params def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end diff --git a/lib/flipper/api/v1/actions/boolean_gate.rb b/lib/flipper/api/v1/actions/boolean_gate.rb index e85cd021a..2c0222362 100644 --- a/lib/flipper/api/v1/actions/boolean_gate.rb +++ b/lib/flipper/api/v1/actions/boolean_gate.rb @@ -6,7 +6,7 @@ module Api module V1 module Actions class BooleanGate < Api::Action - REGEX = %r{\A/features/(.*)/boolean/?\Z} + REGEX = %r{\A/features/(?.*)/boolean/?\Z} match { |request| request.path_info =~ REGEX } def post @@ -28,7 +28,7 @@ def delete def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end end diff --git a/lib/flipper/api/v1/actions/clear_feature.rb b/lib/flipper/api/v1/actions/clear_feature.rb index 1359cce93..00feb4b4c 100644 --- a/lib/flipper/api/v1/actions/clear_feature.rb +++ b/lib/flipper/api/v1/actions/clear_feature.rb @@ -6,7 +6,7 @@ module Api module V1 module Actions class ClearFeature < Api::Action - REGEX = %r{\A/features/(.*)/clear/?\Z} + REGEX = %r{\A/features/(?.*)/clear/?\Z} match { |request| request.path_info =~ REGEX } def delete @@ -20,7 +20,7 @@ def delete def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end end diff --git a/lib/flipper/api/v1/actions/feature.rb b/lib/flipper/api/v1/actions/feature.rb index 873a428af..ce0a40ddf 100644 --- a/lib/flipper/api/v1/actions/feature.rb +++ b/lib/flipper/api/v1/actions/feature.rb @@ -6,7 +6,7 @@ module Api module V1 module Actions class Feature < Api::Action - REGEX = %r{\A/features/(.*)/?\Z} + REGEX = %r{\A/features/(?.*)/?\Z} match { |request| request.path_info =~ REGEX } def get @@ -25,7 +25,7 @@ def delete def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end diff --git a/lib/flipper/api/v1/actions/groups_gate.rb b/lib/flipper/api/v1/actions/groups_gate.rb index 12e6c115e..7232bae03 100644 --- a/lib/flipper/api/v1/actions/groups_gate.rb +++ b/lib/flipper/api/v1/actions/groups_gate.rb @@ -6,7 +6,7 @@ module Api module V1 module Actions class GroupsGate < Api::Action - REGEX = %r{\A/features/(.*)/groups/?\Z} + REGEX = %r{\A/features/(?.*)/groups/?\Z} match { |request| request.path_info =~ REGEX } def post @@ -50,7 +50,7 @@ def disallow_unregistered_groups? def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end diff --git a/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb b/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb index bc54a701b..e88f45726 100644 --- a/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb +++ b/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb @@ -6,7 +6,7 @@ module Api module V1 module Actions class PercentageOfActorsGate < Api::Action - REGEX = %r{\A/features/(.*)/percentage_of_actors/?\Z} + REGEX = %r{\A/features/(?.*)/percentage_of_actors/?\Z} match { |request| request.path_info =~ REGEX } def post @@ -32,7 +32,7 @@ def delete def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end diff --git a/lib/flipper/api/v1/actions/percentage_of_time_gate.rb b/lib/flipper/api/v1/actions/percentage_of_time_gate.rb index 76398762a..2d1a5f53d 100644 --- a/lib/flipper/api/v1/actions/percentage_of_time_gate.rb +++ b/lib/flipper/api/v1/actions/percentage_of_time_gate.rb @@ -6,7 +6,7 @@ module Api module V1 module Actions class PercentageOfTimeGate < Api::Action - REGEX = %r{\A/features/(.*)/percentage_of_time/?\Z} + REGEX = %r{\A/features/(?.*)/percentage_of_time/?\Z} match { |request| request.path_info =~ REGEX } def post @@ -33,7 +33,7 @@ def delete def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end diff --git a/lib/flipper/ui/actions/actors_gate.rb b/lib/flipper/ui/actions/actors_gate.rb index 53f02bf8f..8f3ec187a 100644 --- a/lib/flipper/ui/actions/actors_gate.rb +++ b/lib/flipper/ui/actions/actors_gate.rb @@ -6,7 +6,7 @@ module Flipper module UI module Actions class ActorsGate < UI::Action - REGEX = %r{\A/features/(.*)/actors/?\Z} + REGEX = %r{\A/features/(?.*)/actors/?\Z} match { |request| request.path_info =~ REGEX } def get @@ -47,7 +47,7 @@ def post def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end end diff --git a/lib/flipper/ui/actions/boolean_gate.rb b/lib/flipper/ui/actions/boolean_gate.rb index 038142b02..df957d769 100644 --- a/lib/flipper/ui/actions/boolean_gate.rb +++ b/lib/flipper/ui/actions/boolean_gate.rb @@ -5,7 +5,7 @@ module Flipper module UI module Actions class BooleanGate < UI::Action - REGEX = %r{\A/features/(.*)/boolean/?\Z} + REGEX = %r{\A/features/(?.*)/boolean/?\Z} match { |request| request.path_info =~ REGEX } def post @@ -26,7 +26,7 @@ def post def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end end diff --git a/lib/flipper/ui/actions/feature.rb b/lib/flipper/ui/actions/feature.rb index 9e70b256a..7da7bf938 100644 --- a/lib/flipper/ui/actions/feature.rb +++ b/lib/flipper/ui/actions/feature.rb @@ -5,7 +5,7 @@ module Flipper module UI module Actions class Feature < UI::Action - REGEX = %r{\A/features/(.*)\Z} + REGEX = %r{\A/features/(?.*)\Z} match { |request| request.path_info =~ REGEX } def get @@ -40,7 +40,7 @@ def delete def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end end diff --git a/lib/flipper/ui/actions/groups_gate.rb b/lib/flipper/ui/actions/groups_gate.rb index b5d5d1ab6..7ab2648b0 100644 --- a/lib/flipper/ui/actions/groups_gate.rb +++ b/lib/flipper/ui/actions/groups_gate.rb @@ -5,7 +5,7 @@ module Flipper module UI module Actions class GroupsGate < UI::Action - REGEX = %r{\A/features/(.*)/groups/?\Z} + REGEX = %r{\A/features/(?.*)/groups/?\Z} match { |request| request.path_info =~ REGEX } def get @@ -44,7 +44,7 @@ def post def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end end diff --git a/lib/flipper/ui/actions/percentage_of_actors_gate.rb b/lib/flipper/ui/actions/percentage_of_actors_gate.rb index b0cd8b327..2ce28a262 100644 --- a/lib/flipper/ui/actions/percentage_of_actors_gate.rb +++ b/lib/flipper/ui/actions/percentage_of_actors_gate.rb @@ -5,7 +5,7 @@ module Flipper module UI module Actions class PercentageOfActorsGate < UI::Action - REGEX = %r{\A/features/(.*)/percentage_of_actors/?\Z} + REGEX = %r{\A/features/(?.*)/percentage_of_actors/?\Z} match { |request| request.path_info =~ REGEX } def post @@ -27,7 +27,7 @@ def post def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end end diff --git a/lib/flipper/ui/actions/percentage_of_time_gate.rb b/lib/flipper/ui/actions/percentage_of_time_gate.rb index 51f72b97a..491797b15 100644 --- a/lib/flipper/ui/actions/percentage_of_time_gate.rb +++ b/lib/flipper/ui/actions/percentage_of_time_gate.rb @@ -5,7 +5,7 @@ module Flipper module UI module Actions class PercentageOfTimeGate < UI::Action - REGEX = %r{\A/features/(.*)/percentage_of_time/?\Z} + REGEX = %r{\A/features/(?.*)/percentage_of_time/?\Z} match { |request| request.path_info =~ REGEX } def post @@ -27,7 +27,7 @@ def post def feature_name @feature_name ||= begin match = request.path_info.match(REGEX) - match ? match[1] : nil + match ? match[:feature_name] : nil end end end From 8fa06a54d0438794124845f35e89e54f8ac534a1 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Tue, 31 Jul 2018 13:53:27 -0400 Subject: [PATCH 7/9] Move back to single regex from overly flexible request match We just need to match the path info to a regex. We don't need the whole request. --- lib/flipper/api/action.rb | 25 ++++++++----------- lib/flipper/api/action_collection.rb | 2 +- lib/flipper/api/v1/actions/actors_gate.rb | 2 +- lib/flipper/api/v1/actions/boolean_gate.rb | 2 +- lib/flipper/api/v1/actions/clear_feature.rb | 2 +- lib/flipper/api/v1/actions/feature.rb | 2 +- lib/flipper/api/v1/actions/features.rb | 3 ++- lib/flipper/api/v1/actions/groups_gate.rb | 2 +- .../v1/actions/percentage_of_actors_gate.rb | 2 +- .../api/v1/actions/percentage_of_time_gate.rb | 2 +- lib/flipper/ui/action.rb | 20 ++++++++------- lib/flipper/ui/action_collection.rb | 2 +- lib/flipper/ui/actions/actors_gate.rb | 2 +- lib/flipper/ui/actions/add_feature.rb | 5 ++-- lib/flipper/ui/actions/boolean_gate.rb | 2 +- lib/flipper/ui/actions/feature.rb | 2 +- lib/flipper/ui/actions/features.rb | 5 ++-- lib/flipper/ui/actions/file.rb | 5 ++-- lib/flipper/ui/actions/groups_gate.rb | 2 +- lib/flipper/ui/actions/home.rb | 4 +-- .../ui/actions/percentage_of_actors_gate.rb | 2 +- .../ui/actions/percentage_of_time_gate.rb | 2 +- 22 files changed, 46 insertions(+), 51 deletions(-) diff --git a/lib/flipper/api/action.rb b/lib/flipper/api/action.rb index 08d14532d..f0aaadb43 100644 --- a/lib/flipper/api/action.rb +++ b/lib/flipper/api/action.rb @@ -17,19 +17,21 @@ class Action # Public: Call this in subclasses so the action knows its route. # - # block - The Block that determines if this can handle a given request. + # regex - The Regexp that this action should run for. # # Returns nothing. - def self.match(&block) - if block_given? - @match = block - else - @match || raise("No match block set for #{name}") - end + def self.route(regex) + @route_regex = regex + end + + # Internal: Does this action's route match the path. + def self.route_match?(path) + path.match(route_regex) end - def self.match?(request) - match.call(request) + # Internal: The regex that matches which routes this action will work for. + def self.route_regex + @route_regex || raise("#{name}.route is not set") end # Internal: Initializes and runs an action for a given request. @@ -42,11 +44,6 @@ def self.run(flipper, request) new(flipper, request).run end - # Internal: The regex that matches which routes this action will work for. - def self.regex - @regex || raise("#{name}.route is not set") - end - # Public: The instance of the Flipper::DSL the middleware was # initialized with. attr_reader :flipper diff --git a/lib/flipper/api/action_collection.rb b/lib/flipper/api/action_collection.rb index 63623e893..7fdaa366b 100644 --- a/lib/flipper/api/action_collection.rb +++ b/lib/flipper/api/action_collection.rb @@ -12,7 +12,7 @@ def add(action_class) def action_for_request(request) @action_classes.detect do |action_class| - action_class.match?(request) + action_class.route_match?(request.path_info) end end end diff --git a/lib/flipper/api/v1/actions/actors_gate.rb b/lib/flipper/api/v1/actions/actors_gate.rb index b8e672ee5..5c8b45ed8 100644 --- a/lib/flipper/api/v1/actions/actors_gate.rb +++ b/lib/flipper/api/v1/actions/actors_gate.rb @@ -7,7 +7,7 @@ module V1 module Actions class ActorsGate < Api::Action REGEX = %r{\A/features/(?.*)/actors/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def post ensure_valid_params diff --git a/lib/flipper/api/v1/actions/boolean_gate.rb b/lib/flipper/api/v1/actions/boolean_gate.rb index 2c0222362..5bf488be8 100644 --- a/lib/flipper/api/v1/actions/boolean_gate.rb +++ b/lib/flipper/api/v1/actions/boolean_gate.rb @@ -7,7 +7,7 @@ module V1 module Actions class BooleanGate < Api::Action REGEX = %r{\A/features/(?.*)/boolean/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def post feature = flipper[feature_name] diff --git a/lib/flipper/api/v1/actions/clear_feature.rb b/lib/flipper/api/v1/actions/clear_feature.rb index 00feb4b4c..9c6209f57 100644 --- a/lib/flipper/api/v1/actions/clear_feature.rb +++ b/lib/flipper/api/v1/actions/clear_feature.rb @@ -7,7 +7,7 @@ module V1 module Actions class ClearFeature < Api::Action REGEX = %r{\A/features/(?.*)/clear/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def delete feature = flipper[feature_name] diff --git a/lib/flipper/api/v1/actions/feature.rb b/lib/flipper/api/v1/actions/feature.rb index ce0a40ddf..40e58355e 100644 --- a/lib/flipper/api/v1/actions/feature.rb +++ b/lib/flipper/api/v1/actions/feature.rb @@ -7,7 +7,7 @@ module V1 module Actions class Feature < Api::Action REGEX = %r{\A/features/(?.*)/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def get return json_error_response(:feature_not_found) unless feature_exists?(feature_name) diff --git a/lib/flipper/api/v1/actions/features.rb b/lib/flipper/api/v1/actions/features.rb index ddced99fe..00a7dd49c 100644 --- a/lib/flipper/api/v1/actions/features.rb +++ b/lib/flipper/api/v1/actions/features.rb @@ -6,7 +6,8 @@ module Api module V1 module Actions class Features < Api::Action - match { |request| request.path_info =~ %r{\A/features/?\Z} } + REGEX = %r{\A/features/?\Z} + route REGEX def get keys = params['keys'] diff --git a/lib/flipper/api/v1/actions/groups_gate.rb b/lib/flipper/api/v1/actions/groups_gate.rb index 7232bae03..972fcff61 100644 --- a/lib/flipper/api/v1/actions/groups_gate.rb +++ b/lib/flipper/api/v1/actions/groups_gate.rb @@ -7,7 +7,7 @@ module V1 module Actions class GroupsGate < Api::Action REGEX = %r{\A/features/(?.*)/groups/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def post ensure_valid_params diff --git a/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb b/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb index e88f45726..35593dc55 100644 --- a/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb +++ b/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb @@ -7,7 +7,7 @@ module V1 module Actions class PercentageOfActorsGate < Api::Action REGEX = %r{\A/features/(?.*)/percentage_of_actors/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def post if percentage < 0 || percentage > 100 diff --git a/lib/flipper/api/v1/actions/percentage_of_time_gate.rb b/lib/flipper/api/v1/actions/percentage_of_time_gate.rb index 2d1a5f53d..067c814fd 100644 --- a/lib/flipper/api/v1/actions/percentage_of_time_gate.rb +++ b/lib/flipper/api/v1/actions/percentage_of_time_gate.rb @@ -7,7 +7,7 @@ module V1 module Actions class PercentageOfTimeGate < Api::Action REGEX = %r{\A/features/(?.*)/percentage_of_time/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def post if percentage < 0 || percentage > 100 diff --git a/lib/flipper/ui/action.rb b/lib/flipper/ui/action.rb index 6731127bc..c56518bbf 100644 --- a/lib/flipper/ui/action.rb +++ b/lib/flipper/ui/action.rb @@ -17,19 +17,21 @@ class Action # Public: Call this in subclasses so the action knows its route. # - # block - The Block that determines if this can handle a given request. + # regex - The Regexp that this action should run for. # # Returns nothing. - def self.match(&block) - if block_given? - @match = block - else - @match || raise("No match block set for #{name}") - end + def self.route(regex) + @route_regex = regex + end + + # Internal: Does this action's route match the path. + def self.route_match?(path) + path.match(route_regex) end - def self.match?(request) - match.call(request) + # Internal: The regex that matches which routes this action will work for. + def self.route_regex + @route_regex || raise("#{name}.route is not set") end # Internal: Initializes and runs an action for a given request. diff --git a/lib/flipper/ui/action_collection.rb b/lib/flipper/ui/action_collection.rb index 44b551c7e..f4f86d679 100644 --- a/lib/flipper/ui/action_collection.rb +++ b/lib/flipper/ui/action_collection.rb @@ -12,7 +12,7 @@ def add(action_class) def action_for_request(request) @action_classes.detect do |action_class| - action_class.match?(request) + action_class.route_match?(request.path_info) end end end diff --git a/lib/flipper/ui/actions/actors_gate.rb b/lib/flipper/ui/actions/actors_gate.rb index 8f3ec187a..db2307c07 100644 --- a/lib/flipper/ui/actions/actors_gate.rb +++ b/lib/flipper/ui/actions/actors_gate.rb @@ -7,7 +7,7 @@ module UI module Actions class ActorsGate < UI::Action REGEX = %r{\A/features/(?.*)/actors/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def get feature = flipper[feature_name] diff --git a/lib/flipper/ui/actions/add_feature.rb b/lib/flipper/ui/actions/add_feature.rb index 8a81b1258..a066ee6e0 100644 --- a/lib/flipper/ui/actions/add_feature.rb +++ b/lib/flipper/ui/actions/add_feature.rb @@ -5,9 +5,8 @@ module Flipper module UI module Actions class AddFeature < UI::Action - match do |request| - request.path_info =~ %r{\A/features/new/?\Z} - end + REGEX = %r{\A/features/new/?\Z} + route REGEX def get unless Flipper::UI.configuration.feature_creation_enabled diff --git a/lib/flipper/ui/actions/boolean_gate.rb b/lib/flipper/ui/actions/boolean_gate.rb index df957d769..740dc8b2a 100644 --- a/lib/flipper/ui/actions/boolean_gate.rb +++ b/lib/flipper/ui/actions/boolean_gate.rb @@ -6,7 +6,7 @@ module UI module Actions class BooleanGate < UI::Action REGEX = %r{\A/features/(?.*)/boolean/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def post feature = flipper[feature_name] diff --git a/lib/flipper/ui/actions/feature.rb b/lib/flipper/ui/actions/feature.rb index 7da7bf938..710ddfee2 100644 --- a/lib/flipper/ui/actions/feature.rb +++ b/lib/flipper/ui/actions/feature.rb @@ -6,7 +6,7 @@ module UI module Actions class Feature < UI::Action REGEX = %r{\A/features/(?.*)\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def get @feature = Decorators::Feature.new(flipper[feature_name]) diff --git a/lib/flipper/ui/actions/features.rb b/lib/flipper/ui/actions/features.rb index d31d60267..ff22bd7f3 100644 --- a/lib/flipper/ui/actions/features.rb +++ b/lib/flipper/ui/actions/features.rb @@ -6,9 +6,8 @@ module Flipper module UI module Actions class Features < UI::Action - match do |request| - request.path_info =~ %r{\A/features/?\Z} - end + REGEX = %r{\A/features/?\Z} + route REGEX def get @page_title = 'Features' diff --git a/lib/flipper/ui/actions/file.rb b/lib/flipper/ui/actions/file.rb index cce80f3aa..b59428b34 100644 --- a/lib/flipper/ui/actions/file.rb +++ b/lib/flipper/ui/actions/file.rb @@ -5,9 +5,8 @@ module Flipper module UI module Actions class File < UI::Action - match do |request| - request.path_info =~ %r{(images|css|js|octicons|fonts)/.*\Z} - end + REGEX = %r{(images|css|js|octicons|fonts)/.*\Z} + route REGEX def get Rack::File.new(public_path).call(request.env) diff --git a/lib/flipper/ui/actions/groups_gate.rb b/lib/flipper/ui/actions/groups_gate.rb index 7ab2648b0..bb75e235c 100644 --- a/lib/flipper/ui/actions/groups_gate.rb +++ b/lib/flipper/ui/actions/groups_gate.rb @@ -6,7 +6,7 @@ module UI module Actions class GroupsGate < UI::Action REGEX = %r{\A/features/(?.*)/groups/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def get feature = flipper[feature_name] diff --git a/lib/flipper/ui/actions/home.rb b/lib/flipper/ui/actions/home.rb index 5f2415043..ea2b7a769 100644 --- a/lib/flipper/ui/actions/home.rb +++ b/lib/flipper/ui/actions/home.rb @@ -5,9 +5,7 @@ module Flipper module UI module Actions class Home < UI::Action - match do |request| - request.path_info =~ %r{\A/?\Z} - end + route %r{\A/?\Z} def get redirect_to '/features' diff --git a/lib/flipper/ui/actions/percentage_of_actors_gate.rb b/lib/flipper/ui/actions/percentage_of_actors_gate.rb index 2ce28a262..64f5a075d 100644 --- a/lib/flipper/ui/actions/percentage_of_actors_gate.rb +++ b/lib/flipper/ui/actions/percentage_of_actors_gate.rb @@ -6,7 +6,7 @@ module UI module Actions class PercentageOfActorsGate < UI::Action REGEX = %r{\A/features/(?.*)/percentage_of_actors/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def post feature = flipper[feature_name] diff --git a/lib/flipper/ui/actions/percentage_of_time_gate.rb b/lib/flipper/ui/actions/percentage_of_time_gate.rb index 491797b15..e1990604a 100644 --- a/lib/flipper/ui/actions/percentage_of_time_gate.rb +++ b/lib/flipper/ui/actions/percentage_of_time_gate.rb @@ -6,7 +6,7 @@ module UI module Actions class PercentageOfTimeGate < UI::Action REGEX = %r{\A/features/(?.*)/percentage_of_time/?\Z} - match { |request| request.path_info =~ REGEX } + route REGEX def post feature = flipper[feature_name] From d0ad3f835a033c5f8d49d8da578dc956a31ecded Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Tue, 31 Jul 2018 14:07:49 -0400 Subject: [PATCH 8/9] Dry up feature name from route --- lib/flipper/api/action.rb | 10 ++++++++++ lib/flipper/api/v1/actions/actors_gate.rb | 12 +++--------- lib/flipper/api/v1/actions/boolean_gate.rb | 14 +++----------- lib/flipper/api/v1/actions/clear_feature.rb | 14 +++----------- lib/flipper/api/v1/actions/feature.rb | 12 +++--------- lib/flipper/api/v1/actions/features.rb | 3 +-- lib/flipper/api/v1/actions/groups_gate.rb | 12 +++--------- .../api/v1/actions/percentage_of_actors_gate.rb | 12 +++--------- .../api/v1/actions/percentage_of_time_gate.rb | 12 +++--------- lib/flipper/ui/action.rb | 10 ++++++++++ lib/flipper/ui/actions/actors_gate.rb | 14 +++----------- lib/flipper/ui/actions/add_feature.rb | 3 +-- lib/flipper/ui/actions/boolean_gate.rb | 14 +++----------- lib/flipper/ui/actions/feature.rb | 14 +++----------- lib/flipper/ui/actions/features.rb | 3 +-- lib/flipper/ui/actions/file.rb | 3 +-- lib/flipper/ui/actions/groups_gate.rb | 14 +++----------- .../ui/actions/percentage_of_actors_gate.rb | 14 +++----------- lib/flipper/ui/actions/percentage_of_time_gate.rb | 14 +++----------- 19 files changed, 63 insertions(+), 141 deletions(-) diff --git a/lib/flipper/api/action.rb b/lib/flipper/api/action.rb index f0aaadb43..cb2bb4f6a 100644 --- a/lib/flipper/api/action.rb +++ b/lib/flipper/api/action.rb @@ -6,6 +6,16 @@ module Flipper module Api class Action + module FeatureNameFromRoute + def feature_name + @feature_name ||= begin + match = request.path_info.match(self.class.route_regex) + match ? match[:feature_name] : nil + end + end + private :feature_name + end + extend Forwardable VALID_REQUEST_METHOD_NAMES = Set.new([ diff --git a/lib/flipper/api/v1/actions/actors_gate.rb b/lib/flipper/api/v1/actions/actors_gate.rb index 5c8b45ed8..0c319a757 100644 --- a/lib/flipper/api/v1/actions/actors_gate.rb +++ b/lib/flipper/api/v1/actions/actors_gate.rb @@ -6,8 +6,9 @@ module Api module V1 module Actions class ActorsGate < Api::Action - REGEX = %r{\A/features/(?.*)/actors/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/actors/?\Z} def post ensure_valid_params @@ -33,13 +34,6 @@ def ensure_valid_params json_error_response(:flipper_id_invalid) if flipper_id.nil? end - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end - def flipper_id @flipper_id ||= params['flipper_id'] end diff --git a/lib/flipper/api/v1/actions/boolean_gate.rb b/lib/flipper/api/v1/actions/boolean_gate.rb index 5bf488be8..80114ca3f 100644 --- a/lib/flipper/api/v1/actions/boolean_gate.rb +++ b/lib/flipper/api/v1/actions/boolean_gate.rb @@ -6,8 +6,9 @@ module Api module V1 module Actions class BooleanGate < Api::Action - REGEX = %r{\A/features/(?.*)/boolean/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/boolean/?\Z} def post feature = flipper[feature_name] @@ -22,15 +23,6 @@ def delete decorated_feature = Decorators::Feature.new(feature) json_response(decorated_feature.as_json, 200) end - - private - - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end end end end diff --git a/lib/flipper/api/v1/actions/clear_feature.rb b/lib/flipper/api/v1/actions/clear_feature.rb index 9c6209f57..60a1b9499 100644 --- a/lib/flipper/api/v1/actions/clear_feature.rb +++ b/lib/flipper/api/v1/actions/clear_feature.rb @@ -6,23 +6,15 @@ module Api module V1 module Actions class ClearFeature < Api::Action - REGEX = %r{\A/features/(?.*)/clear/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/clear/?\Z} def delete feature = flipper[feature_name] feature.clear json_response({}, 204) end - - private - - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end end end end diff --git a/lib/flipper/api/v1/actions/feature.rb b/lib/flipper/api/v1/actions/feature.rb index 40e58355e..46e301032 100644 --- a/lib/flipper/api/v1/actions/feature.rb +++ b/lib/flipper/api/v1/actions/feature.rb @@ -6,8 +6,9 @@ module Api module V1 module Actions class Feature < Api::Action - REGEX = %r{\A/features/(?.*)/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/?\Z} def get return json_error_response(:feature_not_found) unless feature_exists?(feature_name) @@ -22,13 +23,6 @@ def delete private - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end - def feature_exists?(feature_name) flipper.features.map(&:key).include?(feature_name) end diff --git a/lib/flipper/api/v1/actions/features.rb b/lib/flipper/api/v1/actions/features.rb index 00a7dd49c..b14560fbe 100644 --- a/lib/flipper/api/v1/actions/features.rb +++ b/lib/flipper/api/v1/actions/features.rb @@ -6,8 +6,7 @@ module Api module V1 module Actions class Features < Api::Action - REGEX = %r{\A/features/?\Z} - route REGEX + route %r{\A/features/?\Z} def get keys = params['keys'] diff --git a/lib/flipper/api/v1/actions/groups_gate.rb b/lib/flipper/api/v1/actions/groups_gate.rb index 972fcff61..ea53a86d4 100644 --- a/lib/flipper/api/v1/actions/groups_gate.rb +++ b/lib/flipper/api/v1/actions/groups_gate.rb @@ -6,8 +6,9 @@ module Api module V1 module Actions class GroupsGate < Api::Action - REGEX = %r{\A/features/(?.*)/groups/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/groups/?\Z} def post ensure_valid_params @@ -47,13 +48,6 @@ def disallow_unregistered_groups? !allow_unregistered_groups? end - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end - def group_name @group_name ||= params['name'] end diff --git a/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb b/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb index 35593dc55..66a030f8e 100644 --- a/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb +++ b/lib/flipper/api/v1/actions/percentage_of_actors_gate.rb @@ -6,8 +6,9 @@ module Api module V1 module Actions class PercentageOfActorsGate < Api::Action - REGEX = %r{\A/features/(?.*)/percentage_of_actors/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/percentage_of_actors/?\Z} def post if percentage < 0 || percentage > 100 @@ -29,13 +30,6 @@ def delete private - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end - def percentage @percentage ||= begin Integer(params['percentage']) diff --git a/lib/flipper/api/v1/actions/percentage_of_time_gate.rb b/lib/flipper/api/v1/actions/percentage_of_time_gate.rb index 067c814fd..4feec4b10 100644 --- a/lib/flipper/api/v1/actions/percentage_of_time_gate.rb +++ b/lib/flipper/api/v1/actions/percentage_of_time_gate.rb @@ -6,8 +6,9 @@ module Api module V1 module Actions class PercentageOfTimeGate < Api::Action - REGEX = %r{\A/features/(?.*)/percentage_of_time/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/percentage_of_time/?\Z} def post if percentage < 0 || percentage > 100 @@ -30,13 +31,6 @@ def delete private - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end - def percentage @percentage ||= begin Integer(params['percentage']) diff --git a/lib/flipper/ui/action.rb b/lib/flipper/ui/action.rb index c56518bbf..a13dc5592 100644 --- a/lib/flipper/ui/action.rb +++ b/lib/flipper/ui/action.rb @@ -6,6 +6,16 @@ module Flipper module UI class Action + module FeatureNameFromRoute + def feature_name + @feature_name ||= begin + match = request.path_info.match(self.class.route_regex) + match ? match[:feature_name] : nil + end + end + private :feature_name + end + extend Forwardable VALID_REQUEST_METHOD_NAMES = Set.new([ diff --git a/lib/flipper/ui/actions/actors_gate.rb b/lib/flipper/ui/actions/actors_gate.rb index db2307c07..2344a8fa8 100644 --- a/lib/flipper/ui/actions/actors_gate.rb +++ b/lib/flipper/ui/actions/actors_gate.rb @@ -6,8 +6,9 @@ module Flipper module UI module Actions class ActorsGate < UI::Action - REGEX = %r{\A/features/(?.*)/actors/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/actors/?\Z} def get feature = flipper[feature_name] @@ -41,15 +42,6 @@ def post redirect_to("/features/#{feature.key}") end - - private - - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end end end end diff --git a/lib/flipper/ui/actions/add_feature.rb b/lib/flipper/ui/actions/add_feature.rb index a066ee6e0..bd22f1a00 100644 --- a/lib/flipper/ui/actions/add_feature.rb +++ b/lib/flipper/ui/actions/add_feature.rb @@ -5,8 +5,7 @@ module Flipper module UI module Actions class AddFeature < UI::Action - REGEX = %r{\A/features/new/?\Z} - route REGEX + route %r{\A/features/new/?\Z} def get unless Flipper::UI.configuration.feature_creation_enabled diff --git a/lib/flipper/ui/actions/boolean_gate.rb b/lib/flipper/ui/actions/boolean_gate.rb index 740dc8b2a..a5e22ac0e 100644 --- a/lib/flipper/ui/actions/boolean_gate.rb +++ b/lib/flipper/ui/actions/boolean_gate.rb @@ -5,8 +5,9 @@ module Flipper module UI module Actions class BooleanGate < UI::Action - REGEX = %r{\A/features/(?.*)/boolean/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/boolean/?\Z} def post feature = flipper[feature_name] @@ -20,15 +21,6 @@ def post redirect_to "/features/#{@feature.key}" end - - private - - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end end end end diff --git a/lib/flipper/ui/actions/feature.rb b/lib/flipper/ui/actions/feature.rb index 710ddfee2..ea2b0876c 100644 --- a/lib/flipper/ui/actions/feature.rb +++ b/lib/flipper/ui/actions/feature.rb @@ -5,8 +5,9 @@ module Flipper module UI module Actions class Feature < UI::Action - REGEX = %r{\A/features/(?.*)\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)\Z} def get @feature = Decorators::Feature.new(flipper[feature_name]) @@ -34,15 +35,6 @@ def delete feature.remove redirect_to '/features' end - - private - - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end end end end diff --git a/lib/flipper/ui/actions/features.rb b/lib/flipper/ui/actions/features.rb index ff22bd7f3..be5aa38ca 100644 --- a/lib/flipper/ui/actions/features.rb +++ b/lib/flipper/ui/actions/features.rb @@ -6,8 +6,7 @@ module Flipper module UI module Actions class Features < UI::Action - REGEX = %r{\A/features/?\Z} - route REGEX + route %r{\A/features/?\Z} def get @page_title = 'Features' diff --git a/lib/flipper/ui/actions/file.rb b/lib/flipper/ui/actions/file.rb index b59428b34..173b5b424 100644 --- a/lib/flipper/ui/actions/file.rb +++ b/lib/flipper/ui/actions/file.rb @@ -5,8 +5,7 @@ module Flipper module UI module Actions class File < UI::Action - REGEX = %r{(images|css|js|octicons|fonts)/.*\Z} - route REGEX + route %r{(images|css|js|octicons|fonts)/.*\Z} def get Rack::File.new(public_path).call(request.env) diff --git a/lib/flipper/ui/actions/groups_gate.rb b/lib/flipper/ui/actions/groups_gate.rb index bb75e235c..2de569cc7 100644 --- a/lib/flipper/ui/actions/groups_gate.rb +++ b/lib/flipper/ui/actions/groups_gate.rb @@ -5,8 +5,9 @@ module Flipper module UI module Actions class GroupsGate < UI::Action - REGEX = %r{\A/features/(?.*)/groups/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/groups/?\Z} def get feature = flipper[feature_name] @@ -38,15 +39,6 @@ def post redirect_to("/features/#{feature.key}/groups?error=#{error}") end end - - private - - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end end end end diff --git a/lib/flipper/ui/actions/percentage_of_actors_gate.rb b/lib/flipper/ui/actions/percentage_of_actors_gate.rb index 64f5a075d..89f166bdd 100644 --- a/lib/flipper/ui/actions/percentage_of_actors_gate.rb +++ b/lib/flipper/ui/actions/percentage_of_actors_gate.rb @@ -5,8 +5,9 @@ module Flipper module UI module Actions class PercentageOfActorsGate < UI::Action - REGEX = %r{\A/features/(?.*)/percentage_of_actors/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/percentage_of_actors/?\Z} def post feature = flipper[feature_name] @@ -21,15 +22,6 @@ def post redirect_to "/features/#{@feature.key}" end - - private - - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end end end end diff --git a/lib/flipper/ui/actions/percentage_of_time_gate.rb b/lib/flipper/ui/actions/percentage_of_time_gate.rb index e1990604a..bf4ff6e1d 100644 --- a/lib/flipper/ui/actions/percentage_of_time_gate.rb +++ b/lib/flipper/ui/actions/percentage_of_time_gate.rb @@ -5,8 +5,9 @@ module Flipper module UI module Actions class PercentageOfTimeGate < UI::Action - REGEX = %r{\A/features/(?.*)/percentage_of_time/?\Z} - route REGEX + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/percentage_of_time/?\Z} def post feature = flipper[feature_name] @@ -21,15 +22,6 @@ def post redirect_to "/features/#{@feature.key}" end - - private - - def feature_name - @feature_name ||= begin - match = request.path_info.match(REGEX) - match ? match[:feature_name] : nil - end - end end end end From 04aead852bf9de30e308cba351b21adc43c604d5 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Tue, 31 Jul 2018 14:18:45 -0400 Subject: [PATCH 9/9] Another order change to fix test failure --- spec/flipper/api/v1/actions/feature_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/flipper/api/v1/actions/feature_spec.rb b/spec/flipper/api/v1/actions/feature_spec.rb index 43231358f..f738ba667 100644 --- a/spec/flipper/api/v1/actions/feature_spec.rb +++ b/spec/flipper/api/v1/actions/feature_spec.rb @@ -170,11 +170,6 @@ 'name' => 'boolean', 'value' => 'true', }, - { - 'key' => 'groups', - 'name' => 'group', - 'value' => [], - }, { 'key' => 'actors', 'name' => 'actor', @@ -190,6 +185,11 @@ 'name' => 'percentage_of_time', 'value' => nil, }, + { + 'key' => 'groups', + 'name' => 'group', + 'value' => [], + }, ], }