Index: /trunk/src/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJob.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJob.java (revision 8432) +++ /trunk/src/org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJob.java (revision 8433) @@ -85,4 +85,5 @@ public static ThreadFactory getNamedThreadFactory(final String name) { return new ThreadFactory(){ + @Override public Thread newThread(Runnable r) { Thread t = Executors.defaultThreadFactory().newThread(r); @@ -185,5 +186,5 @@ if (first || force) { ensureCacheElement(); - if (!force && cacheElement != null && isCacheElementValid() && (isObjectLoadable())) { + if (!force && cacheElement != null && isCacheElementValid() && isObjectLoadable()) { // we got something in cache, and it's valid, so lets return it log.log(Level.FINE, "JCS - Returning object from cache: {0}", getCacheKey()); @@ -215,9 +216,11 @@ /** + * 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 * - * @return cache object as empty, regardless of what remote resource has returned (ex. based on headers) - */ - protected boolean cacheAsEmpty(Map> headers, int statusCode, byte[] content) { - return false; + * @return true if we should put empty object into cache, regardless of what remote resource has returned + */ + protected boolean cacheAsEmpty() { + return attributes.getResponseCode() < 500; } @@ -262,5 +265,4 @@ } - private void finishLoading(LoadResult result) { Set listeners = null; @@ -272,17 +274,7 @@ return; } - try { - for (ICachedLoaderListener l: listeners) { - l.loadingFinished(cacheData, attributes, result); - } - } catch (Exception e) { - log.log(Level.WARNING, "JCS - Error while loading object from cache: {0}; {1}", new Object[]{e.getMessage(), getUrl()}); - Main.warn(e); - for (ICachedLoaderListener l: listeners) { - l.loadingFinished(cacheData, attributes, LoadResult.FAILURE); - } - - } - + for (ICachedLoaderListener l: listeners) { + l.loadingFinished(cacheData, attributes, result); + } } @@ -362,6 +354,5 @@ byte[] raw = read(urlConn); - if (!cacheAsEmpty(urlConn.getHeaderFields(), urlConn.getResponseCode(), raw) && - raw != null && raw.length > 0) { + if (isResponseLoadable(urlConn.getHeaderFields(), urlConn.getResponseCode(), raw)) { // we need to check cacheEmpty, so for cases, when data is returned, but we want to store // as empty (eg. empty tile images) to save some space @@ -371,15 +362,32 @@ new Object[] {getCacheKey(), raw.length, getUrl()}); return true; - } else { + } else if (cacheAsEmpty()) { cacheData = createCacheEntry(new byte[]{}); cache.put(getCacheKey(), cacheData, attributes); log.log(Level.FINE, "JCS - Caching empty object {0}", getUrl()); return true; + } else { + log.log(Level.FINE, "JCS - failure during load - reponse is not loadable nor cached as empty"); + return false; } } } catch (FileNotFoundException e) { log.log(Level.FINE, "JCS - Caching empty object as server returned 404 for: {0}", getUrl()); - cache.put(getCacheKey(), createCacheEntry(new byte[]{}), attributes); - return handleNotFound(); + attributes.setResponseCode(404); + boolean doCache = isResponseLoadable(null, 404, null) || cacheAsEmpty(); + if (doCache) { + cacheData = createCacheEntry(new byte[]{}); + cache.put(getCacheKey(), cacheData, attributes); + } + return doCache; + } catch (IOException e) { + log.log(Level.FINE, "JCS - IOExecption during communication with server for: {0}", getUrl()); + 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; } catch (Exception e) { log.log(Level.WARNING, "JCS - Exception during download {0}", getUrl()); @@ -392,7 +400,20 @@ /** - * @return if we should treat this object as properly loaded - */ - protected abstract boolean handleNotFound(); + * Check if the object is loadable. This means, if the data will be parsed, and if this response + * will finish as successful retrieve. + * + * This simple implementation doesn't load empty response, nor client (4xx) and server (5xx) errors + * + * @param headerFields headers sent by server + * @param responseCode http status code + * @param raw data read from server + * @return true if object should be cached and returned to listener + */ + protected boolean isResponseLoadable(Map> headerFields, int responseCode, byte[] raw) { + if (raw == null || raw.length == 0 || responseCode >= 400) { + return false; + } + return true; + } protected abstract V createCacheEntry(byte[] content); Index: /trunk/src/org/openstreetmap/josm/data/imagery/TMSCachedTileLoaderJob.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/imagery/TMSCachedTileLoaderJob.java (revision 8432) +++ /trunk/src/org/openstreetmap/josm/data/imagery/TMSCachedTileLoaderJob.java (revision 8433) @@ -137,13 +137,16 @@ @Override - protected boolean cacheAsEmpty(Map> headers, int statusCode, byte[] content) { - // cacheAsEmpty is called for every successful download, so we can put - // metadata handling here + protected boolean isResponseLoadable(Map> headers, int statusCode, byte[] content) { attributes.setMetadata(tile.getTileSource().getMetadata(headers)); if (tile.getTileSource().isNoTileAtZoom(headers, statusCode, content)) { attributes.setNoTileAtZoom(true); - return true; - } - return false; + return false; // do no try to load data from no-tile at zoom, cache empty object instead + } + return super.isResponseLoadable(headers, statusCode, content); + } + + @Override + protected boolean cacheAsEmpty() { + return isNoTileAtZoom() || super.cacheAsEmpty(); } @@ -171,4 +174,5 @@ listeners = inProgress.remove(getCacheKey()); } + boolean status = result.equals(LoadResult.SUCCESS); try { @@ -194,4 +198,5 @@ if (!isNoTileAtZoom() && httpStatusCode >= 400) { tile.setError(tr("HTTP error {0} when loading tiles", httpStatusCode)); + status = false; } break; @@ -207,5 +212,5 @@ if (listeners != null) { // listeners might be null, if some other thread notified already about success for(TileLoaderListener l: listeners) { - l.tileLoadingFinished(tile, result.equals(LoadResult.SUCCESS)); + l.tileLoadingFinished(tile, status); } } @@ -259,14 +264,4 @@ } - @Override - protected boolean handleNotFound() { - if (tile.getSource().isNoTileAtZoom(null, 404, null)) { - tile.setError("No tile at this zoom level"); - tile.putValue("tile-info", "no-tile"); - return true; - } - return false; - } - /** * For TMS use BaseURL as settings discovery, so for different paths, we will have different settings (useful for developer servers)