Skip to content

Commit

Permalink
feat: DAH-2556 check translations cache LastModifiedDate (#2283)
Browse files Browse the repository at this point in the history
* feat: check translations cache LastModifiedDate

* feat: handle outdated translations and outdated listing cases

* fix: invalidate translation cache

* fix: npm issue

* fix: move feature flag check up a level

* fix: flag check

* test: cover cache invalidation checks

* fix: use each

* fix: translations object for test
  • Loading branch information
cade-exygy authored Sep 17, 2024
1 parent d2fe4b7 commit 456c5fc
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 37 deletions.
1 change: 1 addition & 0 deletions app/services/cache_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def process_translations(listing)
ServiceHelper.convert_to_salesforce_field_name(key)
end,
translations,
listing['LastModifiedDate'],
)
end

Expand Down
1 change: 1 addition & 0 deletions app/services/force/event_subscriber_translate_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ def translate_and_cache(event)
event.listing_id,
event.updated_values.keys,
translations,
event.last_modified_date,
)
logger(
"Event Translations: #{cached_response.inspect}",
Expand Down
74 changes: 55 additions & 19 deletions app/services/force/listing_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
module Force
# encapsulate all Salesforce Listing querying functions
class ListingService
CACHE_KEY_PREFIX = '/ListingDetails/'
DATE_FORMAT = '%Y-%m-%dT%H:%M:%SZ'
# get all open listings or specific set of listings by id
# `ids` is a comma-separated list of ids
# `type` designates "ownership" or "rental" listings
Expand All @@ -29,36 +31,70 @@ def self.eligible_listings(filters)
# get one detailed listing result by id
def self.listing(id, opts = {})
@cache = Rails.cache
endpoint = "/ListingDetails/#{CGI.escape(id)}"
endpoint = "#{CACHE_KEY_PREFIX}#{CGI.escape(id)}"
force = opts[:force] || false
Rails.logger.info("Calling self.listing for #{id} with force: #{force}")

results = Request.new(parse_response: true).cached_get(endpoint, nil, force)
listing = process_listing_images(results)

if ::UNLEASH.is_enabled? 'GoogleCloudTranslate'
listing['translations'] = get_listing_translations(listing) || {}
end
listing
end

def self.process_listing_images(results)
results_with_cached_listing_images = add_cloudfront_urls_for_listing_images(results)
listing = add_image_urls(results_with_cached_listing_images).first
add_image_urls(results_with_cached_listing_images).first
end

if Unleash::Client.new.is_enabled? 'GoogleCloudTranslate'
listing_translations = @cache.fetch("/ListingDetails/#{id}/translations") do
Rails.logger.info("Nothing in cache for Listing #{id} translations")
{}
end
def self.get_listing_translations(listing)
listing_id = listing['Id']
listing_translations = fetch_listing_translations_from_cache(listing_id)
if translations_invalid?(listing_translations) || translations_are_outdated?(
listing_translations[:LastModifiedDate], listing['LastModifiedDate']
)
Rails.logger.info("Translations are not valid for #{listing_id}")
return CacheService.new.process_translations(listing)
end

unless translations_valid?(listing_translations)
Rails.logger.info("Translations are not valid for #{id}")
results = Request.new(parse_response: true).cached_get(endpoint, nil, true)
listing_translations = CacheService.new.process_translations(results.first)
end
if listing_is_outdated?(listing_translations[:LastModifiedDate],
listing['LastModifiedDate'])
Rails.logger.info("Listing is outdated for #{listing_id}")
refresh_listing_cache(listing_id)
end
listing_translations
end

listing['translations'] = listing_translations || {}
def self.translations_invalid?(listing_translations)
ServiceHelper.listing_field_names_salesforce.any? do |field_name|
!listing_translations.key?(field_name.to_sym)
end
listing
end

def self.translations_valid?(listing_translations)
listing_field_names_salesforce = ServiceHelper.listing_field_names_salesforce
listing_field_names_salesforce.each do |field_name|
return false unless listing_translations.key?(field_name.to_sym)
def self.translations_are_outdated?(cache_last_modified, listing_last_modified)
parse_time(cache_last_modified) < parse_time(listing_last_modified)
end

def self.listing_is_outdated?(cache_last_modified, listing_last_modified)
parse_time(cache_last_modified) > parse_time(listing_last_modified)
end

def self.fetch_listing_translations_from_cache(listing_id)
@cache.fetch("#{CACHE_KEY_PREFIX}#{listing_id}/translations") do
Rails.logger.info("Nothing in cache for Listing #{listing_id} translations")
{}
end
true
end

def self.refresh_listing_cache(listing_id)
endpoint = "#{CACHE_KEY_PREFIX}#{CGI.escape(listing_id)}"
Request.new(parse_response: true).cached_get(endpoint, nil, true)
end

def self.parse_time(time_str)
Time.parse(time_str)
end

# get all units for a given listing
Expand Down
9 changes: 5 additions & 4 deletions app/services/google_translation_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ def translate(text, to)
end
end

def cache_listing_translations(listing_id, keys, translations)
translations = transform_translations_for_caching(listing_id, keys, translations)
def cache_listing_translations(listing_id, keys, translations, last_modified)
translations = transform_translations_for_caching(listing_id, keys, translations,
last_modified)
if @cache.write("/ListingDetails/#{listing_id}/translations", translations)
google_translation_logger(
"Successfully cached listing translations for listing id: #{listing_id}",
Expand All @@ -49,13 +50,13 @@ def parse_translations(results)
results.map(&:text)
end

def transform_translations_for_caching(listing_id, keys, translations)
def transform_translations_for_caching(listing_id, keys, translations, last_modified)
prev_cached_translations = @cache.read("/ListingDetails/#{listing_id}/translations")

# keys can come from updated_values.keys in the event_subscriber_translate_service
# they will be in the same order as the translations because the translation service
# uses the values from that object and the api returns 1 for each key
return_value = {}
return_value = { LastModifiedDate: last_modified }
translations.each do |target|
target[:translation].each_with_index do |value, i|
field = keys[i].to_sym
Expand Down
11 changes: 9 additions & 2 deletions spec/services/google_translation_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@
expected_result = [{ to: 'ES', translation: %w[Hello World] }]
listing_id = 'a0W0P00000F8YG4UAN'
let(:mock_translate) { instance_double(Google::Cloud::Translate::V2::Api) }
let(:mock_request) { instance_double(Force::Request) }

before do
allow(Google::Cloud::Translate::V2).to receive(:new).and_return(mock_translate)
allow(Force::Request).to receive(:new).and_return(mock_request)
allow(mock_request).to receive(:cached_get).and_return(true)
end

it 'translates text to the target language' do
Expand All @@ -32,9 +35,11 @@
listing_id,
fields,
expected_result,
'2021-07-01T00:00:00Z',
)

expect(result).to eq({ Hello: { ES: 'Hello' }, World: { ES: 'World' } })
expect(result).to eq({ LastModifiedDate: '2021-07-01T00:00:00Z',
Hello: { ES: 'Hello' }, World: { ES: 'World' } })
end

it 'updates translation object for a listing' do
Expand All @@ -51,9 +56,11 @@
listing_id,
fields_updated,
translations,
'2021-07-01T00:00:00Z',
)

