/hg/icedtea-web: Minor code enhancements: Enum for HEAD and GET,...

jvanek at icedtea.classpath.org jvanek at icedtea.classpath.org
Fri May 23 12:13:36 UTC 2014


changeset c97ba9284e7e in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=c97ba9284e7e
author: Jiri Vanek <jvanek at redhat.com>
date: Fri May 23 14:13:26 2014 +0200

	Minor code enhancements: Enum for HEAD and GET, urlutils classes moved to UrlUtil


diffstat:

 ChangeLog                                                           |  40 ++++-
 netx/net/sourceforge/jnlp/cache/CacheEntry.java                     |  40 +--
 netx/net/sourceforge/jnlp/cache/CacheUtil.java                      |  70 +-------
 netx/net/sourceforge/jnlp/cache/Resource.java                       |   3 +-
 netx/net/sourceforge/jnlp/cache/ResourceTracker.java                |  61 ++++---
 netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java          |  47 +++--
 netx/net/sourceforge/jnlp/util/UrlUtils.java                        |  80 ++++++++-
 tests/netx/unit/net/sourceforge/jnlp/cache/CacheUtilTest.java       |  15 +-
 tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTest.java        |   3 +-
 tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTrackerTest.java |  21 +-
 tests/netx/unit/net/sourceforge/jnlp/util/UrlUtilsTest.java         |  83 +++++++++-
 tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java        |  20 +-
 tests/test-extensions/net/sourceforge/jnlp/TinyHttpdImpl.java       |   5 +-
 13 files changed, 299 insertions(+), 189 deletions(-)

diffs (truncated from 916 to 500 lines):

diff -r 7703f77be921 -r c97ba9284e7e ChangeLog
--- a/ChangeLog	Fri May 23 13:38:14 2014 +0200
+++ b/ChangeLog	Fri May 23 14:13:26 2014 +0200
@@ -1,4 +1,42 @@
-2014-05-21  Jiri Vanek  <jvanek at redhat.com>
+2014-05-23  Jiri Vanek  <jvanek at redhat.com>
+
+	Minor code enhancements: Enum for HEAD and GET, urlutils classes moved to UrlUtils
+	* netx/net/sourceforge/jnlp/cache/CacheEntry.java: "*" imports replaced by full
+	ones location, version and properties made final. (isCurrent) rewritten
+	* netx/net/sourceforge/jnlp/cache/CacheUtil.java: (urlEquals) and (notNullUrlEquals)
+	and (compare) moved to UrlUtils. (getReadPermission) got javadoc. urlList, keep,
+	remove colelctions redeclared to diamond
+	* netx/net/sourceforge/jnlp/cache/Resource.java: adapted imports and calls to
+	CacheUtil UrlUtils change.
+	* netx/net/sourceforge/jnlp/cache/ResourceTracker.java: (requestMethods) hidden into
+	public inner enum of RequestMethods. prefetchTrackers, queue, downloadOptions, active,
+	resources, listeners marked final and redeclared with diamond. (getUrlResponseCode)
+	adapted to new enum, removed javadoc. (getUrlResponseCodeWithRedirectonResult) adapted
+	to new enum
+	* netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java: file group loader
+	appContext weakWindows weakWindows made final and redeclared with diamond. Added
+	override annotations. (shouldCreateShortcut) changed to switch.
+	* netx/net/sourceforge/jnlp/util/UrlUtils.java: used multi catch where possible.
+	urlEquals, notNullUrlEquals, compareNullableStrings moved from CacheUtils
+	* tests/netx/unit/net/sourceforge/jnlp/cache/CacheUtilTest.java: adapted to 
+	CacheUtils->UrlUtils method movement.
+	* tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTest.java: assertNotEquals
+	replaced by assertFalse and equals. Specific versions of JUnit have problems with
+	notEquals.
+	* tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTrackerTest.java: Adapted
+	to new enum.
+	* tests/netx/unit/net/sourceforge/jnlp/util/UrlUtilsTest.java: added testUrlEquals
+	verifyNotNullUrlEqualsThrowsExceptionWhenBothArgumentsAreNull
+	nverifyNotNullUrlEqualsThrowsExceptionWhenFirstArgumentIsNull
+	verifyNotNullUrlEqualsThrowsExceptionWhenSecondArgumentIsNull
+	notNullUrlValuesEqualsCaseSensitiveIssuesTest
+	notNullUrlComapreWithPorts (known to fail), testCompareNullableStrings and
+	testCompareNullableStrings tests
+	* tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java: removed unused
+	imports, used diamonds, removed dead code.
+	* tests/test-extensions/net/sourceforge/jnlp/TinyHttpdImpl.java: Adapted to new enum.
+
+2014-05-23  Jiri Vanek  <jvanek at redhat.com>
 
 	Minor javadoc enhancements
 	* netx/net/sourceforge/jnlp/cache/CacheEntry.java: added or filled some javadocs
