/hg/icedtea-web: Resource status field refactored to enum

aazores at icedtea.classpath.org aazores at icedtea.classpath.org
Wed May 14 19:05:25 UTC 2014


changeset d96760e31283 in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=d96760e31283
author: Andrew Azores <aazores at redhat.com>
date: Wed May 14 15:04:16 2014 -0400

	Resource status field refactored to enum

	2014-05-14  Andrew Azores  <aazores at redhat.com>

	    * netx/net/sourceforge/jnlp/cache/Resource.java: (Status) new enum
	    replacing int bitfield statuses. (transferred, size) made volatile for
	    atomic read/write. (isSet, getStatusString, changeStatus) refactored for
	    Status enum. (hasFlags, setStatusFlag, setStatusFlags, unsetStatusFlag,
	    resetStatus, isInitialized) new methods. (hashCode) newly overridden since
	    equals was already overridden.
	    * netx/net/sourceforge/jnlp/cache/ResourceTracker.java: all references to
	    Resource int bitfield status refactored. (selectByFilter) new method since
	    UNINITIALIZED is no longer an actual flag in Resource Status, allows for
	    filtering by uninitialized resources anyway.
	    * tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTest.java: refactored
	    for Status enum
	    * tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTrackerTest.java:
	    refactored for Status enum and selectByFilter


diffstat:

 ChangeLog                                                           |   17 +
 netx/net/sourceforge/jnlp/cache/Resource.java                       |  196 ++++++---
 netx/net/sourceforge/jnlp/cache/ResourceTracker.java                |  134 ++++--
 tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTest.java        |   82 +--
 tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTrackerTest.java |   48 +-
 5 files changed, 294 insertions(+), 183 deletions(-)

diffs (truncated from 885 to 500 lines):

diff -r a784a0f1f821 -r d96760e31283 ChangeLog
--- a/ChangeLog	Tue May 13 12:21:07 2014 -0400
+++ b/ChangeLog	Wed May 14 15:04:16 2014 -0400
@@ -1,3 +1,20 @@
+2014-05-14  Andrew Azores  <aazores at redhat.com>
+
+	* netx/net/sourceforge/jnlp/cache/Resource.java: (Status) new enum
+	replacing int bitfield statuses. (transferred, size) made volatile for
+	atomic read/write. (isSet, getStatusString, changeStatus) refactored for
+	Status enum. (hasFlags, setStatusFlag, setStatusFlags, unsetStatusFlag,
+	resetStatus, isInitialized) new methods. (hashCode) newly overridden since
+	equals was already overridden.
+	* netx/net/sourceforge/jnlp/cache/ResourceTracker.java: all references to
+	Resource int bitfield status refactored. (selectByFilter) new method since
+	UNINITIALIZED is no longer an actual flag in Resource Status, allows for
+	filtering by uninitialized resources anyway.
+	* tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTest.java: refactored
+	for Status enum
+	* tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTrackerTest.java:
+	refactored for Status enum and selectByFilter
+
 2014-05-13  Omair Majid  <omajid at redhat.com>
 
 	* netx/net/sourceforge/jnlp/cache/CacheEntry.java
diff -r a784a0f1f821 -r d96760e31283 netx/net/sourceforge/jnlp/cache/Resource.java
--- a/netx/net/sourceforge/jnlp/cache/Resource.java	Tue May 13 12:21:07 2014 -0400
+++ b/netx/net/sourceforge/jnlp/cache/Resource.java	Wed May 14 15:04:16 2014 -0400
@@ -16,14 +16,14 @@
 
 package net.sourceforge.jnlp.cache;
 
-import net.sourceforge.jnlp.util.logging.OutputController;
-import java.io.*;
-import java.net.*;
-import java.util.*;
+import java.io.File;
+import java.net.URL;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.List;
 
