Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] g11n-java-client - ComponentService.getMessages is not retrieving "matchedLocale" properly #781

Closed
jessiejuachon opened this issue Sep 14, 2020 · 7 comments · Fixed by #782 or #805
Assignees
Labels
area/java-client kind/bug Something isn't working
Milestone

Comments

@jessiejuachon
Copy link
Contributor

jessiejuachon commented Sep 14, 2020

Describe the bug
ComponentService.getMessages is not retrieving "matchedLocale" from cache properly.

To Reproduce

  1. Have VIP service support 2 locales: en and fr.
  2. Set initializeCache=true on client side and start the application to populate client-side cache.
  3. Call TranslationMessage.getMessages for "fr_CA" locale. When ComponentService.getMessages is invoked, notice that CacheService.isContainComponent returns false.
  4. A call to VIP service for fr_CA locale will be sent. The VIP service will reply with the message for the "fr" locale as best match.

Expected behavior
Because cache has been initialized at app start up, there should have been NO call to VIP service for fr_CA in step #4 above anymore because it matches "fr". CacheService.getCacheOfComponent should have immediately returned the messages of "fr" locale from client-side cache.

@jessiejuachon
Copy link
Contributor Author

jessiejuachon commented Sep 14, 2020

@lyiyu66 , FYI. Please review.

@gong-yu gong-yu added this to the Sprint 89 milestone Sep 15, 2020
@jessiejuachon jessiejuachon modified the milestones: Sprint 89, Sprint 90 Sep 16, 2020
@jessiejuachon jessiejuachon changed the title [BUG] g11n-java-client - L3's CacheService.getSupportedLocalesFromCache is incorrectly using L2's data. [BUG] g11n-java-client - CacheService.getCacheOfComponent cannot get "matchedLocale" properly if L2 supported locale list cache is not yet populated. Sep 16, 2020
@jessiejuachon jessiejuachon changed the title [BUG] g11n-java-client - CacheService.getCacheOfComponent cannot get "matchedLocale" properly if L2 supported locale list cache is not yet populated. [BUG] g11n-java-client - CacheService.getCacheOfComponent cannot get "matchedLocale" properly Sep 17, 2020
@jessiejuachon jessiejuachon changed the title [BUG] g11n-java-client - CacheService.getCacheOfComponent cannot get "matchedLocale" properly [BUG] g11n-java-client - CacheService.getCacheOfComponent is not retrieving "matchedLocale" properly Sep 17, 2020
@jessiejuachon jessiejuachon changed the title [BUG] g11n-java-client - CacheService.getCacheOfComponent is not retrieving "matchedLocale" properly [BUG] g11n-java-client - ComponentService.getMessages is not retrieving "matchedLocale" from cache properly Sep 17, 2020
@Xiaochao8
Copy link
Contributor

Does this mean that it's impossible to get translation of fr-CA when both fr and fr-CA are supported?

@lyiyu66
Copy link

lyiyu66 commented Sep 21, 2020

@jessiejuachon

Expected behavior
Because cache has been initialized at app start up, there should have been NO call to VIP service for fr_FR in step #4 above anymore because it matches "fr". CacheService.getCacheOfComponent should have immediately returned the messages of "fr" locale from client-side cache.

For the expected behavior, is it a typo "fr-FR"on there should have been NO call to VIP service for fr_FR in step #4 above anymore because it matches "fr"? Should be fr-CA?

In current behavior, when user requests fr-CA, client will send a http request with fr-CA to service. I think it is correct and expected behavior. For the locale that can't be found in cache, client should send out the request to fetch from service even data has been cached initially since below possible scenarios:

  • missing data in cache initialized
  • has translation updated/added after cache initialized

@huihuiw01
Copy link
Contributor

huihuiw01 commented Sep 21, 2020

Suggest solution:

  1. get matched locale first(compare request locale with supported locale list, get supported locale list by call L2 API)
    1.1. if matched locale is not null, query translation for matched locale(query cache first, if not found query data source, then only add matched locale's translation to cache)
    1.2. if matched locale is null, query translation for fallback locale(query cache first, if not found query data source)

Refer to suggestion on the PR.

I think this also can resolve issue #754

@jessiejuachon jessiejuachon changed the title [BUG] g11n-java-client - ComponentService.getMessages is not retrieving "matchedLocale" from cache properly [BUG] g11n-java-client - ComponentService.getMessages is not retrieving "matchedLocale" properly Sep 22, 2020
@jessiejuachon
Copy link
Contributor Author

Does this mean that it's impossible to get translation of fr-CA when both fr and fr-CA are supported?

No, fr-CA will be queried from service because it is supported.

@jessiejuachon
Copy link
Contributor Author

jessiejuachon commented Sep 25, 2020

@jessiejuachon

Expected behavior
Because cache has been initialized at app start up, there should have been NO call to VIP service for fr_FR in step #4 above anymore because it matches "fr". CacheService.getCacheOfComponent should have immediately returned the messages of "fr" locale from client-side cache.

For the expected behavior, is it a typo "fr-FR"on there should have been NO call to VIP service for fr_FR in step #4 above anymore because it matches "fr"? Should be fr-CA?

In current behavior, when user requests fr-CA, client will send a http request with fr-CA to service. I think it is correct and expected behavior. For the locale that can't be found in cache, client should send out the request to fetch from service even data has been cached initially since below possible scenarios:

  • missing data in cache initialized
  • has translation updated/added after cache initialized

Yes, it was a typo.
The client should not send a request to service if the requested locale matches a supported locale. See more details in next comment.

@jessiejuachon
Copy link
Contributor Author

jessiejuachon commented Sep 25, 2020

Suggest solution:

  1. get matched locale first(compare request locale with supported locale list, get supported locale list by call L2 API)
    1.1. if matched locale is not null, query translation for matched locale(query cache first, if not found query data source, then only add matched locale's translation to cache)
    1.2. if matched locale is null, query translation for fallback locale(query cache first, if not found query data source)

Refer to suggestion on the PR.

I think this also can resolve issue #754

Yes, you got it! :) This issue and associated solution is is for 1.1 above. Number 1.2 will be resolved in issue 754.

I decided to split up issue 754 because the initial PR was too big..code reviewers were having a hard time.
Also, I will do a code clean up in 754 to remove logic of adding a cacheItem with an empty dataMap to cache. We do not need this anymore because we are using the list of supported locales to check.

Regarding your following comment:

For the locale that can't be found in cache, client should send out the request to fetch from service even data has been cached initially since below possible scenarios:

  • missing data in cache initialized
  • has translation updated/added after cache initialized

The list of supported locales is in cache with an expiry logic. When it expires, it will be fetched again from the service so any missing locale or newly added locale will be included then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment