From 71794cf40a0aff42c19d98e9c0aa2a654ed49d0b Mon Sep 17 00:00:00 2001 From: Lucy Fu Date: Thu, 24 Aug 2017 17:03:00 -0400 Subject: [PATCH 1/4] Use save! over save to be API call ready. --- app/models/generic_object.rb | 4 ++-- app/models/generic_object_definition.rb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/generic_object.rb b/app/models/generic_object.rb index 94179312ce8..66b70376b6d 100644 --- a/app/models/generic_object.rb +++ b/app/models/generic_object.rb @@ -48,7 +48,7 @@ def add_to_property_association(name, objs) klass = property_associations[name].constantize selected = objs.select { |obj| obj.kind_of?(klass) } properties[name] = (properties[name] + selected.pluck(:id)).uniq if selected - save + save! end def delete_from_property_association(name, objs) @@ -59,7 +59,7 @@ def delete_from_property_association(name, objs) klass = property_associations[name].constantize selected = objs.select { |obj| obj.kind_of?(klass) } properties[name] = properties[name] - selected.pluck(:id) - save + save! end def inspect diff --git a/app/models/generic_object_definition.rb b/app/models/generic_object_definition.rb index 7ba6df1971e..bee7daa7a70 100644 --- a/app/models/generic_object_definition.rb +++ b/app/models/generic_object_definition.rb @@ -90,7 +90,7 @@ def properties=(props) def add_property_attribute(name, type) properties[:attributes][name.to_s] = type.to_sym - save + save! end def delete_property_attribute(name) @@ -107,7 +107,7 @@ def add_property_association(name, type) raise "invalid model for association: [#{type}]" unless type.in?(ALLOWED_ASSOCIATION_TYPES) properties[:associations][name.to_s] = type - save + save! end def delete_property_association(name) @@ -121,12 +121,12 @@ def delete_property_association(name) def add_property_method(name) properties[:methods] << name.to_s unless properties[:methods].include?(name.to_s) - save + save! end def delete_property_method(name) properties[:methods].delete(name.to_s) - save + save! end private From ded7d76bbb75fe178e13801d8d259f502229f39a Mon Sep 17 00:00:00 2001 From: Lucy Fu Date: Fri, 25 Aug 2017 10:21:22 -0400 Subject: [PATCH 2/4] Keywords in properties hash are all in string format. --- app/models/generic_object.rb | 2 +- app/models/generic_object_definition.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/generic_object.rb b/app/models/generic_object.rb index 66b70376b6d..17928462f0f 100644 --- a/app/models/generic_object.rb +++ b/app/models/generic_object.rb @@ -114,7 +114,7 @@ def respond_to_missing?(method_name, _include_private = false) end def _property_getter(name) - generic_object_definition.property_getter(name, properties[name]) + generic_object_definition.property_getter(name.to_s, properties[name.to_s]) end def _property_setter(name, value) diff --git a/app/models/generic_object_definition.rb b/app/models/generic_object_definition.rb index bee7daa7a70..8a586608e76 100644 --- a/app/models/generic_object_definition.rb +++ b/app/models/generic_object_definition.rb @@ -80,7 +80,7 @@ def property_getter(attr, val) end def type_cast(attr, value) - TYPE_MAP.fetch(property_attributes[attr]).cast(value) + TYPE_MAP.fetch(property_attributes[attr.to_s]).cast(value) end def properties=(props) @@ -132,7 +132,7 @@ def delete_property_method(name) private def get_objects_of_association(attr, values) - property_associations[attr].constantize.where(:id => values).to_a + property_associations[attr.to_s].constantize.where(:id => values).to_a end def normalize_property_attributes From f944cd4d9f798471e7e4a8ebd51e6a1c6c160395 Mon Sep 17 00:00:00 2001 From: Lucy Fu Date: Fri, 25 Aug 2017 10:52:25 -0400 Subject: [PATCH 3/4] Enhance method delete_property to work on defined property attributes and associations. --- app/models/generic_object.rb | 11 +++++++++-- app/models/generic_object_definition.rb | 8 ++++---- spec/models/generic_object_spec.rb | 23 +++++++++++++++++++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/app/models/generic_object.rb b/app/models/generic_object.rb index 17928462f0f..0d48befe73c 100644 --- a/app/models/generic_object.rb +++ b/app/models/generic_object.rb @@ -36,8 +36,15 @@ def property_attributes end def delete_property(name) - properties.delete(name.to_s) - save! + if !property_attribute_defined?(name) && !property_association_defined?(name) + valid_property_names = generic_object_definition.property_attributes.keys + generic_object_definition.property_associations.keys + raise "Invalid property [#{name}]: must be one of #{valid_property_names.join(", ")}" + end + + _property_getter(name).tap do + properties.delete(name.to_s) + save! + end end def add_to_property_association(name, objs) diff --git a/app/models/generic_object_definition.rb b/app/models/generic_object_definition.rb index 8a586608e76..b12ff64a621 100644 --- a/app/models/generic_object_definition.rb +++ b/app/models/generic_object_definition.rb @@ -95,10 +95,10 @@ def add_property_attribute(name, type) def delete_property_attribute(name) transaction do + generic_objects.find_each { |o| o.delete_property(name) } + properties[:attributes].delete(name.to_s) save! - - generic_objects.find_each { |o| o.delete_property(name) } end end @@ -112,10 +112,10 @@ def add_property_association(name, type) def delete_property_association(name) transaction do + generic_objects.find_each { |o| o.delete_property(name) } + properties[:associations].delete(name.to_s) save! - - generic_objects.find_each { |o| o.delete_property(name) } end end diff --git a/spec/models/generic_object_spec.rb b/spec/models/generic_object_spec.rb index c2eb61b8c2b..2998a487bfd 100644 --- a/spec/models/generic_object_spec.rb +++ b/spec/models/generic_object_spec.rb @@ -221,4 +221,27 @@ expect(go.my_host).to eq("some_return_value") end end + + describe '#delete_property' do + it 'an attriute' do + max = go.max_number + expect(go.delete_property("max_number")).to eq(max) + expect(go.max_number).to be_nil + end + + it 'an association' do + vm2 = FactoryGirl.create(:vm_vmware) + go.vms = [vm1, vm2] + expect(go.delete_property("vms")).to match_array([vm1, vm2]) + expect(go.vms).to be_empty + end + + it 'a method' do + expect { go.delete_property("my_host") }.to raise_error(RuntimeError) + end + + it 'an invalid property name' do + expect { go.delete_property("some_attribute_not_defined") }.to raise_error(RuntimeError) + end + end end From 20096636a9ae332f6b7c7573155a8862bd722917 Mon Sep 17 00:00:00 2001 From: Lucy Fu Date: Fri, 25 Aug 2017 10:58:06 -0400 Subject: [PATCH 4/4] Add GenericObject#property_associations to return the objects associations. --- app/models/generic_object.rb | 23 +++++++--- spec/models/generic_object_spec.rb | 68 ++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/app/models/generic_object.rb b/app/models/generic_object.rb index 0d48befe73c..7a39d998a9f 100644 --- a/app/models/generic_object.rb +++ b/app/models/generic_object.rb @@ -8,7 +8,7 @@ class GenericObject < ApplicationRecord delegate :property_attribute_defined?, :property_defined?, :type_cast, - :property_associations, :property_association_defined?, + :property_association_defined?, :property_methods, :property_method_defined?, :to => :generic_object_definition, :allow_nil => true @@ -35,6 +35,12 @@ def property_attributes end end + def property_associations + properties.select { |k, _| property_association_defined?(k) }.each_with_object({}) do |(k, _), h| + h[k] = _property_getter(k) + end + end + def delete_property(name) if !property_attribute_defined?(name) && !property_association_defined?(name) valid_property_names = generic_object_definition.property_attributes.keys + generic_object_definition.property_associations.keys @@ -52,7 +58,7 @@ def add_to_property_association(name, objs) name = name.to_s properties[name] ||= [] - klass = property_associations[name].constantize + klass = generic_object_definition.property_associations[name].constantize selected = objs.select { |obj| obj.kind_of?(klass) } properties[name] = (properties[name] + selected.pluck(:id)).uniq if selected save! @@ -63,10 +69,14 @@ def delete_from_property_association(name, objs) name = name.to_s properties[name] ||= [] - klass = property_associations[name].constantize + klass = generic_object_definition.property_associations[name].constantize selected = objs.select { |obj| obj.kind_of?(klass) } - properties[name] = properties[name] - selected.pluck(:id) + common_ids = properties[name] & selected.pluck(:id) + properties[name] = properties[name] - common_ids + return unless properties_changed? + save! + klass.where(:id => common_ids).to_a end def inspect @@ -75,7 +85,7 @@ def inspect end attributes_as_string += ["attributes: #{property_attributes}"] - attributes_as_string += ["associations: #{property_associations.keys}"] + attributes_as_string += ["associations: #{generic_object_definition.property_associations.keys}"] attributes_as_string += ["methods: #{property_methods}"] prefix = Kernel.instance_method(:inspect).bind(self).call.split(' ', 2).first @@ -126,13 +136,14 @@ def _property_getter(name) def _property_setter(name, value) name = name.to_s + val = if property_attribute_defined?(name) # property attribute is of single value, for now type_cast(name, value) elsif property_association_defined?(name) # property association is of multiple values - value.select { |v| v.kind_of?(property_associations[name].constantize) }.uniq.map(&:id) + value.select { |v| v.kind_of?(generic_object_definition.property_associations[name].constantize) }.uniq.map(&:id) end self.properties = properties.merge(name => val) diff --git a/spec/models/generic_object_spec.rb b/spec/models/generic_object_spec.rb index 2998a487bfd..ff28ac25f3e 100644 --- a/spec/models/generic_object_spec.rb +++ b/spec/models/generic_object_spec.rb @@ -159,6 +159,15 @@ go_assoc.save! expect(go_assoc.vms.count).to eq(1) end + + it 'method returns all associations' do + host = FactoryGirl.create(:host) + go_assoc.hosts = [host] + + result = go_assoc.property_associations + expect(result["vms"]).to match_array([vm1, vm2]) + expect(result["hosts"]).to match_array([host]) + end end describe 'property methods' do @@ -244,4 +253,63 @@ expect { go.delete_property("some_attribute_not_defined") }.to raise_error(RuntimeError) end end + + describe '#add_to_property_association' do + let(:new_vm) { FactoryGirl.create(:vm_vmware) } + subject { go.add_to_property_association("vms", vm1) } + + it 'adds objects into association' do + subject + expect(go.vms.count).to eq(1) + + go.add_to_property_association(:vms, [new_vm]) + expect(go.vms.count).to eq(2) + end + + it 'does not add duplicate object' do + subject + expect(go.vms.count).to eq(1) + + subject + expect(go.vms.count).to eq(1) + end + + it 'does not add object from differnt class' do + go.add_to_property_association("vms", FactoryGirl.create(:host)) + expect(go.vms.count).to eq(0) + end + + it 'does not accept object id' do + go.add_to_property_association(:vms, new_vm.id) + expect(go.vms.count).to eq(0) + end + end + + describe '#delete_from_property_association' do + before { go.add_to_property_association("vms", [vm1]) } + let(:new_vm) { FactoryGirl.create(:vm_vmware) } + + it 'deletes objects from association' do + result = go.delete_from_property_association(:vms, [vm1]) + expect(go.vms.count).to eq(0) + expect(result).to match_array([vm1]) + end + + it 'does not delete object that is not in association' do + expect(go.vms.count).to eq(1) + result = go.delete_from_property_association(:vms, [new_vm]) + expect(go.vms).to match_array([vm1]) + expect(result).to be_nil + end + + it 'does not delete object from differnt class' do + result = go.delete_from_property_association(:vms, [FactoryGirl.create(:host)]) + expect(go.vms).to match_array([vm1]) + expect(result).to be_nil + end + + it 'does not accept object id' do + expect(go.delete_from_property_association(:vms, vm1.id)).to be_nil + end + end end