[RFC][icedtea-web]: DownloadService implementation

Saad Mohammad smohammad at redhat.com
Thu Oct 11 13:47:10 PDT 2012


Adam,

A very minor change below with comments.

[..snip...]
> +    /**
> +     * Manages DownloadService jars which are not mentioned in the JNLP file
> +     * @param ref Path to the resource.
> +     * @param version The version of resource. If null, no version is specified.
> +     * @param action The action to perform with the resource. Either DOWNLOADTOCACHE, REMOVEFROMCACHE, or CHECKCACHE.
> +     * @return true if CHECKCACHE and the resource is cached.
> +     */
> +    boolean manageExternalJars(URL ref, String version, DownloadAction action) {
> +        boolean approved = false;
> +        JNLPClassLoader foundLoader = LocateJnlpClassLoader.getLoaderByResourceUrl(this, ref, version);
> +        Version resourceVersion = (version == null) ? null : new Version(version);
> +
> +        if (foundLoader != null)
> +            approved = true;
> +
> +        else if (ref.toString().startsWith(file.getCodeBase().toString()))
> +            approved = true;
> +        else if (SecurityDesc.ALL_PERMISSIONS.equals(security.getSecurityType()))
> +            approved = true;
> +
> +        if (approved) {
> +            if (foundLoader == null)
> +                foundLoader = this;
> +
> +            if (action == DownloadAction.DOWNLOAD_TO_CACHE) {
> +                JARDesc jarToCache = new JARDesc(ref, resourceVersion, null, false, true, false, true);
> +                if (JNLPRuntime.isDebug())
> +                    System.out.println("Downloading and initializing jar: " + ref.toString());
> +
> +                foundLoader.addNewJar(jarToCache);

I didn't think there was a reason to repost the full patch for a one-line
change. As I mentioned on IRC, I found a small bug in the code above. When we
repeatedly load and unload external resources, the managed resource is not
cached correctly. Therefore, the line above should be changed to:

                  foundLoader.addNewJar(jarToCache, UpdatePolicy.FORCE);

I added additional tests to the reproducer to test this behaviour. I assume this
change is fine?

> +
> +            } else if (action == DownloadAction.REMOVE_FROM_CACHE) {
> +                JARDesc[] jarToRemove = { new JARDesc(ref, resourceVersion, null, false, true, false, true) };
> +                foundLoader.removeJars(jarToRemove);
> +
> +            } else if (action == DownloadAction.CHECK_CACHE) {
> +                return CacheUtil.isCached(ref, resourceVersion);
> +            }
> +        }
> +        return false;
> +    }
> +
> +    /**
>       * Decrements loader use count by 1
>       * 
>       * If count reaches 0, loader is removed from list of available loaders
> @@ -2305,4 +2421,6 @@
>              return null;
>          }
>      }
> +
> +
>  }

[..snip..]

Thanks for the reviews!

-- 
Cheers,
Saad Mohammad



More information about the distro-pkg-dev mailing list