Merge pull request #906 from AdoptOpenJDK/improve-cache-key

Improve cache key

Author: sclassen

core/src/main/java/net/adoptopenjdk/icedteaweb/client/parts/browser/HtmlBrowserPanel.java

@@ -178,7 +178,7 @@ private void put(URL url, List<URL> where) {
             if (url != null) {
                 if (where.isEmpty()) {
                     where.add(0, url);
-                } else if (!where.get(0).equals(url)) {
+                } else if (!where.get(0).toString().equals(url.toString())) {
                     where.add(0, url);
                 }
             }

core/src/main/java/net/adoptopenjdk/icedteaweb/resources/cache/CacheImpl.java

@@ -214,6 +214,10 @@ boolean isCached(CacheKey key) {
      */
     boolean isUpToDate(CacheKey key, long lastModified) {
         final Boolean isUpToDate = getResourceInfo(key)
+                .map(cachedFile -> {
+                    LOG.debug("found {} for {}", cachedFile.getCacheKey(), key);
+                    return cachedFile;
+                })
                 .map(cachedFile -> cachedFile.isCurrent(lastModified))
                 .orElse(false);
         LOG.info("isUpToDate: {} = {}", key, isUpToDate);

core/src/main/java/net/adoptopenjdk/icedteaweb/resources/cache/CacheIndexEntry.java

@@ -50,7 +50,7 @@ long getLastAccessed() {
     }
 
     boolean matches(URL resource) {
-        return key.getLocation().equals(resource);
+        return key.matches(resource);
     }
 
     boolean matches(CacheKey key) {

core/src/main/java/net/adoptopenjdk/icedteaweb/resources/cache/CacheKey.java

@@ -10,10 +10,12 @@
 class CacheKey {
 
     private final URL location;
+    private final String locationString;
     private final VersionId version;
 
     public CacheKey(final URL location, final VersionId version) {
         this.location = requireNonNull(location, "location");
+        this.locationString = location.toString();
         this.version = version;
     }
 
@@ -28,7 +30,7 @@ public VersionId getVersion() {
     @Override
     public String toString() {
         return "CacheKey{" +
-                "location=" + location +
+                "location=" + locationString +
                 ", version=" + version +
                 '}';
     }
@@ -38,11 +40,15 @@ public boolean equals(final Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         CacheKey cacheKey = (CacheKey) o;
-        return location.equals(cacheKey.location) && Objects.equals(version, cacheKey.version);
+        return Objects.equals(locationString, cacheKey.locationString) && Objects.equals(version, cacheKey.version);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(location, version);
+        return Objects.hash(locationString, version);
+    }
+
+    public boolean matches(URL resource) {
+        return resource != null && locationString.equals(resource.toString());
     }
 }

core/src/main/java/net/sourceforge/jnlp/Launcher.java

@@ -309,7 +309,7 @@ private JNLPFile fromUrl(URL location) throws LaunchException {
             }
             if (!isLocal && haveHref) {
                 //this is case when remote file have href to different file
-                if (!location.equals(file.getSourceLocation())) {
+                if (file.getSourceLocation() == null || !location.toString().equals(file.getSourceLocation().toString())) {
                     //mark local true, so the following condition will be true and
                     //new jnlp file will be downloaded
                     isLocal = true;

core/src/main/java/net/sourceforge/jnlp/runtime/classloader/JNLPClassLoader.java

@@ -490,7 +490,7 @@ private static JNLPClassLoader getInstance(JNLPFile file, UpdatePolicy policy, S
             // for this codebase/jnlp yet. Create one.
             if (baseLoader == null
                     || (file.isApplication()
-                    && !baseLoader.getJNLPFile().getFileLocation().equals(file.getFileLocation()))) {
+                    && (file.getFileLocation() == null || !baseLoader.getJNLPFile().getFileLocation().toString().equals(file.getFileLocation().toString())))) {
 
                 loader = createInstance(file, policy, mainName, enableCodeBase);
             } else {
@@ -536,7 +536,7 @@ private static JNLPClassLoader getInstance(final URL location, final String uniq
         synchronized (getUniqueKeyLock(uniqueKey)) {
             loader = uniqueKeyToLoader.get(uniqueKey);
 
-            if (loader == null || !location.equals(loader.getJNLPFile().getFileLocation())) {
+            if (loader == null || loader.getJNLPFile().getFileLocation() == null || !location.toString().equals(loader.getJNLPFile().getFileLocation().toString())) {
                 final JNLPFile jnlpFile = new JNLPFileFactory().create(location, uniqueKey, version, settings, policy);
 
                 loader = getInstance(jnlpFile, policy, mainName, enableCodeBase);

core/src/main/java/net/sourceforge/jnlp/runtime/classloader/LocateJnlpClassLoader.java

@@ -52,16 +52,17 @@ class LocateJnlpClassLoader {
      */
     static JNLPClassLoader getLoaderByJnlpFile(final JNLPClassLoader rootClassLoader, URL urlToJnlpFile) {
 
-        if (rootClassLoader == null)
+        if (rootClassLoader == null) {
             return null;
+        }
 
         JNLPFile file = rootClassLoader.getJNLPFile();
 
-        if (urlToJnlpFile == null)
+        if (urlToJnlpFile == null) {
             urlToJnlpFile = rootClassLoader.getJNLPFile().getFileLocation();
-
-        if (file.getFileLocation().equals(urlToJnlpFile))
+        } else if (file.getFileLocation().toString().equals(urlToJnlpFile.toString())) {
             return rootClassLoader;
+        }
 
         for (JNLPClassLoader loader : rootClassLoader.getLoaders()) {
             if (rootClassLoader != loader) {
@@ -89,7 +90,8 @@ static JNLPClassLoader getLoaderByResourceUrl(final JNLPClassLoader rootClassLoa
             ResourcesDesc resources = loader.getJNLPFile().getResources();
 
             for (JARDesc eachJar : resources.getJARs()) {
-                if (ref.equals(eachJar.getLocation()) &&
+                if (eachJar.getLocation() != null &&
+                        ref.toString().equals(eachJar.getLocation().toString()) &&
                         (resourceVersion == null || resourceVersion.equals(eachJar.getVersion())))
                     return loader;
             }

core/src/test/java/net/adoptopenjdk/icedteaweb/resources/ResourceTrackerTest.java

@@ -82,10 +82,10 @@ public void testNormalizeUrl() throws Exception {
         Assert.assertNull("first url should be null", u[0]);
         Assert.assertNull("first normalized url should be null", n[0]);
         for (int i = 1; i < CHANGE_BORDER; i++) {
-            Assert.assertTrue("url " + i + " must be equals too normalized url " + i, u[i].equals(n[i]));
+            Assert.assertTrue("url " + i + " must be equals too normalized url " + i, u[i].toString().equals(n[i].toString()));
         }
         for (int i = CHANGE_BORDER; i < n.length; i++) {
-            Assert.assertFalse("url " + i + " must be normalized (and so not equals) too normalized url " + i, u[i].equals(n[i]));
+            Assert.assertFalse("url " + i + " must be normalized (and so not equals) too normalized url " + i, u[i].toString().equals(n[i].toString()));
         }
     }