Changeset 10558 in josm for trunk

Timestamp:

2016-07-17T20:43:22+02:00 (8 years ago)

Author:

wiktorn

Message:

Do not cache IOException in JCS, they should be cached only in memory.

Caching IOException in JCS Cache leads to the situation, where errors
are cached for DEFAULT_EXPIRE_TIME which causes the error message to
stay too long on user screen.

Fix tests for JCSCachedTileLoaderJob

See: #13128

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJob.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJobTest.java (modified) (8 diffs)

trunk/src/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJob.java

@@ -391 +391 @@
             } else {
                 attributes.setError(e);
-                attributes.setResponseCode(499); // set dummy error code
-                boolean doCache = isResponseLoadable(null, 499, null) || cacheAsEmpty(); //generic 499 error code returned
-                if (doCache) {
-                    cacheData = createCacheEntry(new byte[]{});
-                    cache.put(getCacheKey(), createCacheEntry(new byte[]{}), attributes);
-                }
-                return doCache;
+                attributes.setResponseCode(599); // set dummy error code, greater than 500 so it will be not cached
+                return false;
             }
 

trunk/test/unit/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJobTest.java

@@ -3 +3 @@
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 
 import java.io.IOException;
@@ -13 +14 @@
 import org.junit.Test;
 import org.openstreetmap.josm.JOSMFixture;
+import org.openstreetmap.josm.data.cache.ICachedLoaderListener.LoadResult;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -23 +25 @@
     private static class TestCachedTileLoaderJob extends JCSCachedTileLoaderJob<String, CacheEntry> {
         private String url;
+        private String key;
 
-        TestCachedTileLoaderJob(String url) throws IOException {
+        TestCachedTileLoaderJob(String url, String key) throws IOException {
             super(getCache(), 30000, 30000, null);
+
             this.url = url;
-        }
-
-        private static ICacheAccess<String, CacheEntry> getCache() throws IOException {
-         return JCSCacheManager.getCache("test");
+            this.key = key;
         }
 
         @Override
         public String getCacheKey() {
-            return "cachekey" + url;
+            return key;
         }
 
@@ -56 +57 @@
         private CacheEntryAttributes attributes;
         private boolean ready;
+        private LoadResult result;
 
         @Override
@@ -61 +63 @@
             this.attributes = attributes;
             this.ready = true;
+            this.result = result;
             this.notifyAll();
         }
@@ -100 +103 @@
     @SuppressFBWarnings(value = "WA_NOT_IN_LOOP")
     public void testUnknownHost() throws IOException, InterruptedException {
-        TestCachedTileLoaderJob job = new TestCachedTileLoaderJob("http://unkownhost.unkownhost/unkown");
+        String key = "key_unknown_host";
+        TestCachedTileLoaderJob job = new TestCachedTileLoaderJob("http://unkownhost.unkownhost/unkown", key);
         Listener listener = new Listener();
         job.submit(listener, true);
@@ -109 +113 @@
         }
         assertEquals("java.net.UnknownHostException: unkownhost.unkownhost", listener.attributes.getErrorMessage());
+        assertEquals(LoadResult.FAILURE, listener.result); // because response will be cached, and that is checked below
+
+        ICacheAccess<String, CacheEntry> cache = getCache();
+        CacheEntry e = new CacheEntry(new byte[]{0,1,2,3});
+        CacheEntryAttributes attributes = new CacheEntryAttributes();
+        attributes.setExpirationTime(2);
+        cache.put(key, e, attributes);
+
+        job = new TestCachedTileLoaderJob("http://unkownhost.unkownhost/unkown", key);
+        listener = new Listener();
+        job.submit(listener, true);
+        synchronized (listener) {
+            if (!listener.ready) {
+                listener.wait();
+            }
+        }
+        assertEquals(LoadResult.SUCCESS, listener.result);
+        assertFalse(job.isCacheElementValid());
     }
 
@@ -125 +147 @@
 
     private static TestCachedTileLoaderJob getStatusLoaderJob(int responseCode) throws IOException {
-        return new TestCachedTileLoaderJob("http://httpstat.us/" + responseCode);
+        return new TestCachedTileLoaderJob("http://httpstat.us/" + responseCode, "key_" + responseCode);
+    }
+
+    private static ICacheAccess<String, CacheEntry> getCache() throws IOException {
+        return JCSCacheManager.getCache("test");
     }
 }