Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Slashes in Feature Names in UI/API #362

Merged
merged 12 commits into from
Jul 31, 2018
Merged

Support Slashes in Feature Names in UI/API #362

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3033e3c
Add support for slashes in feature names to UI
jnunemaker May 20, 2018
cccf29f
Make it easy to only log requests but still log feature stuff if you …
jnunemaker May 20, 2018
569d8c3
Add a slash feature to the example
jnunemaker May 20, 2018
1602d2e
Add support for slashes in feature names to API
jnunemaker Jun 3, 2018
de10683
Make rubo happy by removing double negation
jnunemaker Jun 3, 2018
1acf979
Merge branch 'master' into slash-names
jnunemaker Jun 4, 2018
f61bbe9
Switch to named captures for route regex
jnunemaker Jul 30, 2018
8fa06a5
Move back to single regex from overly flexible request match
jnunemaker Jul 31, 2018
d0ad3f8
Dry up feature name from route
jnunemaker Jul 31, 2018
560927c
Merge branch 'master' into slash-names
jnunemaker Jul 31, 2018
f66b22f
Merge branch 'master' into slash-names
jnunemaker Jul 31, 2018
04aead8
Another order change to fix test failure
jnunemaker Jul 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions examples/ui/basic.ru
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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)
Expand All @@ -42,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"
Expand Down
14 changes: 11 additions & 3 deletions lib/flipper/api/action.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion lib/flipper/api/action_collection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 = []
Expand All @@ -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
Expand Down
8 changes: 6 additions & 2 deletions lib/flipper/api/v1/actions/actors_gate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super minor suggestion these regexes could use named capture groups that might read slightly better, but this also works great as is

REGEX = %r{\A/features/(?<feature_name>.*)/actors/?\Z}

def feature_name
  if match = request.path_info.match(REGEX)
    match[:feature_name]
  end
end

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost went for a capture group, but this took so much longer than I was planning I lost steam. I think it is worth updating to use them so I'll spin back around on it soon.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in f61bbe9

end
end

def flipper_id
Expand Down
14 changes: 11 additions & 3 deletions lib/flipper/api/v1/actions/boolean_gate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,31 @@ 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)
json_response(decorated_feature.as_json, 200)
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
Expand Down
13 changes: 11 additions & 2 deletions lib/flipper/api/v1/actions/clear_feature.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions lib/flipper/api/v1/actions/feature.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/flipper/api/v1/actions/features.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down
8 changes: 6 additions & 2 deletions lib/flipper/api/v1/actions/groups_gate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions lib/flipper/api/v1/actions/percentage_of_actors_gate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 6 additions & 2 deletions lib/flipper/api/v1/actions/percentage_of_time_gate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
19 changes: 11 additions & 8 deletions lib/flipper/ui/action.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion lib/flipper/ui/action_collection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 13 additions & 5 deletions lib/flipper/ui/actions/actors_gate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', '/'
Expand All @@ -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)
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion lib/flipper/ui/actions/add_feature.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 12 additions & 3 deletions lib/flipper/ui/actions/boolean_gate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
Expand Down
Loading