Fixing issue #75

ads_common/ChangeLog

@@ -1,3 +1,6 @@
+0.12.5:
+  - Fixed issue #75.
+
 0.12.4:
   - Started enforcing minumum Ruby runtime version via gemspec.
 

ads_common/lib/ads_common/parameters_validator.rb

@@ -88,6 +88,19 @@ def validate_arguments(args_hash, fields_list, type_ns = nil)
     def validate_choice_argument(item, parent, key, item_type)
       result = false
       if item_type.kind_of?(Hash) && item_type.include?(:choices)
+        # If we have an array of choices, we need to go over them individually.
+        # We drop original array and replace it with the generated one that's
+        # nested one level more.
+        if item.kind_of?(Array)
+          parent[key] = []
+          item.each do |sub_item|
+            unless validate_choice_argument(sub_item, parent, key, item_type)
+              validate_arg(sub_item, parent, key, item_type)
+            end
+          end
+          return true
+        end
+        # New root needed for extra nesting we have (choice field).
         new_root = {}
         choice_items = arrayize(item)
         choice_items.each do |choice_item|
@@ -98,22 +111,44 @@ def validate_choice_argument(item, parent, key, item_type)
             raise AdsCommon::Errors::TypeMismatchError.new(
               'choice subtype', choice_type, choice_item.to_s())
           end
-          new_root[choice_type] ||= []
-          new_root[choice_type] << choice_item
+          choice_item[:xsi_type] = choice_type
+          # Note we use original name that produces a string like
+          # "BasicUserList". That's needed as the name is generated out of
+          # standard naming ("basicUserList") which would otherwise be produced.
+          choice_key = choice_item_type[:original_name]
+          new_root[choice_key] = choice_item
           type_signature = get_full_type_signature(choice_type)
-          validate_arg(choice_item, new_root, choice_type, type_signature)
+          validate_arg(choice_item, new_root, choice_key, type_signature)
+        end
+        if parent[key].kind_of?(Array)
+          parent[key] << new_root
+        else
+          parent[key] = new_root
         end
-        parent[key] = new_root
         result = true
       end
       return result
     end
 
     def find_choice_by_xsi_type(xsi_type, item_def)
-      index = item_def.index {|item| xsi_type.eql?(item[:original_name])}
+      return nil if xsi_type.nil?
+      # Method may allow this class name or any of it's ancestors.
+      possible_names = collect_parent_types(xsi_type)
+      index = item_def.index do |item|
+        possible_names.include?(item[:original_name])
+      end
       return index.nil? ? nil : item_def[index]
     end
 
+    def collect_parent_types(xsi_type)
+      result = [xsi_type]
+      type_signature = @registry.get_type_signature(xsi_type)
+      if !type_signature.nil? && !type_signature[:base].nil?
+        result += collect_parent_types(type_signature[:base])
+      end
+      return result
+    end
+
     # Checks if no extra fields provided outside of known ones.
     def check_extra_fields(args_hash, known_fields)
       extra_fields = args_hash.keys - known_fields - IGNORED_HASH_KEYS

ads_common/lib/ads_common/version.rb

@@ -19,6 +19,6 @@
 
 module AdsCommon
   module ApiConfig
-    CLIENT_LIB_VERSION = '0.12.4'
+    CLIENT_LIB_VERSION = '0.12.5'
   end
 end

adwords_api/google-adwords-api.gemspec

@@ -37,7 +37,7 @@ Gem::Specification.new do |s|
   s.files = Dir.glob('{lib,test}/**/*') + Dir.glob('examples/v*/**/*') +
       %w(COPYING README.md ChangeLog adwords_api.yml)
   s.test_files = ['test/suite_unittests.rb']
-  s.add_runtime_dependency('google-ads-common', '~> 0.12.3')
+  s.add_runtime_dependency('google-ads-common', '~> 0.12.5')
   s.add_development_dependency('rr', '~> 1.1.2')
   s.add_development_dependency('webmock', '~> 1.21.0')
 end

adwords_api/test/adwords_api/test_choices.rb

@@ -38,12 +38,14 @@ def test_choices_one()
         :key => {:name => 'foo'}, :op => 'EQUALS', :value => 42
       }
     ])
-    expected = get_expected_for_items({
-      'NumberRuleItem' => [
-          {:key => {:name => 'foo', :'order!' => [:name]},
-           :op => 'EQUALS', :value => 42,
-           :'order!' => [:key, :op, :value]}]
-    })
+    expected = get_expected_for_items([
+      {
+        'NumberRuleItem' => {
+          :key => {:name => 'foo', :'order!' => [:name]},
+          :op => 'EQUALS', :value => 42, :'order!' => [:key, :op, :value]},
+        :'attributes!'=>{'NumberRuleItem'=>{'xsi:type'=>'NumberRuleItem'}},
+      }
+    ])
     result = @validator.validate_args('mutate', [[op]])[:operations][0]
     assert_equal(expected, result[:operand][:rule])
   end
