[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