expect(result).to eq({ Hello: { ES: 'Hello' }, World: { ES: 'Mundo' } })
expect(result).to eq({ LastModifiedDate: '2021-07-01T00:00:00Z',
Hello: { ES: 'Hello' }, World: { ES: 'Mundo' } })
end
end
end
99 changes: 87 additions & 12 deletions spec/services/listing_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@
end
end

# before do
# allow_any_instance_of(Force::Request).to receive(:oauth_token)
# end

describe '.listings' do
it 'should pass type down to Salesforce request for ownership listing' do
expect_any_instance_of(Force::Request).to receive(:cached_get)
Expand Down Expand Up @@ -63,32 +59,111 @@
end
end

describe '.listing with GoogleCloudTranslate feature flag enabled' do
describe '.listing with GoogleCloudTranslate feature flag disabled' do
it 'should return details of a listing' do
endpoint = '/ListingDetails/' + listing_id
endpoint = "/ListingDetails/#{listing_id}"
expect_any_instance_of(Force::Request).to receive(:cached_get)
.with(endpoint, nil, false).and_return([single_listing])
Force::ListingService.listing(listing_id)
end
end

describe '.listing with GoogleCloudTranslate feature flag disabled' do
describe '.listing with GoogleCloudTranslate feature flag enabled' do
let(:cache_store) { instance_double(ActiveSupport::Cache::MemoryStore) }
let(:endpoint) { "/ListingDetails/#{listing_id}" }
let(:single_listing) do
{ 'Id' => listing_id, 'LastModifiedDate' => '2024-03-08T16:51:35.000+0000' }
end
let(:request_instance) { instance_double(Force::Request) }
let(:cache_service) { instance_double(CacheService) }

