G11n java client - Fix for expiration-related issues (issues 604, 664, and 662)

src/main/java/com/vmware/vipclient/i18n/base/cache/MessageCacheItem.java

@@ -14,77 +14,60 @@ public class MessageCacheItem implements CacheItem {
 	public MessageCacheItem() {
 
 	}
-
-	public MessageCacheItem (Map<String, String> dataMap, String etag, long timestamp, Long maxAgeMillis) {
-		super();
-		this.addCachedData(dataMap);
-		this.etag = etag;
-		this.timestamp = timestamp;
-		this.maxAgeMillis = maxAgeMillis;
-	}
-
-	public MessageCacheItem (Map<String, String> dataMap) {
-		super();
+
+	public MessageCacheItem(Map<String, String> dataMap) {
 		if (dataMap != null)
-			this.addCachedData(dataMap);
+			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);
+	}
+
+	private String locale;
 	private String etag;
 	private long timestamp;
 	private Long maxAgeMillis = 86400000l;
 
-	private final Map<String, String> cachedData = new HashMap<String, String>();
-
-	public void addCacheData(String key, String value) {
-		this.cachedData.put(key, value);
+	private final Map<String, String> cachedData = new HashMap<>();
+
+	public synchronized void setCacheItem(String locale, Map<String, String> dataToCache, String etag, long timestamp, Long maxAgeMillis) {
+		if (dataToCache != null)
+			this.cachedData.putAll(dataToCache);
+		this.setCacheItem(locale, etag, timestamp, maxAgeMillis);
 	}
-
-	public synchronized void addCachedData(Map<String, String> cachedData) {
-		if (cachedData != null) 
-			this.cachedData.putAll(cachedData);
+	public synchronized void setCacheItem(String locale, String etag, long timestamp, Long maxAgeMillis) {
+		this.locale = locale;
+		if (etag != null && !etag.isEmpty())
+			this.etag = etag;
+		this.timestamp = timestamp;
+		if (maxAgeMillis != null)
+			this.maxAgeMillis = maxAgeMillis;
 	}
-	
+
 	public synchronized void setCacheItem (MessageCacheItem cacheItem) {
-		// Do not update cacheItem if timestamp is earlier than current. 
-		// An older timestamp comes from an old thread that was blocked.
-		if (cacheItem.getTimestamp() < this.timestamp) 
-			return;
-		this.addCachedData(cacheItem.getCachedData());
-		this.etag = cacheItem.etag;
-		this.timestamp = cacheItem.timestamp;
-		this.maxAgeMillis = cacheItem.maxAgeMillis;
+		this.setCacheItem(cacheItem.getLocale(), cacheItem.getCachedData(), cacheItem.getEtag(), cacheItem.getTimestamp(), cacheItem.getMaxAgeMillis());
 	}
 
-	public synchronized String getEtag() {
+	public String getEtag() {
 		return etag;
 	}
 
-	public synchronized void setEtag(String etag) {
-		this.etag = etag;
-	}
-
-	public synchronized long getTimestamp() {
+	public long getTimestamp() {
 		return timestamp;
 	}
-
-	public synchronized void setTimestamp(long timestamp) {
-		this.timestamp = timestamp;
-	}
 
     public Map<String, String> getCachedData() {
 		return cachedData;
 	}
 
-	public synchronized Long getMaxAgeMillis() {
+	public Long getMaxAgeMillis() {
 		return maxAgeMillis;
 	}
 
-	public synchronized void setMaxAgeMillis(Long maxAgeMillis) {
-		this.maxAgeMillis = maxAgeMillis;
-	}
+	public String getLocale() { return locale; }
 
-	public boolean isExpired() {
+	public synchronized boolean isExpired() {
 		// If offline mode only, cache never expires.
 		if (VIPCfg.getInstance().getVipServer() == null) {
 			return false;

src/main/java/com/vmware/vipclient/i18n/messages/api/opt/local/LocalMessagesOpt.java

@@ -67,9 +67,7 @@ public void getComponentMessages(MessageCacheItem cacheItem) {
 				path = Paths.get(uri);
 				messages = JSONBundleUtil.getMessages(path);
 			}
-
-	    	cacheItem.addCachedData(messages);
-	    	cacheItem.setTimestamp(System.currentTimeMillis());
+			cacheItem.setCacheItem(dto.getLocale(), messages, null, System.currentTimeMillis(), null);
 		} catch (Exception e) {
 			logger.debug(e.getMessage());
 			// Do not update cacheItem

src/main/java/com/vmware/vipclient/i18n/messages/api/opt/server/ComponentBasedOpt.java

@@ -9,11 +9,6 @@
 import java.util.List;
 import java.util.Map;
 
-import org.json.simple.JSONObject;
-import org.json.simple.JSONValue;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import com.vmware.vipclient.i18n.VIPCfg;
 import com.vmware.vipclient.i18n.base.cache.MessageCacheItem;
 import com.vmware.vipclient.i18n.messages.api.opt.BaseOpt;
@@ -23,7 +18,10 @@
 import com.vmware.vipclient.i18n.messages.api.url.V2URL;
 import com.vmware.vipclient.i18n.messages.dto.MessagesDTO;
 import com.vmware.vipclient.i18n.util.ConstantsKeys;
-import com.vmware.vipclient.i18n.util.LocaleUtility;
+import org.json.simple.JSONObject;
+import org.json.simple.JSONValue;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class ComponentBasedOpt extends BaseOpt implements Opt, MessageOpt {
     private final Logger      logger = LoggerFactory.getLogger(ComponentBasedOpt.class.getName());
@@ -52,33 +50,32 @@ public void getComponentMessages(MessageCacheItem cacheItem) {
 
         if (responseCode != null && (responseCode.equals(HttpURLConnection.HTTP_OK) || 
         		responseCode.equals(HttpURLConnection.HTTP_NOT_MODIFIED))) {
-
-        	// If already in cache (timestamp > 0), always extend the timestamp.
-        	if (cacheItem.getTimestamp() != 0 && response.get(URLUtils.RESPONSE_TIMESTAMP) != null)
-	        	cacheItem.setTimestamp((long) response.get(URLUtils.RESPONSE_TIMESTAMP) );
+        	long timestamp = 0;
+        	String etag = null;
+        	Long maxAgeMillis = null;
+
+        	if (response.get(URLUtils.RESPONSE_TIMESTAMP) != null)
+        	    timestamp = (long) response.get(URLUtils.RESPONSE_TIMESTAMP);
         	if (response.get(URLUtils.HEADERS) != null)
-	        	cacheItem.setEtag(URLUtils.createEtagString((Map<String, List<String>>) response.get(URLUtils.HEADERS)));
+        	    etag = URLUtils.createEtagString((Map<String, List<String>>) response.get(URLUtils.HEADERS));
 	        if (response.get(URLUtils.MAX_AGE_MILLIS) != null)
-	        	cacheItem.setMaxAgeMillis((Long) response.get(URLUtils.MAX_AGE_MILLIS));
-			      
+                maxAgeMillis = (Long) response.get(URLUtils.MAX_AGE_MILLIS);
+
 	        if (responseCode.equals(HttpURLConnection.HTTP_OK)) {
 		        JSONObject respObj = (JSONObject) JSONValue.parse((String) response.get(URLUtils.BODY));
 		        try {
-	        		if (getResponseCode(respObj) == 200){ 
-	        			// If not yet in cache (timestamp = 0), store the timestamp only when successful (business code 200).
-	        			if (cacheItem.getTimestamp() == 0 && response.get(URLUtils.RESPONSE_TIMESTAMP) != null) {
-	        				cacheItem.setTimestamp((long) response.get(URLUtils.RESPONSE_TIMESTAMP) );
-	        			}
+	        		if (getResponseCode(respObj) == 200) {
 				        Map<String,String> messages = this.getMsgsJson(response);
 				        if (messages != null) {
-				        	cacheItem.addCachedData(messages);
+				        	cacheItem.setCacheItem(this.dto.getLocale(), messages, etag, timestamp, maxAgeMillis);
 				        }
         			}
 	        	} catch (Exception e) {
 	        		logger.error("Failed to get messages");
 	        	}
-	        }
-
+	        } else {
+                cacheItem.setCacheItem(this.dto.getLocale(), etag, timestamp, maxAgeMillis);
+            }
         } 
     }
 

src/main/java/com/vmware/vipclient/i18n/messages/api/opt/server/ComponentsBasedOpt.java

@@ -48,9 +48,10 @@ public JSONObject queryFromServer(final Set<String> components, final Set<String
         Map<String, Object> response = VIPCfg.getInstance().getVipService().getHttpRequester().request(url, ConstantsKeys.GET,
                 requestData);
         this.responseStr = (String) response.get(URLUtils.BODY);
-        cacheItem.setEtag(URLUtils.createEtagString((Map<String, List<String>>) response.get(URLUtils.HEADERS)));
-        cacheItem.setTimestamp((long) response.get(URLUtils.RESPONSE_TIMESTAMP));
-        cacheItem.setMaxAgeMillis((Long) response.get(URLUtils.MAX_AGE_MILLIS));
+        String etag = URLUtils.createEtagString((Map<String, List<String>>) response.get(URLUtils.HEADERS));
+        long timestamp = (long) response.get(URLUtils.RESPONSE_TIMESTAMP);
+        Long maxAgeMillis = (Long) response.get(URLUtils.MAX_AGE_MILLIS);
+        cacheItem.setCacheItem(null, etag, timestamp, maxAgeMillis);
         if (StringUtil.isEmpty(this.responseStr))
             throw new VIPJavaClientException(ConstantsMsg.SERVER_RETURN_EMPTY);
 

src/main/java/com/vmware/vipclient/i18n/messages/service/CacheService.java

@@ -4,12 +4,7 @@
  */
 package com.vmware.vipclient.i18n.messages.service;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 import com.vmware.vipclient.i18n.VIPCfg;
 import com.vmware.vipclient.i18n.base.cache.Cache;

src/main/java/com/vmware/vipclient/i18n/messages/service/ComponentService.java

@@ -32,37 +32,43 @@ public ComponentService(MessagesDTO dto) {
         this.dto = dto;
     }
 
-    /**
-     * Fetch messages from either remote vip service or local bundle
-     * 
-     * @param cacheItem MessageCacheItem object to store the messages
-     * @param msgSourceQueueIter Iterator of the msgSourceQueue (e.g. [DataSourceEnum.VIP, DataSourceEnum.Bundle])
-     */
-    @SuppressWarnings("unchecked")
-    private void fetchMessages(final MessageCacheItem cacheItem, Iterator<DataSourceEnum> msgSourceQueueIter) {
-    	if (!msgSourceQueueIter.hasNext()) 
-    		return;
-
-    	long timestampOld = cacheItem.getTimestamp();
-    	DataSourceEnum dataSource = msgSourceQueueIter.next();
-    	dataSource.createMessageOpt(dto).getComponentMessages(cacheItem);
-    	long timestamp = cacheItem.getTimestamp();
-    	if (timestampOld == timestamp) {
-    		logger.debug(FormatUtils.format(ConstantsMsg.GET_MESSAGES_FAILED, dto.getComponent(), dto.getLocale(), dataSource.toString()));
-    	}
-
-    	// Skip this block if timestamp is not 0 (which means cacheItem is in the cache) regardless if cacheItem is expired or not.
-    	// Otherwise, try the next dataSource in the queue.
-    	if (timestamp == 0) {
-    		// Try the next dataSource in the queue
-    		if (msgSourceQueueIter.hasNext()) {
-    			fetchMessages(cacheItem, msgSourceQueueIter);
-    		// If no more data source in queue, log the error. This means that neither online nor offline fetch succeeded.
-    		} else {
-    			logger.debug(FormatUtils.format(ConstantsMsg.GET_MESSAGES_FAILED_ALL, dto.getComponent(), dto.getLocale()));
-    		}
-    	}
-    }
+	/**
+	 * Fetches data and populates the MessageCacheItem
+	 *
+	 * @param cacheItem The MessageCacheItem to populate/refresh data in. The following properties of cacheItem will be populated/refreshed:
+	 * <ul>
+	 * 		<li>The cachedData map which holds the localized messages</li>
+	 * 		<li>The timestamp of when the messages were fetched</li>
+	 * 		<li>The maxAgeMillis which tells how long before the cacheData map is considered to be expired.</li>
+	 * 	    <li>The eTag, if any, which will be used in the succeeding cache refresh.</li>
+	 * </ul>
+	 *  @param msgSourceQueueIter Iterator of the message source queue (e.g. [DataSourceEnum.VIP, DataSourceEnum.Bundle])
+	 */
+	@SuppressWarnings("unchecked")
+	private void refreshCacheItem(final MessageCacheItem cacheItem, Iterator<DataSourceEnum> msgSourceQueueIter) {
+		if (!msgSourceQueueIter.hasNext())
+			return;
+
+		long timestampOld = cacheItem.getTimestamp();
+		DataSourceEnum dataSource = msgSourceQueueIter.next();
+		dataSource.createMessageOpt(dto).getComponentMessages(cacheItem);
+		long timestamp = cacheItem.getTimestamp();
+		if (timestampOld == timestamp) {
+			logger.debug(FormatUtils.format(ConstantsMsg.GET_MESSAGES_FAILED, dto.getComponent(), dto.getLocale(), dataSource.toString()));
+		}
+
+		// Skip this block if timestamp is not 0 (which means cacheItem is in the cache) regardless if cacheItem is expired or not.
+		// Otherwise, try the next dataSource in the queue.
+		if (timestamp == 0) {
+			// Try the next dataSource in the queue
+			if (msgSourceQueueIter.hasNext()) {
+				refreshCacheItem(cacheItem, msgSourceQueueIter);
+				// If no more data source in queue, log the error. This means that neither online nor offline fetch succeeded.
+			} else {
+				logger.debug(FormatUtils.format(ConstantsMsg.GET_MESSAGES_FAILED_ALL, dto.getComponent(), dto.getLocale()));
+			}
+		}
+	}
 
 	/**
 	 * Get MessageCacheItem from cache.
@@ -100,44 +106,78 @@ public MessageCacheItem getMessages(Iterator<Locale> fallbackLocalesIter) {
     	if (cacheService.isContainComponent()) { // Item is in cache
     		cacheItem = cacheService.getCacheOfComponent();
     		if (cacheItem.isExpired()) { // cacheItem has expired
-    			// Update the cache in a separate thread
-    			populateCacheTask(cacheItem);
+    			// Refresh the cacheItem in a separate thread
+    			refreshCacheItemTask(cacheItem);
     		}
-    	} else { // Item is not in cache
-    		// Create a new cacheItem object to be stored in cache
-    		cacheItem = new MessageCacheItem();
-    		fetchMessages(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, 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()) {
-					cacheService.addCacheOfComponent(cacheItem);
-				}
+			// If the cacheItem is for a fallback locale, create and store cacheItem for the requested locale in a separate thread.
+    		if (!LocaleUtility.isSameLocale(cacheItem.getLocale(), this.dto.getLocale())) {
+				this.createCacheItemTask(fallbackLocalesIter);
 			}
+    	} else { // Item is not in cache. Create and store cacheItem for the requested locale
+    		cacheItem = createCacheItem(fallbackLocalesIter);
     	}
     	return cacheItem;
     }
-
-	private void populateCacheTask(MessageCacheItem cacheItem) {
+
+	/**
+	 * Creates a new MessageCacheItem for the DTO and stores it in cache.
+	 *
+	 * @param fallbackLocalesIter The fallback locale queue to use in case of failure.
+	 *
+	 */
+    private MessageCacheItem createCacheItem(Iterator<Locale> fallbackLocalesIter) {
+		CacheService cacheService = new CacheService(dto);
+		// Create a new cacheItem object to be stored in cache
+		MessageCacheItem cacheItem = new MessageCacheItem();
+		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 requetsed 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()) {
+				cacheService.addCacheOfComponent(cacheItem);
+			}
+		}
+		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 = () -> {
     		try {
-    			// Pass cacheItem to getMessages so that:
-				// 1. A previously stored etag, if any, can be used for the next HTTP request.
-				// 2. CacheItem properties such as etag, timestamp and maxAgeMillis can be refreshed 
-				// 	 with new properties from the next HTTP response.
-				fetchMessages(cacheItem, VIPCfg.getInstance().getMsgOriginsQueue().listIterator());
-    			return cacheItem;
+    			// 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;
 		    }
 		};
-		FutureTask<MessageCacheItem> task = new FutureTask<MessageCacheItem>(callable); 
+		FutureTask<MessageCacheItem> task = new FutureTask<>(callable);
 		Thread thread = new Thread(task);
 		thread.start();	
 	}