Fix a case when v1/action/query returns only InternalServerError

.rubocop.yml

@@ -26,6 +26,7 @@ Metrics/AbcSize:
   Exclude:
     - lib/iron_bank/associations.rb
     - lib/iron_bank/client.rb
+    - lib/iron_bank/describe/excluded_fields/deduce_from_query.rb
 
 Metrics/BlockLength:
   Exclude:
@@ -44,9 +45,13 @@ Metrics/MethodLength:
     - lib/iron_bank/resources/product_rate_plan_charge_tier.rb
     - lib/iron_bank/error.rb
     - lib/iron_bank/client.rb
+    - lib/iron_bank/describe/excluded_fields/deduce_from_query.rb
 
 Style/ClassAndModuleChildren:
   EnforcedStyle: nested
+  Exclude:
+    - lib/iron_bank/describe/excluded_fields/deduce_from_query.rb
+    - lib/iron_bank/describe/excluded_fields/extract_from_message.rb
 
 Style/MixinUsage:
   Exclude:

Rakefile

@@ -31,6 +31,9 @@ task :excluded_fields, [:filename] do |_t, args|
   setup_iron_bank
 
   destination = args[:filename] || IronBank.configuration.excluded_fields_file
+  # When Zuora return InternalServerError we can not extract fields from the a message.
+  # For this case we are doing binary search through the query fields and it could be
+  # expensive due to repeatedly querying.
   fields      = IronBank::Schema.excluded_fields.sort.to_h
 
   File.write(destination, Psych.dump(fields))

lib/iron_bank.rb

@@ -83,6 +83,8 @@ module Operations; end
 require "iron_bank/client"
 require "iron_bank/describe/field"
 require "iron_bank/describe/excluded_fields"
+require "iron_bank/describe/excluded_fields/deduce_from_query"
+require "iron_bank/describe/excluded_fields/extract_from_message"
 require "iron_bank/describe/object"
 require "iron_bank/describe/related"
 require "iron_bank/describe/tenant"

lib/iron_bank/describe/excluded_fields.rb

@@ -9,28 +9,7 @@ module Describe
     class ExcludedFields
       extend Forwardable
 
-      FAULT_FIELD_MESSAGES = Regexp.union(
-        # Generic fault field
-        /invalid field for query: \w+\.(\w+)/,
-        # Invoice Bill Run is not selectable in ZOQL
-        /Cannot use the (BillRunId) field in the select clause/,
-        # Invoice Body, implemented as a separate call
-        /Can only query one invoice (body) at a time/,
-        # Catalog tier should only query the price field
-        /use Price or (DiscountAmount) or (DiscountPercentage)/,
-        # Catalog plan currencies, implemented as a separate call
-        /When querying for (active currencies)/,
-        # Catalog charge rollover balance
-        /You can only query (RolloverBalance) in particular/,
-        # (Subscription) charge should only query the price field
-        %r{
-          (OveragePrice),
-          \ Price,
-          \ (IncludedUnits),
-          \ (DiscountAmount)
-          \ or\ (DiscountPercentage)
-        }x
-      ).freeze
+      INVALID_OBJECT_ID = "InvalidObjectId"
 
       private_class_method :new
 
@@ -46,8 +25,6 @@ def call
 
       private
 
-      INVALID_OBJECT_ID = "InvalidObjectId"
-
       attr_reader :object_name, :last_failed_fields
 
       def_delegators "IronBank.logger", :info
@@ -87,25 +64,13 @@ def valid_query?
         info "Successful query for #{object_name}"
 
         true
-      rescue IronBank::InternalServerError, IronBank::BadRequestError => e
-        @last_failed_fields = extract_fields_from_exception(e)
-
-        false
-      end
-
-      def extract_fields_from_exception(exception)
-        message = exception.message
-
-        raise "Could not parse error message: #{message}" unless FAULT_FIELD_MESSAGES.match(message)
+      rescue IronBank::BadRequestError, InternalServerError => e
+        @last_failed_fields = ExtractFromMessage.call(e.message) ||
+                              DeduceFromQuery.call(object)
 
-        failed_fields = Regexp.last_match.
-                        captures.
-                        compact.
-                        map { |capture| capture.delete(" ") }
+        info "Invalid fields '#{@last_failed_fields}' for #{object_name} query"
 
-        info "Invalid fields '#{failed_fields}' for #{object_name} query"
-
-        failed_fields
+        false
       end
     end
   end

lib/iron_bank/describe/excluded_fields/deduce_from_query.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+module IronBank
+  module Describe
+    # Makes a binary search by query fields to get failed fields
+    class ExcludedFields::DeduceFromQuery
+      INVALID_OBJECT_ID = "InvalidObjectId"
+
+      private_class_method :new
+
+      def self.call(object)
+        new(object).call
+      end
+
+      def call
+        exctract_from_dividing
+      end
+
+      private
+
+      attr_reader :object
+
+      def initialize(object)
+        @object = object
+      end
+
+      def exctract_from_dividing
+        @working_fields = []
+        @failed_fields = []
+        query_fields = object.query_fields.clone
+
+        divide_and_execute(query_fields)
+        # Set initial state for object.query_fields
+        object.query_fields.concat query_fields
+
+        @failed_fields
+      end
+
+      def divide_and_execute(query_fields)
+        # Clear state before queries
+        object.query_fields.clear
+        # We repeat dividing until only one field has left
+        @failed_fields.push(query_fields.pop) if query_fields.one?
+        return if query_fields.empty?
+
+        mid = query_fields.size / 2
+        left = query_fields[0..mid - 1]
+        right = query_fields[mid..]
+
+        if execute_query(left)
+          @working_fields.concat(left)
+        else
+          divide_and_execute(left)
+        end
+
+        if execute_query(right)
+          @working_fields.concat(right)
+        else
+          divide_and_execute(right)
+        end
+      end
+
+      def execute_query(fields)
+        object.query_fields.concat(@working_fields + fields)
+
+        object.where({ id: INVALID_OBJECT_ID })
+
+        true
+      rescue IronBank::InternalServerError
+        false
+      end
+    end
+  end
+end

lib/iron_bank/describe/excluded_fields/extract_from_message.rb

@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module IronBank
+  module Describe
+    # Extracts failed fields from an exception message.
+    # Returns from a call if an exception message does not contain failed field
+    class ExcludedFields::ExtractFromMessage
+      FAULT_FIELD_MESSAGES = Regexp.union(
+        # Generic fault field
+        /invalid field for query: \w+\.(\w+)/,
+        # Invoice Bill Run is not selectable in ZOQL
+        /Cannot use the (BillRunId) field in the select clause/,
+        # Invoice Body, implemented as a separate call
+        /Can only query one invoice (body) at a time/,
+        # Catalog tier should only query the price field
+        /use Price or (DiscountAmount) or (DiscountPercentage)/,
+        # Catalog plan currencies, implemented as a separate call
+        /When querying for (active currencies)/,
+        # Catalog charge rollover balance
+        /You can only query (RolloverBalance) in particular/,
+        # (Subscription) charge should only query the price field
+        /
+          (OveragePrice),
+          \ Price,
+          \ (IncludedUnits),
+          \ (DiscountAmount)
+          \ or\ (DiscountPercentage)
+        /x
+      ).freeze
+
+      private_class_method :new
+
+      def self.call(message)
+        new(message).call
+      end
+
+      def call
+        return unless FAULT_FIELD_MESSAGES.match(message)
+
+        Regexp.last_match.captures.compact.map { |capture| capture.delete(" ") }
+      end
+
+      private
+
+      attr_reader :message
+
+      def initialize(message)
+        @message = message
+      end
+    end
+  end
+end

lib/iron_bank/schema.rb

@@ -39,6 +39,9 @@ def self.reset
     end
 
     def self.excluded_fields
+      # When Zuora return InternalServerError we can not extract fields from the a message.
+      # For this case we are doing binary search through the query fields and it could be
+      # expensive due to repeatedly querying.
       @excluded_fields ||= IronBank::Resources.constants.each.with_object({}) do |resource, fields|
         fields[resource.to_s] =
           IronBank::Describe::ExcludedFields.call(object_name: resource)

spec/iron_bank/describe/excluded_fields/deduce_from_query_spec.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+RSpec.describe IronBank::Describe::ExcludedFields::DeduceFromQuery do
+  let(:working_fields) { %w[ValidField1 ValidField2] }
+  let(:failed_fields) { %w[InvalidField1 InvalidField2] }
+  let(:query_fields) { failed_fields + working_fields }
+  let(:object)     { IronBank::Resources.const_get(object_name) }
+  let(:invalid_id) { described_class::INVALID_OBJECT_ID }
+
+  subject(:call) { described_class.call(object) }
+
+  before do
+    allow(object).to receive(:query_fields).and_return(query_fields)
+
+    allow(object).to receive(:where).with({ id: invalid_id }) do
+      if query_fields.include?("InvalidField1") || query_fields.include?("InvalidField2")
+        raise IronBank::InternalServerError
+      end
+
+      true
+    end
+  end
+
+  describe "Deduce failed fields from query" do
+    let(:object_name) { "Product" }
+
+    it "extracts failed fields by binary search" do
+      expect(call).to contain_exactly(*failed_fields)
+    end
+  end
+end

spec/iron_bank/describe/excluded_fields/extract_from_message_spec.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+RSpec.describe IronBank::Describe::ExcludedFields::ExtractFromMessage do
+  subject(:call) { described_class.call(message) }
+
+  describe "when a message could be processed" do
+    context "and includes generic fault field" do
+      let(:message) { "invalid field for query: product.invalidfield" }
+
+      it "returns extracted failed fields" do
+        expect(call).to include("invalidfield")
+      end
+    end
+
+    context "and includes Invoice Bill Run" do
+      let(:message) { "Cannot use the BillRunId field in the select clause" }
+
+      it "returns extracted failed fields" do
+        expect(call).to include("BillRunId")
+      end
+    end
+
+    context "and includes Invoice Body" do
+      let(:message) { "Can only query one invoice body at a time" }
+
+      it "returns extracted failed fields" do
+        expect(call).to include("body")
+      end
+    end
+
+    context "and catalog tier should only query the price field" do
+      let(:message) { "use Price or DiscountAmount or DiscountPercentage" }
+
+      it "returns extracted failed fields" do
+        expect(call).to include("DiscountAmount", "DiscountPercentage")
+      end
+    end
+
+    context "and there are catalog plan currencies" do
+      let(:message) { "When querying for active currencies" }
+
+      it "returns extracted failed fields" do
+        expect(call).to include("activecurrencies")
+      end
+    end
+
+    context "and included rollover balance" do
+      let(:message) { "You can only query RolloverBalance in particular" }
+
+      it "returns extracted failed fields" do
+        expect(call).to include("RolloverBalance")
+      end
+    end
+
+    context "and subscription should only query the price field" do
+      let(:message) { "OveragePrice, Price, IncludedUnits, DiscountAmount or DiscountPercentage" }
+
+      it "returns extracted failed fields" do
+        expect(call).to_not include("Price")
+      end
+    end
+  end
+
+  describe "when a message could not be processed" do
+    let(:message) { "some invalid message" }
+
+    it "returns from a call" do
+      expect(call).to be_nil
+    end
+  end
+end