diff -r 7703f77be921 -r c97ba9284e7e netx/net/sourceforge/jnlp/cache/CacheEntry.java
--- a/netx/net/sourceforge/jnlp/cache/CacheEntry.java	Fri May 23 13:38:14 2014 +0200
+++ b/netx/net/sourceforge/jnlp/cache/CacheEntry.java	Fri May 23 14:13:26 2014 +0200
@@ -16,14 +16,13 @@
 
 package net.sourceforge.jnlp.cache;
 
-import net.sourceforge.jnlp.util.logging.OutputController;
 import static net.sourceforge.jnlp.runtime.Translator.R;
 
-import java.io.*;
-import java.net.*;
-
-import net.sourceforge.jnlp.*;
-import net.sourceforge.jnlp.util.*;
+import java.io.File;
+import java.net.URL;
+import net.sourceforge.jnlp.Version;
+import net.sourceforge.jnlp.util.PropertiesFile;
+import net.sourceforge.jnlp.util.logging.OutputController;
 
 /**
  * Describes an entry in the cache.
@@ -41,13 +40,13 @@
     private static final String KEY_LAST_UPDATED = "last-updated";
 
     /** the remote resource location */
-    private URL location;
+    private final URL location;
 
     /** the requested version */
-    private Version version;
+    private final Version version;
 
     /** info about the cached file */
-    private PropertiesFile properties;
+    private final PropertiesFile properties;
 
     /**
      * Create a CacheEntry for the resources specified as a remote
@@ -152,30 +151,23 @@
 
     /**
      * Returns whether there is a version of the URL contents in
-     * the cache and it is up to date.  This method may not return
-     * immediately.
+     * the cache and it is up to date.
      *
-     * @param lastModified
+     * @param lastModified - current time as get from server (in ms). Mostly value of "Last-Modified" http header'? 
      * @return whether the cache contains the version
      */
     public boolean isCurrent(long lastModified) {
         boolean cached = isCached();
 
-        if (!cached)
+        if (!cached) {
             return false;
-
+        }
         try {
-            long remoteModified = lastModified;
             long cachedModified = Long.parseLong(properties.getProperty(KEY_LAST_MODIFIED));
-
-            if (remoteModified > 0 && remoteModified <= cachedModified)
-                return true;
-            else
-                return false;
-        } catch (Exception ex) {
-            OutputController.getLogger().log(ex);;
-
-            return cached; // if can't connect return whether already in cache
+            return lastModified > 0 && lastModified <= cachedModified;
+        } catch (Exception ex){
+            OutputController.getLogger().log(ex);
+            return cached;
         }
     }
 
