Skip to content

Commit a0c0365

Browse files
sivaallisivaalli
committed
adding config to get the delay in ms and also making a CookieEvictionTask to evict expired cookies.
1 parent cedd3c9 commit a0c0365

File tree

10 files changed

+115
-49
lines changed

10 files changed

+115
-49
lines changed

‎client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,13 @@ public interface AsyncHttpClientConfig {
198198
*/
199199
CookieStore getCookieStore();
200200

201+
/**
202+
* Return the delay in milliseconds to evict expired cookies from {@linkplain CookieStore}
203+
*
204+
* @return the delay in milliseconds to evict expired cookies from {@linkplain CookieStore}
205+
*/
206+
int expiredCookieEvictionDelay();
207+
201208
/**
202209
* Return the number of time the library will retry when an {@link java.io.IOException} is throw by the remote server
203210
*

‎client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.netty.util.Timer;
2323
import io.netty.util.concurrent.DefaultThreadFactory;
2424
import org.asynchttpclient.channel.ChannelPool;
25+
import org.asynchttpclient.cookie.CookieEvictionTask;
2526
import org.asynchttpclient.filter.FilterContext;
2627
import org.asynchttpclient.filter.FilterException;
2728
import org.asynchttpclient.filter.RequestFilter;
@@ -31,7 +32,6 @@
3132
import org.slf4j.Logger;
3233
import org.slf4j.LoggerFactory;
3334

34-
import java.io.IOException;
3535
import java.util.List;
3636
import java.util.concurrent.ThreadFactory;
3737
import java.util.concurrent.TimeUnit;
@@ -91,6 +91,8 @@ public DefaultAsyncHttpClient(AsyncHttpClientConfig config) {
9191
channelManager = new ChannelManager(config, nettyTimer);
9292
requestSender = new NettyRequestSender(config, channelManager, nettyTimer, new AsyncHttpClientState(closed));
9393
channelManager.configureBootstraps(requestSender);
94+
nettyTimer.newTimeout(new CookieEvictionTask(config.expiredCookieEvictionDelay(), config.getCookieStore()),
95+
config.expiredCookieEvictionDelay(), TimeUnit.MILLISECONDS);
9496
}
9597

9698
private Timer newNettyTimer(AsyncHttpClientConfig config) {
@@ -115,13 +117,6 @@ public void close() {
115117
LOGGER.warn("Unexpected error on HashedWheelTimer close", t);
116118
}
117119
}
118-
if (config.getCookieStore() != null) {
119-
try {
120-
config.getCookieStore().close();
121-
} catch (IOException e) {
122-
LOGGER.warn("IOException closing CookieStore", e);
123-
}
124-
}
125120
}
126121
}
127122

‎client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ public class DefaultAsyncHttpClientConfig implements AsyncHttpClientConfig {
109109

110110
// cookie store
111111
private final CookieStore cookieStore;
112+
private final int expiredCookieEvictionDelay;
112113

113114
// internals
114115
private final String threadPoolName;
@@ -192,6 +193,7 @@ private DefaultAsyncHttpClientConfig(// http
192193

193194
// cookie store
194195
CookieStore cookieStore,
196+
int expiredCookieEvictionDelay,
195197

196198
// tuning
197199
boolean tcpNoDelay,
@@ -283,6 +285,7 @@ private DefaultAsyncHttpClientConfig(// http
283285

284286
// cookie store
285287
this.cookieStore = cookieStore;
288+
this.expiredCookieEvictionDelay = expiredCookieEvictionDelay;
286289

287290
// tuning
288291
this.tcpNoDelay = tcpNoDelay;
@@ -558,6 +561,11 @@ public CookieStore getCookieStore() {
558561
return cookieStore;
559562
}
560563

564+
@Override
565+
public int expiredCookieEvictionDelay() {
566+
return expiredCookieEvictionDelay;
567+
}
568+
561569
// tuning
562570
@Override
563571
public boolean isTcpNoDelay() {
@@ -746,6 +754,7 @@ public static class Builder {
746754

747755
// cookie store
748756
private CookieStore cookieStore = new ThreadSafeCookieStore();
757+
private int expiredCookieEvictionDelay = defaultExpiredCookieEvictionDelay();
749758

750759
// tuning
751760
private boolean tcpNoDelay = defaultTcpNoDelay();
@@ -1146,6 +1155,11 @@ public Builder setCookieStore(CookieStore cookieStore) {
11461155
return this;
11471156
}
11481157

1158+
public Builder setExpiredCookieEvictionDelay(int expiredCookieEvictionDelay) {
1159+
this.expiredCookieEvictionDelay = expiredCookieEvictionDelay;
1160+
return this;
1161+
}
1162+
11491163
// tuning
11501164
public Builder setTcpNoDelay(boolean tcpNoDelay) {
11511165
this.tcpNoDelay = tcpNoDelay;
@@ -1330,6 +1344,7 @@ public DefaultAsyncHttpClientConfig build() {
13301344
responseFilters.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(responseFilters),
13311345
ioExceptionFilters.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(ioExceptionFilters),
13321346
cookieStore,
1347+
expiredCookieEvictionDelay,
13331348
tcpNoDelay,
13341349
soReuseAddress,
13351350
soKeepAlive,

‎client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ public final class AsyncHttpClientConfigDefaults {
7373
public static final String IO_THREADS_COUNT_CONFIG = "ioThreadsCount";
7474
public static final String HASHED_WHEEL_TIMER_TICK_DURATION = "hashedWheelTimerTickDuration";
7575
public static final String HASHED_WHEEL_TIMER_SIZE = "hashedWheelTimerSize";
76+
public static final String EXPIRED_COOKIE_EVICTION_DELAY = "expiredCookieEvictionDelay";
7677

7778
public static final String AHC_VERSION;
7879

@@ -304,4 +305,8 @@ public static int defaultHashedWheelTimerTickDuration() {
304305
public static int defaultHashedWheelTimerSize() {
305306
return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getInt(ASYNC_CLIENT_CONFIG_ROOT + HASHED_WHEEL_TIMER_SIZE);
306307
}
308+
309+
public static int defaultExpiredCookieEvictionDelay() {
310+
return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getInt(ASYNC_CLIENT_CONFIG_ROOT + EXPIRED_COOKIE_EVICTION_DELAY);
311+
}
307312
}

‎client/src/main/java/org/asynchttpclient/cookie/CookieEvictionTask.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package org.asynchttpclient.cookie;
2+
3+
import java.util.concurrent.TimeUnit;
4+
5+
import org.asynchttpclient.AsyncHttpClientConfig;
6+
7+
import io.netty.util.Timeout;
8+
import io.netty.util.TimerTask;
9+
10+
/**
11+
* Evicts expired cookies from the {@linkplain CookieStore} periodically.
12+
* The default delay is 30 seconds. Ypu may override the default using
13+
* {@linkplain AsyncHttpClientConfig#expiredCookieEvictionDelay()}.
14+
*/
15+
public class CookieEvictionTask implements TimerTask {
16+
17+
private long evictDelayInMs;
18+
private CookieStore cookieStore;
19+
20+
public CookieEvictionTask(long evictDelayInMs, CookieStore cookieStore) {
21+
this.evictDelayInMs = evictDelayInMs;
22+
this.cookieStore = cookieStore;
23+
}
24+
25+
@Override
26+
public void run(Timeout timeout) throws Exception {
27+
cookieStore.evictExpired();
28+
timeout.timer().newTimeout(this, evictDelayInMs, TimeUnit.MILLISECONDS);
29+
}
30+
}

‎client/src/main/java/org/asynchttpclient/cookie/CookieStore.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@
3232
*
3333
* @since 2.1
3434
*/
35-
public interface CookieStore extends Closeable {
35+
public interface CookieStore {
3636
/**
3737
* Adds one {@link Cookie} to the store. This is called for every incoming HTTP response.
38-
* If the given cookie has already expired it will not be added, but existing values will still be removed.
38+
* If the given cookie has already expired it will not be added.
3939
*
4040
* <p>A cookie to store may or may not be associated with an URI. If it
4141
* is not associated with an URI, the cookie's domain and path attribute
@@ -83,4 +83,9 @@ public interface CookieStore extends Closeable {
8383
* @return true if any cookies were purged.
8484
*/
8585
boolean clear();
86+
87+
/**
88+
* Evicts all the cookies that expired as of the time this method is run.
89+
*/
90+
void evictExpired();
8691
}

‎client/src/main/java/org/asynchttpclient/cookie/ThreadSafeCookieStore.java

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,19 @@
1515
package org.asynchttpclient.cookie;
1616

1717
import io.netty.handler.codec.http.cookie.Cookie;
18+
1819
import org.asynchttpclient.uri.Uri;
1920
import org.asynchttpclient.util.Assertions;
2021
import org.asynchttpclient.util.MiscUtils;
21-
import org.slf4j.Logger;
22-
import org.slf4j.LoggerFactory;
2322

24-
import java.io.IOException;
2523
import java.util.*;
2624
import java.util.concurrent.ConcurrentHashMap;
27-
import java.util.concurrent.Executors;
28-
import java.util.concurrent.ScheduledExecutorService;
29-
import java.util.concurrent.TimeUnit;
3025
import java.util.function.Predicate;
3126
import java.util.stream.Collectors;
3227

3328
public final class ThreadSafeCookieStore implements CookieStore {
34-
private static final Logger log = LoggerFactory.getLogger(ThreadSafeCookieStore.class);
3529

3630
private final Map<String, Map<CookieKey, StoredCookie>> cookieJar = new ConcurrentHashMap<>();
37-
private final ScheduledExecutorService scheduledExecutor = Executors.newScheduledThreadPool(1);
3831

3932
@Override
4033
public void add(Uri uri, Cookie cookie) {
@@ -44,20 +37,6 @@ public void add(Uri uri, Cookie cookie) {
4437
add(thisRequestDomain, thisRequestPath, cookie);
4538
}
4639

47-
public ThreadSafeCookieStore() {
48-
scheduleExpiredCookiesRemoval();
49-
}
50-
51-
private void scheduleExpiredCookiesRemoval() {
52-
scheduledExecutor.scheduleWithFixedDelay(() -> {
53-
if (cookieJar.isEmpty()) {
54-
return;
55-
}
56-
57-
removeExpired();
58-
}, 10, 10, TimeUnit.SECONDS);
59-
}
60-
6140
@Override
6241
public List<Cookie> get(Uri uri) {
6342
return get(requestDomain(uri), requestPath(uri), uri.isSecured());
@@ -97,8 +76,18 @@ public boolean clear() {
9776
return result;
9877
}
9978

79+
@Override
80+
public void evictExpired() {
81+
removeExpired();
82+
}
83+
10084
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
10185

86+
// Visible for test
87+
public Map<String, Map<CookieKey, StoredCookie>> getUnderlying() {
88+
return new HashMap<>(cookieJar);
89+
}
90+
10291
private String requestDomain(Uri requestUri) {
10392
return requestUri.getHost().toLowerCase();
10493
}
@@ -206,27 +195,13 @@ private List<Cookie> getStoredCookies(String domain, String path, boolean secure
206195

207196
private void removeExpired() {
208197
final boolean[] removed = {false};
209-
cookieJar.values().forEach(cookieMap -> {
210-
if (!removed[0]) {
211-
removed[0] = cookieMap.entrySet().removeIf(v -> hasCookieExpired(v.getValue().cookie, v.getValue().createdAt));
212-
}
213-
});
198+
cookieJar.values().forEach(cookieMap -> removed[0] |= cookieMap.entrySet().removeIf(
199+
v -> hasCookieExpired(v.getValue().cookie, v.getValue().createdAt)));
214200
if (removed[0]) {
215201
cookieJar.entrySet().removeIf(entry -> entry.getValue() == null || entry.getValue().isEmpty());
216202
}
217203
}
218204

219-
@Override
220-
public void close() throws IOException {
221-
scheduledExecutor.shutdown();
222-
try {
223-
scheduledExecutor.awaitTermination(5, TimeUnit.SECONDS);
224-
} catch (InterruptedException e) {
225-
log.debug("Interrupted while shutting down " + ThreadSafeCookieStore.class.getName());
226-
Thread.currentThread().interrupt();
227-
}
228-
}
229-
230205
private static class CookieKey implements Comparable<CookieKey> {
231206
final String name;
232207
final String path;

‎client/src/main/resources/org/asynchttpclient/config/ahc-default.properties

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,4 @@ org.asynchttpclient.useNativeTransport=false
5252
org.asynchttpclient.ioThreadsCount=0
5353
org.asynchttpclient.hashedWheelTimerTickDuration=100
5454
org.asynchttpclient.hashedWheelTimerSize=512
55+
org.asynchttpclient.expiredCookieEvictionDelay=30000

‎client/src/test/java/org/asynchttpclient/CookieStoreTest.java

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import io.netty.handler.codec.http.cookie.ClientCookieDecoder;
1818
import io.netty.handler.codec.http.cookie.ClientCookieEncoder;
1919
import io.netty.handler.codec.http.cookie.Cookie;
20+
import io.netty.handler.codec.http.cookie.DefaultCookie;
21+
2022
import org.asynchttpclient.cookie.CookieStore;
2123
import org.asynchttpclient.cookie.ThreadSafeCookieStore;
2224
import org.asynchttpclient.uri.Uri;
@@ -26,11 +28,14 @@
2628
import org.testng.annotations.BeforeClass;
2729
import org.testng.annotations.Test;
2830

31+
import java.util.Collection;
2932
import java.util.List;
3033
import java.util.stream.Collectors;
3134

3235
import static org.testng.Assert.assertTrue;
3336

37+
import com.google.common.collect.Sets;
38+
3439
public class CookieStoreTest {
3540

3641
private final Logger logger = LoggerFactory.getLogger(getClass());
@@ -47,7 +52,7 @@ public void tearDownGlobal() {
4752
}
4853

4954
@Test
50-
public void runAllSequentiallyBecauseNotThreadSafe() {
55+
public void runAllSequentiallyBecauseNotThreadSafe() throws Exception {
5156
addCookieWithEmptyPath();
5257
dontReturnCookieForAnotherDomain();
5358
returnCookieWhenItWasSetOnSamePath();
@@ -78,6 +83,7 @@ public void runAllSequentiallyBecauseNotThreadSafe() {
7883
shouldAlsoServeNonSecureCookiesBasedOnTheUriScheme();
7984
shouldNotServeSecureCookiesForDefaultRetrievedHttpUriScheme();
8085
shouldServeSecureCookiesForSpecificallyRetrievedHttpUriScheme();
86+
shouldCleanExpiredCookieFromUnderlyingDataStructure();
8187
}
8288

8389
private void addCookieWithEmptyPath() {
@@ -339,4 +345,26 @@ private void shouldServeSecureCookiesForSpecificallyRetrievedHttpUriScheme() {
339345
assertTrue(store.get(uri).get(0).value().equals("VALUE3"));
340346
assertTrue(store.get(uri).get(0).isSecure());
341347
}
348+
349+
private void shouldCleanExpiredCookieFromUnderlyingDataStructure() throws Exception {
350+
ThreadSafeCookieStore store = new ThreadSafeCookieStore();
351+
store.add(Uri.create("https://foo.org/moodle/"), getCookie("JSESSIONID", "FOO", 1));
352+
store.add(Uri.create("https://bar.org/moodle/"), getCookie("JSESSIONID", "BAR", 1));
353+
store.add(Uri.create("https://bar.org/moodle/"), new DefaultCookie("UNEXPIRED_BAR", "BAR"));
354+
store.add(Uri.create("https://foobar.org/moodle/"), new DefaultCookie("UNEXPIRED_FOOBAR", "FOOBAR"));
355+
356+
357+
assertTrue(store.getAll().size() == 4);
358+
Thread.sleep(2000);
359+
store.evictExpired();
360+
assertTrue(store.getUnderlying().size() == 2);
361+
Collection<String> unexpiredCookieNames = store.getAll().stream().map(Cookie::name).collect(Collectors.toList());
362+
assertTrue(unexpiredCookieNames.containsAll(Sets.newHashSet("UNEXPIRED_BAR", "UNEXPIRED_FOOBAR")));
363+
}
364+
365+
private static Cookie getCookie(String key, String value, int maxAge) {
366+
DefaultCookie cookie = new DefaultCookie(key, value);
367+
cookie.setMaxAge(maxAge);
368+
return cookie;
369+
}
342370
}

‎extras/typesafeconfig/src/main/java/org/asynchttpclient/extras/typesafeconfig/AsyncHttpClientTypesafeConfig.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,11 @@ public CookieStore getCookieStore() {
164164
return new ThreadSafeCookieStore();
165165
}
166166

167+
@Override
168+
public int expiredCookieEvictionDelay() {
169+
return getIntegerOpt(EXPIRED_COOKIE_EVICTION_DELAY).orElse(defaultExpiredCookieEvictionDelay());
170+
}
171+
167172
@Override
168173
public int getMaxRequestRetry() {
169174
return getIntegerOpt(MAX_REQUEST_RETRY_CONFIG).orElse(defaultMaxRequestRetry());

0 commit comments

Comments
 (0)