-import net.sourceforge.jnlp.*;
-import net.sourceforge.jnlp.runtime.*;
-import net.sourceforge.jnlp.util.*;
+import net.sourceforge.jnlp.Version;
+import net.sourceforge.jnlp.util.WeakList;
 
 /**
  * <p>
@@ -51,25 +51,26 @@
     // todo: IIRC, any resource is checked for being up-to-date
     // only once, regardless of UpdatePolicy.  verify and fix.
 
-    /** status bits */
-    public static final int UNINITIALIZED = 0;
-    public static final int CONNECT = 1;
-    public static final int CONNECTING = 2;
-    public static final int CONNECTED = 4;
-    public static final int DOWNLOAD = 8;
-    public static final int DOWNLOADING = 16;
-    public static final int DOWNLOADED = 32;
-    public static final int ERROR = 64;
-    public static final int STARTED = 128; // enqueued or being worked on
+    public enum Status {
+        CONNECT,
+        CONNECTING,
+        CONNECTED,
+        DOWNLOAD,
+        DOWNLOADING,
+        DOWNLOADED,
+        ERROR,
+        STARTED // enqueued or being worked on
+    }
+
 
     /** list of weak references of resources currently in use */
-    private static WeakList<Resource> resources = new WeakList<Resource>();
+    private static final WeakList<Resource> resources = new WeakList<>();
 
     /** weak list of trackers monitoring this resource */
-    private WeakList<ResourceTracker> trackers = new WeakList<ResourceTracker>();
+    private final WeakList<ResourceTracker> trackers = new WeakList<>();
 
     /** the remote location of the resource */
-    URL location;
+    final URL location;
 
     /** the location to use when downloading */
     private URL downloadLocation;
@@ -78,22 +79,22 @@
     File localFile;
 
     /** the requested version */
-    Version requestVersion;
+    final Version requestVersion;
 
     /** the version downloaded from server */
     Version downloadVersion;
 
     /** amount in bytes transferred */
-    long transferred = 0;
+    volatile long transferred = 0;
 
     /** total size of the resource, or -1 if unknown */
-    long size = -1;
+    volatile long size = -1;
 
     /** the status of the resource */
-    int status = UNINITIALIZED;
+    final EnumSet<Status> status = EnumSet.noneOf(Status.class);
 
     /** Update policy for this resource */
-    UpdatePolicy updatePolicy;
+    final UpdatePolicy updatePolicy;
 
     /**
      * Create a resource.
@@ -118,8 +119,9 @@
             int index = resources.indexOf(resource);
             if (index >= 0) { // return existing object
                 Resource result = resources.get(index);
-                if (result != null)
+                if (result != null) {
                     return result;
+                }
             }
 
             resources.add(resource);
@@ -161,21 +163,34 @@
     ResourceTracker getTracker() {
         synchronized (trackers) {
             List<ResourceTracker> t = trackers.hardList();
-            if (t.size() > 0)
+            if (t.size() > 0) {
                 return t.get(0);
+            }
 
             return null;
         }
     }
 
     /**
-     * Returns true if any of the specified flags are set.
+     * Check if the specified flag is set.
+     * @param flag a status flag
+     * @return true iff the flag is set
      */
