Align ModelContract validate/valid? methods with ActiveModel::Validations (#8955)

* Align ModelContract validate/valid? methods with ActiveModel::Validations

* Add BaseContract to allow contracts on non ActiveRecord resources

* Avoid using alias_method as that won't work with subclasses

Co-authored-by: Oliver Günther <mail@oliverguenther.de>
pull/8970/head
Wieland Lindenthal 4 years ago committed by GitHub
parent 3c9be3bdbe
commit 3044b5ce5e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 272
      app/contracts/base_contract.rb
  2. 2
      app/contracts/custom_actions/execute_contract.rb
  3. 231
      app/contracts/model_contract.rb

@ -0,0 +1,272 @@
#-- encoding: UTF-8
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2021 the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See docs/COPYRIGHT.rdoc for more details.
#++
class BaseContract < Disposable::Twin
require "disposable/twin/composition" # Expose.
include Expose
feature Setup
feature Setup::SkipSetter
feature Default
include ActiveModel::Validations
extend ActiveModel::Naming
extend ActiveModel::Translation
# Allows human_attribute_name to resolve custom fields correctly
extend Redmine::Acts::Customizable::HumanAttributeName
delegate :id,
to: :model
class << self
def writable_attributes
@writable_attributes ||= []
end
def writable_conditions
@writable_conditions ||= []
end
def attribute_permissions
@attribute_permissions ||= {}
end
def attribute_aliases
@attribute_aliases ||= {}
end
def attribute_alias(db, outside)
raise "Cannot define the alias to #{db} to be the same: #{outside}" if db == outside
attribute_aliases[db] = outside
end
def property(name, options = {}, &block)
if (twin = options.delete(:form))
options[:twin] = twin
end
if (validates_options = options[:validates])
validates name, validates_options
end
super
end
def attribute(attribute, options = {}, &block)
property attribute
add_writable(attribute, options[:writeable])
attribute_permission(attribute, options[:permission])
validate(attribute, &block) if block
end
def default_attribute_permission(permission)
attribute_permission(:default_permission, permission)
end
def attribute_permission(attribute, permission)
return unless permission
attribute_permissions[attribute] = Array(permission)
end
private
def add_writable(attribute, writeable)
attribute_name = attribute.to_s.gsub /_id\z/, ''
unless writeable == false
writable_attributes << attribute_name
# allow the _id variant as well
writable_attributes << "#{attribute_name}_id"
end
if writeable.respond_to?(:call)
writable_conditions << [attribute_name, writeable]
end
end
end
attr_reader :user
attr_accessor :options
def initialize(model, user, options: {})
super(model)
@user = user
@options = options
end
# we want to add a validation error whenever someone sets a property that we don't know.
# However AR will cleverly try to resolve the value for erroneous properties. Thus we need
# to hook into this method and return nil for unknown properties to avoid NoMethod errors...
def read_attribute_for_validation(attribute)
if respond_to? attribute
send attribute
end
end
def writable_attributes
@writable_attributes ||= begin
reduce_writable_attributes(collect_writable_attributes)
end
end
def writable?(attribute)
writable_attributes.include?(attribute.to_s)
end
def valid?(*args)
super()
errors.empty?
end
# Provide same interface with valid? and validate
# as with AM::Validations
#
# Do not use alias_method as this will not work when
# valid? is overridden in subclasses
def validate(*args)
valid?(*args)
end
# Methods required to get ActiveModel error messages working
extend ActiveModel::Naming
def self.model_name
ActiveModel::Name.new(model, nil)
end
def self.model
@model ||= begin
name.deconstantize.singularize.constantize
rescue NameError
ActiveRecord::Base
end
end
# use activerecord as the base scope instead of 'activemodel' to be compatible
# to the messages we have already stored
def self.i18n_scope
:activerecord
end
# end Methods required to get ActiveModel error messages working
protected
def collect_ancestor_attribute_aliases
@ancestor_attribute_aliases ||= collect_ancestor_attributes(:attribute_aliases)
end
# Traverse ancestor hierarchy to collect contract information.
# This allows to define attributes on a common base class of two or more contracts.
def collect_ancestor_attributes(attribute_to_collect)
combination_method, cleanup_method = if self.class.send(attribute_to_collect).is_a?(Hash)
%i[merge with_indifferent_access]
else
%i[concat uniq]
end
collect_ancestor_attributes_by(attribute_to_collect, combination_method, cleanup_method)
end
def collect_ancestor_attributes_by(attribute_to_collect, combination_method, cleanup_method)
klass = self.class
# `dup` is very important here.
# As the array/hash queried for here is memoized in the class (e.g. writable_conditions) and that
# object is later on combined (e.g. with #concat which alters the called on object) with a
# similar object from the superclass, every call would alter the memoized object as an unwanted side effect.
# Not only would that lead to the subclass now having all the attributes of the superclass,
# but those attributes would also be duplicated so that performance suffers significantly.
attributes = klass.send(attribute_to_collect).dup
while klass.superclass != ModelContract
# Collect all the attribute_to_collect from ancestors
klass = klass.superclass
attributes = attributes.send(combination_method, klass.send(attribute_to_collect))
end
attributes.send(cleanup_method)
end
def collect_writable_attributes
writable = collect_ancestor_attributes(:writable_attributes)
writable.each do |attribute|
if collect_ancestor_attribute_aliases[attribute]
writable << collect_ancestor_attribute_aliases[attribute].to_s
end
end
if model.respond_to?(:available_custom_fields)
writable += model.available_custom_fields.map { |cf| "custom_field_#{cf.id}" }
end
writable
end
def reduce_writable_attributes(attributes)
attributes = reduce_by_writable_conditions(attributes)
reduce_by_writable_permissions(attributes)
end
def reduce_by_writable_conditions(attributes)
collect_ancestor_attributes(:writable_conditions).each do |attribute, condition|
attributes -= [attribute, "#{attribute}_id"] unless instance_exec(&condition)
end
attributes
end
def reduce_by_writable_permissions(attributes)
attribute_permissions = collect_ancestor_attributes(:attribute_permissions)
attributes.reject do |attribute|
canonical_attribute = attribute.gsub(/_id\z/, '')
permissions = attribute_permissions[canonical_attribute] ||
attribute_permissions["#{canonical_attribute}_id"] ||
attribute_permissions[:default_permission]
next unless permissions
# This will break once a model that does not respond to project is used.
# This is intended to be worked on then with the additional knowledge.
next if permissions.any? { |p| user.allowed_to?(p, model.project, global: model.project.nil?) }
true
end
end
end

@ -29,7 +29,7 @@
#++
module CustomActions
class ExecuteContract < ModelContract
class ExecuteContract < BaseContract
property :lock_version
property :work_package_id

@ -28,125 +28,13 @@
# See docs/COPYRIGHT.rdoc for more details.
#++
class ModelContract < Disposable::Twin
require "disposable/twin/composition" # Expose.
include Expose
require_relative './base_contract'
feature Setup
feature Setup::SkipSetter
feature Default
include ActiveModel::Validations
extend ActiveModel::Naming
extend ActiveModel::Translation
delegate :id,
to: :model
# Allows human_attribute_name to resolve custom fields correctly
extend Redmine::Acts::Customizable::HumanAttributeName
class << self
def writable_attributes
@writable_attributes ||= []
end
def writable_conditions
@writable_conditions ||= []
end
def attribute_permissions
@attribute_permissions ||= {}
end
def attribute_aliases
@attribute_aliases ||= {}
end
def attribute_alias(db, outside)
raise "Cannot define the alias to #{db} to be the same: #{outside}" if db == outside
attribute_aliases[db] = outside
end
def property(name, options = {}, &block)
if (twin = options.delete(:form))
options[:twin] = twin
end
if (validates_options = options[:validates])
validates name, validates_options
end
super
end
def attribute(attribute, options = {}, &block)
property attribute
add_writable(attribute, options[:writeable])
attribute_permission(attribute, options[:permission])
validate(attribute, &block) if block
end
def default_attribute_permission(permission)
attribute_permission(:default_permission, permission)
end
def attribute_permission(attribute, permission)
return unless permission
attribute_permissions[attribute] = Array(permission)
end
private
def add_writable(attribute, writeable)
attribute_name = attribute.to_s.gsub /_id\z/, ''
unless writeable == false
writable_attributes << attribute_name
# allow the _id variant as well
writable_attributes << "#{attribute_name}_id"
end
if writeable.respond_to?(:call)
writable_conditions << [attribute_name, writeable]
end
end
end
attr_reader :user
attr_accessor :options
def initialize(model, user, options: {})
super(model)
@user = user
@options = options
end
# we want to add a validation error whenever someone sets a property that we don't know.
# However AR will cleverly try to resolve the value for erroneous properties. Thus we need
# to hook into this method and return nil for unknown properties to avoid NoMethod errors...
def read_attribute_for_validation(attribute)
if respond_to? attribute
send attribute
end
end
def writable_attributes
@writable_attributes ||= begin
reduce_writable_attributes(collect_writable_attributes)
end
end
def writable?(attribute)
writable_attributes.include?(attribute.to_s)
end
def validate(*args)
##
# Model contract for AR records that
# support change tracking
class ModelContract < BaseContract
def valid?(*args)
super()
readonly_attributes_unchanged
@ -164,28 +52,6 @@ class ModelContract < Disposable::Twin
errors.empty?
end
# Methods required to get ActiveModel error messages working
extend ActiveModel::Naming
def self.model_name
ActiveModel::Name.new(model, nil)
end
def self.model
@model ||= begin
name.deconstantize.singularize.constantize
rescue NameError
ActiveRecord::Base
end
end
# use activerecord as the base scope instead of 'activemodel' to be compatible
# to the messages we have already stored
def self.i18n_scope
:activerecord
end
# end Methods required to get ActiveModel error messages working
protected
##
@ -223,89 +89,4 @@ class ModelContract < Disposable::Twin
def changed_attributes
model.changed
end
def collect_ancestor_attribute_aliases
@ancestor_attribute_aliases ||= collect_ancestor_attributes(:attribute_aliases)
end
# Traverse ancestor hierarchy to collect contract information.
# This allows to define attributes on a common base class of two or more contracts.
def collect_ancestor_attributes(attribute_to_collect)
combination_method, cleanup_method = if self.class.send(attribute_to_collect).is_a?(Hash)
%i[merge with_indifferent_access]
else
%i[concat uniq]
end
collect_ancestor_attributes_by(attribute_to_collect, combination_method, cleanup_method)
end
def collect_ancestor_attributes_by(attribute_to_collect, combination_method, cleanup_method)
klass = self.class
# `dup` is very important here.
# As the array/hash queried for here is memoized in the class (e.g. writable_conditions) and that
# object is later on combined (e.g. with #concat which alters the called on object) with a
# similar object from the superclass, every call would alter the memoized object as an unwanted side effect.
# Not only would that lead to the subclass now having all the attributes of the superclass,
# but those attributes would also be duplicated so that performance suffers significantly.
attributes = klass.send(attribute_to_collect).dup
while klass.superclass != ModelContract
# Collect all the attribute_to_collect from ancestors
klass = klass.superclass
attributes = attributes.send(combination_method, klass.send(attribute_to_collect))
end
attributes.send(cleanup_method)
end
def collect_writable_attributes
writable = collect_ancestor_attributes(:writable_attributes)
writable.each do |attribute|
if collect_ancestor_attribute_aliases[attribute]
writable << collect_ancestor_attribute_aliases[attribute].to_s
end
end
if model.respond_to?(:available_custom_fields)
writable += model.available_custom_fields.map { |cf| "custom_field_#{cf.id}" }
end
writable
end
def reduce_writable_attributes(attributes)
attributes = reduce_by_writable_conditions(attributes)
reduce_by_writable_permissions(attributes)
end
def reduce_by_writable_conditions(attributes)
collect_ancestor_attributes(:writable_conditions).each do |attribute, condition|
attributes -= [attribute, "#{attribute}_id"] unless instance_exec(&condition)
end
attributes
end
def reduce_by_writable_permissions(attributes)
attribute_permissions = collect_ancestor_attributes(:attribute_permissions)
attributes.reject do |attribute|
canonical_attribute = attribute.gsub(/_id\z/, '')
permissions = attribute_permissions[canonical_attribute] ||
attribute_permissions["#{canonical_attribute}_id"] ||
attribute_permissions[:default_permission]
next unless permissions
# This will break once a model that does not respond to project is used.
# This is intended to be worked on then with the additional knowledge.
next if permissions.any? { |p| user.allowed_to?(p, model.project, global: model.project.nil?) }
true
end
end
end

Loading…
Cancel
Save