diff --git a/src/main/java/org/rometools/feed/module/cc/types/License.java b/src/main/java/org/rometools/feed/module/cc/types/License.java index 0b2d5d6..c7c358c 100644 --- a/src/main/java/org/rometools/feed/module/cc/types/License.java +++ b/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"); diff --git a/src/test/java/org/rometools/feed/module/cc/types/LicenseTest.java b/src/test/java/org/rometools/feed/module/cc/types/LicenseTest.java new file mode 100644 index 0000000..46ee8f0 --- /dev/null +++ b/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()); + } +}