Filter out duplicated flags from user provided XML

This can happen either when posting a _meta (in this case an error
could be good enough, but isn't really helpful) or when receiving
_meta from local or remote backend. In this case we can't error out,
but need to recover the situation

Fixes #6823

src/api/app/helpers/flag_helper.rb

@@ -40,6 +40,9 @@ def update_flags(xmlhash, flagtype, position)
     # translate the flag types as used in the xml to model name + s
     validate_type flagtype
 
+    # we need to catch duplicates - and prefer the last
+    flags_to_create = {}
+
     # select each build flag from xml
     xmlhash.elements(flagtype.to_s) do |xmlflags|
       xmlflags.keys.each do |status|
@@ -52,17 +55,18 @@ def update_flags(xmlhash, flagtype, position)
 
           repo = xmlflag['repository']
 
-          # instantiate new flag object
-          flags.new(status: status, position: position, flag: flagtype) do |flag|
-            # set the flag attributes
-            flag.repo = repo
-            flag.architecture = arch
-          end
+          key = "#{repo}-#{arch}"
+          # overwrite duplicates - but prefer disables
+          next if flags_to_create[key] && flags_to_create[key][:status] == 'disable'
+          flags_to_create[key] = { status: status, position: position, repo: repo, architecture: arch }
           position += 1
         end
       end
     end
 
+    flags_to_create.values.each do |flag|
+      flags.build(flag.merge(flag: flagtype))
+    end
     position
   end
 

src/api/app/models/flag/validations.rb

@@ -0,0 +1,12 @@
+module Flag::Validations
+  extend ActiveSupport::Concern
+
+  included do
+    validate :validate_no_overlapping_flags
+  end
+
+  def validate_no_overlapping_flags
+    flags = self.flags.map { |flag| "#{flag.flag}-#{flag.architecture_id}-#{flag.repo}" }
+    errors.add(:flags, 'Duplicated flags') if flags.size != flags.uniq.size
+  end
+end

src/api/app/models/package.rb

@@ -7,6 +7,7 @@
 
 class Package < ApplicationRecord
   include FlagHelper
+  include Flag::Validations
   include CanRenderModel
   include HasRelationships
   include Package::Errors

src/api/app/models/project.rb

@@ -3,6 +3,7 @@
 # rubocop:disable Metrics/ClassLength
 class Project < ApplicationRecord
   include FlagHelper
+  include Flag::Validations
   include CanRenderModel
   include HasRelationships
   include HasRatings

src/api/spec/db/data/remove_duplicated_flags_spec.rb

@@ -23,6 +23,6 @@
       RemoveDuplicatedFlags.new.up
     end
 
-    it { expect(project.flags).to contain_exactly(flag_3, flag_2) }
+    it { expect(project.reload.flags).to contain_exactly(flag_3, flag_2) }
   end
 end

src/api/spec/models/package_spec.rb

@@ -756,5 +756,41 @@
       end
     end
   end
+
+  describe '#update_from_xml' do
+    let(:invalid_meta_xml) do
+      <<-XML_DATA
+      <package>
+        <title/>
+        <description/>
+        <build>
+          <enable/>
+          <disable/>
+          <enable arch="i586"/>
+          <disable arch="x86_64"/>
+          <enable arch="x86_64"/>
+        </build>
+      </package>
+      XML_DATA
+    end
+    let(:corrected_meta_xml) do
+      <<~XML_DATA2
+        <package name="test_package" project="home:tom">
+          <title/>
+          <description/>
+          <build>
+            <disable/>
+            <enable arch="i586"/>
+            <disable arch="x86_64"/>
+          </build>
+        </package>
+      XML_DATA2
+    end
+
+    it "doesn't crash on duplicated flags" do
+      package.update_from_xml(Xmlhash.parse(invalid_meta_xml))
+      expect(package.render_xml).to eq(corrected_meta_xml)
+    end
+  end
 end
 # rubocop:enable Metrics/BlockLength

src/api/spec/models/project_spec.rb

@@ -3,6 +3,7 @@
 # WARNING: If you need to make a Backend call uncomment the following line
 # CONFIG['global_write_through'] = true
 
+# rubocop:disable Metrics/BlockLength
 RSpec.describe Project, vcr: true do
   let!(:project) { create(:project, name: 'openSUSE_41') }
   let(:remote_project) { create(:remote_project, name: 'openSUSE.org') }
@@ -583,4 +584,44 @@ def reset_project_in_backend
       expect(project_release.packages.where(name: 'my_release_target')).to exist
     end
   end
+
+  describe '#update_from_xml' do
+    let(:project) { create(:project) }
+    let(:invalid_meta_xml) do
+      <<-XML_DATA
+      <project name="#{project.name}">
+        <title>Mine</title>
+        <description/>
+        <build>
+          <enable/>
+          <disable/>
+          <enable arch="i586"/>
+          <disable arch="x86_64"/>
+          <disable/>
+          <enable/>
+          <enable arch="x86_64"/>
+        </build>
+      </project>
+      XML_DATA
+    end
+
+    let(:new_xml) do
+      project.update_from_xml!(Xmlhash.parse(invalid_meta_xml))
+      project.save!
+      Xmlhash.parse(project.render_xml)
+    end
+
+    it 'ignores duplicated flags' do
+      expect(new_xml['build']['disable']).to contain_exactly({}, 'arch' => 'x86_64')
+    end
+
+    it 'erases all enable flags shadowed' do
+      expect(new_xml['build']['enable'].to_s).to eq('{"arch"=>"i586"}')
+    end
+
+    it 'updates basics' do
+      expect(new_xml).to include('title' => 'Mine', 'description' => {}, 'name' => project.name)
+    end
+  end
 end
+# rubocop:enable Metrics/BlockLength