Commit 6f07f99f authored by Mehmet Emin INAC's avatar Mehmet Emin INAC

Apply model layer validations on ingestion process

With this change, the ingestion service classes are now using the
`BulkInsertSafe` utility module which validates the records before
executing the UPSERT queries.

Some of the validations require additional DB calls like the uniqueness
and presence of associated records therefore we disable them to prevent
running N+1 queries.
parent 2c4aaa87
...@@ -11,10 +11,6 @@ module Vulnerabilities ...@@ -11,10 +11,6 @@ module Vulnerabilities
# https://gitlab.com/gitlab-org/gitlab/-/issues/214563#note_370782508 is why the table names are not renamed # https://gitlab.com/gitlab-org/gitlab/-/issues/214563#note_370782508 is why the table names are not renamed
self.table_name = "vulnerability_occurrences" self.table_name = "vulnerability_occurrences"
# This is necessary to prevent updating the
# created_at attribute with upsert queries.
attr_readonly(:created_at)
FINDINGS_PER_PAGE = 20 FINDINGS_PER_PAGE = 20
MAX_NUMBER_OF_IDENTIFIERS = 20 MAX_NUMBER_OF_IDENTIFIERS = 20
REPORT_TYPES_WITH_LOCATION_IMAGE = %w[container_scanning cluster_image_scanning].freeze REPORT_TYPES_WITH_LOCATION_IMAGE = %w[container_scanning cluster_image_scanning].freeze
......
...@@ -4,10 +4,6 @@ module Vulnerabilities ...@@ -4,10 +4,6 @@ module Vulnerabilities
class FindingIdentifier < ApplicationRecord class FindingIdentifier < ApplicationRecord
self.table_name = "vulnerability_occurrence_identifiers" self.table_name = "vulnerability_occurrence_identifiers"
# This is necessary to prevent updating the
# created_at attribute with upsert queries.
attr_readonly(:created_at)
alias_attribute :finding_id, :occurrence_id alias_attribute :finding_id, :occurrence_id
belongs_to :finding, class_name: 'Vulnerabilities::Finding', inverse_of: :finding_identifiers, foreign_key: 'occurrence_id' belongs_to :finding, class_name: 'Vulnerabilities::Finding', inverse_of: :finding_identifiers, foreign_key: 'occurrence_id'
......
...@@ -5,10 +5,6 @@ module Vulnerabilities ...@@ -5,10 +5,6 @@ module Vulnerabilities
class FindingRemediation < ApplicationRecord class FindingRemediation < ApplicationRecord
self.table_name = 'vulnerability_findings_remediations' self.table_name = 'vulnerability_findings_remediations'
# This is necessary to prevent updating the
# created_at attribute with upsert queries.
attr_readonly(:created_at)
belongs_to :finding, class_name: 'Vulnerabilities::Finding', inverse_of: :finding_remediations, foreign_key: 'vulnerability_occurrence_id', optional: false belongs_to :finding, class_name: 'Vulnerabilities::Finding', inverse_of: :finding_remediations, foreign_key: 'vulnerability_occurrence_id', optional: false
belongs_to :remediation, class_name: 'Vulnerabilities::Remediation', inverse_of: :finding_remediations, foreign_key: 'vulnerability_remediation_id', optional: false belongs_to :remediation, class_name: 'Vulnerabilities::Remediation', inverse_of: :finding_remediations, foreign_key: 'vulnerability_remediation_id', optional: false
......
...@@ -7,10 +7,6 @@ module Vulnerabilities ...@@ -7,10 +7,6 @@ module Vulnerabilities
self.table_name = 'vulnerability_finding_signatures' self.table_name = 'vulnerability_finding_signatures'
# This is necessary to prevent updating the
# created_at attribute with upsert queries.
attr_readonly(:created_at)
belongs_to :finding, foreign_key: 'finding_id', inverse_of: :signatures, class_name: 'Vulnerabilities::Finding' belongs_to :finding, foreign_key: 'finding_id', inverse_of: :signatures, class_name: 'Vulnerabilities::Finding'
enum algorithm_type: VulnerabilityFindingSignatureHelpers::ALGORITHM_TYPES, _prefix: :algorithm enum algorithm_type: VulnerabilityFindingSignatureHelpers::ALGORITHM_TYPES, _prefix: :algorithm
validates :finding, presence: true validates :finding, presence: true
......
...@@ -4,10 +4,6 @@ module Vulnerabilities ...@@ -4,10 +4,6 @@ module Vulnerabilities
class Flag < ApplicationRecord class Flag < ApplicationRecord
self.table_name = 'vulnerability_flags' self.table_name = 'vulnerability_flags'
# This is necessary to prevent updating the
# created_at attribute with upsert queries.
attr_readonly(:created_at)
belongs_to :finding, class_name: 'Vulnerabilities::Finding', foreign_key: 'vulnerability_occurrence_id', inverse_of: :vulnerability_flags, optional: false belongs_to :finding, class_name: 'Vulnerabilities::Finding', foreign_key: 'vulnerability_occurrence_id', inverse_of: :vulnerability_flags, optional: false
validates :origin, length: { maximum: 255 } validates :origin, length: { maximum: 255 }
......
...@@ -28,31 +28,72 @@ module Security ...@@ -28,31 +28,72 @@ module Security
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
def self.included(base) def self.included(base)
base.singleton_class.attr_accessor :model, :unique_by, :uses base.singleton_class.attr_accessor(:model, :unique_by, :uses)
base.extend(ClassMethods)
end
module ClassMethods
# Creates a proxy class to be used in UPSERT queries
# which will also run the model layer validations except
# the uniquness and presence of associations validations.
def klass
@klass ||= Class.new(model).tap do |klass|
remove_validations(klass)
end.include(BulkInsertSafe)
end
private
def remove_validations(klass)
klass.validators.each do |validator|
remove_validation_if_necessary(klass, validator)
end
end
def remove_validation_if_necessary(klass, validator)
return unless uniqunesss_validator?(validator) || presence_of_association_validator?(klass, validator)
klass.skip_callback(:validate, :before, validator)
end
def uniqunesss_validator?(validator)
validator.instance_of?(ActiveRecord::Validations::UniquenessValidator)
end
def presence_of_association_validator?(klass, validator)
validator.instance_of?(ActiveRecord::Validations::PresenceValidator) &&
(klass.reflections.keys & validator.attributes.map(&:to_s)).any?
end
end end
def execute def execute
result_set return_data
after_ingest if uses after_ingest if uses
end end
private private
delegate :unique_by, :model, :uses, :cast_values, to: :'self.class', private: true delegate :unique_by, :model, :klass, :uses, :cast_values, to: :'self.class', private: true
def return_data def return_data
@return_data ||= result_set&.cast_values(model.attribute_types).to_a strong_memoize(:return_data) do
end if insert_objects.present?
unique_by.present? ? bulk_upsert : bulk_insert
def result_set else
strong_memoize(:result_set) do []
if insert_attributes.present?
ActiveRecord::InsertAll.new(model, insert_attributes, on_duplicate: on_duplicate, returning: uses, unique_by: unique_by).execute
end end
end end
end end
def bulk_insert
klass.bulk_insert!(insert_objects, skip_duplicates: true, returns: uses)
end
def bulk_upsert
klass.bulk_upsert!(insert_objects, unique_by: unique_by, returns: uses, &method(:slice_attributes))
end
def after_ingest def after_ingest
raise "Implement the `after_ingest` template method!" raise "Implement the `after_ingest` template method!"
end end
...@@ -61,16 +102,28 @@ module Security ...@@ -61,16 +102,28 @@ module Security
raise "Implement the `attributes` template method!" raise "Implement the `attributes` template method!"
end end
def insert_objects
@insert_objects ||= insert_attributes.map { |attributes| klass.new(attributes) }
end
def insert_attributes def insert_attributes
@insert_attributes ||= attributes.map { |values| values.merge(timestamps) } @insert_attributes ||= attributes.map { |values| values.merge(timestamps) }
end end
def timestamps # `BulkInsertSafe` module is trying to update all the attributes
@timestamps ||= Time.zone.now.then { |time| { created_at: time, updated_at: time } } # of a record which overrides the columns with NULL values if the
# attribute is not provided. For this reason, we need to slice the
# attributes with this callback.
def slice_attributes(item_attributes)
item_attributes.slice!(*attribute_names)
end end
def on_duplicate def attribute_names
unique_by.present? ? :update : :skip @attribute_names ||= insert_attributes.first.keys.map(&:to_s)
end
def timestamps
@timestamps ||= Time.zone.now.then { |time| { created_at: time, updated_at: time } }
end end
end end
end end
......
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