-    public boolean isSet(int flag) {
-        if (flag == UNINITIALIZED)
-            return status == UNINITIALIZED;
-        else
-            return (status & flag) != 0;
+    public boolean isSet(Status flag) {
+        synchronized (status) {
+            return status.contains(flag);
+        }
+    }
+
+    /**
+     * Check if all the specified flags are set.
+     * @param flags a collection of flags
+     * @return true iff all the flags are set
+     */
+    public boolean hasFlags(Collection<Status> flags) {
+        synchronized (status) {
+            return status.containsAll(flags);
+        }
     }
 
     /**
@@ -190,55 +205,85 @@
     /**
      * Returns a human-readable status string.
      */
-    private String getStatusString(int flag) {
-        StringBuffer result = new StringBuffer();
+    private String getStatusString() {
+        StringBuilder result = new StringBuilder();
 
-        if (flag == 0)
-            result.append("<> ");
-        if ((flag & CONNECT) != 0)
-            result.append("CONNECT ");
-        if ((flag & CONNECTING) != 0)
-            result.append("CONNECTING ");
-        if ((flag & CONNECTED) != 0)
-            result.append("CONNECTED ");
-        if ((flag & DOWNLOAD) != 0)
-            result.append("DOWNLOAD ");
-        if ((flag & DOWNLOADING) != 0)
-            result.append("DOWNLOADING ");
-        if ((flag & DOWNLOADED) != 0)
-            result.append("DOWNLOADED ");
-        if ((flag & ERROR) != 0)
-            result.append("ERROR ");
-        if ((flag & STARTED) != 0)
-            result.append("STARTED ");
+        synchronized (status) {
+            if (status.isEmpty()) {
+                return "<>";
+            }
+            for (Status stat : status) {
+                result.append(stat.toString()).append(" ");
+            }
+        }
 
-        return result.deleteCharAt(result.length() - 1).toString();
+        return result.toString().trim();
     }
 
     /**
      * Changes the status by clearing the flags in the first
      * parameter and setting the flags in the second.  This method
      * is synchronized on this resource.
+     * @param clear a collection of status flags to unset
+     * @param add a collection of status flags to set
      */
-    public void changeStatus(int clear, int add) {
-        int orig = 0;
+    public void changeStatus(Collection<Status> clear, Collection<Status> add) {
+        synchronized (status) {
+            if (clear != null) {
+                status.removeAll(clear);
+            }
+            if (add != null) {
+                status.addAll(add);
+            }
+        }
+    }
 
-        synchronized (this) {
-            orig = status;
+    /**
+     * Set status flag
+     * @param flag a flag to set
+     */
+    public void setStatusFlag(Status flag) {
+        synchronized (status) {
+            status.add(flag);
+        }
+    }
 
-            this.status &= ~clear;
-            this.status |= add;
+    /**
+     * Set flags
+     * @param flags a collection of flags to set
+     */
+    public void setStatusFlags(Collection<Status> flags) {
+        synchronized (status) {
+            status.addAll(flags);
         }
+    }
 
-           if (status != orig) {
-            OutputController.getLogger().log("Status: " + getStatusString(status));
-            if ((status & ~orig) != 0) {
-                OutputController.getLogger().log(" +(" + getStatusString(status & ~orig) + ")");
-            }
-            if ((~status & orig) != 0) {
-                OutputController.getLogger().log(" -(" + getStatusString(~status & orig) + ")");
-            }
-            OutputController.getLogger().log(" @ " + location.getPath());
+    /**
+     * Unset flags
+     * @param flags a collection of flags to unset
+     */
+    public void unsetStatusFlag(Collection<Status> flags) {
+        synchronized (status) {
+            status.removeAll(flags);
+        }
+    }
+
+    /**
+     * Clear all flags
+     */
+    public void resetStatus() {
+        synchronized (status) {
+            status.clear();
+        }
+    }
+
+    /**
+     * Check if this resource has been initialized
+     * @return true iff any flags have been set
+     */
+    public boolean isInitialized() {
+        synchronized (status) {
+            return !status.isEmpty();
         }
     }
 
@@ -284,6 +329,16 @@
         }
     }
 
+    @Override
+    public int hashCode() {
+        // FIXME: should probably have a better hashcode than this, but considering
+        // #equals(Object) was already defined first (without also overriding hashcode!),
+        // this is just being implemented in line with that so we don't break HashMaps,
+        // HashSets, etc
+        return location.hashCode();
+    }
+
+    @Override
     public boolean equals(Object other) {
         if (other instanceof Resource) {
             // this prevents the URL handler from looking up the IP
@@ -296,8 +351,9 @@
         return false;
     }
 
+    @Override
     public String toString() {
-        return "location=" + location.toString() + " state=" + getStatusString(status);
+        return "location=" + location.toString() + " state=" + getStatusString();
     }
 
 }
diff -r a784a0f1f821 -r d96760e31283 netx/net/sourceforge/jnlp/cache/ResourceTracker.java
--- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java	Tue May 13 12:21:07 2014 -0400
+++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java	Wed May 14 15:04:16 2014 -0400
@@ -16,6 +16,8 @@
 
 package net.sourceforge.jnlp.cache;
 
+import static net.sourceforge.jnlp.cache.Resource.Status.*;
+
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
 import java.io.File;
@@ -31,6 +33,8 @@
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -46,9 +50,9 @@
 import net.sourceforge.jnlp.event.DownloadListener;
 import net.sourceforge.jnlp.runtime.JNLPRuntime;
 import net.sourceforge.jnlp.util.HttpUtils;
-import net.sourceforge.jnlp.util.logging.OutputController;
 import net.sourceforge.jnlp.util.UrlUtils;
 import net.sourceforge.jnlp.util.WeakList;
+import net.sourceforge.jnlp.util.logging.OutputController;
 
 /**
  * This class tracks the downloading of various resources of a
@@ -104,17 +108,6 @@
     /** notified on initialization or download of a resource */
     private static final Object lock = new Object(); // used to lock static structures
 
-    // shortcuts
-    private static final int UNINITIALIZED = Resource.UNINITIALIZED;
-    private static final int CONNECT = Resource.CONNECT;
-    private static final int CONNECTING = Resource.CONNECTING;
-    private static final int CONNECTED = Resource.CONNECTED;
-    private static final int DOWNLOAD = Resource.DOWNLOAD;
-    private static final int DOWNLOADING = Resource.DOWNLOADING;
-    private static final int DOWNLOADED = Resource.DOWNLOADED;
-    private static final int ERROR = Resource.ERROR;
-    private static final int STARTED = Resource.STARTED;
-
     /** max threads */
     private static final int maxThreads = 5;
     
@@ -251,7 +244,7 @@
             // they will just 'pass through' the tracker as if they were
             // never added (for example, not affecting the total download size).
             synchronized (resource) {
-                resource.changeStatus(0, DOWNLOADED | CONNECTED | STARTED);
+                resource.changeStatus(EnumSet.noneOf(Resource.Status.class), EnumSet.of(DOWNLOADED, CONNECTED, STARTED));
             }
             fireDownloadEvent(resource);
             return true;
@@ -267,7 +260,7 @@
                     resource.localFile = CacheUtil.getCacheFile(resource.location, resource.downloadVersion);
                     resource.size = resource.localFile.length();
                     resource.transferred = resource.localFile.length();
-                    resource.changeStatus(0, DOWNLOADED | CONNECTED | STARTED);
+                    resource.changeStatus(EnumSet.noneOf(Resource.Status.class), EnumSet.of(DOWNLOADED, CONNECTED, STARTED));
                 }
                 fireDownloadEvent(resource);
                 return true;
@@ -276,7 +269,7 @@
 
         if (updatePolicy == UpdatePolicy.FORCE) { // ALWAYS update
             // When we are "always" updating, we update for each instance. Reset resource status.
-            resource.changeStatus(Integer.MAX_VALUE, 0);
+            resource.resetStatus();
         }
 
         // may or may not be cached, but check update when connection
@@ -322,18 +315,18 @@
             l = listeners.toArray(new DownloadListener[0]);
         }
 
-        int status;
+        Collection<Resource.Status> status;
         synchronized (resource) {
             status = resource.status;
         }
 
         DownloadEvent event = new DownloadEvent(this, resource);
         for (DownloadListener dl : l) {
-            if (0 != ((ERROR | DOWNLOADED) & status))
+            if (status.contains(ERROR) || status.contains(DOWNLOADED))
                 dl.downloadCompleted(event);
-            else if (0 != (DOWNLOADING & status))
+            else if (status.contains(DOWNLOADING))
                 dl.downloadStarted(event);
-            else if (0 != (CONNECTING & status))
+            else if (status.contains(CONNECTING))
                 dl.updateStarted(event);
         }
     }