before do
allow(Rails).to receive(:cache).and_return(cache_store)

allow(Force::Request).to receive(:new).and_return(request_instance)
allow(request_instance).to receive(:cached_get).with(endpoint, nil,
false).and_return([single_listing])
allow(::UNLEASH).to receive(:is_enabled?) # rubocop:disable Style/RedundantConstantBase
.with('GoogleCloudTranslate')
.and_return(true)

allow(cache_store).to receive(:fetch)
.with("/ListingDetails/#{listing_id}/translations")
.and_return({ LastModifiedDate: '2024-03-08T16:51:35.000+0000' })

allow(CacheService).to receive(:new).and_return(cache_service)

allow(cache_service).to receive(:process_translations).and_return({ LastModifiedDate: '2024-03-08T16:51:35.000+0000' })
end
after do
allow(::UNLEASH).to receive(:is_enabled?) # rubocop:disable Style/RedundantConstantBase
.and_return(false)
it 'should return details of a listing with cached listing_translations attached' do
endpoint = "/ListingDetails/#{listing_id}"
expect(request_instance).to receive(:cached_get)
.with(endpoint, nil, false).and_return([single_listing])

expect(Force::ListingService).to receive(:translations_invalid?).and_return(false)

expect(cache_store).to receive(:fetch).and_return({ LastModifiedDate: '2024-03-08T16:51:35.000+0000' })

listing = Force::ListingService.listing(listing_id)
expect(listing).to have_key('translations')
end
it 'should return details of a listing with listing_translations attached' do
it 'should return details of a listing with fresh listing_translations attached' do
endpoint = "/ListingDetails/#{listing_id}"
expect_any_instance_of(Force::Request).to receive(:cached_get)
expect(request_instance).to receive(:cached_get)
.with(endpoint, nil, false).and_return([single_listing])

expect(Force::ListingService).to receive(:translations_invalid?).and_return(true)

expect(cache_store).to receive(:fetch).and_return({ LastModifiedDate: '2024-03-08T16:51:35.000+0000' })

expect(cache_service).to receive(:process_translations).and_return({ LastModifiedDate: '2024-03-08T16:51:35.000+0000' })

listing = Force::ListingService.listing(listing_id)
expect(listing).to have_key('translations')
end

it 'should return details of a listing with fresh listing_translations attached when translations are outdated' do
endpoint = "/ListingDetails/#{listing_id}"
expect(request_instance).to receive(:cached_get)
.with(endpoint, nil, false).and_return([single_listing])

expect(Force::ListingService).to receive(:translations_invalid?).and_return(false)

expect(cache_store).to receive(:fetch).and_return({ LastModifiedDate: '2024-02-08T16:51:35.000+0000' })

expect(cache_service).to receive(:process_translations).and_return({ LastModifiedDate: '2024-03-08T16:51:35.000+0000' })

listing = Force::ListingService.listing(listing_id)
expect(listing).to have_key('translations')
end

it 'should return details of a listing with cached listing_translations attached when listing is outdated' do
endpoint = "/ListingDetails/#{listing_id}"

expect(Force::ListingService).to receive(:translations_invalid?).and_return(false)

expect(cache_store).to receive(:fetch).and_return({ LastModifiedDate: '2024-04-08T16:51:35.000+0000' })

expect(request_instance).to receive(:cached_get)
.with(endpoint, nil, true).and_return([single_listing])

listing = Force::ListingService.listing(listing_id)
expect(listing).to have_key('translations')
end

it 'translations_invalid if missing fields' do
translations_invalid = Force::ListingService.translations_invalid?({})
expect(translations_invalid).to be_truthy
end
it 'translations are valid if all fields are present' do
translations = {}
ServiceHelper.listing_field_names_salesforce.each do |field|
translations[field.to_sym] = 'value'
end

translations_invalid = Force::ListingService.translations_invalid?(translations)
expect(translations_invalid).to be_falsey
end
end

describe '.eligible_listings' do
Expand Down

0 comments on commit 456c5fc

Please sign in to comment.