Kiwi image editor: Improve #to_xml to decrease the resulting diff

src/api/app/models/kiwi/image.rb

@@ -14,7 +14,6 @@ class Kiwi::Image < ApplicationRecord
 '.freeze
 
   #### Self config
-
   #### Attributes
 
   #### Associations macros (Belongs to, Has one, Has many)
@@ -91,29 +90,7 @@ def self.build_from_xml(xml_string, md5)
   end
 
   def to_xml
-    if package
-      kiwi_file = package.kiwi_image_file
-      return nil unless kiwi_file
-      kiwi_body = package.source_file(kiwi_file)
-    else
-      kiwi_body = DEFAULT_KIWI_BODY
-    end
-
-    doc = Nokogiri::XML::DocumentFragment.parse(kiwi_body)
-    image = doc.at_css('image')
-
-    return nil unless image && image.first_element_child
-
-    doc.xpath("image/packages").remove
-    xml_packages = package_groups.map(&:to_xml).join("\n")
-    image.first_element_child.after(xml_packages)
-
-    doc.xpath("image/repository").remove
-    xml_repos = repositories_for_xml.map(&:to_xml).join("\n")
-    image.first_element_child.after(xml_repos)
-
-    # Reparser for pretty printing
-    Nokogiri::XML(doc.to_xml, &:noblanks).to_xml
+    Kiwi::Image::Xml.new(self).to_xml
   end
 
   def write_to_backend
@@ -157,14 +134,6 @@ def self.binaries_available(project, use_project_repositories, repositories)
 
   private
 
-  def repositories_for_xml
-    if use_project_repositories?
-      [Kiwi::Repository.new(source_path: 'obsrepositories:/', repo_type: 'rpm-md')]
-    else
-      repositories
-    end
-  end
-
   def check_use_project_repositories
     return unless use_project_repositories? && repositories.present?
 

src/api/app/models/kiwi/image/xml.rb

@@ -0,0 +1,66 @@
+# TODO: Please overwrite this comment with something explaining the model target
+module Kiwi
+  class Image
+    class Xml
+      def initialize(image)
+        @image = image
+      end
+
+      def to_xml
+        doc = Nokogiri::XML::DocumentFragment.parse(kiwi_body)
+        image = doc.at_css('image')
+
+        return nil unless image && image.first_element_child
+
+        doc = update_packages(doc)
+        doc = update_repositories(doc)
+        Nokogiri::XML(doc.to_xml, &:noblanks).to_xml(indent: kiwi_indentation(doc))
+      end
+
+      private
+
+      def update_packages(document)
+        # for now we only write the default package group
+        xml_packages = @image.default_package_group.to_xml
+        packages = document.xpath('image/packages[@type="image"]').first
+        if packages
+          packages.replace(xml_packages)
+        else
+          document.at_css('image').last_element_child.after(xml_packages)
+        end
+        document
+      end
+
+      def update_repositories(document)
+        repository_position = document.xpath("image/repository").first.try(:previous) || document.at_css('image').last_element_child
+        document.xpath("image/repository").remove
+        xml_repos = repositories_for_xml.map(&:to_xml).join("\n")
+        repository_position.after(xml_repos)
+        document
+      end
+
+      def repositories_for_xml
+        if @image.use_project_repositories?
+          [Kiwi::Repository.new(source_path: 'obsrepositories:/', repo_type: 'rpm-md')]
+        else
+          @image.repositories
+        end
+      end
+
+      def kiwi_body
+        if @image.package
+          kiwi_file = @image.package.kiwi_image_file
+          return nil unless kiwi_file
+          @image.package.source_file(kiwi_file)
+        else
+          Kiwi::Image::DEFAULT_KIWI_BODY
+        end
+      end
+
+      def kiwi_indentation(xml)
+        content = xml.xpath('image').children.first.try(:content)
+        content ? content.delete("\n").length : 2
+      end
+    end
+  end
+end

src/api/app/models/kiwi/package_group.rb

@@ -13,6 +13,7 @@ class Kiwi::PackageGroup < ApplicationRecord
   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?

src/api/spec/models/kiwi/image_spec.rb

@@ -169,7 +169,7 @@
       context 'with some repositories and packages' do
         before do
           kiwi_image.repositories << create(:kiwi_repository)
-          kiwi_image.package_groups << create(:kiwi_package_group_non_empty)
+          kiwi_image.package_groups << create(:kiwi_package_group_non_empty, kiwi_type: 'image')
         end
 
         subject { Nokogiri::XML::DocumentFragment.parse(kiwi_image.to_xml) }
@@ -211,7 +211,8 @@
       it { expect(subject.errors).to be_empty }
       it { expect(subject.xpath('.//image').length).to be(1) }
       it { expect(subject.xpath('.//image/description').length).to be(1) }
-      it { expect(subject.xpath('.//image/packages/package').length).to be(0) }
+      it { expect(subject.xpath(".//image/packages[@type='image']/package").length).to be(0) }
+      it { expect(subject.xpath(".//image/packages[@type='delete']/package").length).to be(0) }
       it { expect(subject.xpath('.//image/repository').length).to be(0) }
     end
 
@@ -241,6 +242,44 @@
 
       it { expect(subject.to_xml).to be_nil }
     end
+
+    context 'with a kiwi file with packages and repositories' do
+      let(:package) { create(:package) }
+      let(:kiwi_image) { Kiwi::Image.build_from_xml(kiwi_xml, 'some_md5') }
+      subject { Nokogiri::XML::DocumentFragment.parse(kiwi_image.to_xml) }
+
+      before do
+        allow(package).to receive(:kiwi_image_file).and_return('config.kiwi')
+        allow(package).to receive(:source_file).and_return(kiwi_xml)
+        kiwi_image.package = package
+        kiwi_image.save
+      end
+
+      it { expect(subject.children[2].children[3].name).to eq('packages') }
+      it { expect(subject.children[2].children[3].attributes['type'].value).to eq('image') }
+      it { expect(subject.children[2].children[7].name).to eq('repository') }
+      it { expect(subject.children[2].children[9].name).to eq('repository') }
+      it { expect(subject.children[2].children[11].name).to eq('repository') }
+      it { expect(subject.children[2].children[13].name).to eq('repository') }
+    end
+
+    context 'with a kiwi file without packages and repositories' do
+      let(:package) { create(:package) }
+      let(:kiwi_image) { Kiwi::Image.build_from_xml(Kiwi::Image::DEFAULT_KIWI_BODY, 'some_md5') }
+      subject { Nokogiri::XML::DocumentFragment.parse(kiwi_image.to_xml) }
+
+      before do
+        allow(package).to receive(:kiwi_image_file).and_return('config.kiwi')
+        allow(package).to receive(:source_file).and_return(Kiwi::Image::DEFAULT_KIWI_BODY)
+        kiwi_image.package = package
+        kiwi_image.package_groups << create(:kiwi_package_group_non_empty, kiwi_type: 'image')
+        kiwi_image.repositories << create(:kiwi_repository)
+        kiwi_image.save
+      end
+
+      it { expect(subject.children[2].children[5].name).to eq('packages') }
+      it { expect(subject.children[2].children[7].name).to eq('repository') }
+    end
   end
 
   describe '.write_to_backend' do