Skip to content

Commit

Permalink
G11n java client code clean up - remove logic of adding a cacheItem w…
Browse files Browse the repository at this point in the history 
…ith an empty dataMap to cache for non-supported locales (#807)

Code clean up - remove logic of adding a cacheItem with an empty dataMap to cache for non-supported locales
  • Loading branch information
@jessiejuachon
jessiejuachon authored Oct 13, 2020
1 parent b3d67e7 commit 370e4e0
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 118 deletions.
19 changes: 7 additions & 12 deletions src/main/java/com/vmware/vipclient/i18n/base/cache/MessageCacheItem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,23 @@ public MessageCacheItem(Map<String, String> dataMap) {
if (dataMap != null)
this.cachedData.putAll(dataMap);
}
public MessageCacheItem (String locale, Map<String, String> dataMap, String etag, long timestamp, Long maxAgeMillis) {
this.setCacheItem(locale, dataMap, etag, timestamp, maxAgeMillis);

public MessageCacheItem (Map<String, String> dataMap, String etag, long timestamp, Long maxAgeMillis) {
this.setCacheItem(dataMap, etag, timestamp, maxAgeMillis);
}

private String locale;
private String etag;
private long timestamp;
private Long maxAgeMillis = 86400000l;

private final Map<String, String> cachedData = new HashMap<>();

public synchronized void setCacheItem(String locale, Map<String, String> dataToCache, String etag, long timestamp, Long maxAgeMillis) {
public synchronized void setCacheItem(Map<String, String> dataToCache, String etag, long timestamp, Long maxAgeMillis) {
if (dataToCache != null)
this.cachedData.putAll(dataToCache);
maxAgeMillis = maxAgeMillis == null ? this.maxAgeMillis : maxAgeMillis;
this.setCacheItem(locale, etag, timestamp, maxAgeMillis);
this.setCacheItem(etag, timestamp, maxAgeMillis);
}
public synchronized void setCacheItem(String locale, String etag, long timestamp, Long maxAgeMillis) {
this.locale = locale;
public synchronized void setCacheItem(String etag, long timestamp, Long maxAgeMillis) {
if (etag != null && !etag.isEmpty())
this.etag = etag;
this.timestamp = timestamp;
Expand All @@ -47,7 +44,7 @@ public synchronized void setCacheItem(String locale, String etag, long timestamp
}

public synchronized void setCacheItem (MessageCacheItem cacheItem) {
this.setCacheItem(cacheItem.getLocale(), cacheItem.getCachedData(), cacheItem.getEtag(), cacheItem.getTimestamp(), cacheItem.getMaxAgeMillis());
this.setCacheItem(cacheItem.getCachedData(), cacheItem.getEtag(), cacheItem.getTimestamp(), cacheItem.getMaxAgeMillis());
}

public String getEtag() {
Expand All @@ -66,8 +63,6 @@ public Long getMaxAgeMillis() {
return maxAgeMillis;
}

public String getLocale() { return locale; }

public synchronized boolean isExpired() {
// If offline mode only, cache never expires.
if (VIPCfg.getInstance().getVipServer() == null) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/vmware/vipclient/i18n/messages/api/opt/local/LocalMessagesOpt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void getComponentMessages(MessageCacheItem cacheItem) {
path = Paths.get(uri);
messages = JSONBundleUtil.getMessages(path);
}
cacheItem.setCacheItem(dto.getLocale(), messages, null, System.currentTimeMillis(), null);
cacheItem.setCacheItem(messages, null, System.currentTimeMillis(), null);
} catch (Exception e) {
logger.debug(e.getMessage());
// Do not update cacheItem
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/vmware/vipclient/i18n/messages/api/opt/local/LocalProductOpt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void getSupportedLocales(MessageCacheItem cacheItem) {
for (String languageTag: supportedLocales) {
languageTagMap.put(languageTag, languageTag);
}
cacheItem.setCacheItem(null, languageTagMap, null, System.currentTimeMillis(), null);
cacheItem.setCacheItem(languageTagMap, null, System.currentTimeMillis(), null);
}

public List<String> getComponents() {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/vmware/vipclient/i18n/messages/api/opt/server/ComponentBasedOpt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ public void getComponentMessages(MessageCacheItem cacheItem) {
if (getResponseCode(respObj) == 200) {
Map<String,String> messages = this.getMsgsJson(response);
if (messages != null) {
cacheItem.setCacheItem(this.dto.getLocale(), messages, etag, timestamp, maxAgeMillis);
cacheItem.setCacheItem(messages, etag, timestamp, maxAgeMillis);
}
}
} catch (Exception e) {
logger.error("Failed to get messages");
}
} else {
cacheItem.setCacheItem(this.dto.getLocale(), etag, timestamp, maxAgeMillis);
cacheItem.setCacheItem(etag, timestamp, maxAgeMillis);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/vmware/vipclient/i18n/messages/api/opt/server/RemoteProductOpt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void getSupportedLocales(MessageCacheItem cacheItem) {
for (String languageTag : supportedLocales) {
languageTags.put(languageTag, languageTag);
}
cacheItem.setCacheItem(null, languageTags, etag, timestamp, maxAgeMillis);
cacheItem.setCacheItem(languageTags, etag, timestamp, maxAgeMillis);
}
}
}
Expand Down
80 changes: 16 additions & 64 deletions src/main/java/com/vmware/vipclient/i18n/messages/service/ComponentService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,19 +102,21 @@ public MessageCacheItem getMessages(Iterator<Locale> fallbackLocalesIter) {
CacheService cacheService = new CacheService(dto);
MessageCacheItem cacheItem = cacheService.getCacheOfComponent();
if (cacheItem != null) { // Item is in cache
if (cacheItem.getCachedData().isEmpty()) { // This means that the data to be used is from a fallback locale.
// If expired, try to first create and store cacheItem for the requested locale in a separate thread.
if (cacheItem.isExpired())
this.createCacheItemTask(null); // Pass null so that locale fallback will not be applied.

// Use cached data of cacheItem.getLocale() --> fallback locale
MessagesDTO fallbackLocaleDTO = new MessagesDTO(dto.getComponent(), cacheItem.getLocale(), dto.getProductID(), dto.getVersion());
cacheItem = new ComponentService(fallbackLocaleDTO).getMessages(null);
} else if (cacheItem.isExpired()) {
if (cacheItem.isExpired()) {
// Refresh the cacheItem in a separate thread
refreshCacheItemTask(cacheItem);
}
} else { // Item is not in cache. Create and store cacheItem for the requested locale
} else { // Item is not in cache.
ProductService ps = new ProductService(dto);
if (!ps.isSupportedLocale(Locale.forLanguageTag(dto.getLocale()))) { // Requested locale is not supported
Locale matchedLocale = LocaleUtility.pickupLocaleFromList(new LinkedList<>(ps.getSupportedLocales()), Locale.forLanguageTag(dto.getLocale()));
if (ps.isSupportedLocale(matchedLocale)) { // Requested locale matches a supported locale (eg. requested locale "fr_CA matches supported locale "fr")
MessagesDTO matchedLocaleDTO = new MessagesDTO(dto.getComponent(), matchedLocale.toLanguageTag(), dto.getProductID(), dto.getVersion());
return new ComponentService(matchedLocaleDTO).getMessages();
}
}

//Create and store cacheItem for the requested locale
cacheItem = createCacheItem(fallbackLocalesIter);
}
return cacheItem;
Expand All @@ -131,76 +133,26 @@ private MessageCacheItem createCacheItem(Iterator<Locale> fallbackLocalesIter) {
// Create a new cacheItem object to be stored in cache
MessageCacheItem cacheItem = new MessageCacheItem();

// If the requested locale is not supported, but matches a supported locale (eg. requested locale "fr_CA matches supported locale "fr"),
// return the messages of the supported locale that best matches the requested locale.
ProductService ps = new ProductService(dto);
if (!ps.isSupportedLocale(Locale.forLanguageTag(dto.getLocale()))) {
Locale matchedLocale = LocaleUtility.pickupLocaleFromList(new LinkedList<>(ps.getSupportedLocales()), Locale.forLanguageTag(dto.getLocale()));
if (ps.isSupportedLocale(matchedLocale)) {
MessagesDTO matchedLocaleDTO = new MessagesDTO(dto.getComponent(), matchedLocale.toLanguageTag(), dto.getProductID(), dto.getVersion());
cacheItem = new ComponentService(matchedLocaleDTO).getMessages(null);
MessageCacheItem cacheItemCopy = new MessageCacheItem(matchedLocale.toLanguageTag(), null, null, System.currentTimeMillis(), cacheItem.getMaxAgeMillis());
cacheService.addCacheOfComponent(cacheItemCopy);
return cacheItem;
}
}

// Will proceed with the following code if
// a. requested locale is supported OR
// b. requested locale is not supported and does not match any supported locale
refreshCacheItem(cacheItem, VIPCfg.getInstance().getMsgOriginsQueue().iterator());
if (!cacheItem.getCachedData().isEmpty()) {
cacheService.addCacheOfComponent(cacheItem);
} else if (!dto.getLocale().equals(ConstantsKeys.SOURCE) && fallbackLocalesIter != null && fallbackLocalesIter.hasNext()) {
// If failed to fetch message for the requested DTO, use MessageCacheItem of the next fallback locale.
MessagesDTO fallbackLocaleDTO = new MessagesDTO(dto.getComponent(), fallbackLocalesIter.next().toLanguageTag(), dto.getProductID(), dto.getVersion());
cacheItem = new ComponentService(fallbackLocaleDTO).getMessages(fallbackLocalesIter);
if (!cacheItem.getCachedData().isEmpty()) {
// Cache a copy of the fallback locale's cacheItem, but with only the locale and maxAgeMillis. Use current timestamp.
MessageCacheItem cacheItemCopy = new MessageCacheItem(cacheItem.getLocale(), null, null, System.currentTimeMillis(), cacheItem.getMaxAgeMillis());
cacheService.addCacheOfComponent(cacheItemCopy);
}
}

return cacheItem;
}

private void createCacheItemTask(Iterator<Locale> fallbackLocalesIter) {
Callable<MessageCacheItem> callable = () -> {
try {
return this.createCacheItem(fallbackLocalesIter);
} catch (Exception e) {
// To make sure that the thread will close
// even when an exception is thrown
return null;
}
};
FutureTask<MessageCacheItem> task = new FutureTask<>(callable);
Thread thread = new Thread(task);
thread.start();
}

private void refreshCacheItemTask(MessageCacheItem cacheItem) {
Callable<MessageCacheItem> callable = () -> {
Runnable runnable = () -> {
try {
// Get the locale of the cacheItem object. It may not be the same as the requested DTO's locale (e.g. the cacheItem is for a fallback locale).
String cacheItemLocale = cacheItem.getLocale();

// Refresh the properties of the cacheItem accordingly by passing a DTO with the correct locale
// to ComponentService, so that it will fetch messages for the correct locale to refresh the cacheItem.
MessagesDTO cacheItemDTO = new MessagesDTO(dto.getComponent(), cacheItemLocale, dto.getProductID(), dto.getVersion());
new ComponentService(cacheItemDTO).refreshCacheItem(cacheItem, VIPCfg.getInstance().getMsgOriginsQueue().listIterator());

return cacheItem;
} catch (Exception e) {
// To make sure that the thread will close
// even when an exception is thrown
return null;
refreshCacheItem(cacheItem, VIPCfg.getInstance().getMsgOriginsQueue().listIterator());
} catch (Exception e) {
}
};
FutureTask<MessageCacheItem> task = new FutureTask<>(callable);
Thread thread = new Thread(task);
thread.start();
new Thread(runnable).start();
}

public boolean isComponentAvailable() {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/vmware/vipclient/i18n/messages/service/ComponentsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public Map<Locale, Map<String, Map<String, String>>> getTranslation(final Set<St
final MessagesDTO dto = new MessagesDTO();
dto.setComponent(comp);
dto.setLocale(locale);
new CacheService(dto).addCacheOfComponent(new MessageCacheItem(locale, messages, null,
new CacheService(dto).addCacheOfComponent(new MessageCacheItem(messages, null,
cacheItem.getTimestamp(), cacheItem.getMaxAgeMillis()));

// update map to return.
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/vmware/vipclient/i18n/messages/service/ProductService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,10 @@ public Set<String> getSupportedLanguageTags(DataSourceEnum dataSource) {
}

public boolean isSupportedLocale(Locale locale) {
return getSupportedLocales().contains(LocaleUtility.fmtToMappedLocale(locale));
return getSupportedLanguageTags().contains(LocaleUtility.fmtToMappedLocale(locale).toLanguageTag());
}


private void refreshCacheItem(final MessageCacheItem cacheItem, DataSourceEnum dataSource) {
long timestampOld = cacheItem.getTimestamp();
dataSource.createProductOpt(dto).getSupportedLocales(cacheItem);
Expand Down Expand Up @@ -142,7 +143,6 @@ private MessageCacheItem createCacheItem (DataSourceEnum dataSource) {
}
return null;
}

private Set<Locale> langTagtoLocaleSet (Set<String> languageTags) {
Set<Locale> locales = new HashSet<>();
if (languageTags != null) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/vmware/vip/i18n/MessageCacheTest1.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public void testExpired() {
String v = "It's a test";
data.put(k, v);
String cachedKey = "key";
MessageCacheItem cacheItem = new MessageCacheItem(null, data, null, System.currentTimeMillis(), null);
MessageCacheItem cacheItem = new MessageCacheItem(data, null, System.currentTimeMillis(), null);
c.put(cachedKey, cacheItem);
long expired = 20000;
c.setExpiredTime(expired);
Expand Down
6 changes: 2 additions & 4 deletions src/test/java/com/vmware/vip/i18n/SharedComponentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,11 @@ public void testGetSharedModuleTranslation() {
VIPCfg gc = VIPCfg.getInstance();
Cache c = TranslationCacheManager.getCache(VIPCfg.CACHE_L3);
Map<String, MessageCacheItem> m = ((MessageCache) c).getCachedTranslationMap();
Assert.assertTrue(m.size() == 3);

Assert.assertTrue(m.size() == 2);
// TODO Null values are not allowed to be stored in the cache anymore.
// The following key must have non-null values to be stored.
//Assert.assertTrue(m.containsKey("JavaclientTest1_2.0.0_JSP_false_#de"));

Assert.assertTrue(m.containsKey("JavaclientTest_1.0.0_JAVA_false_#zh"));
Assert.assertTrue(m.containsKey("JavaclientTest_1.0.0_JAVA_false_#en"));
}
}
2 changes: 1 addition & 1 deletion src/test/java/com/vmware/vipclient/i18n/messages/service/CacheServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public void testExpireUsingCacheControlMaxAge() throws InterruptedException {
Long responseTime = (Long) cacheItem.getTimestamp();

// Set max age to 0 to explicitly expire the cache for testing purposes.
cacheItem.setCacheItem(cacheItem.getLocale(), cacheItem.getEtag(), cacheItem.getTimestamp(), 0l);
cacheItem.setCacheItem(cacheItem.getEtag(), cacheItem.getTimestamp(), 0l);

// Second request for the same message triggers an HTTP request because cacheItem has expired.
// The http request includes an If-None-Match header that is set to the previously received eTag value.
Expand Down
56 changes: 28 additions & 28 deletions src/test/java/com/vmware/vipclient/i18n/messages/service/OfflineModeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class OfflineModeTest extends BaseTestClass {
String comment = "comment";
String messageFil = "[{0}] Alerto sa pagsusuri";
String messageFr ="[{0}] Alerte de test";
String messageEn ="[{0}] Test alert";
Object[] args = { "a" };

MessagesDTO dto = new MessagesDTO();
Expand Down Expand Up @@ -91,15 +92,15 @@ public void testGetMsgsOfflineMode() {

dto.setProductID(VIPCfg.getInstance().getProductName());
dto.setVersion(VIPCfg.getInstance().getVersion());

CacheService cs = new CacheService(dto);
String message = translation.getMessage(locale, component, key, args);
assertEquals(FormatUtils.format(messageFil, locale, args), message);

// cs.getCacheOfComponent() for fil-PH returns MessageCacheItem of fil
MessageCacheItem filPHCacheItem = cs.getCacheOfComponent();
Assert.assertTrue(filPHCacheItem.getLocale().equals("fil"));

// fil-PH cacheItem does not exist
CacheService cs = new CacheService(dto);
assertNull(cs.getCacheOfComponent());

// fil-PH matches fil cacheItem
String messageFilPh = translation.getMessage(locale, component, key, args);
assertEquals(FormatUtils.format(messageFil, locale, args), messageFilPh);

cfg.setOfflineResourcesBaseUrl(offlineResourcesBaseUrlOrig);
cfg.setMsgOriginsQueue(msgOriginsQueueOrig);
}
Expand Down Expand Up @@ -178,12 +179,11 @@ public void testGetMsgsFailedUseDefault() {
String message = translation.getMessage(newLocale, component, key, args);
// Returns the message in the default locale
assertEquals(FormatUtils.format(source, args), message);

// cacheItem for "es" locale has an empty data map. It's locale is the fallback locale.

MessageCacheItem cacheItem = cs.getCacheOfComponent();
assertEquals("en", cacheItem.getLocale());
assertTrue(cacheItem.getCachedData().isEmpty());
assertNull(cacheItem);
assertEquals(FormatUtils.format(messageEn, args), message);

cfg.setOfflineResourcesBaseUrl(offlineResourcesBaseUrlOrig);
cfg.setMsgOriginsQueue(msgOriginsQueueOrig);
}
Expand Down Expand Up @@ -257,9 +257,10 @@ public void testGetMsgsOfflineModeAfterOnlineError() {

String message = translation.getMessage(locale, component, key, args);
assertEquals(FormatUtils.format(messageFil, locale, args), message);

MessageCacheItem cacheItem = cs.getCacheOfComponent();
assertEquals(messageFil, cacheItem.getCachedData().get(key));

// fil-PH does not exist in cache
//assertNull(cs.getCacheOfComponent());
assertEquals(FormatUtils.format(messageFil, args), message);

cfg.setOfflineResourcesBaseUrl(offlineResourcesBaseUrlOrig);
cfg.setMsgOriginsQueue(msgOriginsQueueOrig);
Expand Down Expand Up @@ -323,11 +324,14 @@ public void testGetSupportedLocalesOfflineBundles() throws ParseException {

I18nFactory i18n = I18nFactory.getInstance(VIPCfg.getInstance());
TranslationMessage translation = (TranslationMessage) i18n.getMessageInstance(TranslationMessage.class);
translation.getMessage(locale, component, key, args);

// cs.getCacheOfComponent() for fil-PH returns MessageCacheItem of fil
MessageCacheItem filPHCacheItem = cs.getCacheOfComponent();
Assert.assertTrue(filPHCacheItem.getLocale().equals("fil"));
String messageFilPh = translation.getMessage(locale, component, key, args);

// fil-PH cacheItem does not exist
assertNull(cs.getCacheOfComponent());

// fil-PH matches fil cacheItem
assertEquals(FormatUtils.format(messageFil, locale, args), messageFilPh);

// Disable offline mode off for next tests.
cfg.setOfflineResourcesBaseUrl(offlineResourcesBaseUrlOrig);
Expand Down Expand Up @@ -357,20 +361,16 @@ public void testGetMsgsBothOfflineAndOnlineFailed() {

CacheService cs = new CacheService(dto);

translation.getMessage(newLocale, component, key, args);
String message = translation.getMessage(newLocale, component, key, args);

MessageCacheItem cacheItem = cs.getCacheOfComponent();
assertNotNull(cacheItem);
assertNull(cacheItem);

MessagesDTO defaultLocaleDTO = new MessagesDTO(dto.getComponent(),
dto.getKey(), dto.getSource(), LocaleUtility.getDefaultLocale().toLanguageTag(), null);
CacheService csDefault = new CacheService(defaultLocaleDTO);
MessageCacheItem cacheItemDefaultLocale = csDefault.getCacheOfComponent();

// cacheItem of Locale.ITALIAN's data map is empty. Its locale is the fallback locale
assertTrue(cacheItem.getCachedData().isEmpty());
assertEquals("en", cacheItem.getLocale());

assertEquals(message, FormatUtils.format(csDefault.getCacheOfComponent().getCachedData().get(key), args));

cfg.setOfflineResourcesBaseUrl(offlineResourcesBaseUrlOrig);
cfg.setMsgOriginsQueue(msgOriginsQueueOrig);
}
Expand Down

0 comments on commit 370e4e0

Please sign in to comment.