From a5cb0a1d84fe18acac693dabafcef01ce59b62b9 Mon Sep 17 00:00:00 2001 From: brennen-stripe <86444598+brennen-stripe@users.noreply.github.com> Date: Tue, 7 Feb 2023 10:52:43 -0800 Subject: [PATCH] Adding caching to data mapper (#864) * added caching service to mapper * added integer to list * merged in main and cleaned up PR * more cleanup * reverted class change * silenced CI failures for mapper * updated git ignore * Sanitize duration_in_months always (#985) * Support amending orders with coupons (#956) * Productionize coupon order trigger (#934) * Ensure updateCoupons trigger is deterministic (#988) * Update 'add_invoice_items.period.end.type' on proration invoices (#987) * Backend proration with amendment (#847) * Add class access for coupon handler (#990) * Manual Translation in Sync Preferences (#992) * passing in mapper to amendment funcs * removed extra backoff * return empty list for non supported cache types --------- Co-authored-by: Nada Ismail <112021025+nadaismail-stripe@users.noreply.github.com> Co-authored-by: jmather-c <117302272+jmather-c@users.noreply.github.com> Co-authored-by: mbianco-stripe <45374579+mbianco-stripe@users.noreply.github.com> --- .gitignore | 1 + lib/stripe-force/cache_service.rb | 22 +++++++++++++++++-- lib/stripe-force/translate/mapper.rb | 20 ++++------------- lib/stripe-force/translate/order.rb | 9 +++++--- .../translate/order/amendments.rb | 14 +++++++----- lib/stripe-force/translate/translate.rb | 11 ++++------ .../translate/utilities/salesforce_util.rb | 2 +- test/unit/test_mapper.rb | 3 ++- 8 files changed, 46 insertions(+), 36 deletions(-) diff --git a/.gitignore b/.gitignore index 68c4692746..628d4c7337 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,4 @@ test/reports/ .DS_Store /vendor/bundle node_modules +.sfdx/* diff --git a/lib/stripe-force/cache_service.rb b/lib/stripe-force/cache_service.rb index 2054073ed0..6e5dee4182 100644 --- a/lib/stripe-force/cache_service.rb +++ b/lib/stripe-force/cache_service.rb @@ -21,6 +21,15 @@ def initialize(user) @previously_invalidated = [] end + # TODO: Remove this after batch service includes mapper relationships (https://github.com/stripe/stripe-salesforce/issues/959) + def silent(&block) + @silence_failures = true + result = yield + @silence_failures = false + + result + end + sig { params(sf_object: Restforce::SObject).void } def cache_for_object(sf_object) record_list = [] @@ -204,7 +213,8 @@ def get_record_from_cache(sf_record_type, sf_record_id) when SF_PRICEBOOK_ENTRY {product2_ids: [sf_object.Product2Id]} else - raise ArgumentError.new("unexpected salesforce object type when pulling cache #{sf_object.sobject_type}") + log.info("Attempted to retrieve related objects for an SF Object type not currently supported by the batching service", sf_object_type: sf_object.sobject_type) + return [] end url = generate_batch_service_url @@ -222,12 +232,20 @@ def get_record_from_cache(sf_record_type, sf_record_id) false end + # We want to throw hard failures in the following case: + # 1. We are in CI or local tests + # 2. The records have not been previously invalidated + # 3. We are not silencing failures + private def throw_hard_failure?(missing_record_ids) + (ENV['CI'] || Rails.env.test?) && !previously_invalidated?(missing_record_ids) && !@silence_failures + end + private def report_missing_cache(message, metadata, missing_record_ids: []) Integrations::ErrorContext.report_edge_case(message, metadata: metadata) # Error out in tests, provide the meatdata for alerting us to what exactly is missing. - if (ENV['CI'] || Rails.env.test?) && !previously_invalidated?(missing_record_ids) + if throw_hard_failure?(missing_record_ids) raise Integrations::Errors::TranslatorError.new(message, metadata: metadata) end end diff --git a/lib/stripe-force/translate/mapper.rb b/lib/stripe-force/translate/mapper.rb index 97cb73e67b..376c08694b 100644 --- a/lib/stripe-force/translate/mapper.rb +++ b/lib/stripe-force/translate/mapper.rb @@ -14,19 +14,6 @@ class Mapper attr_reader :user - sig do - params( - user: StripeForce::User, - record_to_map: Stripe::APIResource, - source_record: Restforce::SObject, - compound_key: T::Boolean - ).void - end - def self.apply_mapping(user, record_to_map, source_record, compound_key: false) - StripeForce::Mapper.new(user) - .apply_mapping(record_to_map, source_record, compound_key: compound_key) - end - sig { params(stripe_record: T.any(Stripe::APIResource, Class), sf_record: T.nilable(Restforce::SObject)).returns(String) } def self.mapping_key_for_record(stripe_record, sf_record) stripe_record_class = if stripe_record.is_a?(Class) && stripe_record < Stripe::APIResource @@ -49,9 +36,10 @@ def self.mapping_key_for_record(stripe_record, sf_record) end end - sig { params(u: StripeForce::User).void } - def initialize(u) + sig { params(u: StripeForce::User, cache_service: CacheService).void } + def initialize(u, cache_service) @user = u + @cache_service = cache_service end sig { params(record: T.any(Stripe::APIResource, Restforce::SObject), key_path: String).returns(T.nilable(T.any(String, Integer, Float, T::Boolean))) } @@ -105,7 +93,7 @@ def extract_salesforce_object_field(sf_object, key_path) target_class = StripeForce::Utilities::SalesforceUtil.salesforce_type_from_id(@user, target_object) if target_class - target_object = backoff { @user.sf_client.find(target_class, target_object) } + target_object = @cache_service.silent { @cache_service.get_record_from_cache(target_class, target_object) } end end end diff --git a/lib/stripe-force/translate/order.rb b/lib/stripe-force/translate/order.rb index 815ff81bda..7e259e5544 100644 --- a/lib/stripe-force/translate/order.rb +++ b/lib/stripe-force/translate/order.rb @@ -235,10 +235,11 @@ def generate_phases_for_initial_order(sf_order:, invoice_items:, subscription_it invoice_items_for_prorations = OrderAmendment.generate_proration_items_from_phase_items( user: @user, + mapper: mapper, sf_order_amendment: sf_order, phase_items: subscription_items, subscription_term: subscription_term_from_sales_force, - billing_frequency: billing_frequency + billing_frequency: billing_frequency, ) # add special metadata to line items as well @@ -450,10 +451,11 @@ def update_subscription_phases_from_order_amendments(contract_structure) # for the 20 units removed. However, for now we would only issue credit if they terminate the line by removing all 50 units. negative_invoice_items = OrderAmendment.generate_proration_credits_from_terminated_phase_items( user: @user, + mapper: mapper, sf_order_amendment: sf_order_amendment, terminated_phase_items: terminated_phase_items, subscription_term: subscription_term_from_sales_force, - billing_frequency: billing_frequency + billing_frequency: billing_frequency, ) end @@ -485,10 +487,11 @@ def update_subscription_phases_from_order_amendments(contract_structure) if !is_order_terminated && is_prorated invoice_items_for_prorations = OrderAmendment.generate_proration_items_from_phase_items( user: @user, + mapper: mapper, sf_order_amendment: sf_order_amendment, phase_items: aggregate_phase_items, subscription_term: subscription_term_from_sales_force, - billing_frequency: billing_frequency + billing_frequency: billing_frequency, ) end diff --git a/lib/stripe-force/translate/order/amendments.rb b/lib/stripe-force/translate/order/amendments.rb index c4b9d829c1..ccdf9b00e3 100644 --- a/lib/stripe-force/translate/order/amendments.rb +++ b/lib/stripe-force/translate/order/amendments.rb @@ -197,14 +197,15 @@ def self.create_credit_price_data_from_terminated_phase_item(user:, phase_item:, sig do params( user: StripeForce::User, + mapper: StripeForce::Mapper, sf_order_amendment: Restforce::SObject, terminated_phase_items: T::Array[ContractItemStructure], subscription_term: Integer, - billing_frequency: Integer + billing_frequency: Integer, ).returns(T::Array[T::Hash[Symbol, T.untyped]]) end # creating one-time invoice items for terminated lines for the unused prorated amount (which has already been billed) - def self.generate_proration_credits_from_terminated_phase_items(user:, sf_order_amendment:, terminated_phase_items:, subscription_term:, billing_frequency:) + def self.generate_proration_credits_from_terminated_phase_items(user:, mapper:, sf_order_amendment:, terminated_phase_items:, subscription_term:, billing_frequency:) negative_invoice_items_for_prorations = [] terminated_phase_items.each do |phase_item| @@ -259,7 +260,7 @@ def self.generate_proration_credits_from_terminated_phase_items(user:, sf_order_ ), }) - StripeForce::Mapper.apply_mapping(user, credit_stripe_item, phase_item.order_line) + mapper.apply_mapping(credit_stripe_item, phase_item.order_line) negative_invoice_items_for_prorations << credit_stripe_item.to_hash.merge({ quantity: phase_item.reduced_by, @@ -281,13 +282,14 @@ def self.generate_proration_credits_from_terminated_phase_items(user:, sf_order_ sig do params( user: StripeForce::User, + mapper: StripeForce::Mapper, sf_order_amendment: Restforce::SObject, phase_items: T::Array[ContractItemStructure], subscription_term: Integer, - billing_frequency: Integer + billing_frequency: Integer, ).returns(T::Array[T::Hash[Symbol, T.untyped]]) end - def self.generate_proration_items_from_phase_items(user:, sf_order_amendment:, phase_items:, subscription_term:, billing_frequency:) + def self.generate_proration_items_from_phase_items(user:, mapper:, sf_order_amendment:, phase_items:, subscription_term:, billing_frequency:) invoice_items_for_prorations = [] phase_items.each do |phase_item| @@ -345,7 +347,7 @@ def self.generate_proration_items_from_phase_items(user:, sf_order_amendment:, p ), }) - StripeForce::Mapper.apply_mapping(user, proration_stripe_item, phase_item.order_line) + mapper.apply_mapping(proration_stripe_item, phase_item.order_line) invoice_items_for_prorations << proration_stripe_item.to_hash.merge({ quantity: phase_item.quantity, diff --git a/lib/stripe-force/translate/translate.rb b/lib/stripe-force/translate/translate.rb index fb5b323ed6..81d88d2af9 100644 --- a/lib/stripe-force/translate/translate.rb +++ b/lib/stripe-force/translate/translate.rb @@ -64,19 +64,16 @@ def cache_service def translate(sf_object) set_error_context(user: @user, integration_record: sf_object) + # Cache Related Objects + cache_service.cache_for_object(sf_object) + catch_errors_with_salesforce_context(primary: sf_object) do case sf_object.sobject_type when SF_ORDER - # Cache Related Objects - cache_service.cache_for_object(sf_object) translate_order(sf_object) when SF_PRODUCT - # Cache Related Objects - cache_service.cache_for_object(sf_object) translate_product(sf_object) when SF_PRICEBOOK_ENTRY - # Cache Related Objects - cache_service.cache_for_object(sf_object) translate_pricebook(sf_object) when SF_ACCOUNT translate_account(sf_object) @@ -492,7 +489,7 @@ def apply_mapping(record_to_map, source_record, compound_key: false) sig { returns(StripeForce::Mapper) } def mapper - @mapper ||= StripeForce::Mapper.new(@user) + @mapper ||= StripeForce::Mapper.new(@user, cache_service) end sig { params(stripe_record: Stripe::StripeObject).void } diff --git a/lib/stripe-force/translate/utilities/salesforce_util.rb b/lib/stripe-force/translate/utilities/salesforce_util.rb index 764a54dd2b..8a6251d76f 100644 --- a/lib/stripe-force/translate/utilities/salesforce_util.rb +++ b/lib/stripe-force/translate/utilities/salesforce_util.rb @@ -186,7 +186,7 @@ def self.extract_subscription_term_from_order!(mapper, sf_order) subscription_term_order_path = user.field_mappings.dig(*subscription_term_stripe_path) || user.required_mappings.dig(*subscription_term_stripe_path) - quote_subscription_term = T.cast(mapper.extract_key_path_for_record(sf_order, subscription_term_order_path), T.nilable(T.any(String, Float))) + quote_subscription_term = T.cast(mapper.extract_key_path_for_record(sf_order, subscription_term_order_path), T.nilable(T.any(String, Float, Integer))) if quote_subscription_term.nil? raise Integrations::Errors::MissingRequiredFields.new( diff --git a/test/unit/test_mapper.rb b/test/unit/test_mapper.rb index d82284e38c..e13814e41b 100644 --- a/test/unit/test_mapper.rb +++ b/test/unit/test_mapper.rb @@ -7,7 +7,8 @@ module Critic::Unit class MapperTest < Critic::UnitTest before do @user = make_user(random_user_id: true) - @mapper = StripeForce::Mapper.new(@user) + cache_service = CacheService.new(@user) + @mapper = StripeForce::Mapper.new(@user, cache_service) end it 'properly saves the annotator hashes' do