root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Implement uniqueness_of_in_memory_validator

This commit is contained in:
Shinya Maeda 2017-07-03 16:59:57 +09:00
parent cd6aa92207
commit c99f8e8884
3 changed files with 68 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
app/models/ci/pipeline_schedule.rb
View File

@ -15,6 +15,11 @@ module Ci
validates :cron_timezone, cron_timezone: true, presence: { unless: :importing? }
validates :ref, presence: { unless: :importing? }
validates :description, presence: true
validates :variables, uniqueness_of_in_memory: {
:collection => :variables,
:attrs => [:pipeline_schedule_id, :key],
:message => ['variables.key', 'keys are duplicated']
}
before_save :set_next_run_at

37
app/validators/uniqueness_of_in_memory_validator.rb Normal file
View File

@ -0,0 +1,37 @@
# UniquenessOfInMemoryValidator
#
# This validtor is designed for especially the following condition
# - Use `accepts_nested_attributes_for :xxx` in a parent model
# - Use `validates :xxx, uniqueness: { scope: :xxx_id }` in a child model
#
# Inspired by https://stackoverflow.com/a/2883129/2522666
module ActiveRecord
class Base
# Validate that the the objects in +collection+ are unique
# when compared against all their non-blank +attrs+. If not
# add +message+ to the base errors.
def validate_uniqueness_of_in_memory(collection, attrs, message)
hashes = collection.inject({}) do |hash, record|
key = attrs.map { |a| record.send(a).to_s }.join
if key.blank? || record.marked_for_destruction?
key = record.object_id
end
hash[key] = record unless hash[key]
hash
end
if collection.length > hashes.length
self.errors.add(*message)
end
end
end
end
class UniquenessOfInMemoryValidator < ActiveModel::Validator
def validate(record)
record.validate_uniqueness_of_in_memory(
record.public_send(options[:collection]),
options[:attrs],
options[:message])
end
end

59
spec/controllers/projects/pipeline_schedules_controller_spec.rb
View File

@ -150,22 +150,21 @@ describe Projects::PipelineSchedulesController do
end
end
# This test no longer passes, since we removed a custom validation
# context 'when variables_attributes has two variables and duplicted' do
# let(:schedule) do
# basic_param.merge({
# variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }]
# })
# end
context 'when variables_attributes has two variables and duplicted' do
let(:schedule) do
basic_param.merge({
variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }]
})
end
# it 'returns an error that the keys of variable are duplicated' do
# expect { go }
# .to change { Ci::PipelineSchedule.count }.by(0)
# .and change { Ci::PipelineScheduleVariable.count }.by(0)
it 'returns an error that the keys of variable are duplicated' do
expect { go }
.to change { Ci::PipelineSchedule.count }.by(0)
.and change { Ci::PipelineScheduleVariable.count }.by(0)
# expect(assigns(:schedule).errors['variables.key']).not_to be_empty
# end
# end
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
end
end
end
describe 'security' do
@ -194,7 +193,6 @@ describe Projects::PipelineSchedulesController do
before do
project.add_developer(user)
sign_in(user)
end
@ -210,7 +208,6 @@ describe Projects::PipelineSchedulesController do
go
pipeline_schedule.reload
expect(response).to have_http_status(:found)
expect(pipeline_schedule.description).to eq('updated_desc')
expect(pipeline_schedule.cron).to eq('0 1 * * *')
@ -232,7 +229,6 @@ describe Projects::PipelineSchedulesController do
expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(1)
pipeline_schedule.reload
expect(response).to have_http_status(:found)
expect(pipeline_schedule.variables.last.key).to eq('AAA')
expect(pipeline_schedule.variables.last.value).to eq('AAA123')
@ -250,7 +246,6 @@ describe Projects::PipelineSchedulesController do
expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(2)
pipeline_schedule.reload
expect(response).to have_http_status(:found)
expect(pipeline_schedule.variables.first.key).to eq('AAA')
expect(pipeline_schedule.variables.first.value).to eq('AAA123')
@ -259,21 +254,20 @@ describe Projects::PipelineSchedulesController do
end
end
# This test no longer passes, since we removed a custom validation
# context 'when params include two duplicated variables' do
# let(:schedule) do
# basic_param.merge({
# variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }]
# })
# end
context 'when params include two duplicated variables' do
let(:schedule) do
basic_param.merge({
variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }]
})
end
# it 'returns an error that variables are duplciated' do
# put :update, namespace_id: project.namespace.to_param,
# project_id: project, id: pipeline_schedule, schedule: schedule
it 'returns an error that variables are duplciated' do
put :update, namespace_id: project.namespace.to_param,
project_id: project, id: pipeline_schedule, schedule: schedule
# expect(assigns(:schedule).errors['variables.key']).not_to be_empty
# end
# end
expect(assigns(:schedule).errors['variables.key']).not_to be_empty
end
end
end
context 'when a pipeline schedule has one variable' do
@ -293,7 +287,6 @@ describe Projects::PipelineSchedulesController do
go
pipeline_schedule.reload
expect(response).to have_http_status(:found)
expect(pipeline_schedule.description).to eq('updated_desc')
expect(pipeline_schedule.cron).to eq('0 1 * * *')
@ -316,6 +309,7 @@ describe Projects::PipelineSchedulesController do
it 'adds the new variable' do
expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(1)
pipeline_schedule.reload
expect(pipeline_schedule.variables.last.key).to eq('AAA')
end
end
@ -331,7 +325,6 @@ describe Projects::PipelineSchedulesController do
expect { go }.not_to change { Ci::PipelineScheduleVariable.count }
pipeline_schedule_variable.reload
expect(pipeline_schedule_variable.value).to eq('new_value')
end
end