Not storing anything in cache if response is neither 200 nor 304

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

@@ -44,16 +44,25 @@ public void getComponentMessages(MessageCacheItem cacheItem) {
         Map<String, Object> response = VIPCfg.getInstance().getVipService().getHttpRequester()
         		.request(url, ConstantsKeys.GET,null, headers);
 
-        if (response.get(URLUtils.HEADERS) != null)
-        	cacheItem.setEtag(URLUtils.createEtagString((Map<String, List<String>>) response.get(URLUtils.HEADERS)));
-        if (response.get(URLUtils.RESPONSE_TIMESTAMP) != null)
-        	cacheItem.setTimestamp((long) response.get(URLUtils.RESPONSE_TIMESTAMP) );
-        if (response.get(URLUtils.MAX_AGE_MILLIS) != null)
-        	cacheItem.setMaxAgeMillis((Long) response.get(URLUtils.MAX_AGE_MILLIS));
-        Map<String,String> messages = this.getMsgsJson(response);
-        if (messages != null) {
-        	cacheItem.addCachedData(messages);
-        }
+        Integer responseCode = (Integer) response.get(URLUtils.RESPONSE_CODE);
+
+        if (responseCode != null && (responseCode.equals(HttpURLConnection.HTTP_OK) || 
+        		responseCode.equals(HttpURLConnection.HTTP_NOT_MODIFIED))) {
+
+        	if (response.get(URLUtils.RESPONSE_TIMESTAMP) != null)
+	        	cacheItem.setTimestamp((long) response.get(URLUtils.RESPONSE_TIMESTAMP) );
+        	if (response.get(URLUtils.HEADERS) != null)
+	        	cacheItem.setEtag(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));
+
+        	if (responseCode.equals(HttpURLConnection.HTTP_OK)) {
+		        Map<String,String> messages = this.getMsgsJson(response);
+		        if (messages != null) {
+		        	cacheItem.addCachedData(messages);
+		        }
+        	}
+        } 
     }
 
     private JSONObject getMsgsJson(Map<String, Object> response) {

src/main/java/com/vmware/vipclient/i18n/messages/api/url/URLUtils.java

@@ -4,13 +4,10 @@
  */
 package com.vmware.vipclient.i18n.messages.api.url;
 
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
-import com.vmware.vipclient.i18n.base.HttpRequester;
-
 /**
  * 
  * Encapsulates some methods related to vIP Server.

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

@@ -44,7 +44,7 @@ public String getString(MessagesDTO dto) {
 
     		cacheOfComponent = populateCache(cacheService, dto, cacheItem);
 
-    		if (cacheOfComponent != null) {
+    		if (cacheOfComponent != null && !cacheOfComponent.isEmpty()) {
     			cacheService.addCacheOfComponent(cacheItem);
     		}
     	} 

src/test/java/com/vmware/vip/i18n/SharedComponentTest.java

@@ -74,9 +74,11 @@ public void testGetSharedModuleTranslation() {
         Cache c = TranslationCacheManager.getCache(VIPCfg.CACHE_L3);
         Map<String, MessageCacheItem> m = ((MessageCache) c).getCachedTranslationMap();
 
-        Assert.assertTrue(m.size() == 4);
-        Assert.assertTrue(m.containsKey("JavaclientTest_1.0.0_JAVA_false_#zh"));
-        Assert.assertTrue(m.containsKey("JavaclientTest1_2.0.0_JSP_false_#de"));
+        Assert.assertTrue(m.size() == 1);
+        // TODO Null values are not allowed to be stored in the cache anymore. 
+        // The following keys must have non-null values to be stored. 
+        //Assert.assertTrue(m.containsKey("JavaclientTest_1.0.0_JAVA_false_#zh"));
+        //Assert.assertTrue(m.containsKey("JavaclientTest1_2.0.0_JSP_false_#de"));
 
         Assert.assertTrue(m.containsKey("JavaclientTest_1.0.0_JAVA_false_#en-US"));
     }

src/test/java/com/vmware/vipclient/i18n/messages/service/CacheServiceTest.java

@@ -42,6 +42,36 @@ public void init() {
         dto.setLocale(locale.toLanguageTag());
     }
 
+    @Test
+    public void testCacheNoUpdateIfErrorResponse() {
+    	VIPCfg gc = VIPCfg.getInstance();
+        try {
+            gc.initialize("vipconfig");
+        } catch (VIPClientInitException e) {
+            logger.error(e.getMessage());
+        }
+    	gc.initializeVIPService();
+
+        Cache c = VIPCfg.getInstance().createTranslationCache(MessageCache.class);
+        TranslationCacheManager.cleanCache(c);
+        I18nFactory i18n = I18nFactory.getInstance(VIPCfg.getInstance());
+        TranslationMessage translation = (TranslationMessage) i18n.getMessageInstance(TranslationMessage.class);
+
+        dto.setProductID(VIPCfg.getInstance().getProductName());
+        dto.setVersion(VIPCfg.getInstance().getVersion());
+
+        // Explicitly set an empty string component to get an HTTP 404 from the service
+        String emptyComponent = "";
+    	dto.setComponent(emptyComponent);
+    	CacheService cs = new CacheService(dto);
+
+        // This triggers the first http call
+    	translation.getString(locale, emptyComponent, key, source, comment, args);
+
+    	MessageCacheItem cacheItem = cs.getCacheOfComponent();
+    	assertNull(cacheItem);
+    }
+
     @Test
     public void testExpireUsingCacheControlMaxAge() {
     	VIPCfg gc = VIPCfg.getInstance();
@@ -52,8 +82,8 @@ public void testExpireUsingCacheControlMaxAge() {
         }
     	gc.initializeVIPService();
 
-    	// Explicitly set this config to the default which is -1, as if the config property was not set.
-        // This is done so that the cache-control max age form the server response is used instead.
+    	// Explicitly set this config to the default which is 0, as if the config property was not set.
+        // This is done so that the cache-control max age from the server response is used instead.
         VIPCfg.getInstance().setCacheExpiredTime(0l);
 
         Cache c = VIPCfg.getInstance().createTranslationCache(MessageCache.class);
@@ -102,15 +132,14 @@ public void testExpireUsingCacheControlMaxAge() {
 
         // Give time for the separate thread to finish.
         try {
-			Thread.sleep(5000);
+			Thread.sleep(3000);
 		} catch (InterruptedException e) {
 			// TODO Auto-generated catch block
 			e.printStackTrace();
 		}
 
-        // Third request for the same message.
         // This should fetch messages and properties from cache 
-        translation.getString(locale, component, key, source, comment, args);
+        cacheItem = cs.getCacheOfComponent();
 
         // TODO Store response code in cache if we want to test 
         //responseCode = cacheItem.getResponseCode();        
@@ -119,6 +148,7 @@ public void testExpireUsingCacheControlMaxAge() {
         // The cached response time had been updated by the separate thread 
         // to the timestamp of the second response.  
         // This, in effect, extends the cache expiration.
+
         Long responseTime3 = cacheItem.getTimestamp();
         assertTrue(responseTime3 > responseTime); 
         assertTrue(cacheItem.getMaxAgeMillis() > 0l);

src/test/resources/mockserver/__files/body-http404-i18n-api-v2-translation-JavaclientTest-1.0.0-de.json

@@ -0,0 +1,7 @@
+{
+    "timestamp": 1586808206152,
+    "status": 404,
+    "error": "Not Found",
+    "message": "/i18n/api/v2/translation/products/ProductNotExist/versions/1.0.0/locales/fr/components/",
+    "path": "/i18n/api/v2/translation/products/ProductNotExist/versions/1.0.0/locales/fr/components/"
+}

src/test/resources/mockserver/mappings/mapping-http404-i18n-api-v2-translation-JavaclientTest-1.0.0.json

@@ -0,0 +1,19 @@
+{
+  "id" : "513afaa7-2693-3dd0-ac83-b40c6e165c08",
+  "request" : {
+    "url" : "/i18n/api/v2/translation/products/JavaClientTest/versions/1.0.0/locales/de/components/?pseudo=false",
+    "method" : "GET"
+  },
+  "response" : {
+    "status" : 404,
+    "bodyFileName" : "body-http404-i18n-api-v2-translation-JavaclientTest-1.0.0-de.json",
+    "headers" : {
+      "Content-Type" : "application/json",
+      "Date" : "Wed, 23 Oct 2019 08:01:31 GMT",
+      "Content-Encoding" : "gzip",
+      "Vary" : "accept-encoding",
+      "Transfer-Encoding" : "chunked"
+    }
+  },
+  "uuid" : "513afaa7-2693-3dd0-ac83-b40c6e165c08"
+}