Revise based on feedback in pull request from nlevitt

This commit is contained in:
Hunter Stern
2015-11-02 13:07:11 -08:00
parent 44ac1030f2
commit 3142bbd485
3 changed files with 84 additions and 44 deletions
@@ -50,6 +50,8 @@ import org.springframework.context.Lifecycle;
abstract public class AbstractCookieStore implements Lifecycle, Checkpointable,
CookieStore, FetchHTTPCookieStore {
public static final int MAX_COOKIES_FOR_DOMAIN = 50;
protected final Logger logger =
Logger.getLogger(AbstractCookieStore.class.getName());
@@ -276,7 +278,13 @@ abstract public class AbstractCookieStore implements Lifecycle, Checkpointable,
String normalizedHost = normalizeHost(curi.getUURI().getHost());
return cookieStoreFor(normalizedHost);
}
public boolean isCookieCountMaxedForDomain(String domain) {
CookieStore cookieStore = cookieStoreFor(normalizeHost(domain));
return (cookieStore != null && cookieStore.getCookies().size() >= MAX_COOKIES_FOR_DOMAIN);
}
abstract public void addCookie(Cookie cookie);
abstract public void clear();
abstract protected void prepare();
@@ -32,6 +32,7 @@ import java.util.logging.Logger;
import org.apache.commons.collections.collection.CompositeCollection;
import org.apache.http.client.CookieStore;
import org.apache.http.cookie.Cookie;
import org.apache.http.cookie.CookieRestrictionViolationException;
import org.archive.bdb.BdbModule;
import org.archive.checkpointing.Checkpoint;
import org.springframework.beans.factory.annotation.Autowired;
@@ -61,9 +62,8 @@ import com.sleepycat.je.DatabaseException;
public class BdbCookieStore extends AbstractCookieStore implements
FetchHTTPCookieStore, CookieStore {
public static final int MAX_COOKIES_FOR_DOMAIN = 50;
private static Logger logger = Logger.getLogger(BdbCookieStore.class.getName());
protected final Logger logger =
Logger.getLogger(BdbCookieStore.class.getName());
/**
* A {@link List} implementation that wraps a {@link Collection}. Needed
@@ -136,37 +136,28 @@ public class BdbCookieStore extends AbstractCookieStore implements
}
public void addCookie(Cookie cookie) {
synchronized (cookies) {
if (isCookieCountMaxedForDomain(cookie.getDomain())) {
logger.log(
Level.FINEST,
"Maximum number of cookies reached for domain "
+ cookie.getDomain()
+ ". Will not add new cookie "
+ cookie.getName() + " with value "
+ cookie.getValue());
return;
}
if (isCookieCountMaxedForDomain(cookie.getDomain())) {
logger.log(
Level.FINEST,
"Maximum number of cookies reached for domain "
+ cookie.getDomain() + ". Will not add new cookie "
+ cookie.getName() + " with value "
+ cookie.getValue());
return;
}
byte[] key;
try {
key = sortableKey(cookie).getBytes("UTF-8");
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e); // impossible
}
byte[] key;
try {
key = sortableKey(cookie).getBytes("UTF-8");
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e); // impossible
}
if (!cookie.isExpired(new Date())) {
cookies.put(key, cookie);
} else {
cookies.remove(key);
}
if (!cookie.isExpired(new Date())) {
cookies.put(key, cookie);
} else {
cookies.remove(key);
}
}
protected boolean isCookieCountMaxedForDomain(String domain) {
Collection<Cookie> subset = hostSubset(normalizeHost(domain));
return (subset != null && subset.size() >= MAX_COOKIES_FOR_DOMAIN);
}
protected Collection<Cookie> hostSubset(String host) {
@@ -183,7 +174,7 @@ public class BdbCookieStore extends AbstractCookieStore implements
throw new RuntimeException(e); // impossible
}
}
/**
* Returns a {@link LimitedCookieStoreFacade} whose
* {@link LimitedCookieStoreFacade#getCookies()} method returns only cookies
@@ -165,7 +165,7 @@ public class CookieStoreTest extends TmpDirTestCase {
basicCookieStore().addCookie(cookie);
assertCookieStoresEquivalent(basicCookieStore(), bdbCookieStore());
}
public void testMaxCookieDomain() throws IOException {
bdbCookieStore().clear();
@@ -184,7 +184,7 @@ public class CookieStoreTest extends TmpDirTestCase {
bdbCookieStore().addCookie(cookie);
assertCookieStoreCountEquals(bdbCookieStore, BdbCookieStore.MAX_COOKIES_FOR_DOMAIN);
}
public void testPaths() throws IOException {
bdbCookieStore().clear();
basicCookieStore().clear();
@@ -297,7 +297,7 @@ public class CookieStoreTest extends TmpDirTestCase {
assertCookieListsEquivalent(cookiesBefore, cookiesAfter);
}
public void testConcurrentLoad() throws IOException, InterruptedException {
public void testConcurrentLoadNoDomainCookieLimitBreach() throws IOException, InterruptedException {
bdbCookieStore().clear();
basicCookieStore().clear();
final Random rand = new Random();
@@ -308,7 +308,7 @@ public class CookieStoreTest extends TmpDirTestCase {
try {
while (!Thread.interrupted()) {
BasicClientCookie cookie = new BasicClientCookie(UUID.randomUUID().toString(), UUID.randomUUID().toString());
cookie.setDomain("d" + rand.nextInt(10) + ".example.com");
cookie.setDomain("d" + rand.nextInt() + ".example.com");
bdbCookieStore().addCookie(cookie);
basicCookieStore().addCookie(cookie);
}
@@ -334,8 +334,49 @@ public class CookieStoreTest extends TmpDirTestCase {
threads[i].join();
}
List<Cookie> bdbCookieList = bdbCookieStore().getCookies();
assertTrue(bdbCookieList.size() > 3000);
assertCookieListsEquivalent(bdbCookieList, basicCookieStore().getCookies());
}
public void testConcurrentLoad() throws IOException, InterruptedException {
bdbCookieStore().clear();
basicCookieStore().clear();
final Random rand = new Random();
Runnable runnable = new Runnable() {
@Override
public void run() {
try {
while (!Thread.interrupted()) {
BasicClientCookie cookie = new BasicClientCookie(UUID.randomUUID().toString(), UUID.randomUUID().toString());
cookie.setDomain("d" + rand.nextInt(20) + ".example.com");
bdbCookieStore().addCookie(cookie);
basicCookieStore().addCookie(cookie);
}
} catch (Exception e) {
throw new RuntimeException(e);
}
}
};
Thread[] threads = new Thread[200];
for (int i = 0; i < threads.length; i++) {
threads[i] = new Thread(runnable);
threads[i].setName("cookie-load-test-" + i);
threads[i].start();
}
Thread.sleep(1000);
for (int i = 0; i < threads.length; i++) {
threads[i].interrupt();
}
for (int i = 0; i < threads.length; i++) {
threads[i].join();
}
ArrayList<Cookie> bdbCookieArrayList = new ArrayList<Cookie>(bdbCookieStore().getCookies());
Map<String, Integer> domainCounts = new HashMap<String, Integer>();
for (Cookie cookie : bdbCookieArrayList) {
if (domainCounts.get(cookie.getDomain()) == null) {
@@ -347,14 +388,14 @@ public class CookieStoreTest extends TmpDirTestCase {
}
for (String domain: domainCounts.keySet()) {
assertTrue(domainCounts.get(domain) <= BdbCookieStore.MAX_COOKIES_FOR_DOMAIN);
}
}
protected void assertCookieStoreCountEquals(BdbCookieStore bdb, int count) {
assertEquals(bdb.getCookies().size(), count);
assertTrue(domainCounts.get(domain) <= BdbCookieStore.MAX_COOKIES_FOR_DOMAIN + 25);
}
}
protected void assertCookieStoreCountEquals(BdbCookieStore bdb, int count) {
assertEquals(bdb.getCookies().size(), count);
}
protected void assertCookieListsEquivalent(List<Cookie> list1,
List<Cookie> list2) {
Comparator<Cookie> comparator = new Comparator<Cookie>() {