Changeset 17505 in josm for trunk

Timestamp:

2021-02-21T13:23:35+01:00 (4 years ago)

Author:

wiktorn

Message:

Do not cache responses, that are not images.

When server returns content type and it's not an image, then do not put it into
cache and try to extract error message from response.

That way, errors will not be stored in on-disk cache and hopefully they will be
retried when user browses again to the same area (modulo MemoryTileCache).

See: #20443

Location:

trunk/src/org/openstreetmap/josm/data

Files:

• cache/JCSCachedTileLoaderJob.java (modified) (4 diffs)
• imagery/TMSCachedTileLoaderJob.java (modified) (2 diffs)

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

@@ -7 +7 @@
 import java.net.URL;
 import java.security.SecureRandom;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -187 +188 @@
      * Simple implementation. All errors should be cached as empty. Though some JDK (JDK8 on Windows for example)
      * doesn't return 4xx error codes, instead they do throw an FileNotFoundException or IOException
+     * @param responseCode
+     * @param headerFields
      *
      * @return true if we should put empty object into cache, regardless of what remote resource has returned
      */
-    protected boolean cacheAsEmpty() {
+    protected boolean cacheAsEmpty(Map<String, List<String>> headerFields, int responseCode) {
         return attributes.getResponseCode() < 500;
     }
@@ -371 +374 @@
                             getCacheKey(), raw.length, getUrl());
                     return true;
-                } else if (cacheAsEmpty()) {
+                } else if (cacheAsEmpty(urlConn.getHeaderFields(), urlConn.getResponseCode())) {
                     cacheData = createCacheEntry(new byte[]{});
                     cache.put(getCacheKey(), cacheData, attributes);
@@ -386 +389 @@
             attributes.setError(e);
             attributes.setException(e);
-            boolean doCache = isResponseLoadable(null, 404, null) || cacheAsEmpty();
+            boolean doCache = isResponseLoadable(null, 404, null) || cacheAsEmpty(Collections.emptyMap(), 404);
             if (doCache) {
                 cacheData = createCacheEntry(new byte[]{});

trunk/src/org/openstreetmap/josm/data/imagery/TMSCachedTileLoaderJob.java

@@ -135 +135 @@
             return false; // do no try to load data from no-tile at zoom, cache empty object instead
         }
+        if (isNotImage(headers, statusCode)) {
+            String message = detectErrorMessage( new String(content, StandardCharsets.UTF_8));
+            if (message != null && !message.isEmpty()) {
+                tile.setError(message);
+            }
+            return false;
+        }
         return super.isResponseLoadable(headers, statusCode, content);
     }
 
-    @Override
-    protected boolean cacheAsEmpty() {
-        return isNoTileAtZoom() || super.cacheAsEmpty();
+    private boolean isNotImage(Map<String, List<String>> headers, int statusCode) {
+        if (statusCode == 200 && headers.containsKey("Content-Type") && !headers.get("Content-Type").isEmpty()) {
+            String contentType = headers.get("Content-Type").stream().findAny().get();
+            if (contentType !=null && !contentType.startsWith("image")) {
+                Logging.warn("Image not returned for tile: " + url + " content type was: " + contentType);
+                // not an image - do not store response in cache, so next time it will be queried again from the server
+                return true;
+            }
+        }
+        return false;
+    }
+
+    @Override
+    protected boolean cacheAsEmpty(Map<String, List<String>> headerFields, int responseCode) {
+        if (isNotImage(headerFields, responseCode)) {
+            return false;
+        }
+        return isNoTileAtZoom() || super.cacheAsEmpty(headerFields, responseCode);
     }
 
@@ -211 +233 @@
 
     private void handleError(CacheEntryAttributes attributes) {
+        if (tile.hasError() && tile.getErrorMessage() != null) {
+            // tile has already set error message, don't overwrite it
+            return;
+        }
         if (attributes != null) {
             int httpStatusCode = attributes.getResponseCode();