diff -r 7703f77be921 -r c97ba9284e7e netx/net/sourceforge/jnlp/cache/CacheUtil.java
--- a/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Fri May 23 13:38:14 2014 +0200
+++ b/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Fri May 23 14:13:26 2014 +0200
@@ -68,49 +68,6 @@
     private static final HashMap<String, FileLock> propertiesLockPool = new HashMap<String, FileLock>();
 
     /**
-     * Compares a URL using string compare of its protocol, host,
-     * port, path, query, and anchor. This method avoids the host
-     * name lookup that URL.equals does for http: protocol URLs.
-     * It may not return the same value as the URL.equals method
-     * (different hostnames that resolve to the same IP address,
-     * ie sourceforge.net and www.sourceforge.net).
-     */
-    public static boolean urlEquals(URL u1, URL u2) {
-        if (u1 == u2) {
-            return true;
-        }
-        if (u1 == null || u2 == null) {
-            return false;
-        }
-
-        if (notNullUrlEquals(u1, u2)) {
-            return true;
-        }
-        try {
-            URL nu1 = UrlUtils.normalizeUrl(u1);
-            URL nu2 = UrlUtils.normalizeUrl(u2);
-            if (notNullUrlEquals(nu1, nu2)) {
-                return true;
-            }
-        } catch (Exception ex) {
-            //keep silent here and return false
-        }
-        return false;
-    }
-
-    private static boolean notNullUrlEquals(URL u1, URL u2) {
-        if (!compare(u1.getProtocol(), u2.getProtocol(), true)
-                || !compare(u1.getHost(), u2.getHost(), true)
-                || //u1.getDefaultPort() != u2.getDefaultPort() || // only in 1.4
-                !compare(u1.getPath(), u2.getPath(), false)
-                || !compare(u1.getQuery(), u2.getQuery(), false)
-                || !compare(u1.getRef(), u2.getRef(), false)) {
-            return false;
-        } else {
-            return true;
-        }
-    }
-    /**
      * Caches a resource and returns a URL for it in the cache;
      * blocks until resource is cached. If the resource location is
      * not cacheable (points to a local file, etc) then the original
@@ -133,26 +90,11 @@
     }
 
     /**
-     * Compare strings that can be {@code null}.
-     */
-    private static boolean compare(String s1, String s2, boolean ignore) {
-        if (s1 == s2)
-            return true;
-        if (s1 == null || s2 == null)
-            return false;
-
-        if (ignore)
-            return s1.equalsIgnoreCase(s2);
-        else
-            return s1.equals(s2);
-    }
-
-    /**
      * Returns the Permission object necessary to access the
      * resource, or {@code null} if no permission is needed.
-     * @param location
-     * @param version
-     * @return 
+     * @param location location of the resource
+     * @param version the version, or {@code null}
+     * @return permissions of the location
      */
     public static Permission getReadPermission(URL location, Version version) {
         Permission result = null;
@@ -540,7 +482,7 @@
                 return;
 
             // only resources not starting out downloaded are displayed
-            List<URL> urlList = new ArrayList<URL>();
+            List<URL> urlList = new ArrayList<>();
             for (URL url : resources) {
                 if (!tracker.checkResource(url))
                     urlList.add(url);
@@ -591,8 +533,8 @@
 
         if (okToClearCache()) {
             // First we want to figure out which stuff we need to delete.
-            HashSet<String> keep = new HashSet<String>();
-            HashSet<String> remove = new HashSet<String>();
+            HashSet<String> keep = new HashSet<>();
+            HashSet<String> remove = new HashSet<>();
             lruHandler.load();
             
             long maxSize = -1; // Default
diff -r 7703f77be921 -r c97ba9284e7e netx/net/sourceforge/jnlp/cache/Resource.java
--- a/netx/net/sourceforge/jnlp/cache/Resource.java	Fri May 23 13:38:14 2014 +0200
+++ b/netx/net/sourceforge/jnlp/cache/Resource.java	Fri May 23 14:13:26 2014 +0200
@@ -24,6 +24,7 @@
 import java.util.Set;
 
 import net.sourceforge.jnlp.Version;
+import net.sourceforge.jnlp.util.UrlUtils;
 import net.sourceforge.jnlp.util.WeakList;
 
 /**
@@ -430,7 +431,7 @@
             // time spent in synchronized addResource determining if
             // Resource is already in a tracker, and better for offline
             // mode on some OS.
-            return CacheUtil.urlEquals(location, ((Resource) other).location);
+            return UrlUtils.urlEquals(location, ((Resource) other).location);
         }
         return false;
     }
diff -r 7703f77be921 -r c97ba9284e7e netx/net/sourceforge/jnlp/cache/ResourceTracker.java
--- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java	Fri May 23 13:38:14 2014 +0200
+++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java	Fri May 23 14:13:26 2014 +0200
@@ -104,6 +104,17 @@
 
     // separately locks on (in order of aquire order, ie, sync on prefetch never syncs on lock):
     //   lock, prefetch, this.resources, each resource, listeners
+    public static enum RequestMethods{
+        HEAD, GET, TESTING_UNDEF;
+        
+    private static final RequestMethods[] requestMethods = {RequestMethods.HEAD, RequestMethods.GET};
+
+        public static RequestMethods[] getValidRequestMethods() {
+            return requestMethods;
+        }
+    
+    
+    }
 
     /** notified on initialization or download of a resource */
     private static final Object lock = new Object(); // used to lock static structures
@@ -111,31 +122,28 @@
     /** max threads */
     private static final int maxThreads = 5;
     
-    /** methods used to try individual URLs when choosing best*/
-    private static final String[] requestMethods = {"HEAD", "GET"};
-
     /** running threads */
     private static int threads = 0;
 
     /** weak list of resource trackers with resources to prefetch */
-    private static WeakList<ResourceTracker> prefetchTrackers =
-            new WeakList<ResourceTracker>();
+    private static final WeakList<ResourceTracker> prefetchTrackers =
+            new WeakList<>();
 
     /** resources requested to be downloaded */
-    private static ArrayList<Resource> queue = new ArrayList<Resource>();
+    private static final ArrayList<Resource> queue = new ArrayList<>();
 
-    private static ConcurrentHashMap<Resource, DownloadOptions> downloadOptions =
-        new ConcurrentHashMap<Resource, DownloadOptions>();
+    private static final ConcurrentHashMap<Resource, DownloadOptions> downloadOptions =
+        new ConcurrentHashMap<>();
 
     /** resource trackers threads are working for (used for load balancing across multi-tracker downloads) */
-    private static ArrayList<ResourceTracker> active =
-            new ArrayList<ResourceTracker>(); //
+    private final static ArrayList<ResourceTracker> active =
+            new ArrayList<>(); //
 
     /** the resources known about by this resource tracker */
-    private List<Resource> resources = new ArrayList<Resource>();
+    private final List<Resource> resources = new ArrayList<>();
 
     /** download listeners for this tracker */
-    private List<DownloadListener> listeners = new ArrayList<DownloadListener>();
+    private final List<DownloadListener> listeners = new ArrayList<>();
 
     /** whether to download parts before requested */
     private boolean prefetch;
@@ -871,24 +879,24 @@
             entry.unlock();
         }
     }
-    /**
-     * testing wrapper
-     *
-     * @param url
-     * @param requestProperties
-     * @param requestMethod
-     * @return
-     * @throws IOException
-     */
-    static int getUrlResponseCode(URL url, Map<String, String> requestProperties, String requestMethod) throws IOException {
+
+    
+    static int getUrlResponseCode(URL url, Map<String, String> requestProperties, RequestMethods requestMethod) throws IOException {
         return getUrlResponseCodeWithRedirectonResult(url, requestProperties, requestMethod).result;
     }
 
+    /**
+     * Complex wrapper around return code with utility methods
+     * Default is HTTP_OK
+     */
     private static class CodeWithRedirect {
 
         int result = HttpURLConnection.HTTP_OK;
         URL URL;
 
+        /**
+         *  @return  whether the result code is redirect one. Rigth now 301-303 and 307-308
+         */
         public boolean shouldRedirect() {
             return (result == 301
                     || result == 302
@@ -915,7 +923,7 @@
      * HttpURLConnection.HTTP_OK and null if not.
      * @throws IOException
      */
-    static CodeWithRedirect getUrlResponseCodeWithRedirectonResult(URL url, Map<String, String> requestProperties, String requestMethod) throws IOException {
+    static CodeWithRedirect getUrlResponseCodeWithRedirectonResult(URL url, Map<String, String> requestProperties, RequestMethods requestMethod) throws IOException {
         CodeWithRedirect result = new CodeWithRedirect();
         URLConnection connection = url.openConnection();
 
@@ -925,7 +933,7 @@
 
         if (connection instanceof HttpURLConnection) {
             HttpURLConnection httpConnection = (HttpURLConnection) connection;
-            httpConnection.setRequestMethod(requestMethod);
+            httpConnection.setRequestMethod(requestMethod.toString());
 
             int responseCode = httpConnection.getResponseCode();
 
@@ -975,7 +983,7 @@
          OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "All possible urls for "
                  + resource.toString() + " : " + urls);
         
-         for (String requestMethod : requestMethods) {
+         for (RequestMethods requestMethod : RequestMethods.getValidRequestMethods()) {
              for (int i = 0; i < urls.size(); i++) {
                  URL url = urls.get(i);
                  try {
@@ -1185,7 +1193,7 @@
     private Resource getResource(URL location) {
         synchronized (resources) {
             for (Resource resource : resources) {
-                if (CacheUtil.urlEquals(resource.getLocation(), location))
+                if (UrlUtils.urlEquals(resource.getLocation(), location))
                     return resource;
             }
         }
@@ -1293,6 +1301,7 @@
 
                     final Resource fResource = resource;
                     AccessController.doPrivileged(new PrivilegedAction<Void>() {
+                        @Override
                         public Void run() {
                             processResource(fResource);                            
                             return null;
diff -r 7703f77be921 -r c97ba9284e7e netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java
--- a/netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java	Fri May 23 13:38:14 2014 +0200
+++ b/netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java	Fri May 23 14:13:26 2014 +0200
@@ -56,13 +56,13 @@
     // installed by the application.
 
     /** the file */
-    private JNLPFile file;
+    private final JNLPFile file;
 
     /** the thread group */
-    private ThreadGroup group;
+    private final ThreadGroup group;
 
     /** the classloader */
-    private ClassLoader loader;
+    private final ClassLoader loader;
 
     /**
      * <p>
@@ -75,16 +75,16 @@
      * It is set to the AppContext which created this ApplicationInstance
      * </p>
      */
-    private AppContext appContext;
+    private final AppContext appContext;
 
     /** whether the application has stopped running */
     private boolean stopped = false;
 
     /** weak list of windows opened by the application */
-    private WeakList<Window> weakWindows = new WeakList<Window>();
+    private final WeakList<Window> weakWindows = new WeakList<>();
 
     /** list of application listeners  */
-    private EventListenerList listeners = new EventListenerList();
+    private final EventListenerList listeners = new EventListenerList();
 
     /** whether or not this application is signed */
     private boolean isSigned = false;
@@ -195,24 +195,27 @@
          * check configuration and possibly prompt user to find out if a
          * shortcut should be created or not
          */
-        if (currentSetting.equals(ShortcutDesc.CREATE_NEVER)) {
-            createShortcut = false;
-        } else if (currentSetting.equals(ShortcutDesc.CREATE_ALWAYS)) {
-            createShortcut = true;
-        } else if (currentSetting.equals(ShortcutDesc.CREATE_ASK_USER)) {
-            if (SecurityDialogs.showAccessWarningDialog(AccessType.CREATE_DESTKOP_SHORTCUT, file)) {
+        switch (currentSetting) {
+            case ShortcutDesc.CREATE_NEVER:
+                createShortcut = false;
+                break;
+            case ShortcutDesc.CREATE_ALWAYS:
                 createShortcut = true;
-            }
-        } else if (currentSetting.equals(ShortcutDesc.CREATE_ASK_USER_IF_HINTED)) {
-            if (sd != null && sd.onDesktop()) {
+                break;
+            case ShortcutDesc.CREATE_ASK_USER:
                 if (SecurityDialogs.showAccessWarningDialog(AccessType.CREATE_DESTKOP_SHORTCUT, file)) {
                     createShortcut = true;
-                }
-            }
-        } else if (currentSetting.equals(ShortcutDesc.CREATE_ALWAYS_IF_HINTED)) {
-            if (sd != null && sd.onDesktop()) {
-                createShortcut = true;
-            }
+                }   break;
+            case ShortcutDesc.CREATE_ASK_USER_IF_HINTED:
+                if (sd != null && sd.onDesktop()) {
+                    if (SecurityDialogs.showAccessWarningDialog(AccessType.CREATE_DESTKOP_SHORTCUT, file)) {
+                        createShortcut = true;
+                    }
+                }   break;
+            case ShortcutDesc.CREATE_ALWAYS_IF_HINTED:
+                if (sd != null && sd.onDesktop()) {
+                    createShortcut = true;
+                }   break;
         }
 
         return createShortcut;
@@ -225,6 +228,7 @@
      * application would have to close its windows and exit its
      * threads but not call JNLPRuntime.exit).
      */
+    @Override
     public void finalize() {
         destroy();
     }
@@ -246,6 +250,7 @@
         AccessControlContext acc = new AccessControlContext(new ProtectionDomain[] { pd });
 
         PrivilegedAction<Object> installProps = new PrivilegedAction<Object>() {
+            @Override
             public Object run() {
                 for (PropertyDesc propDesc : props) {
                     System.setProperty(propDesc.getKey(), propDesc.getValue());
diff -r 7703f77be921 -r c97ba9284e7e netx/net/sourceforge/jnlp/util/UrlUtils.java
--- a/netx/net/sourceforge/jnlp/util/UrlUtils.java	Fri May 23 13:38:14 2014 +0200
+++ b/netx/net/sourceforge/jnlp/util/UrlUtils.java	Fri May 23 14:13:26 2014 +0200
@@ -56,9 +56,7 @@
             String[] urlParts = url.toString().split("\\?");
             URL strippedUrl = new URL(urlParts[0]); 
             return normalizeUrl(strippedUrl, encodeFileUrls);
-        } catch (IOException e) {
-            OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
-        } catch (URISyntaxException e) {
+        } catch (IOException | URISyntaxException e) {


More information about the distro-pkg-dev mailing list