[icedtea-web] RFC: add support for packEnabled and versionEnabled
Dr Andrew John Hughes
ahughes at redhat.com
Wed Feb 9 14:33:09 PST 2011
On 13:04 Wed 09 Feb , Omair Majid wrote:
> On 02/09/2011 11:42 AM, Dr Andrew John Hughes wrote:
> > On 15:42 Wed 26 Jan , Omair Majid wrote:
> >> Hi,
> >>
> >> The attached patch adds support for packEnabled and versionEnabled
> >> properties in JNLP files. This allows JNLP applications to use pack200
> >> compression and versioned jars without requiring special server
> >> configuration/software. It also fixes bug RH669942.
> >>
> >> The patch adds a new class DownloadOptions which specify download
> >> options for jars (currently just version and pack200 support). Another
> >> new class ResourceUrlCreator is used to get a list of valid urls for a
> >> resource. ResourceTracker now uses ResourceUrlCreator to get the list of
> >> valid urls for downloading a jar and then tries to find the best one.
> >> initializeResource() and downloadResource() both use this url.
> >> downloadResource() has been modified to look at the file extension as
> >> well as the content-encoding when deciding whether a file is compressed
> >> or not.
> >>
> >> The patch is much bigger than I had expected it to be. It also affects
> >> core functionality - downloading jars. I would appreciate any comments
> >> and suggestions for improvements.
> >>
> >> Thanks,
> >> Omair
> >>
> >> [1] https://bugzilla.redhat.com/show_bug.cgi?id=669942
> >
> > Comments inline.
> >
>
> Thanks for looking it over.
>
> >> diff -r 64da2a80df88 netx/net/sourceforge/jnlp/cache/ResourceTracker.java
> >> --- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java Tue Jan 25 10:19:20 2011 -0500
> >> +++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java Wed Jan 26 15:31:51 2011 -0500
> >> @@ -636,16 +648,16 @@
> >> String contentEncoding = con.getContentEncoding();
> >>
> >> if (JNLPRuntime.isDebug()) {
> >> - System.err.println("Content encoding for " + resource.location + ": "
> >> - + contentEncoding);
> >> + System.out.println("Downloading" + resource.location + " using " +
> >> + realLocation + " (encoding : " + contentEncoding + ")");
> >> +
> >
> > Shouldn't this still be System.err?
> >
>
> I like to send most debug output to System.out, but I dont think it's a
> big deal. I can change it if you like.
>
I'd prefer System.err but I could live with System.out as long as we're consistent.
Is there output on System.out that appears regardless of whether debugging is on or off?
If so, the debug dependent stuff should be on System.err so it can be captured separately.
> >> @@ -686,7 +698,8 @@
> >> inputStream.close();
> >> gzInputStream.close();
> >>
> >> - } else if (contentEncoding.equals("pack200-gzip")) {
> >> + } else if ("pack200-gzip".equals(contentEncoding) ||
> >> + realLocation.getPath().endsWith(".pack.gz")) {
> >> GZIPInputStream gzInputStream = new GZIPInputStream(new FileInputStream(
> >> CacheUtil.getCacheFile(downloadLocation, resource.downloadVersion)));
> >> InputStream inputStream = new BufferedInputStream(gzInputStream);
> >
> > I'm not sure these changes are necessary. Although you eliminate the null check, it does create
> > a lot of additional noise for no apparent change. Maybe something for a seperate patch?
> >
>
> No, it's actually required here. It's the simplest way I can think of to
> make the existing code that dealt with content-encoding also work for
> pack.gz files. Before, we would ask the server for foo.jar and the
> server could send it with content-encoding pack200-gzip (by actually
> sending foo.pack.gz). With this patch, we ask the server for foo.pack.gz
> directly. The server sends foo.pack.gz with null content encoding. We
> must deal with this in the exact same way as foo.jar with content
> encoding pack200-gzip.
>
> Notice these lines in the patch:
> - } else if (contentEncoding.equals("pack200-gzip")) {
> + } else if ("pack200-gzip".equals(contentEncoding) ||
> + realLocation.getPath().endsWith(".pack.gz")) {
>
> If realLocation.getPath().endsWith(".pack.gz") is true, then
> contentEncoding is null. Perhaps I should have made it more explicit.
You've snipped some of what I was commenting about. I agree the additional
conditional needs to be there but there's a lot of change in that area just
to flip the equals comparison which hides the actual if extension. The bit
above that you snipped is JUST a change in the equals and could be done
in a separate patch.
>
> >> @@ -777,27 +792,45 @@
> >> }
> >>
> >> /**
> >> - * Returns the versioned url for a resource
> >> - * @param resource the resource to get the url for
> >> + * Returns the best URL to use for downloading the resource
> >> + *
> >> + * @param resource the resource
> >> + * @return a URL or null
> >> */
> >> - private URL getVersionedResourceURL(Resource resource) {
> >> - String actualLocation = resource.location.getProtocol() + "://"
> >> - + resource.location.getHost();
> >> - if (resource.location.getPort() != -1) {
> >> - actualLocation += ":" + resource.location.getPort();
> >> + private URL findBestUrl(Resource resource) {
> >> + DownloadOptions options = downloadOptions.get(resource);
> >> + if (options == null) {
> >> + options = new DownloadOptions(false, false);
> >> }
> >> - actualLocation += resource.location.getPath();
> >> - if (resource.requestVersion != null
> >> -&& resource.requestVersion.isVersionId()) {
> >> - actualLocation += "?version-id=" + resource.requestVersion;
> >> +
> >> + List<URL> urls = new ResourceUrlCreator(resource, options).getUrls();
> >> + if (JNLPRuntime.isDebug()) {
> >> + System.out.println(urls);
> >
> > System.err? Possibly with some kind of description of what is being printed.
> >
>
> I have still left it as System.out, but added a better description.
>
> >> }
> >> - URL versionedURL;
> >> - try {
> >> - versionedURL = new URL(actualLocation);
> >> - } catch (MalformedURLException e) {
> >> - return resource.location;
> >> + URL bestUrl = null;
> >> + for (int i = 0; i< urls.size(); i++) {
> >> + URL url = urls.get(i);
> >> + try {
> >> + URLConnection connection = url.openConnection();
> >> + connection.addRequestProperty("Accept-Encoding", "pack200-gzip, gzip");
> >> + if (connection instanceof HttpURLConnection) {
> >> + HttpURLConnection con = (HttpURLConnection)connection;
> >> + int responseCode = con.getResponseCode();
> >> + if (responseCode == -1 || responseCode< 200 || responseCode>= 300) {
> >> + continue;
> >> + }
> >> + }
> >> + if (JNLPRuntime.isDebug()) {
> >> + System.out.println("best url for " + resource.toString() + " is " + url.toString());
> >> + }
> >> + bestUrl = url;
> >> + break;
> >> + } catch (IOException e) {
> >> + // continue
> >> + }
> >> }
> >> - return versionedURL;
> >> +
> >> + return bestUrl;
> >> }
> >>
> >> /**
> >> diff -r 64da2a80df88 netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java
> >> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> >> +++ b/netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java Wed Jan 26 15:31:51 2011 -0500
> >> @@ -0,0 +1,163 @@
> >> +/* ResourceUrlCreator.java
> >> + Copyright (C) 2011 Red Hat, Inc
> >> +
> >> +This file is part of IcedTea.
> >> +
> >> +IcedTea is free software; you can redistribute it and/or modify
> >> +it under the terms of the GNU General Public License as published by
> >> +the Free Software Foundation; either version 2, or (at your option)
> >> +any later version.
> >> +
> >> +IcedTea is distributed in the hope that it will be useful, but
> >> +WITHOUT ANY WARRANTY; without even the implied warranty of
> >> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >> +General Public License for more details.
> >> +
> >> +You should have received a copy of the GNU General Public License
> >> +along with IcedTea; see the file COPYING. If not, write to the
> >> +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> >> +02110-1301 USA.
> >> +
> >> +Linking this library statically or dynamically with other modules is
> >> +making a combined work based on this library. Thus, the terms and
> >> +conditions of the GNU General Public License cover the whole
> >> +combination.
> >> +
> >> +As a special exception, the copyright holders of this library give you
> >> +permission to link this library with independent modules to produce an
> >> +executable, regardless of the license terms of these independent
> >> +modules, and to copy and distribute the resulting executable under
> >> +terms of your choice, provided that you also meet, for each linked
> >> +independent module, the terms and conditions of the license of that
> >> +module. An independent module is a module which is not derived from
> >> +or based on this library. If you modify this library, you may extend
> >> +this exception to your version of the library, but you are not
> >> +obligated to do so. If you do not wish to do so, delete this
> >> +exception statement from your version. */
> >> +
> >> +package net.sourceforge.jnlp.cache;
> >> +
> >> +import java.net.MalformedURLException;
> >> +import java.net.URL;
> >> +import java.util.LinkedList;
> >> +import java.util.List;
> >> +
> >> +import net.sourceforge.jnlp.DownloadOptions;
> >> +
> >> +public class ResourceUrlCreator {
> >> +
> >> + protected final Resource resource;
> >> + protected final DownloadOptions downloadOptions;
> >> +
> >> + public ResourceUrlCreator(Resource resource, DownloadOptions downloadOptions) {
> >> + this.resource = resource;
> >> + this.downloadOptions = downloadOptions;
> >> + }
> >> +
> >> + /**
> >> + * Returns a list of URLs that the resources might be downloadable from.
> >> + * The Resources may not be downloadable from any of them. The returned order is the order
> >> + * the urls should be attempted in.
> >> + * @return
> >> + */
> >> + public List<URL> getUrls() {
> >> + List<URL> urls = new LinkedList<URL>();
> >> + URL url = null;
> >> +
> >> + if (downloadOptions.useExplicitPack()&& downloadOptions.useExplicitVersion()) {
> >> + url = getUrl(resource, true, true);
> >> + if (url != null) {
> >> + urls.add(url);
> >> + }
> >> + url = getUrl(resource, false, true);
> >> + if (url != null) {
> >> + urls.add(url);
> >> + }
> >> + url = getUrl(resource, true, false);
> >> + if (url != null) {
> >> + urls.add(url);
> >> + }
> >> + } else if (downloadOptions.useExplicitPack()) {
> >> + url = getUrl(resource, true, false);
> >> + if (url != null) {
> >> + urls.add(url);
> >> + }
> >> + } else if (downloadOptions.useExplicitVersion()) {
> >> + url = getUrl(resource, false, true);
> >> + if (url != null) {
> >> + urls.add(url);
> >> + }
> >> + }
> >> +
> >> + url = getVersionedUrlUsingQuery(resource);
> >> + urls.add(url);
> >> +
> >> + urls.add(resource.getLocation());
> >> +
> >> + return urls;
> >> + }
> >> +
> >> + /**
> >> + * Returns a url for the resource.
> >> + * @param resource the resource
> >> + * @param usePack whether the URL should point to the pack200 file
> >> + * @param useVersion whether the URL should be modified to include the version
> >> + * @return a URL for the resource or null if an appropraite URL can not be found
> >> + */
> >> + protected URL getUrl(Resource resource, boolean usePack, boolean useVersion) {
> >
> > getURL?
> >
>
> I have been naming variables/methods as *Url* for a while now, including
> multiple methods in this patch itself (look at the method above this
> one). The advice mentioned on
> http://www.ibm.com/developerworks/library/ws-tip-namingconv.html#h7
> makes sense to me. What do you think?
>
Sounds ok. It just looked odd at first glance.
> >> + if (!(usePack || useVersion)) {
> >> + throw new IllegalArgumentException("either pack200 or version required");
> >> + }
> >> +
> >> + String location = resource.getLocation().toString();
> >> + int lastSlash = resource.getLocation().toString().lastIndexOf('/');
> >> + if (lastSlash == -1) {
> >> + return resource.getLocation();
> >> + }
> >> + String filename = location.substring(lastSlash + 1);
> >> + if (useVersion&& resource.requestVersion != null) {
> >> + String parts[] = filename.split("\\.", 2);
> >> + String name = parts[0];
> >> + String extension = parts[1];
> >> + filename = name + "__V" + resource.requestVersion + "." + extension;
> >> + }
> >> + if (usePack) {
> >> + filename = filename.replace(".jar", ".pack.gz");
> >> + }
> >> +
> >> + location = location.substring(0, lastSlash + 1) + filename;
> >> + try {
> >> + URL newUrl = new URL(location);
> >> + return newUrl;
> >> + } catch (MalformedURLException e) {
> >> + return null;
> >> + }
> >> + }
> >> +
> >> + /**
> >> + * Returns the URL for a resource, relying on HTTP query for getting the
> >> + * right version
> >> + *
> >> + * @param resource the resource to get the url for
> >> + */
> >> + protected URL getVersionedUrlUsingQuery(Resource resource) {
> >> + String actualLocation = resource.getLocation().getProtocol() + "://"
> >> + + resource.getLocation().getHost();
> >> + if (resource.getLocation().getPort() != -1) {
> >> + actualLocation += ":" + resource.getLocation().getPort();
> >> + }
> >> + actualLocation += resource.getLocation().getPath();
> >> + if (resource.requestVersion != null
> >> +&& resource.requestVersion.isVersionId()) {
> >> + actualLocation += "?version-id=" + resource.requestVersion;
> >> + }
> >> + URL versionedURL;
> >> + try {
> >> + versionedURL = new URL(actualLocation);
> >> + } catch (MalformedURLException e) {
> >> + return resource.getLocation();
> >> + }
> >> + return versionedURL;
> >> + }
> >> +
> >> +}
> >> \ No newline at end of file
> >> diff -r 64da2a80df88 netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> >> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Tue Jan 25 10:19:20 2011 -0500
> >> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Wed Jan 26 15:31:51 2011 -0500
> >> @@ -1243,4 +1246,27 @@
> >> jarLocationSecurityMap.put(key, extLoader.jarLocationSecurityMap.get(key));
> >> }
> >> }
> >> +
> >> + private DownloadOptions getDownloadOptionsForJar(JARDesc jar) {
> >> + boolean usePack = false;
> >> + boolean useVersion = false;
> >> +
> >> + ResourcesDesc[] descs = file.getResourceDescs();
> >> + for (ResourcesDesc desc: descs) {
> >> + JARDesc[] jars = desc.getJARs();
> >> + for (JARDesc aJar: jars) {
> >> + if (jar == aJar) {
> >> + if (Boolean.valueOf(desc.getPropertiesMap().get("jnlp.packEnabled"))) {
> >> + usePack = true;
> >> + }
> >> + if (Boolean.valueOf(desc.getPropertiesMap().get("jnlp.versionEnabled"))) {
> >> + useVersion = true;
> >> + }
> >
> > Is Boolean.valueOf necessary? Won't autoboxing handle this?
> >
>
> propertiesMap is a Map<String,String>. It contains key,value pairs for
> system properties (eg "swing.useSystemFontSettings"). So autoboxing cant
> handle it.
Ah sorry, I assumed it was returning a Boolean.
>
> Any other thoughts or comments?
>
Just check the System.out/err stuff and separate the equals flipping into a separate patch.
> Cheers,
> Omair
> diff -r a6b173d91ad1 NEWS
> --- a/NEWS Wed Feb 09 10:02:46 2011 -0500
> +++ b/NEWS Wed Feb 09 12:57:54 2011 -0500
> @@ -11,6 +11,7 @@
> New in release 1.1 (2011-XX-XX):
> * NetX
> - Use Firefox's proxy settings if possible
> + - RH669942: javaws fails to download version/packed files (missing support for jnlp.packEnabled and jnlp.versionEnabled)
>
> New in release 1.0 (2010-XX-XX):
>
> diff -r a6b173d91ad1 netx/net/sourceforge/jnlp/DownloadOptions.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/netx/net/sourceforge/jnlp/DownloadOptions.java Wed Feb 09 12:57:54 2011 -0500
> @@ -0,0 +1,63 @@
> +/* DownloadOptions.java
> + Copyright (C) 2011 Red Hat, Inc
> +
> +This file is part of IcedTea.
> +
> +IcedTea is free software; you can redistribute it and/or modify
> +it under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 2, or (at your option)
> +any later version.
> +
> +IcedTea is distributed in the hope that it will be useful, but
> +WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with IcedTea; see the file COPYING. If not, write to the
> +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +02110-1301 USA.
> +
> +Linking this library statically or dynamically with other modules is
> +making a combined work based on this library. Thus, the terms and
> +conditions of the GNU General Public License cover the whole
> +combination.
> +
> +As a special exception, the copyright holders of this library give you
> +permission to link this library with independent modules to produce an
> +executable, regardless of the license terms of these independent
> +modules, and to copy and distribute the resulting executable under
> +terms of your choice, provided that you also meet, for each linked
> +independent module, the terms and conditions of the license of that
> +module. An independent module is a module which is not derived from
> +or based on this library. If you modify this library, you may extend
> +this exception to your version of the library, but you are not
> +obligated to do so. If you do not wish to do so, delete this
> +exception statement from your version. */
> +
> +package net.sourceforge.jnlp;
> +
> +public class DownloadOptions {
> +
> + private final boolean usePack200;
> + private final boolean useVersion;
> +
> + public DownloadOptions(boolean usePack, boolean useVersion) {
> + this.usePack200 = usePack;
> + this.useVersion = useVersion;
> + }
> +
> + public boolean useExplicitPack() {
> + return usePack200;
> + }
> +
> + public boolean useExplicitVersion() {
> + return useVersion;
> + }
> +
> + @Override
> + public String toString() {
> + return "use pack: " + usePack200 + "; use version: " + useVersion;
> + }
> +
> +}
> diff -r a6b173d91ad1 netx/net/sourceforge/jnlp/JNLPFile.java
> --- a/netx/net/sourceforge/jnlp/JNLPFile.java Wed Feb 09 10:02:46 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/JNLPFile.java Wed Feb 09 12:57:54 2011 -0500
> @@ -248,7 +248,7 @@
>
> try {
> ResourceTracker tracker = new ResourceTracker(false); // no prefetch
> - tracker.addResource(location, version, policy);
> + tracker.addResource(location, version, null, policy);
>
> return tracker.getInputStream(location);
> } catch (Exception ex) {
> @@ -362,7 +362,7 @@
> }
>
> /**
> - * Returns the information section of the JNLP file for the
> + * Returns the resources section of the JNLP file for the
> * specified locale, os, and arch.
> */
> public ResourcesDesc getResources(final Locale locale, final String os, final String arch) {
> @@ -392,6 +392,31 @@
> }
>
> /**
> + * Returns the resources section of the JNLP file as viewed
> + * through the default locale and the os.name and os.arch
> + * properties.
> + */
> + public ResourcesDesc[] getResourceDescs() {
> + return getResourcesDescs(defaultLocale, defaultOS, defaultArch);
> + }
> +
> + /**
> + * Returns the resources section of the JNLP file for the
> + * specified locale, os, and arch.
> + */
> + public ResourcesDesc[] getResourcesDescs(final Locale locale, final String os, final String arch) {
> + List<ResourcesDesc> matchingResources = new ArrayList<ResourcesDesc>();
> + for (ResourcesDesc rescDesc: resources) {
> + if (localMatches(locale, rescDesc.getLocales())
> + && stringMatches(os, rescDesc.getOS())
> + && stringMatches(arch, rescDesc.getArch())) {
> + matchingResources.add(rescDesc);
> + }
> + }
> + return matchingResources.toArray(new ResourcesDesc[0]);
> + }
> +
> + /**
> * Returns an object of one of the following types: AppletDesc,
> * ApplicationDesc and InstallerDesc
> */
> diff -r a6b173d91ad1 netx/net/sourceforge/jnlp/Launcher.java
> --- a/netx/net/sourceforge/jnlp/Launcher.java Wed Feb 09 10:02:46 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/Launcher.java Wed Feb 09 12:57:54 2011 -0500
> @@ -417,7 +417,7 @@
> IconDesc.SPLASH, preferredWidth, preferredHeight);
> if (splashImageURL != null) {
> ResourceTracker resourceTracker = new ResourceTracker(true);
> - resourceTracker.addResource(splashImageURL, file.getFileVersion(), updatePolicy);
> + resourceTracker.addResource(splashImageURL, file.getFileVersion(), null, updatePolicy);
> splashScreen = new JNLPSplashScreen(resourceTracker, null, null);
> splashScreen.setSplashImageURL(splashImageURL);
> if (splashScreen.isSplashScreenValid()) {
> diff -r a6b173d91ad1 netx/net/sourceforge/jnlp/cache/CacheUtil.java
> --- a/netx/net/sourceforge/jnlp/cache/CacheUtil.java Wed Feb 09 10:02:46 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/cache/CacheUtil.java Wed Feb 09 12:57:54 2011 -0500
> @@ -76,7 +76,7 @@
> */
> public static URL getCachedResource(URL location, Version version, UpdatePolicy policy) {
> ResourceTracker rt = new ResourceTracker();
> - rt.addResource(location, version, policy);
> + rt.addResource(location, version, null, policy);
> try {
> File f = rt.getCacheFile(location);
> // TODO: Should be toURI().toURL()
> diff -r a6b173d91ad1 netx/net/sourceforge/jnlp/cache/Resource.java
> --- a/netx/net/sourceforge/jnlp/cache/Resource.java Wed Feb 09 10:02:46 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/cache/Resource.java Wed Feb 09 12:57:54 2011 -0500
> @@ -67,6 +67,9 @@
> /** the remote location of the resource */
> URL location;
>
> + /** the location to use when downloading */
> + private URL downloadLocation;
> +
> /** the local file downloaded to */
> File localFile;
>
> @@ -96,6 +99,7 @@
> */
> private Resource(URL location, Version requestVersion, UpdatePolicy updatePolicy) {
> this.location = location;
> + this.downloadLocation = location;
> this.requestVersion = requestVersion;
> this.updatePolicy = updatePolicy;
> }
> @@ -130,6 +134,24 @@
> }
>
> /**
> + * Returns the URL to use for downloading the resource. This can be
> + * different from the original location since it may use a different
> + * file name to support versioning and compression
> + * @return the url to use when downloading
> + */
> + public URL getDownloadLocation() {
> + return downloadLocation;
> + }
> +
> + /**
> + * Set the url to use for downloading the resource
> + * @param location
> + */
> + public void setDownloadLocation(URL location) {
> + downloadLocation = location;
> + }
> +
> + /**
> * Returns the tracker that first created or monitored the
> * resource, or null if no trackers are monitoring the resource.
> */
> diff -r a6b173d91ad1 netx/net/sourceforge/jnlp/cache/ResourceTracker.java
> --- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java Wed Feb 09 10:02:46 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java Wed Feb 09 12:57:54 2011 -0500
> @@ -29,12 +29,15 @@
> import java.net.URL;
> import java.net.URLConnection;
> import java.util.ArrayList;
> +import java.util.HashMap;
> import java.util.List;
> +import java.util.Map;
> import java.util.jar.JarOutputStream;
> import java.util.jar.Pack200;
> import java.util.jar.Pack200.Unpacker;
> import java.util.zip.GZIPInputStream;
>
> +import net.sourceforge.jnlp.DownloadOptions;
> import net.sourceforge.jnlp.Version;
> import net.sourceforge.jnlp.event.DownloadEvent;
> import net.sourceforge.jnlp.event.DownloadListener;
> @@ -117,6 +120,9 @@
> /** resources requested to be downloaded */
> private static ArrayList<Resource> queue = new ArrayList<Resource>();
>
> + private static Map<Resource, DownloadOptions> downloadOptions =
> + new HashMap<Resource, DownloadOptions>();
> +
> /** resource trackers threads are working for (used for load balancing across multi-tracker downloads) */
> private static ArrayList<ResourceTracker> active =
> new ArrayList<ResourceTracker>(); //
> @@ -163,7 +169,7 @@
> * @param version the resource version
> * @param updatePolicy whether to check for updates if already in cache
> */
> - public void addResource(URL location, Version version, UpdatePolicy updatePolicy) {
> + public void addResource(URL location, Version version, DownloadOptions options, UpdatePolicy updatePolicy) {
> if (location == null)
> throw new IllegalArgumentException("location==null");
>
> @@ -177,6 +183,11 @@
> resources.add(resource);
> }
>
> + if (options == null) {
> + options = new DownloadOptions(false, false);
> + }
> + downloadOptions.put(resource, options);
> +
> // checkCache may take a while (loads properties file). this
> // should really be synchronized on resources, but the worst
> // case should be that the resource will be updated once even
> @@ -621,7 +632,8 @@
>
> try {
> // create out second in case in does not exist
> - URLConnection con = getVersionedResourceURL(resource).openConnection();
> + URL realLocation = resource.getDownloadLocation();
> + URLConnection con = realLocation.openConnection();
> con.addRequestProperty("Accept-Encoding", "pack200-gzip, gzip");
>
> con.connect();
> @@ -636,16 +648,16 @@
> String contentEncoding = con.getContentEncoding();
>
> if (JNLPRuntime.isDebug()) {
> - System.err.println("Content encoding for " + resource.location + ": "
> - + contentEncoding);
> + System.out.println("Downloading" + resource.location + " using " +
> + realLocation + " (encoding : " + contentEncoding + ")");
> +
> }
>
> - if (contentEncoding != null) {
> - if (contentEncoding.equals("gzip")) {
> - downloadLocation = new URL(downloadLocation.toString() + ".gz");
> - } else if (contentEncoding.equals("pack200-gzip")) {
> - downloadLocation = new URL(downloadLocation.toString() + ".pack.gz");
> - }
> + if ("gzip".equals(contentEncoding)) {
> + downloadLocation = new URL(downloadLocation.toString() + ".gz");
> + } else if ("pack200-gzip".equals(contentEncoding) ||
> + realLocation.getPath().endsWith(".pack.gz")) {
> + downloadLocation = new URL(downloadLocation.toString() + ".pack.gz");
> }
>
> InputStream in = new BufferedInputStream(con.getInputStream());
> @@ -668,8 +680,8 @@
> /*
> * If the file was compressed, uncompress it.
> */
> - if (contentEncoding != null) {
> - if (contentEncoding.equals("gzip")) {
> +
> + if ("gzip".equals(contentEncoding)) {
> GZIPInputStream gzInputStream = new GZIPInputStream(new FileInputStream(CacheUtil
> .getCacheFile(downloadLocation, resource.downloadVersion)));
> InputStream inputStream = new BufferedInputStream(gzInputStream);
> @@ -686,7 +698,8 @@
> inputStream.close();
> gzInputStream.close();
>
> - } else if (contentEncoding.equals("pack200-gzip")) {
> + } else if ("pack200-gzip".equals(contentEncoding) ||
> + realLocation.getPath().endsWith(".pack.gz")) {
> GZIPInputStream gzInputStream = new GZIPInputStream(new FileInputStream(
> CacheUtil.getCacheFile(downloadLocation, resource.downloadVersion)));
> InputStream inputStream = new BufferedInputStream(gzInputStream);
> @@ -701,7 +714,7 @@
> inputStream.close();
> gzInputStream.close();
> }
> - }
> +
>
> resource.changeStatus(DOWNLOADING, DOWNLOADED);
> synchronized (lock) {
> @@ -731,7 +744,9 @@
> File localFile = CacheUtil.getCacheFile(resource.location, resource.downloadVersion);
>
> // connect
> - URLConnection connection = getVersionedResourceURL(resource).openConnection(); // this won't change so should be okay unsynchronized
> + URL finalLocation = findBestUrl(resource);
> + resource.setDownloadLocation(finalLocation);
> + URLConnection connection = finalLocation.openConnection(); // this won't change so should be okay unsynchronized
> connection.addRequestProperty("Accept-Encoding", "pack200-gzip, gzip");
>
> int size = connection.getContentLength();
> @@ -777,27 +792,46 @@
> }
>
> /**
> - * Returns the versioned url for a resource
> - * @param resource the resource to get the url for
> + * Returns the best URL to use for downloading the resource
> + *
> + * @param resource the resource
> + * @return a URL or null
> */
> - private URL getVersionedResourceURL(Resource resource) {
> - String actualLocation = resource.location.getProtocol() + "://"
> - + resource.location.getHost();
> - if (resource.location.getPort() != -1) {
> - actualLocation += ":" + resource.location.getPort();
> + private URL findBestUrl(Resource resource) {
> + DownloadOptions options = downloadOptions.get(resource);
> + if (options == null) {
> + options = new DownloadOptions(false, false);
> }
> - actualLocation += resource.location.getPath();
> - if (resource.requestVersion != null
> - && resource.requestVersion.isVersionId()) {
> - actualLocation += "?version-id=" + resource.requestVersion;
> +
> + List<URL> urls = new ResourceUrlCreator(resource, options).getUrls();
> + if (JNLPRuntime.isDebug()) {
> + System.out.println("All possible urls for " +
> + resource.toString() + " : " + urls);
> }
> - URL versionedURL;
> - try {
> - versionedURL = new URL(actualLocation);
> - } catch (MalformedURLException e) {
> - return resource.location;
> + URL bestUrl = null;
> + for (int i = 0; i < urls.size(); i++) {
> + URL url = urls.get(i);
> + try {
> + URLConnection connection = url.openConnection();
> + connection.addRequestProperty("Accept-Encoding", "pack200-gzip, gzip");
> + if (connection instanceof HttpURLConnection) {
> + HttpURLConnection con = (HttpURLConnection)connection;
> + int responseCode = con.getResponseCode();
> + if (responseCode == -1 || responseCode < 200 || responseCode >= 300) {
> + continue;
> + }
> + }
> + if (JNLPRuntime.isDebug()) {
> + System.out.println("best url for " + resource.toString() + " is " + url.toString());
> + }
> + bestUrl = url;
> + break;
> + } catch (IOException e) {
> + // continue
> + }
> }
> - return versionedURL;
> +
> + return bestUrl;
> }
>
> /**
> diff -r a6b173d91ad1 netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java Wed Feb 09 12:57:54 2011 -0500
> @@ -0,0 +1,163 @@
> +/* ResourceUrlCreator.java
> + Copyright (C) 2011 Red Hat, Inc
> +
> +This file is part of IcedTea.
> +
> +IcedTea is free software; you can redistribute it and/or modify
> +it under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 2, or (at your option)
> +any later version.
> +
> +IcedTea is distributed in the hope that it will be useful, but
> +WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with IcedTea; see the file COPYING. If not, write to the
> +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +02110-1301 USA.
> +
> +Linking this library statically or dynamically with other modules is
> +making a combined work based on this library. Thus, the terms and
> +conditions of the GNU General Public License cover the whole
> +combination.
> +
> +As a special exception, the copyright holders of this library give you
> +permission to link this library with independent modules to produce an
> +executable, regardless of the license terms of these independent
> +modules, and to copy and distribute the resulting executable under
> +terms of your choice, provided that you also meet, for each linked
> +independent module, the terms and conditions of the license of that
> +module. An independent module is a module which is not derived from
> +or based on this library. If you modify this library, you may extend
> +this exception to your version of the library, but you are not
> +obligated to do so. If you do not wish to do so, delete this
> +exception statement from your version. */
> +
> +package net.sourceforge.jnlp.cache;
> +
> +import java.net.MalformedURLException;
> +import java.net.URL;
> +import java.util.LinkedList;
> +import java.util.List;
> +
> +import net.sourceforge.jnlp.DownloadOptions;
> +
> +public class ResourceUrlCreator {
> +
> + protected final Resource resource;
> + protected final DownloadOptions downloadOptions;
> +
> + public ResourceUrlCreator(Resource resource, DownloadOptions downloadOptions) {
> + this.resource = resource;
> + this.downloadOptions = downloadOptions;
> + }
> +
> + /**
> + * Returns a list of URLs that the resources might be downloadable from.
> + * The Resources may not be downloadable from any of them. The returned order is the order
> + * the urls should be attempted in.
> + * @return
> + */
> + public List<URL> getUrls() {
> + List<URL> urls = new LinkedList<URL>();
> + URL url = null;
> +
> + if (downloadOptions.useExplicitPack() && downloadOptions.useExplicitVersion()) {
> + url = getUrl(resource, true, true);
> + if (url != null) {
> + urls.add(url);
> + }
> + url = getUrl(resource, false, true);
> + if (url != null) {
> + urls.add(url);
> + }
> + url = getUrl(resource, true, false);
> + if (url != null) {
> + urls.add(url);
> + }
> + } else if (downloadOptions.useExplicitPack()) {
> + url = getUrl(resource, true, false);
> + if (url != null) {
> + urls.add(url);
> + }
> + } else if (downloadOptions.useExplicitVersion()) {
> + url = getUrl(resource, false, true);
> + if (url != null) {
> + urls.add(url);
> + }
> + }
> +
> + url = getVersionedUrlUsingQuery(resource);
> + urls.add(url);
> +
> + urls.add(resource.getLocation());
> +
> + return urls;
> + }
> +
> + /**
> + * Returns a url for the resource.
> + * @param resource the resource
> + * @param usePack whether the URL should point to the pack200 file
> + * @param useVersion whether the URL should be modified to include the version
> + * @return a URL for the resource or null if an appropraite URL can not be found
> + */
> + protected URL getUrl(Resource resource, boolean usePack, boolean useVersion) {
> + if (!(usePack || useVersion)) {
> + throw new IllegalArgumentException("either pack200 or version required");
> + }
> +
> + String location = resource.getLocation().toString();
> + int lastSlash = resource.getLocation().toString().lastIndexOf('/');
> + if (lastSlash == -1) {
> + return resource.getLocation();
> + }
> + String filename = location.substring(lastSlash + 1);
> + if (useVersion && resource.requestVersion != null) {
> + String parts[] = filename.split("\\.", 2);
> + String name = parts[0];
> + String extension = parts[1];
> + filename = name + "__V" + resource.requestVersion + "." + extension;
> + }
> + if (usePack) {
> + filename = filename.replace(".jar", ".pack.gz");
> + }
> +
> + location = location.substring(0, lastSlash + 1) + filename;
> + try {
> + URL newUrl = new URL(location);
> + return newUrl;
> + } catch (MalformedURLException e) {
> + return null;
> + }
> + }
> +
> + /**
> + * Returns the URL for a resource, relying on HTTP query for getting the
> + * right version
> + *
> + * @param resource the resource to get the url for
> + */
> + protected URL getVersionedUrlUsingQuery(Resource resource) {
> + String actualLocation = resource.getLocation().getProtocol() + "://"
> + + resource.getLocation().getHost();
> + if (resource.getLocation().getPort() != -1) {
> + actualLocation += ":" + resource.getLocation().getPort();
> + }
> + actualLocation += resource.getLocation().getPath();
> + if (resource.requestVersion != null
> + && resource.requestVersion.isVersionId()) {
> + actualLocation += "?version-id=" + resource.requestVersion;
> + }
> + URL versionedURL;
> + try {
> + versionedURL = new URL(actualLocation);
> + } catch (MalformedURLException e) {
> + return resource.getLocation();
> + }
> + return versionedURL;
> + }
> +
> +}
> \ No newline at end of file
> diff -r a6b173d91ad1 netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Wed Feb 09 10:02:46 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Wed Feb 09 12:57:54 2011 -0500
> @@ -45,6 +45,7 @@
> import java.util.jar.JarEntry;
> import java.util.jar.JarFile;
>
> +import net.sourceforge.jnlp.DownloadOptions;
> import net.sourceforge.jnlp.ExtensionDesc;
> import net.sourceforge.jnlp.JARDesc;
> import net.sourceforge.jnlp.JNLPFile;
> @@ -406,6 +407,7 @@
>
> tracker.addResource(jars[i].getLocation(),
> jars[i].getVersion(),
> + getDownloadOptionsForJar(jars[i]),
> jars[i].isCacheable() ? JNLPRuntime.getDefaultUpdatePolicy() : UpdatePolicy.FORCE
> );
> }
> @@ -696,7 +698,7 @@
> List<JARDesc> jars = new ArrayList<JARDesc>();
> JARDesc jarDesc = new JARDesc(new File(extractedJarLocation).toURL(), null, null, false, false, false, false);
> jars.add(jarDesc);
> - tracker.addResource(new File(extractedJarLocation).toURL(), null, null);
> + tracker.addResource(new File(extractedJarLocation).toURL(), null, null, null);
> signer.verifyJars(jars, tracker);
>
> if (signer.anyJarsSigned() && !signer.getAlreadyTrustPublisher()) {
> @@ -1007,6 +1009,7 @@
>
> tracker.addResource(desc.getLocation(),
> desc.getVersion(),
> + null,
> JNLPRuntime.getDefaultUpdatePolicy()
> );
>
> @@ -1247,4 +1250,27 @@
> jarLocationSecurityMap.put(key, extLoader.jarLocationSecurityMap.get(key));
> }
> }
> +
> + private DownloadOptions getDownloadOptionsForJar(JARDesc jar) {
> + boolean usePack = false;
> + boolean useVersion = false;
> +
> + ResourcesDesc[] descs = file.getResourceDescs();
> + for (ResourcesDesc desc: descs) {
> + JARDesc[] jars = desc.getJARs();
> + for (JARDesc aJar: jars) {
> + if (jar == aJar) {
> + if (Boolean.valueOf(desc.getPropertiesMap().get("jnlp.packEnabled"))) {
> + usePack = true;
> + }
> + if (Boolean.valueOf(desc.getPropertiesMap().get("jnlp.versionEnabled"))) {
> + useVersion = true;
> + }
> + }
> + }
> + }
> +
> + return new DownloadOptions(usePack, useVersion);
> + }
> +
> }
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list