Merge pull request #25 from buckett/master

Allow concurrent access to Licenses

src/main/java/org/rometools/feed/module/cc/types/License.java

@@ -41,7 +41,9 @@ package org.rometools.feed.module.cc.types;
 
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.Map;
 import java.util.StringTokenizer;
+import java.util.concurrent.ConcurrentHashMap;
 
 import com.sun.syndication.feed.impl.EqualsBean;
 import com.sun.syndication.feed.impl.ToStringBean;
@@ -52,7 +54,7 @@ import com.sun.syndication.feed.impl.ToStringBean;
  */
 public class License {
     private static final String CC_START = "http://creativecommons.org/licenses/";
-    private static final HashMap lookupLicense = new HashMap();
+    private static final Map lookupLicense = new ConcurrentHashMap();
     public static final License NO_DERIVS = new License("http://creativecommons.org/licenses/nd/1.0/", new Behaviour[0], new Behaviour[] {
             Behaviour.DISTRIBUTION, Behaviour.REPRODUCTION });
     public static final License NO_DERIVS_NONCOMMERCIAL = new License("http://creativecommons.org/licenses/nd-nc/1.0/",
@@ -104,8 +106,8 @@ public class License {
         if (found == null && uri.startsWith("http://") && uri.toLowerCase().indexOf("creativecommons.org") != -1) {
             final Iterator it = License.lookupLicense.keySet().iterator();
             while (it.hasNext() && found == null) {
+                final String key = (String) it.next();
                 try {
-                    final String key = (String) it.next();
                     if (key.startsWith(CC_START)) {
                         final String licensePath = key.substring(CC_START.length(), key.length());
                         final StringTokenizer tok = new StringTokenizer(licensePath, "/");
@@ -128,6 +130,13 @@ public class License {
         return found;
     }
 
+    /**
+     * This is just useful for testing to allow clearing of the looked up licenses.
+     */
+    static void clear() {
+        lookupLicense.clear();
+    }
+
     public Behaviour[] getPermits() {
         return permits;
     }
@@ -160,7 +169,7 @@ public class License {
     }
 
     public static class Behaviour {
-        private static final HashMap lookup = new HashMap();
+        private static final Map lookup = new HashMap();
         public static final Behaviour REPRODUCTION = new Behaviour("http://web.resource.org/cc/Reproduction");
         public static final Behaviour DISTRIBUTION = new Behaviour("http://web.resource.org/cc/Distribution");
         public static final Behaviour DERIVATIVE = new Behaviour("http://web.resource.org/cc/DerivativeWorks");

src/test/java/org/rometools/feed/module/cc/types/LicenseTest.java

@@ -0,0 +1,83 @@
+package org.rometools.feed.module.cc.types;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Random;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+
+import static org.junit.Assert.assertFalse;
+
+/**
+ * @author Matthew Buckett
+ */
+public class LicenseTest {
+
+    @Before
+    public void setUp() {
+        // As the looked up licenses are held statically we need to clear this between tests.
+        License.clear();
+    }
+
+    @Test(timeout = 1000)
+    public void testConcurrent() throws InterruptedException {
+        final AtomicBoolean run = new AtomicBoolean(true);
+        final AtomicLong type = new AtomicLong(0);
+        // Tracking any problems.
+        final AtomicBoolean hadProblem = new AtomicBoolean(false);
+        final AtomicBoolean hadException = new AtomicBoolean(false);
+
+        // This thread keeps on adding new licenses (not very realistic but shows the bug)
+        Thread addNew = new Thread(){
+            @Override
+            public void run() {
+                try {
+                    while(run.get()) {
+                        License license = License.findByValue("http://creativecommons.org/licenses/"+
+                                type.incrementAndGet()+ "/1");
+                        if (license == null) {
+                            hadProblem.set(true);
+                        }
+                    }
+                } catch (Exception e) {
+                    hadException.set(true);
+                }
+            }
+        };
+
+        // This thread attempts to get ones we know have already been put in.
+        Thread getExisting = new Thread() {
+            @Override
+            public void run() {
+                Random rnd = new Random();
+                try {
+                    while(run.get()) {
+                        License license = License.findByValue("http://creativecommons.org/licenses/"+
+                                rnd.nextInt(type.intValue())+"/1");
+                        if (license == null) {
+                            hadProblem.set(true);
+                        }
+                    }
+                } catch (Exception e) {
+                    hadException.set(true);
+                }
+            }
+        };
+
+        addNew.start();
+        getExisting.start();
+        // Let them do some stuff.
+        Thread.sleep(400);
+        // Get them to both stop.
+        run.set(false);
+        // Allow them a little time to stop.
+        addNew.join(50);
+        getExisting.join(50);
+        // Check we didn't have any problems and they have both stopped.
+        assertFalse(hadProblem.get());
+        assertFalse(hadException.get());
+        assertFalse(addNew.isAlive());
+        assertFalse(getExisting.isAlive());
+    }
+}