[icedtea-web] RFC: PR1533: Inherit jnlp.packEnabled and jnlp.versionEnabled like other properties

Jiri Vanek jvanek at redhat.com
Mon Sep 9 06:49:53 PDT 2013


On 09/06/2013 07:46 PM, Omair Majid wrote:
> Hi,
>
> The attached patch fixes the inheritance of jnlp.packEnabled and
> jnlp.versionEnabled properties as seen by JNLPFile for the purposes of
> DownloadOptions. If one of those two properties is specified anywhere
> acceptable (in a general or os- or arch-specific section), it is applied
> to all jars.
>
> Okay to push?

Looks like yes.

except the nit on  bottom,
ok head + 1.4
>
> Thanks,
> Omair
> -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F
> 6648 4681
>
>
> pr1533-02.patch
>
>
> diff --git a/ChangeLog b/ChangeLog
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,18 @@
> +2013-09-06  Omair Majid<omajid at redhat.com>
> +
> +	* netx/net/sourceforge/jnlp/JNLPFile.java
> +	(getDownloadOptionsForJar): Rename to ...
> +	(getDownloadOptions): New method. Look up jnlp.packEnabled and
> +	jnlp.versionEnabled in any resources element.
> +	* netx/net/sourceforge/jnlp/PluginBridge.java
> +	(getDownloadOptionsForJar): Rename to ...
> +	(getDownloadOptions): New method.
> +	* netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> +	(initializeResources): Invoke file.getDownloadResources.
> +	(getDownloadOptionsForJar): Remove.
> +	* tests/netx/unit/net/sourceforge/jnlp/JNLPFileTest.java
> +	(testDownloadOptions): New method.
> +
>   2013-09-04  Andrew Azores<aazores at redhat.com>
>
>   	* netx/net/sourceforge/jnlp/config/Defaults.java: (USER_CACHE_HOME) made
> diff --git a/netx/net/sourceforge/jnlp/JNLPFile.java b/netx/net/sourceforge/jnlp/JNLPFile.java
> --- a/netx/net/sourceforge/jnlp/JNLPFile.java
> +++ b/netx/net/sourceforge/jnlp/JNLPFile.java
> @@ -20,7 +20,6 @@
>
>   import java.io.IOException;
>   import java.io.InputStream;
> -import java.io.Reader;
>   import java.net.URL;
>   import java.util.ArrayList;
>   import java.util.Arrays;
> @@ -791,29 +790,17 @@
>       }
>
>       /**
> -     * XXX: this method does a "==" comparison between the input JARDesc and
> -     * jars it finds through getResourcesDescs(). If ever the implementation
> -     * of that function should change to return copies of JARDescs objects,
> -     * then the "jar == aJar" comparison below should change accordingly.
> -     * @param jar the jar whose download options to get.
> -     * @return the download options.
> +     * @return the download options to use for downloading jars listed in this jnlp file.
>        */
> -    public DownloadOptions getDownloadOptionsForJar(JARDesc jar) {
> +    public DownloadOptions getDownloadOptions() {
>           boolean usePack = false;
>           boolean useVersion = false;
> -        ResourcesDesc[] descs = getResourcesDescs();
> -        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;
> -                    }
> -                }
> -            }
> +        ResourcesDesc desc = getResources();
> +        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);
>       }
> diff --git a/netx/net/sourceforge/jnlp/PluginBridge.java b/netx/net/sourceforge/jnlp/PluginBridge.java
> --- a/netx/net/sourceforge/jnlp/PluginBridge.java
> +++ b/netx/net/sourceforge/jnlp/PluginBridge.java
> @@ -228,7 +228,7 @@
>        * {@inheritDoc }
>        */
>       @Override
> -    public DownloadOptions getDownloadOptionsForJar(JARDesc jar) {
> +    public DownloadOptions getDownloadOptions() {
>           return new DownloadOptions(usePack, useVersion);
>       }
>
> diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> @@ -626,7 +626,7 @@
>
>               tracker.addResource(jars[i].getLocation(),
>                                   jars[i].getVersion(),
> -                                getDownloadOptionsForJar(jars[i]),
> +                                file.getDownloadOptions(),
>                                   jars[i].isCacheable() ? JNLPRuntime.getDefaultUpdatePolicy() : UpdatePolicy.FORCE
>                                  );
>           }
> @@ -1966,10 +1966,6 @@
>           }
>       }
>
> -    private DownloadOptions getDownloadOptionsForJar(JARDesc jar) {
> -        return file.getDownloadOptionsForJar(jar);
> -    }
> -
>       /**
>        * Returns a set of paths that indicate the Class-Path entries in the
>        * manifest file. The paths are rooted in the same directory as the
> diff --git a/tests/netx/unit/net/sourceforge/jnlp/JNLPFileTest.java b/tests/netx/unit/net/sourceforge/jnlp/JNLPFileTest.java
> --- a/tests/netx/unit/net/sourceforge/jnlp/JNLPFileTest.java
> +++ b/tests/netx/unit/net/sourceforge/jnlp/JNLPFileTest.java
> @@ -45,6 +45,7 @@
>   import java.util.Map;
>
>   import net.sourceforge.jnlp.JNLPFile.Match;
> +import net.sourceforge.jnlp.annotations.Bug;
>   import net.sourceforge.jnlp.mock.MockJNLPFile;
>
>   import org.junit.Assert;
> @@ -208,4 +209,39 @@
>           Assert.assertEquals("os2", properties.get("os"));
>           Assert.assertEquals("arch2", properties.get("arch"));
>       }
> +
> +    @Bug(id={"PR1533"})
> +    @Test
> +    public void testDownloadOptions() throws MalformedURLException, ParseException {
> +        String os = System.getProperty("os.name");
> +        String arch = System.getProperty("os.arch");
> +
> +        String jnlpContents = "<?xml version='1.0'?>\n" +
> +                "<jnlp spec='1.5' href='foo' codebase='bar'>\n" +
> +                "  <information>\n" +
> +                "    <title>Parsing Test</title>\n" +
> +                "    <vendor>IcedTea</vendor>\n" +
> +                "    <offline-allowed/>\n" +
> +                "  </information>\n" +
> +                "  <resources>\n" +
> +                "    <property name='jnlp.packEnabled' value='false'/>" +
> +                "    <property name='jnlp.versionEnabled' value='false'/>" +
> +                "  </resources>\n" +
> +                "  <resources os='" + os + "'>" +
> +                "    <property name='jnlp.packEnabled' value='true'/>" +
> +                "  </resources>\n" +
> +                "  <resources arch='" + arch + "'>" +
> +                "    <property name='jnlp.versionEnabled' value='true'/>" +
> +                "  </resources>\n" +
> +                "  <installer-desc/>\n" +
> +                "</jnlp>";
> +
> +        URL codeBase = new URL("http://icedtea.classpath.org");
> +        InputStream is = new ByteArrayInputStream(jnlpContents.getBytes());
> +        JNLPFile jnlpFile = new JNLPFile(is, codeBase, new ParserSettings(false,false,false));
> +        DownloadOptions downloadOptions = jnlpFile.getDownloadOptions();
> +
> +        Assert.assertTrue(downloadOptions.useExplicitPack());
> +        Assert.assertTrue(downloadOptions.useExplicitVersion());


Maybe add test which will  set different os and arch, and and will assert false on above?
> +    }
>   }
>




More information about the distro-pkg-dev mailing list