@@ -382,7 +375,7 @@
     public File getCacheFile(URL location) {
         try {
             Resource resource = getResource(location);
-            if (!resource.isSet(DOWNLOADED | ERROR))
+            if (!(resource.isSet(DOWNLOADED) || resource.isSet(ERROR)))
                 waitForResource(location, 0);
 
             if (resource.isSet(ERROR))
@@ -420,7 +413,7 @@
     public InputStream getInputStream(URL location) throws IOException {
         try {
             Resource resource = getResource(location);
-            if (!resource.isSet(DOWNLOADED | ERROR))
+            if (!(resource.isSet(DOWNLOADED) || resource.isSet(ERROR)))
                 waitForResource(location, 0);
 
             if (resource.localFile != null)
@@ -491,7 +484,8 @@
      * @throws IllegalResourceDescriptorException if the resource is not being tracked
      */
     public boolean checkResource(URL location) {
-        return getResource(location).isSet(DOWNLOADED | ERROR); // isSet atomic
+        Resource resource = getResource(location);
+        return resource.isSet(DOWNLOADED) || resource.isSet(ERROR);
     }
 
     /**
@@ -526,12 +520,12 @@
 
             enqueue = !resource.isSet(STARTED);
 
-            if (!resource.isSet(CONNECTED | CONNECTING))
-                resource.changeStatus(0, CONNECT | STARTED);
-            if (!resource.isSet(DOWNLOADED | DOWNLOADING))
-                resource.changeStatus(0, DOWNLOAD | STARTED);
+            if (!(resource.isSet(CONNECTED) || resource.isSet(CONNECTING)))
+                resource.changeStatus(EnumSet.noneOf(Resource.Status.class), EnumSet.of(CONNECT, STARTED));
+            if (!(resource.isSet(DOWNLOADED) || resource.isSet(DOWNLOADING)))
+                resource.changeStatus(EnumSet.noneOf(Resource.Status.class), EnumSet.of(DOWNLOAD, STARTED));
 
-            if (!resource.isSet(DOWNLOAD | CONNECT))
+            if (!(resource.isSet(DOWNLOAD) || resource.isSet(CONNECT)))
                 enqueue = false;
         }
 
@@ -604,7 +598,7 @@
      */
     private void queueResource(Resource resource) {
         synchronized (lock) {
-            if (!resource.isSet(CONNECT | DOWNLOAD))
+            if (!(resource.isSet(CONNECT) || resource.isSet(DOWNLOAD)))
                 throw new IllegalResourceDescriptorException("Invalid resource state (resource: " + resource + ")");
 
             queue.add(resource);
@@ -764,14 +758,14 @@
                 downloadEntry.store();
             }
 
-            resource.changeStatus(DOWNLOADING, DOWNLOADED);
+            resource.changeStatus(EnumSet.of(DOWNLOADING), EnumSet.of(DOWNLOADED));
             synchronized (lock) {
                 lock.notifyAll(); // wake up wait's to check for completion
             }
             resource.fireDownloadEvent(); // fire DOWNLOADED
         } catch (Exception ex) {
             OutputController.getLogger().log(ex);
-            resource.changeStatus(0, ERROR);
+            resource.changeStatus(EnumSet.noneOf(Resource.Status.class), EnumSet.of(ERROR));
             synchronized (lock) {
                 lock.notifyAll(); // wake up wait's to check for completion
             }
@@ -830,11 +824,11 @@
                 resource.localFile = localFile;
                 // resource.connection = connection;
                 resource.size = size;
-                resource.changeStatus(CONNECT | CONNECTING, CONNECTED);
+                resource.changeStatus(EnumSet.of(CONNECT, CONNECTING), EnumSet.of(CONNECTED));
 
                 // check if up-to-date; if so set as downloaded


More information about the distro-pkg-dev mailing list