From 356e48213e59dc7a57dfc36028fea4a9700f7dd5 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 26 Oct 2017 12:05:28 +0200 Subject: [PATCH] [frontend] Use nested module syntax for Kiwi classes simplecov has problems to calculate coverage otherwise... --- src/api/app/models/kiwi/image.rb | 208 ++++++++++++----------- src/api/app/models/kiwi/package.rb | 24 +-- src/api/app/models/kiwi/package_group.rb | 48 +++--- src/api/app/models/kiwi/repository.rb | 166 +++++++++--------- src/api/spec/models/kiwi/image_spec.rb | 2 +- 5 files changed, 228 insertions(+), 220 deletions(-) diff --git a/src/api/app/models/kiwi/image.rb b/src/api/app/models/kiwi/image.rb index 6299b5c77f7..8389d68b155 100644 --- a/src/api/app/models/kiwi/image.rb +++ b/src/api/app/models/kiwi/image.rb @@ -1,126 +1,128 @@ -class Kiwi::Image < ApplicationRecord - #### Includes and extends - - #### Constants - DEFAULT_KIWI_BODY = ' - - - - - - - -'.freeze - - #### Self config - #### Attributes - - #### Associations macros (Belongs to, Has one, Has many) - has_one :package, foreign_key: 'kiwi_image_id', class_name: '::Package', dependent: :nullify, inverse_of: :kiwi_image - has_many :repositories, -> { order(order: :asc) }, dependent: :destroy, index_errors: true - has_many :package_groups, -> { order(:id) }, dependent: :destroy, index_errors: true - has_many :kiwi_packages, -> { where(kiwi_package_groups: { kiwi_type: Kiwi::PackageGroup.kiwi_types[:image] }) }, - through: :package_groups, source: :packages, inverse_of: :kiwi_image - - #### Callbacks macros: before_save, after_save, etc. - - #### Scopes (first the default_scope macro if is used) - - #### Validations macros - validates :name, presence: true - validate :check_use_project_repositories - validate :check_package_groups - accepts_nested_attributes_for :repositories, allow_destroy: true - accepts_nested_attributes_for :package_groups, allow_destroy: true - accepts_nested_attributes_for :kiwi_packages, allow_destroy: true - - #### Class methods using self. (public and then private) - - #### To define class methods as private use private_class_method - #### private - - #### Instance methods (public and then protected/private) - - #### Alias of methods - def self.build_from_xml(xml_string, md5) - Kiwi::Image::XmlParser.new(xml_string, md5).parse - end +module Kiwi + class Image < ApplicationRecord + #### Includes and extends + + #### Constants + DEFAULT_KIWI_BODY = ' + + + + + + + + '.freeze + + #### Self config + #### Attributes + + #### Associations macros (Belongs to, Has one, Has many) + has_one :package, foreign_key: 'kiwi_image_id', class_name: '::Package', dependent: :nullify, inverse_of: :kiwi_image + has_many :repositories, -> { order(order: :asc) }, dependent: :destroy, index_errors: true + has_many :package_groups, -> { order(:id) }, dependent: :destroy, index_errors: true + has_many :kiwi_packages, -> { where(kiwi_package_groups: { kiwi_type: Kiwi::PackageGroup.kiwi_types[:image] }) }, + through: :package_groups, source: :packages, inverse_of: :kiwi_image + + #### Callbacks macros: before_save, after_save, etc. + + #### Scopes (first the default_scope macro if is used) + + #### Validations macros + validates :name, presence: true + validate :check_use_project_repositories + validate :check_package_groups + accepts_nested_attributes_for :repositories, allow_destroy: true + accepts_nested_attributes_for :package_groups, allow_destroy: true + accepts_nested_attributes_for :kiwi_packages, allow_destroy: true + + #### Class methods using self. (public and then private) + + #### To define class methods as private use private_class_method + #### private + + #### Instance methods (public and then protected/private) + + #### Alias of methods + def self.build_from_xml(xml_string, md5) + Kiwi::Image::XmlParser.new(xml_string, md5).parse + end - def to_xml - Kiwi::Image::XmlBuilder.new(self).build - end + def to_xml + Kiwi::Image::XmlBuilder.new(self).build + end - def write_to_backend - return false unless package + def write_to_backend + return false unless package - Package.transaction do - file_name = package.kiwi_image_file || "#{package.name}.kiwi" - package.save_file({ filename: file_name, file: to_xml }) - self.md5_last_revision = package.kiwi_file_md5 - save! + Package.transaction do + file_name = package.kiwi_image_file || "#{package.name}.kiwi" + package.save_file({ filename: file_name, file: to_xml }) + self.md5_last_revision = package.kiwi_file_md5 + save! + end end - end - def outdated? - return false unless package + def outdated? + return false unless package - package.kiwi_image_outdated? - end + package.kiwi_image_outdated? + end - def default_package_group - package_groups.type_image.first || package_groups.create(kiwi_type: :image, pattern_type: 'onlyRequired') - end + def default_package_group + package_groups.type_image.first || package_groups.create(kiwi_type: :image, pattern_type: 'onlyRequired') + end - def self.find_binaries_by_name(query, project, repositories, options = {}) - finder = /\A#{Regexp.quote(query)}/ - binaries_available(project, options[:use_project_repositories], repositories).select { |package, _| finder.match(package.to_s) } - end + def self.find_binaries_by_name(query, project, repositories, options = {}) + finder = /\A#{Regexp.quote(query)}/ + binaries_available(project, options[:use_project_repositories], repositories).select { |package, _| finder.match(package.to_s) } + end - def self.binaries_available(project, use_project_repositories, repositories) - Rails.cache.fetch("kiwi_image_binaries_available_#{project}_#{use_project_repositories}_#{repositories}", expires_in: 5.minutes) do - if use_project_repositories - Backend::Api::BuildResults::Binaries.available_in_project(project) - else - return [] if repositories.blank? - obs_repository_paths = repositories.select { |url| url.starts_with?("obs://")}.map {|url| url[6..-1] } - non_obs_repository_urls = repositories.reject { |url| url.starts_with?("obs://")} - Backend::Api::BuildResults::Binaries.available_in_repositories(project, non_obs_repository_urls, obs_repository_paths) + def self.binaries_available(project, use_project_repositories, repositories) + Rails.cache.fetch("kiwi_image_binaries_available_#{project}_#{use_project_repositories}_#{repositories}", expires_in: 5.minutes) do + if use_project_repositories + Backend::Api::BuildResults::Binaries.available_in_project(project) + else + return [] if repositories.blank? + obs_repository_paths = repositories.select { |url| url.starts_with?("obs://")}.map {|url| url[6..-1] } + non_obs_repository_urls = repositories.reject { |url| url.starts_with?("obs://")} + Backend::Api::BuildResults::Binaries.available_in_repositories(project, non_obs_repository_urls, obs_repository_paths) + end end end - end - # This method is for parse the error messages and group them to make them more understandable. - # The nested errors messages are like `Model[0] attributes` and this is not easy to understand. - def parsed_errors(title, packages) - message = { title: title } - message["Image Errors:"] = errors.messages[:base] if errors.messages[:base].present? - - { repository: repositories, package: packages }.each do |key, records| - records.each do |record| - if record.errors.present? - message["#{key.capitalize}: #{record.name}"] ||= [] - message["#{key.capitalize}: #{record.name}"] << record.errors.messages.values - message["#{key.capitalize}: #{record.name}"].flatten! + # This method is for parse the error messages and group them to make them more understandable. + # The nested errors messages are like `Model[0] attributes` and this is not easy to understand. + def parsed_errors(title, packages) + message = { title: title } + message["Image Errors:"] = errors.messages[:base] if errors.messages[:base].present? + + { repository: repositories, package: packages }.each do |key, records| + records.each do |record| + if record.errors.present? + message["#{key.capitalize}: #{record.name}"] ||= [] + message["#{key.capitalize}: #{record.name}"] << record.errors.messages.values + message["#{key.capitalize}: #{record.name}"].flatten! + end end end - end - message - end + message + end - private + private - def check_use_project_repositories - return unless use_project_repositories? && repositories.present? + def check_use_project_repositories + return unless use_project_repositories? && repositories.present? - errors.add(:base, - "A repository with source_path \"obsrepositories:/\" has been set. If you want to use it, please remove the other repositories.") - end + errors.add(:base, + "A repository with source_path \"obsrepositories:/\" has been set. If you want to use it, please remove the other repositories.") + end - def check_package_groups - return if package_groups.group_by(&:kiwi_type).select { |_key, value| value.count > 1 }.keys.empty? + def check_package_groups + return if package_groups.group_by(&:kiwi_type).select { |_key, value| value.count > 1 }.keys.empty? - errors.add(:base, "Multiple package groups with same type are not allowed.") + errors.add(:base, "Multiple package groups with same type are not allowed.") + end end end diff --git a/src/api/app/models/kiwi/package.rb b/src/api/app/models/kiwi/package.rb index 259f4e03d5b..7fbba732118 100644 --- a/src/api/app/models/kiwi/package.rb +++ b/src/api/app/models/kiwi/package.rb @@ -1,16 +1,18 @@ -class Kiwi::Package < ApplicationRecord - belongs_to :package_group - has_one :kiwi_image, through: :package_groups +module Kiwi + class Package < ApplicationRecord + belongs_to :package_group + has_one :kiwi_image, through: :package_groups - validates :name, presence: { message: 'Package name can\'t be blank'} + validates :name, presence: { message: 'Package name can\'t be blank'} - def to_h - hash = { name: name } - hash[:arch] = arch if arch.present? - hash[:replaces] = replaces if replaces.present? - hash[:bootinclude] = bootinclude if bootinclude.present? - hash[:bootdelete] = bootdelete if bootdelete.present? - hash + def to_h + hash = { name: name } + hash[:arch] = arch if arch.present? + hash[:replaces] = replaces if replaces.present? + hash[:bootinclude] = bootinclude if bootinclude.present? + hash[:bootdelete] = bootdelete if bootdelete.present? + hash + end end end diff --git a/src/api/app/models/kiwi/package_group.rb b/src/api/app/models/kiwi/package_group.rb index 62b2b6299cc..9d19254074c 100644 --- a/src/api/app/models/kiwi/package_group.rb +++ b/src/api/app/models/kiwi/package_group.rb @@ -1,35 +1,37 @@ -class Kiwi::PackageGroup < ApplicationRecord - has_many :packages, dependent: :destroy - belongs_to :image +module Kiwi + class PackageGroup < ApplicationRecord + has_many :packages, dependent: :destroy + belongs_to :image - # we need to add a prefix, to avoid generating class methods that already - # exist in Active Record, such as "delete" - enum kiwi_type: %i[bootstrap delete docker image iso lxc oem pxe split testsuite vmx], _prefix: :type + # we need to add a prefix, to avoid generating class methods that already + # exist in Active Record, such as "delete" + enum kiwi_type: %i[bootstrap delete docker image iso lxc oem pxe split testsuite vmx], _prefix: :type - scope :type_image, -> { where(kiwi_type: :image) } + scope :type_image, -> { where(kiwi_type: :image) } - validates :kiwi_type, presence: true + validates :kiwi_type, presence: true - accepts_nested_attributes_for :packages, reject_if: :all_blank, allow_destroy: true + accepts_nested_attributes_for :packages, reject_if: :all_blank, allow_destroy: true - def to_xml - return '' if packages.empty? - group_attributes = { type: kiwi_type } - group_attributes[:profiles] = profiles if profiles.present? - group_attributes[:patternType] = pattern_type if pattern_type.present? + def to_xml + return '' if packages.empty? + group_attributes = { type: kiwi_type } + group_attributes[:profiles] = profiles if profiles.present? + group_attributes[:patternType] = pattern_type if pattern_type.present? - builder = Nokogiri::XML::Builder.new - builder.packages(group_attributes) do |group| - packages.each do |package| - group.package(package.to_h) + builder = Nokogiri::XML::Builder.new + builder.packages(group_attributes) do |group| + packages.each do |package| + group.package(package.to_h) + end end - end - builder.to_xml save_with: Nokogiri::XML::Node::SaveOptions::NO_DECLARATION | Nokogiri::XML::Node::SaveOptions::FORMAT - end + builder.to_xml save_with: Nokogiri::XML::Node::SaveOptions::NO_DECLARATION | Nokogiri::XML::Node::SaveOptions::FORMAT + end - def kiwi_type_image? - kiwi_type == 'image' + def kiwi_type_image? + kiwi_type == 'image' + end end end diff --git a/src/api/app/models/kiwi/repository.rb b/src/api/app/models/kiwi/repository.rb index 2187b67a357..eaaaedf2752 100644 --- a/src/api/app/models/kiwi/repository.rb +++ b/src/api/app/models/kiwi/repository.rb @@ -1,108 +1,110 @@ # TODO: Please overwrite this comment with something explaining the model target -class Kiwi::Repository < ApplicationRecord - #### Includes and extends +module Kiwi + class Repository < ApplicationRecord + #### Includes and extends - #### Constants - REPO_TYPES = ['rpm-md', 'apt-deb'].freeze + #### Constants + REPO_TYPES = ['rpm-md', 'apt-deb'].freeze - #### Self config + #### Self config - #### Attributes + #### Attributes - #### Associations macros (Belongs to, Has one, Has many) - belongs_to :image + #### Associations macros (Belongs to, Has one, Has many) + belongs_to :image - #### Callbacks macros: before_save, after_save, etc. - before_validation :map_to_allowed_repository_types, on: :create + #### Callbacks macros: before_save, after_save, etc. + before_validation :map_to_allowed_repository_types, on: :create - #### Scopes (first the default_scope macro if is used) + #### Scopes (first the default_scope macro if is used) - #### Validations macros - validates :alias, :source_path, uniqueness: { scope: :image, message: "%{attribute} '%{value}' has already been taken."} - validates :source_path, presence: { message: '%{attribute} can\'t be nil.'} - validate :source_path_format - validates :priority, numericality: { only_integer: true, allow_nil: true, greater_than_or_equal_to: 0, - less_than: 100, message: '%{attribute} must be between 0 and 99.' } - validates :order, numericality: { only_integer: true, greater_than_or_equal_to: 1 } - # TODO: repo_type value depends on packagemanager element - # https://doc.opensuse.org/projects/kiwi/doc/#sec.description.repository - validates :repo_type, inclusion: { in: REPO_TYPES, message: "%{attribute} '%{value}' is not included in the list." } - validates :replaceable, inclusion: { in: [true, false], message: "%{attribute} has to be a boolean" } - validates :imageinclude, :prefer_license, inclusion: { in: [true, false], message: "%{attribute} has to be a boolean" }, allow_nil: true - validates_associated :image, on: :update + #### Validations macros + validates :alias, :source_path, uniqueness: { scope: :image, message: "%{attribute} '%{value}' has already been taken."} + validates :source_path, presence: { message: '%{attribute} can\'t be nil.'} + validate :source_path_format + validates :priority, numericality: { only_integer: true, allow_nil: true, greater_than_or_equal_to: 0, + less_than: 100, message: '%{attribute} must be between 0 and 99.' } + validates :order, numericality: { only_integer: true, greater_than_or_equal_to: 1 } + # TODO: repo_type value depends on packagemanager element + # https://doc.opensuse.org/projects/kiwi/doc/#sec.description.repository + validates :repo_type, inclusion: { in: REPO_TYPES, message: "%{attribute} '%{value}' is not included in the list." } + validates :replaceable, inclusion: { in: [true, false], message: "%{attribute} has to be a boolean" } + validates :imageinclude, :prefer_license, inclusion: { in: [true, false], message: "%{attribute} has to be a boolean" }, allow_nil: true + validates_associated :image, on: :update - #### Class methods using self. (public and then private) + #### Class methods using self. (public and then private) - #### To define class methods as private use private_class_method - #### private + #### To define class methods as private use private_class_method + #### private - #### Instance methods (public and then protected/private) - def name - return source_path.to_s.tr('/', '_') if attributes['alias'].blank? - attributes['alias'] - end - - def source_path_format - return if source_path == 'obsrepositories:/' - return if source_path =~ /^(dir|iso|smb|this):\/\/.+/ - return if source_path =~ /\A#{URI.regexp(['ftp', 'http', 'https', 'plain'])}\z/ - if source_path_for_obs_repository? - return if repo_type == 'rpm-md' - errors.add(:repo_type, "Repo type should be 'rpm-md' for obs:// repositories.") + #### Instance methods (public and then protected/private) + def name + return source_path.to_s.tr('/', '_') if attributes['alias'].blank? + attributes['alias'] end - return if source_path_for_opensuse_repository? - errors.add(:source_path, "Source path has an invalid format.") - end - def to_xml - repo_attributes = { type: repo_type } - repo_attributes[:priority] = priority if priority.present? - repo_attributes[:alias] = self.alias if self.alias.present? - if username.present? - repo_attributes[:username] = username - repo_attributes[:password] = password + def source_path_format + return if source_path == 'obsrepositories:/' + return if source_path =~ /^(dir|iso|smb|this):\/\/.+/ + return if source_path =~ /\A#{URI.regexp(['ftp', 'http', 'https', 'plain'])}\z/ + if source_path_for_obs_repository? + return if repo_type == 'rpm-md' + errors.add(:repo_type, "Repo type should be 'rpm-md' for obs:// repositories.") + end + return if source_path_for_opensuse_repository? + errors.add(:source_path, "Source path has an invalid format.") end - repo_attributes[:status] = 'replaceable' if replaceable - repo_attributes[:imageinclude] = true if imageinclude - repo_attributes['prefer-license'] = true if prefer_license - builder = Nokogiri::XML::Builder.new - builder.repository(repo_attributes) do |repo| - repo.source(path: source_path) + def to_xml + repo_attributes = { type: repo_type } + repo_attributes[:priority] = priority if priority.present? + repo_attributes[:alias] = self.alias if self.alias.present? + if username.present? + repo_attributes[:username] = username + repo_attributes[:password] = password + end + repo_attributes[:status] = 'replaceable' if replaceable + repo_attributes[:imageinclude] = true if imageinclude + repo_attributes['prefer-license'] = true if prefer_license + + builder = Nokogiri::XML::Builder.new + builder.repository(repo_attributes) do |repo| + repo.source(path: source_path) + end + + builder.to_xml save_with: Nokogiri::XML::Node::SaveOptions::NO_DECLARATION | Nokogiri::XML::Node::SaveOptions::FORMAT end - builder.to_xml save_with: Nokogiri::XML::Node::SaveOptions::NO_DECLARATION | Nokogiri::XML::Node::SaveOptions::FORMAT - end - - def obs_source_path? - source_path && source_path.match(/^obs:\/\/([^\/]+)\/([^\/]+)$/).present? - end + def obs_source_path? + source_path && source_path.match(/^obs:\/\/([^\/]+)\/([^\/]+)$/).present? + end - def project_for_type_obs - return '' unless source_path - source_path.match(/^obs:\/\/([^\/]+)\/([^\/]+)$/).try(:[], 1) - end + def project_for_type_obs + return '' unless source_path + source_path.match(/^obs:\/\/([^\/]+)\/([^\/]+)$/).try(:[], 1) + end - def repository_for_type_obs - return '' unless source_path - source_path.match(/^obs:\/\/([^\/]+)\/([^\/]+)$/).try(:[], 2) - end + def repository_for_type_obs + return '' unless source_path + source_path.match(/^obs:\/\/([^\/]+)\/([^\/]+)$/).try(:[], 2) + end - private + private - def source_path_for_obs_repository? - source_path =~ /^obs:\/\/([^\/]+)\/([^\/]+)$/ && Project.valid_name?(Regexp.last_match(1)) && Project.valid_name?(Regexp.last_match(2)) - end + def source_path_for_obs_repository? + source_path =~ /^obs:\/\/([^\/]+)\/([^\/]+)$/ && Project.valid_name?(Regexp.last_match(1)) && Project.valid_name?(Regexp.last_match(2)) + end - def source_path_for_opensuse_repository? - # $1 must be a project name. $2 must be a repository name - source_path =~ /^opensuse:\/\/([^\/]+)\/([^\/]+)$/ && - Project.valid_name?(Regexp.last_match(1)) && - Regexp.last_match(2) =~ /\A[^_:\/\000-\037][^:\/\000-\037]*\Z/ - end + def source_path_for_opensuse_repository? + # $1 must be a project name. $2 must be a repository name + source_path =~ /^opensuse:\/\/([^\/]+)\/([^\/]+)$/ && + Project.valid_name?(Regexp.last_match(1)) && + Regexp.last_match(2) =~ /\A[^_:\/\000-\037][^:\/\000-\037]*\Z/ + end - def map_to_allowed_repository_types - self.repo_type = 'rpm-md' unless repo_type.in?(REPO_TYPES) + def map_to_allowed_repository_types + self.repo_type = 'rpm-md' unless repo_type.in?(REPO_TYPES) + end end end diff --git a/src/api/spec/models/kiwi/image_spec.rb b/src/api/spec/models/kiwi/image_spec.rb index a48aea2ddd5..ca773d94c02 100644 --- a/src/api/spec/models/kiwi/image_spec.rb +++ b/src/api/spec/models/kiwi/image_spec.rb @@ -163,7 +163,7 @@ describe '#to_xml' do context 'without a package' do context 'without any repository or package' do - it { expect(kiwi_image.to_xml).to eq(Kiwi::Image::DEFAULT_KIWI_BODY) } + it { expect(kiwi_image.to_xml.delete(' ')).to eq(Kiwi::Image::DEFAULT_KIWI_BODY.delete(' ')) } end context 'with some repositories and packages' do