Issue 6

pom.xml

@@ -138,13 +138,13 @@
                 <configuration>
                     <signature>
                         <groupId>org.codehaus.mojo.signature</groupId>
-                        <artifactId>java15</artifactId>
+                        <artifactId>java16</artifactId>
                         <version>1.0</version>
                     </signature>
                 </configuration>
                 <executions>
                     <execution>
-                        <id>check-java-1.5-compat</id>
+                        <id>check-java-1.6-compat</id>
                         <phase>process-classes</phase>
                         <goals>
                             <goal>check</goal>
@@ -195,7 +195,7 @@
                                     <version>2.0.9</version>
                                 </requireMavenVersion>
                                 <requireJavaVersion>
-                                    <version>1.5</version>
+                                    <version>1.6</version>
                                 </requireJavaVersion>
                             </rules>
                         </configuration>

src/main/java/org/sonatype/spice/jersey/client/ahc/AhcClientHandler.java

@@ -12,9 +12,11 @@
 package org.sonatype.spice.jersey.client.ahc;
 
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListSet;
 
 import javax.ws.rs.core.Context;
 
@@ -60,8 +62,8 @@ public final class AhcClientHandler implements ClientHandler {
     private final AhcConfig config;
 
     private final AhcRequestWriter requestWriter = new AhcRequestWriter();
-
-    private final List<Cookie> cookies = new ArrayList<Cookie>();
+    //ConcurrentSkipListSet can be iterated with and modified in a thread safe manner
+    private final ConcurrentSkipListSet<Cookie> cookies = new ConcurrentSkipListSet<Cookie>(new CookieComparator());
 
     @Context
     private MessageBodyWorkers workers;
@@ -146,30 +148,37 @@ public ClientResponse handle(final ClientRequest cr)
     private void applyResponseCookies(final List<Cookie> responseCookies) {
         if (responseCookies != null) {
             for (final Cookie rc : responseCookies) {
-                // remove existing cookie
-                final Iterator<Cookie> it = cookies.iterator();
-                while (it.hasNext()) {
-                    final Cookie c = it.next();
-                    if (isSame(rc, c)) {
-                        it.remove();
-                        break;
-                    }
-                }
-                // add new cookie
+                // add or replace new cookie
                 cookies.add(rc);
             }
         }
     }
 
-    private boolean isSame(final Cookie c, final Cookie o) {
-        return isEquals(c.getDomain(), o.getDomain()) &&
-                isEquals(c.getPath(), o.getPath()) &&
-                isEquals(c.getName(), o.getName());
+    public final static class CookieComparator  implements Comparator<Cookie>{
+
+        @Override
+        public int compare(Cookie o1, Cookie o2) {
+            if (o1 == o2) return 0;
+            if (o1 == null) return -1;
+            if (o2 == null) return 1;
+            int retval = 0;
+            retval = compareStrings(o1.getDomain(),o2.getDomain());
+            if (retval != 0) return retval;
+            retval = compareStrings(o1.getPath(),o2.getPath());
+            if (retval != 0) return retval;
+            retval = compareStrings(o1.getName(),o2.getName());
+            return retval;
+        }
+        public int compareStrings(String o1, String o2) {
+            if (o1 == o2) return 0;
+            if (o1 == null) return -1;
+            if (o2 == null) return 1;
+            return o1.compareTo(o2);
+
+        }
     }
+
 
-    private boolean isEquals(final Object o, final Object o2) {
-        return (o == null && o2 == null) || o != null && o.equals(o2);
-    }
 
     /**
      * Check if a body needs to be constructed based on a method's name.

src/test/java/org/sonatype/spice/jersey/client/ahc/tests/tests/CookieTest.java

@@ -40,10 +40,15 @@
 
 package org.sonatype.spice.jersey.client.ahc.tests.tests;
 
+import java.util.concurrent.CountDownLatch;
+
 import com.sun.jersey.api.client.WebResource;
 import com.sun.jersey.api.container.filter.LoggingFilter;
 import com.sun.jersey.api.core.DefaultResourceConfig;
 import com.sun.jersey.api.core.ResourceConfig;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.sonatype.spice.jersey.client.ahc.AhcHttpClient;
 import org.sonatype.spice.jersey.client.ahc.config.DefaultAhcConfig;
 
@@ -56,6 +61,7 @@
  * @author [email protected]
  */
 public class CookieTest extends AbstractGrizzlyServerTester {
+    private final static Logger log = LoggerFactory.getLogger(CookieTest.class);
     @Path("/")
     public static class CookieResource {
         @GET
@@ -123,4 +129,56 @@ public void testSessionCookie() {
         assertEquals("wo-cookie", r.post(String.class));
         assertEquals("value", r.get(String.class));
     }
+
+    public void testCookieThreading() throws Exception {
+        ResourceConfig rc = new DefaultResourceConfig(CookieResource.class);
+        rc.getProperties().put(ResourceConfig.PROPERTY_CONTAINER_REQUEST_FILTERS,
+                LoggingFilter.class.getName());
+        startServer(rc);
+
+        DefaultAhcConfig config = new DefaultAhcConfig();
+        AhcHttpClient c = AhcHttpClient.create(config);
+
+        final WebResource r = c.resource(getUri().build());
+        // Set first cookie
+        assertEquals("NO-COOKIE", r.get(String.class));
+        CountDownLatch latch = new CountDownLatch(2);
+        // Test concurrent requests
+        CookieThreadTest a = new CookieThreadTest(r, latch);
+        CookieThreadTest b = new CookieThreadTest(r, latch);
+        a.start();
+        b.start();
+        latch.await();
+        if (a.e != null) {
+            fail("Unexpected Exception " + a.e);
+        }
+        if (b.e != null) {
+            fail("Unexpected Exception " + a.e);
+        }
+    }
+    public static class CookieThreadTest extends Thread {
+        private WebResource r;
+        private CountDownLatch latch;
+        Exception e;
+        public CookieThreadTest(WebResource r, CountDownLatch latch) {
+            this.r =r;
+            this.latch = latch;
+        }
+        @Override
+        public void run() {
+            try {
+                for (int i=0; i<1000; i++) {
+                    assertEquals("value", r.get(String.class));
+                }
+            } catch (RuntimeException e) {
+                log.error("Threading error",e);
+                this.e = e;
+                throw e;
+            } finally {
+                latch.countDown();
+            }
+        }
+    }
+
+
 }