Commit 7e3604dd authored by Jason Goodman's avatar Jason Goodman Committed by Shinya Maeda

Add associations for feature flag strategy user lists

Require user list for gitlabUserList strategy
Forbid user list for other strategies
parent 4ee562ce
......@@ -53,7 +53,7 @@ module Operations
end
def for_unleash_client(project, environment)
includes(strategies: :scopes)
includes(strategies: [:scopes, :user_list])
.where(project: project)
.merge(Operations::FeatureFlags::Scope.on_environment(environment))
.reorder(:id)
......
......@@ -4,10 +4,12 @@ module Operations
module FeatureFlags
class Strategy < ApplicationRecord
STRATEGY_DEFAULT = 'default'
STRATEGY_GITLABUSERLIST = 'gitlabUserList'
STRATEGY_GRADUALROLLOUTUSERID = 'gradualRolloutUserId'
STRATEGY_USERWITHID = 'userWithId'
STRATEGIES = {
STRATEGY_DEFAULT => [].freeze,
STRATEGY_GITLABUSERLIST => [].freeze,
STRATEGY_GRADUALROLLOUTUSERID => %w[groupId percentage].freeze,
STRATEGY_USERWITHID => ['userIds'].freeze
}.freeze
......@@ -17,6 +19,8 @@ module Operations
belongs_to :feature_flag
has_many :scopes, class_name: 'Operations::FeatureFlags::Scope'
has_one :strategy_user_list
has_one :user_list, through: :strategy_user_list
validates :name,
inclusion: {
......@@ -25,11 +29,20 @@ module Operations
}
validate :parameters_validations, if: -> { errors[:name].blank? }
validates :user_list, presence: true, if: -> { name == STRATEGY_GITLABUSERLIST }
validates :user_list, absence: true, if: -> { name != STRATEGY_GITLABUSERLIST }
validate :same_project_validation, if: -> { user_list.present? }
accepts_nested_attributes_for :scopes, allow_destroy: true
private
def same_project_validation
unless user_list.project_id == feature_flag.project_id
errors.add(:user_list, 'must belong to the same project')
end
end
def parameters_validations
validate_parameters_type &&
validate_parameters_keys &&
......
# frozen_string_literal: true
module Operations
module FeatureFlags
class StrategyUserList < ApplicationRecord
self.table_name = 'operations_strategies_user_lists'
belongs_to :strategy
belongs_to :user_list
end
end
end
......@@ -7,7 +7,27 @@ module EE
expose :name
expose :description, unless: ->(feature) { feature.description.nil? }
expose :active, as: :enabled
expose :strategies, using: UnleashStrategy
expose :strategies do |flag|
flag.strategies.map do |strategy|
if legacy_strategy?(strategy)
UnleashLegacyStrategy.represent(strategy)
elsif gitlab_user_list_strategy?(strategy)
UnleashGitlabUserListStrategy.represent(strategy)
else
UnleashStrategy.represent(strategy)
end
end
end
private
def legacy_strategy?(strategy)
!strategy.respond_to?(:name)
end
def gitlab_user_list_strategy?(strategy)
strategy.name == ::Operations::FeatureFlags::Strategy::STRATEGY_GITLABUSERLIST
end
end
end
end
......
# frozen_string_literal: true
module EE
module API
module Entities
class UnleashGitlabUserListStrategy < Grape::Entity
expose :name do |_strategy|
::Operations::FeatureFlags::Strategy::STRATEGY_USERWITHID
end
expose :parameters do |strategy|
{ userIds: strategy.user_list.user_xids }
end
end
end
end
end
# frozen_string_literal: true
module EE
module API
module Entities
class UnleashLegacyStrategy < Grape::Entity
expose :name do |strategy|
strategy['name']
end
expose :parameters do |strategy|
strategy['parameters']
end
end
end
end
end
......@@ -4,20 +4,8 @@ module EE
module API
module Entities
class UnleashStrategy < Grape::Entity
expose :name do |strategy|
if strategy.respond_to?(:name)
strategy.name
else
strategy['name']
end
end
expose :parameters do |strategy|
if strategy.respond_to?(:parameters)
strategy.parameters
else
strategy['parameters']
end
end
expose :name
expose :parameters
end
end
end
......
......@@ -3,10 +3,12 @@
require 'spec_helper'
describe Operations::FeatureFlags::Strategy do
let_it_be(:project) { create(:project) }
describe 'validations' do
it do
is_expected.to validate_inclusion_of(:name)
.in_array(%w[default gradualRolloutUserId userWithId])
.in_array(%w[default gradualRolloutUserId userWithId gitlabUserList])
.with_message('strategy name is invalid')
end
......@@ -17,7 +19,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'skips parameters validation' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: invalid_name, parameters: { bad: 'params' })
......@@ -34,7 +36,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'must have valid parameters for the strategy' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gradualRolloutUserId', parameters: invalid_parameters)
......@@ -43,7 +45,7 @@ describe Operations::FeatureFlags::Strategy do
end
it 'allows the parameters in any order' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gradualRolloutUserId',
parameters: { percentage: '10', groupId: 'mygroup' })
......@@ -59,7 +61,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gradualRolloutUserId',
parameters: { groupId: 'mygroup', percentage: invalid_value })
......@@ -73,7 +75,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gradualRolloutUserId',
parameters: { groupId: 'mygroup', percentage: valid_value })
......@@ -90,7 +92,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'must be a string value of up to 32 lowercase characters' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gradualRolloutUserId',
parameters: { groupId: invalid_value, percentage: '40' })
......@@ -104,7 +106,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'must be a string value of up to 32 lowercase characters' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gradualRolloutUserId',
parameters: { groupId: valid_value, percentage: '40' })
......@@ -121,7 +123,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'must have valid parameters for the strategy' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'userWithId', parameters: invalid_parameters)
......@@ -138,7 +140,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'is valid with a string of comma separated values' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'userWithId', parameters: { userIds: valid_value })
......@@ -153,7 +155,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'is invalid' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'userWithId', parameters: { userIds: invalid_value })
......@@ -171,7 +173,7 @@ describe Operations::FeatureFlags::Strategy do
end
with_them do
it 'must be empty' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'default',
parameters: invalid_value)
......@@ -181,7 +183,7 @@ describe Operations::FeatureFlags::Strategy do
end
it 'must be empty' do
feature_flag = create(:operations_feature_flag)
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'default',
parameters: {})
......@@ -189,6 +191,133 @@ describe Operations::FeatureFlags::Strategy do
expect(strategy.errors[:parameters]).to be_empty
end
end
context 'when the strategy name is gitlabUserList' do
where(:invalid_value) do
[{ groupId: "default", percentage: "7" }, "", "nothing", 7, nil, [], 2.5, { userIds: 'user1' }]
end
with_them do
it 'must be empty' do
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gitlabUserList',
parameters: invalid_value)
expect(strategy.errors[:parameters]).to eq(['parameters are invalid'])
end
end
it 'must be empty' do
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gitlabUserList',
parameters: {})
expect(strategy.errors[:parameters]).to be_empty
end
end
end
describe 'associations' do
context 'when name is gitlabUserList' do
it 'is valid when associated with a user list' do
feature_flag = create(:operations_feature_flag, project: project)
user_list = create(:operations_feature_flag_user_list, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gitlabUserList',
user_list: user_list,
parameters: {})
expect(strategy.errors[:user_list]).to be_empty
end
it 'is invalid without a user list' do
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gitlabUserList',
parameters: {})
expect(strategy.errors[:user_list]).to eq(["can't be blank"])
end
it 'is invalid when associated with a user list from another project' do
other_project = create(:project)
feature_flag = create(:operations_feature_flag, project: project)
user_list = create(:operations_feature_flag_user_list, project: other_project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gitlabUserList',
user_list: user_list,
parameters: {})
expect(strategy.errors[:user_list]).to eq(['must belong to the same project'])
end
end
context 'when name is default' do
it 'is invalid when associated with a user list' do
feature_flag = create(:operations_feature_flag, project: project)
user_list = create(:operations_feature_flag_user_list, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'default',
user_list: user_list,
parameters: {})
expect(strategy.errors[:user_list]).to eq(['must be blank'])
end
it 'is valid without a user list' do
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'default',
parameters: {})
expect(strategy.errors[:user_list]).to be_empty
end
end
context 'when name is userWithId' do
it 'is invalid when associated with a user list' do
feature_flag = create(:operations_feature_flag, project: project)
user_list = create(:operations_feature_flag_user_list, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'userWithId',
user_list: user_list,
parameters: { userIds: 'user1' })
expect(strategy.errors[:user_list]).to eq(['must be blank'])
end
it 'is valid without a user list' do
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'userWithId',
parameters: { userIds: 'user1' })
expect(strategy.errors[:user_list]).to be_empty
end
end
context 'when name is gradualRolloutUserId' do
it 'is invalid when associated with a user list' do
feature_flag = create(:operations_feature_flag, project: project)
user_list = create(:operations_feature_flag_user_list, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gradualRolloutUserId',
user_list: user_list,
parameters: { groupId: 'default', percentage: '10' })
expect(strategy.errors[:user_list]).to eq(['must be blank'])
end
it 'is valid without a user list' do
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'gradualRolloutUserId',
parameters: { groupId: 'default', percentage: '10' })
expect(strategy.errors[:user_list]).to be_empty
end
end
end
end
end
......@@ -494,6 +494,28 @@ describe API::Unleash do
}]
}])
end
it 'returns a userWithId strategy for a gitlabUserList strategy' do
feature_flag = create(:operations_feature_flag, :new_version_flag, project: project,
name: 'myfeature', active: true)
user_list = create(:operations_feature_flag_user_list, project: project,
name: 'My List', user_xids: 'user1,user2')
strategy = create(:operations_strategy, feature_flag: feature_flag,
name: 'gitlabUserList', parameters: {}, user_list: user_list)
create(:operations_scope, strategy: strategy, environment_scope: 'production')
get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' }
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['features']).to eq([{
'name' => 'myfeature',
'enabled' => true,
'strategies' => [{
'name' => 'userWithId',
'parameters' => { 'userIds' => 'user1,user2' }
}]
}])
end
end
context 'when mixing version 1 and version 2 feature flags' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment