From d109663cb91dd8783113a6442c4a45206839ce92 Mon Sep 17 00:00:00 2001 From: Hauke Klement Date: Wed, 18 Mar 2015 09:10:58 +0100 Subject: [PATCH] extracted boolean validation logic into stand-alone validator --- app/models/code_ocean/file.rb | 5 +++-- app/models/execution_environment.rb | 4 +++- app/models/exercise.rb | 4 +++- app/models/file_type.rb | 8 +++++--- .../validations/boolean_presence_validator.rb | 12 ++++++++++++ spec/models/code_ocean/file_spec.rb | 4 ++++ spec/models/execution_environment_spec.rb | 2 ++ spec/models/exercise_spec.rb | 2 ++ spec/models/file_type_spec.rb | 6 ++++++ 9 files changed, 40 insertions(+), 7 deletions(-) create mode 100644 lib/active_model/validations/boolean_presence_validator.rb diff --git a/app/models/code_ocean/file.rb b/app/models/code_ocean/file.rb index f00dccc5..4220618d 100644 --- a/app/models/code_ocean/file.rb +++ b/app/models/code_ocean/file.rb @@ -1,4 +1,5 @@ require File.expand_path('../../../uploaders/file_uploader', __FILE__) +require File.expand_path('../../../../lib/active_model/validations/boolean_presence_validator', __FILE__) module CodeOcean class File < ActiveRecord::Base @@ -35,9 +36,9 @@ module CodeOcean validates :feedback_message, absence: true, unless: :teacher_defined_test? validates :file_type_id, presence: true validates :hashed_content, if: :content_present?, presence: true - validates :hidden, inclusion: {in: [true, false]} + validates :hidden, boolean_presence: true validates :name, presence: true - validates :read_only, inclusion: {in: [true, false]} + validates :read_only, boolean_presence: true validates :role, inclusion: {in: ROLES} validates :weight, if: :teacher_defined_test?, numericality: true, presence: true validates :weight, absence: true, unless: :teacher_defined_test? diff --git a/app/models/execution_environment.rb b/app/models/execution_environment.rb index e49a869c..3a4efdde 100644 --- a/app/models/execution_environment.rb +++ b/app/models/execution_environment.rb @@ -1,3 +1,5 @@ +require File.expand_path('../../../lib/active_model/validations/boolean_presence_validator', __FILE__) + class ExecutionEnvironment < ActiveRecord::Base include Creation include DefaultValues @@ -16,7 +18,7 @@ class ExecutionEnvironment < ActiveRecord::Base validate :working_docker_image?, if: :validate_docker_image? validates :docker_image, presence: true validates :memory_limit, numericality: {greater_than_or_equal_to: DockerClient::MINIMUM_MEMORY_LIMIT, only_integer: true}, presence: true - validates :network_enabled, inclusion: {in: [true, false]} + validates :network_enabled, boolean_presence: true validates :name, presence: true validates :permitted_execution_time, numericality: {only_integer: true}, presence: true validates :pool_size, numericality: {only_integer: true}, presence: true diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 8c3abc9b..6e7e090a 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -1,3 +1,5 @@ +require File.expand_path('../../../lib/active_model/validations/boolean_presence_validator', __FILE__) + class Exercise < ActiveRecord::Base include Context include Creation @@ -16,7 +18,7 @@ class Exercise < ActiveRecord::Base validate :valid_main_file? validates :description, presence: true validates :execution_environment_id, presence: true - validates :public, inclusion: {in: [true, false]} + validates :public, boolean_presence: true validates :title, presence: true validates :token, presence: true, uniqueness: true diff --git a/app/models/file_type.rb b/app/models/file_type.rb index b4eb02a5..53bf18dc 100644 --- a/app/models/file_type.rb +++ b/app/models/file_type.rb @@ -1,3 +1,5 @@ +require File.expand_path('../../../lib/active_model/validations/boolean_presence_validator', __FILE__) + class FileType < ActiveRecord::Base include Creation include DefaultValues @@ -11,12 +13,12 @@ class FileType < ActiveRecord::Base has_many :execution_environments has_many :files - validates :binary, inclusion: {in: [true, false]} + validates :binary, boolean_presence: true validates :editor_mode, presence: true, unless: :binary? - validates :executable, inclusion: {in: [true, false]} + validates :executable, boolean_presence: true validates :indent_size, presence: true, unless: :binary? validates :name, presence: true - validates :renderable, inclusion: {in: [true, false]} + validates :renderable, boolean_presence: true [:audio, :image, :video].each do |type| define_method("#{type}?") do diff --git a/lib/active_model/validations/boolean_presence_validator.rb b/lib/active_model/validations/boolean_presence_validator.rb new file mode 100644 index 00000000..f1fac7dd --- /dev/null +++ b/lib/active_model/validations/boolean_presence_validator.rb @@ -0,0 +1,12 @@ +module ActiveModel + module Validations + class BooleanPresenceValidator < EachValidator + def validate(record) + [attributes].flatten.each do |attribute| + value = record.send(:read_attribute_for_validation, attribute) + record.errors.add(attribute, nil, options) unless [false, true].include?(value) + end + end + end + end +end diff --git a/spec/models/code_ocean/file_spec.rb b/spec/models/code_ocean/file_spec.rb index c2d83a1c..1f0524a2 100644 --- a/spec/models/code_ocean/file_spec.rb +++ b/spec/models/code_ocean/file_spec.rb @@ -9,6 +9,8 @@ describe CodeOcean::File do it 'validates the presence of the hidden flag' do expect(file.errors[:hidden]).to be_present + file.update(hidden: false) + expect(file.errors[:hidden]).to be_blank end it 'validates the presence of a name' do @@ -17,6 +19,8 @@ describe CodeOcean::File do it 'validates the presence of the read-only flag' do expect(file.errors[:read_only]).to be_present + file.update(read_only: false) + expect(file.errors[:read_only]).to be_blank end context 'as a teacher-defined test' do diff --git a/spec/models/execution_environment_spec.rb b/spec/models/execution_environment_spec.rb index f5a207bf..a55eda25 100644 --- a/spec/models/execution_environment_spec.rb +++ b/spec/models/execution_environment_spec.rb @@ -34,6 +34,8 @@ describe ExecutionEnvironment do it 'validates the presence of the network enabled flag' do expect(execution_environment.errors[:network_enabled]).to be_present + execution_environment.update(network_enabled: false) + expect(execution_environment.errors[:network_enabled]).to be_blank end it 'validates the numericality of the permitted run time' do diff --git a/spec/models/exercise_spec.rb b/spec/models/exercise_spec.rb index d1c8e800..bd7e2545 100644 --- a/spec/models/exercise_spec.rb +++ b/spec/models/exercise_spec.rb @@ -28,6 +28,8 @@ describe Exercise do it 'validates the presence of the public flag' do expect(exercise.errors[:public]).to be_present + exercise.update(public: false) + expect(exercise.errors[:public]).to be_blank end it 'validates the presence of a title' do diff --git a/spec/models/file_type_spec.rb b/spec/models/file_type_spec.rb index 89de68b5..0193a9fd 100644 --- a/spec/models/file_type_spec.rb +++ b/spec/models/file_type_spec.rb @@ -5,6 +5,8 @@ describe FileType do it 'validates the presence of the binary flag' do expect(file_type.errors[:binary]).to be_present + file_type.update(binary: false) + expect(file_type.errors[:binary]).to be_blank end context 'when binary' do @@ -33,6 +35,8 @@ describe FileType do it 'validates the presence of the executable flag' do expect(file_type.errors[:executable]).to be_present + file_type.update(executable: false) + expect(file_type.errors[:executable]).to be_blank end it 'validates the presence of a name' do @@ -41,6 +45,8 @@ describe FileType do it 'validates the presence of the renderable flag' do expect(file_type.errors[:renderable]).to be_present + file_type.update(renderable: false) + expect(file_type.errors[:renderable]).to be_blank end it 'validates the presence of a user' do