[PATCH aeolus-conductor] BZ#790434 Check unique assembly names on deployable xml

Jason Guiditta jguiditt at redhat.com
Wed Feb 22 20:49:33 UTC 2012


On 16/02/12 10:53 +0000, mtaylor at redhat.com wrote:
>From: Martyn Taylor <mtaylor at redhat.com>
>
Conditional ACK, minor point inline, I am making this change and
pushing so we can get it in a build tonight.

>---
> src/app/models/deployable.rb       |    4 +++-
> src/app/util/deployable_xml.rb     |    4 ++++
> src/config/locales/cs.yml          |    1 +
> src/config/locales/en.yml          |    1 +
> src/spec/factories/deployable.rb   |   15 +++++++++++++++
> src/spec/models/deployable_spec.rb |    5 +++++
> 6 files changed, 29 insertions(+), 1 deletions(-)
>
>diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb
>index c9df4c5..3c8bdbd 100644
>--- a/src/app/models/deployable.rb
>+++ b/src/app/models/deployable.rb
>@@ -48,8 +48,10 @@ class Deployable < ActiveRecord::Base
>   def valid_deployable_xml?
>     begin
>       deployable_xml = DeployableXML.new(xml)
>-      unless deployable_xml.validate!
>+      if !deployable_xml.validate!
>         errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid'))
>+      elsif !deployable_xml.unique_assembly_names?
>+        errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid_duplicate_assembly_names'))
>       end
>     rescue Nokogiri::XML::SyntaxError => e
>       errors.add(:base, I18n.t("deployments.errors.not_valid_deployable_xml", :msg => "#{e.message}"))
>diff --git a/src/app/util/deployable_xml.rb b/src/app/util/deployable_xml.rb
>index 3032b5e..272476d 100644
>--- a/src/app/util/deployable_xml.rb
>+++ b/src/app/util/deployable_xml.rb
>@@ -212,6 +212,10 @@ class DeployableXML
>     errors.empty?
>   end
>
>+  def unique_assembly_names?
>+    @root.xpath("/deployable/assemblies/assembly/@name").collect { |e| e.value }.uniq!.nil?
>+  end
>+
>   def assemblies
>     @assemblies ||=
>       @root.xpath('/deployable/assemblies/assembly').collect do |assembly_node|
>diff --git a/src/config/locales/cs.yml b/src/config/locales/cs.yml
>index 7ada0cc..e004fa1 100644
>--- a/src/config/locales/cs.yml
>+++ b/src/config/locales/cs.yml
>@@ -216,6 +216,7 @@ cs:
>       warning:
>         not_valid_or_reachable: mcdmhig jiama afc nfdi ef jhifen bagekcj di dd bjokkm kefddkmcb dn
>         not_valid: cdjodmkfmo cmg jmjkbee namcgnb foblo kio hkgk
>+        not_valid_duplicate_assembly_names: dsadf asdfsad oi ppoi jflf opifas
>     properties:
>       properties_for: nmiknfjdii hdk
>   permissions:
>diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
>index fbacbc0..6de7502 100644
>--- a/src/config/locales/en.yml
>+++ b/src/config/locales/en.yml
>@@ -662,6 +662,7 @@ en:
>         not_valid_or_reachable: "Deployable XML file is either invalid or no longer reachable at %{url}"
>         no_url_provided: "No URL provided for the deployable XML file."
>         not_valid: "Deployable XML file doesn't resolve valid XML"
>+        not_valid_duplicate_assembly_names: "Deployable XML must contain unique assembly names"
>         failed: Deployable was not created.
>       notice:
>         added: Deployable added to catalog %{catalog}.
>diff --git a/src/spec/factories/deployable.rb b/src/spec/factories/deployable.rb
>index d5afa1e..efbfc6e 100644
>--- a/src/spec/factories/deployable.rb
>+++ b/src/spec/factories/deployable.rb
>@@ -34,6 +34,21 @@ FactoryGirl.define do
>
>   end
>
>+  factory :deployable_unique_name_violation, :parent => :deployable do
>+    xml "<?xml version=\"1.0\"?>
>+            <deployable version='1.0' name='my'>
>+              <description>This is my testing image</description>
>+              <assemblies>
>+                <assembly name='frontend' hwp='front_hwp1'>
>+                  <image id='53d2a281-448b-4872-b1b0-680edaad5922' build='63838705-8608-44c6-aded-7c243137172c'></image>
>+                </assembly>
>+                <assembly name='frontend' hwp='front_hwp2'>
>+                  <image id='53d2a281-448b-4872-b1b0-680edaad5922' build='63838705-8608-44c6-aded-7c243137172c'></image>
>+                </assembly>
>+              </assemblies>
>+            </deployable>"
>+  end
>+
>   factory :deployable_with_parameters, :parent => :deployable do
>     xml "<?xml version=\"1.0\"?>
>             <deployable version='1.0' name='deployable_with_launch_parameters'>
>diff --git a/src/spec/models/deployable_spec.rb b/src/spec/models/deployable_spec.rb
>index 7e91ae2..af3f4b0 100644
>--- a/src/spec/models/deployable_spec.rb
>+++ b/src/spec/models/deployable_spec.rb
>@@ -38,4 +38,9 @@ describe Deployable do
>     deployable.should_not be_valid
>   end
>
>+  it "should not be valid if xml has multiple assemblies with the same name" do
>+    deployable = FactoryGirl.build(:deployable_unique_name_violation)
>+    deployable.valid_deployable_xml?
>+    deployable.errors[:xml].should == ["Deployable XML must contain unique assembly names"]

The above will not work when the i18n string is overriden (which it
will be in product for this case), so I think it is less error-prone
to change this to:

deployable.errors[:xml].should ==  [I18n.t('catalog_entries.flash.warning.not_valid_duplicate_assembly_names')]


>+  end
> end
>-- 
>1.7.6.4
>



More information about the aeolus-devel mailing list