@@ -55,17 +57,22 @@ def test_choices_multiple_of_same_type()
       {:xsi_type => 'NumberRuleItem',
        :key => {:name => 'bar'}, :op => 'EQUALS', :value => 84}
     ])
-    expected = get_expected_for_items({
-      'NumberRuleItem' => [
-          {:key => {:name => 'foo', :'order!' => [:name]},
-           :op => 'EQUALS',
-           :value => 42,
-           :'order!' => [:key, :op, :value]},
-          {:key => {:name => 'bar', :'order!' => [:name]},
-           :op => 'EQUALS',
-           :value => 84,
-           :'order!' => [:key, :op, :value]}]
-    })
+    expected = get_expected_for_items([
+      {
+        'NumberRuleItem' => {
+          :key => {:name => 'foo', :'order!' => [:name]},
+          :op => 'EQUALS', :value => 42, :'order!' => [:key, :op, :value]},
+        :'attributes!' =>
+          {'NumberRuleItem' => {'xsi:type' => 'NumberRuleItem'}}
+      },
+      {
+        'NumberRuleItem' => {
+          :key => {:name => 'bar', :'order!' => [:name]},
+          :op => 'EQUALS', :value => 84, :'order!' => [:key, :op, :value]},
+        :'attributes!' =>
+          {'NumberRuleItem' => {'xsi:type' => 'NumberRuleItem'}}
+      }
+    ])
     result = @validator.validate_args('mutate', [[op]])[:operations][0]
     assert_equal(expected, result[:operand][:rule])
   end
@@ -77,14 +84,23 @@ def test_choices_different_types()
       {:xsi_type => 'StringRuleItem',
        :key => {:name => 'bar'}, :op => 'EQUALS', :value => 'baz'}
     ])
-    expected = get_expected_for_items({
-      'NumberRuleItem' => [{:key => {:name => 'foo', :'order!' => [:name]},
-                            :op => 'EQUALS', :value => 42,
-                            :'order!' => [:key, :op, :value]}],
-      'StringRuleItem' => [{:key => {:name => 'bar', :'order!' => [:name]},
-                            :op => 'EQUALS', :value => 'baz',
-                            :'order!' => [:key, :op, :value]}],
-    })
+    expected = get_expected_for_items([
+      {
+          'NumberRuleItem' =>
+            {:key => {:name => 'foo', :'order!' => [:name]},
+             :op => 'EQUALS', :value => 42, :'order!' => [:key, :op, :value]},
+          :'attributes!' =>
+            {'NumberRuleItem' => {'xsi:type' => 'NumberRuleItem'}}
+      },
+      {
+          'StringRuleItem' =>
+            {:key => {:name => 'bar', :'order!' => [:name]},
+             :op => 'EQUALS', :value=> 'baz', :'order!' => [:key, :op, :value]},
+          :'attributes!' => {
+            'StringRuleItem' => {'xsi:type' => 'StringRuleItem'}
+          }
+      }
+    ])
     result = @validator.validate_args('mutate', [[op]])[:operations][0]
     assert_equal(expected, result[:operand][:rule])
   end
@@ -108,6 +124,43 @@ def test_choices_missing_xsi_type()
     end
   end
 
+  def test_inherited_choices()
+    result = nil
+    op = {
+      :operator => 'ADD',
+      :operand => {
+        :xsi_type => 'LogicalUserList',
+        :name => 'Sample Logical List',
+        :status => 'OPEN',
+        :rules => [
+          {
+            :operator => 'ANY',
+            :rule_operands => [
+              {:xsi_type => 'LogicalUserList', :id => 123456781},
+              {:xsi_type => 'BasicUserList', :id => 123456782},
+              {:xsi_type => 'LogicalUserList', :id => 123456783},
+              {:xsi_type => 'ExpressionRuleUserList', :id => 123456784}
+            ]
+          }
+        ]
+      }
+    }
+    assert_nothing_raised do
+      result = @validator.validate_args('mutate', [[op]])
+    end
+
+    assert_not_nil(result)
+    assert_kind_of(Array,
+        result[:operations][0][:operand][:rules][0][:rule_operands])
+
+    list = result[:operations][0][:operand][:rules][0][:rule_operands]
+    expected = ['LogicalUserList', 'BasicUserList', 'LogicalUserList',
+        'ExpressionRuleUserList']
+    expected.each_with_index do |name, i|
+      assert_equal(name, list[i][:'attributes!']['UserList']['xsi:type'])
+    end
+  end
+
   private
 
   def get_op_for_items(items)