[java-support] branch master updated: JSPT-33 - Refactor HttpClient builders and factory beans
Scott Cantor
cantor.2 at osu.edu
Wed Mar 6 18:28:14 EST 2019
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch master
in repository java-support.
View the commit online:
http://git.shibboleth.net/view/?p=java-support.git;a=commit;h=894a87f9a34cc0006eadc53bd544509433f2ada9
The following commit(s) were added to refs/heads/master by this push:
new 894a87f JSPT-33 - Refactor HttpClient builders and factory beans
894a87f is described below
commit 894a87f9a34cc0006eadc53bd544509433f2ada9
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Mar 6 18:28:05 2019 -0500
JSPT-33 - Refactor HttpClient builders and factory beans
https://issues.shibboleth.net/jira/browse/JSPT-33
Also converts duration properties to Duration type.
---
.../httpclient/FileCachingHttpClientBuilder.java | 28 +++----
.../java/support/httpclient/HttpClientBuilder.java | 94 +++++++++++-----------
.../support/httpclient/HttpClientBuilderTest.java | 17 +++-
3 files changed, 75 insertions(+), 64 deletions(-)
diff --git a/src/main/java/net/shibboleth/utilities/java/support/httpclient/FileCachingHttpClientBuilder.java b/src/main/java/net/shibboleth/utilities/java/support/httpclient/FileCachingHttpClientBuilder.java
index 68e8cdb..3d1f927 100644
--- a/src/main/java/net/shibboleth/utilities/java/support/httpclient/FileCachingHttpClientBuilder.java
+++ b/src/main/java/net/shibboleth/utilities/java/support/httpclient/FileCachingHttpClientBuilder.java
@@ -19,12 +19,12 @@ package net.shibboleth.utilities.java.support.httpclient;
import java.io.File;
import java.io.IOException;
+import java.time.Duration;
import java.util.Timer;
import java.util.TimerTask;
import javax.annotation.Nonnull;
-import net.shibboleth.utilities.java.support.annotation.Duration;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.component.ComponentSupport;
@@ -102,7 +102,7 @@ public class FileCachingHttpClientBuilder extends HttpClientBuilder {
private long maxCacheEntrySize;
/** Interval at which the storage maintenance task should run. */
- @Duration private long maintentanceTaskInterval;
+ @Nonnull private Duration maintentanceTaskInterval;
/** The current managed storage instance. */
private ManagedHttpCacheStorage managedStorage;
@@ -124,8 +124,7 @@ public class FileCachingHttpClientBuilder extends HttpClientBuilder {
cacheDir = new File(System.getProperty("java.io.tmpdir") + File.separator + "wwwcache");
maxCacheEntries = 100;
maxCacheEntrySize = 10485760;
- // 30 minutes
- maintentanceTaskInterval = 30*60*1000;
+ maintentanceTaskInterval = Duration.ofMinutes(30);
}
/**
@@ -198,20 +197,22 @@ public class FileCachingHttpClientBuilder extends HttpClientBuilder {
/**
* Get the interval at which the storage maintenance task should run.
*
- * @return the maintenance task interval, in milliseconds
+ * @return the maintenance task interval
*/
- public long getMaintentanceTaskInterval() {
+ @Nonnull public Duration getMaintentanceTaskInterval() {
return maintentanceTaskInterval;
}
/**
* Set the interval at which the storage maintenance task should run.
*
- * @param value the new maintenance task interval, in milliseconds
+ * @param value the new maintenance task interval
*/
- public void setMaintentanceTaskInterval(final long value) {
- maintentanceTaskInterval = Constraint.isGreaterThan(0, value,
- "Maintenance task interval must be greater than 0");
+ public void setMaintentanceTaskInterval(@Nonnull final Duration value) {
+ Constraint.isNotNull(value, "Interval cannot be null");
+ Constraint.isFalse(value.isNegative() || value.isZero(), "Interval must be positive");
+
+ maintentanceTaskInterval = value;
}
/** {@inheritDoc} */
@@ -255,14 +256,14 @@ public class FileCachingHttpClientBuilder extends HttpClientBuilder {
final ManagedHttpCacheStorage tempStorage = managedStorage;
// Null this out so we don't keep a reference, inhibiting garbage collection.
managedStorage = null;
- return new StorageManagingHttpClient(client, tempStorage, getMaintentanceTaskInterval());
+ return new StorageManagingHttpClient(client, tempStorage, getMaintentanceTaskInterval().toMillis());
}
/**
* Class which wraps a caching instance of {@link CloseableHttpClient} and its associated
* {@link ManagedHttpCacheStorage}, and manages the scheduled maintenance and lifecycle of the latter.
*/
- public static class StorageManagingHttpClient extends CloseableHttpClient
+ private static class StorageManagingHttpClient extends CloseableHttpClient
implements InitializableComponent, DestructableComponent {
/** Logger. */
@@ -298,7 +299,6 @@ public class FileCachingHttpClientBuilder extends HttpClientBuilder {
*/
public StorageManagingHttpClient(@Nonnull final CloseableHttpClient wrappedClient,
@Nonnull final ManagedHttpCacheStorage managedStorage, final long taskInterval) {
- super();
httpClient = Constraint.isNotNull(wrappedClient, "HttpClient was null");
storage = Constraint.isNotNull(managedStorage, "ManagedHttpCacheStorage was null");
maintenanceTaskInterval = taskInterval;
@@ -378,7 +378,7 @@ public class FileCachingHttpClientBuilder extends HttpClientBuilder {
/**
* Scheduled task to manage an instance of {@link ManagedHttpCacheStorage}.
*/
- public static class StorageMaintenanceTask extends TimerTask {
+ private static class StorageMaintenanceTask extends TimerTask {
/** Logger. */
private Logger log = LoggerFactory.getLogger(StorageMaintenanceTask.class);
diff --git a/src/main/java/net/shibboleth/utilities/java/support/httpclient/HttpClientBuilder.java b/src/main/java/net/shibboleth/utilities/java/support/httpclient/HttpClientBuilder.java
index 7237cbb..032ba4e 100644
--- a/src/main/java/net/shibboleth/utilities/java/support/httpclient/HttpClientBuilder.java
+++ b/src/main/java/net/shibboleth/utilities/java/support/httpclient/HttpClientBuilder.java
@@ -19,6 +19,7 @@ package net.shibboleth.utilities.java.support.httpclient;
import java.net.InetAddress;
import java.net.UnknownHostException;
+import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -44,7 +45,6 @@ import org.apache.http.util.CharsetUtils;
import com.google.common.base.Predicates;
import com.google.common.collect.Collections2;
-import net.shibboleth.utilities.java.support.annotation.Duration;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
import net.shibboleth.utilities.java.support.annotation.constraint.NotLive;
import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
@@ -153,7 +153,7 @@ public class HttpClientBuilder {
/**
* Maximum period inactivity between two consecutive data packets in milliseconds. Default value: 60000 (60 seconds)
*/
- @Duration private int socketTimeout;
+ @Nonnull private Duration socketTimeout;
/** Socket buffer size in bytes. Default size is 8192 bytes. */
private int socketBufferSize;
@@ -162,13 +162,13 @@ public class HttpClientBuilder {
* Maximum length of time in milliseconds to wait for the connection to be established. Default value: 60000 (60
* seconds)
*/
- @Duration private int connectionTimeout;
+ @Nonnull private Duration connectionTimeout;
/**
* Maximum length of time in milliseconds to wait for a connection to be returned from the connection
* manager. Default value: 60000 (60 seconds);
*/
- @Duration private int connectionRequestTimeout;
+ @Nonnull private Duration connectionRequestTimeout;
/**
* Max total simultaneous connections allowed by the pooling connection manager.
@@ -283,9 +283,9 @@ public class HttpClientBuilder {
maxConnectionsPerRoute = -1;
socketLocalAddress = null;
socketBufferSize = 8192;
- socketTimeout = 60*1000;
- connectionTimeout = 60*1000;
- connectionRequestTimeout = 60*1000;
+ socketTimeout = Duration.ofSeconds(60);
+ connectionTimeout = Duration.ofSeconds(60);
+ connectionRequestTimeout = Duration.ofSeconds(60);
connectionDisregardTLSCertificate = false;
connectionCloseAfterResponse = true;
connectionStaleCheck = false;
@@ -364,26 +364,26 @@ public class HttpClientBuilder {
}
/**
- * Gets the maximum period inactivity between two consecutive data packets in milliseconds. A value of less than 1
+ * Gets the maximum period inactivity between two consecutive data packets. A value of less than 1 ms
* indicates no timeout.
*
- * @return maximum period inactivity between two consecutive data packets in milliseconds
+ * @return maximum period inactivity between two consecutive data packets
*/
- @Duration public int getSocketTimeout() {
+ @Nonnull public Duration getSocketTimeout() {
return socketTimeout;
}
/**
- * Sets the maximum period inactivity between two consecutive data packets in milliseconds. A value of less than 1
+ * Sets the maximum period inactivity between two consecutive data packets. A value of less than 1 ms
* indicates no timeout.
*
- * @param timeout maximum period inactivity between two consecutive data packets in milliseconds
+ * @param timeout maximum period inactivity between two consecutive data packets
*/
- public void setSocketTimeout(@Duration final long timeout) {
- if (timeout > Integer.MAX_VALUE) {
- throw new IllegalArgumentException("Timeout was too large");
- }
- this.socketTimeout = (int) timeout;
+ public void setSocketTimeout(@Nonnull final Duration timeout) {
+ Constraint.isNotNull(timeout, "Timeout cannot be null");
+ Constraint.isLessThanOrEqual(Integer.MAX_VALUE, timeout.toMillis(), "Timeout too large");
+
+ socketTimeout = timeout;
}
/**
@@ -405,49 +405,49 @@ public class HttpClientBuilder {
}
/**
- * Gets the maximum length of time in milliseconds to wait for the connection to be established. A value of less
- * than 1 indicates no timeout.
+ * Gets the maximum length of time to wait for the connection to be established. A value of less
+ * than 1 ms indicates no timeout.
*
- * @return maximum length of time in milliseconds to wait for the connection to be established
+ * @return maximum length of time to wait for the connection to be established
*/
- @Duration public int getConnectionTimeout() {
+ @Nonnull public Duration getConnectionTimeout() {
return connectionTimeout;
}
/**
- * Sets the maximum length of time in milliseconds to wait for the connection to be established. A value of less
- * than 1 indicates no timeout.
+ * Sets the maximum length of time to wait for the connection to be established. A value of less
+ * than 1 ms indicates no timeout.
*
- * @param timeout maximum length of time in milliseconds to wait for the connection to be established
+ * @param timeout maximum length of time to wait for the connection to be established
*/
- public void setConnectionTimeout(@Duration final long timeout) {
- if (timeout > Integer.MAX_VALUE) {
- throw new IllegalArgumentException("Timeout was too large");
- }
- connectionTimeout = (int) timeout;
+ public void setConnectionTimeout(@Nonnull final Duration timeout) {
+ Constraint.isNotNull(timeout, "Connection timeout cannot be null");
+ Constraint.isLessThanOrEqual(Integer.MAX_VALUE, timeout.toMillis(), "Connection timeout too large");
+
+ connectionTimeout = timeout;
}
/**
- * Gets the maximum length of time in milliseconds to wait for a connection to be returned from the connection
- * manager. A value of less than 1 indicates no timeout.
+ * Gets the maximum length of time to wait for a connection to be returned from the connection
+ * manager. A value of less than 1 ms indicates no timeout.
*
- * @return maximum length of time in milliseconds to wait for the connection to be established
+ * @return maximum length of time to wait for the connection to be established
*/
- @Duration public int getConnectionRequestTimeout() {
+ @Nonnull public Duration getConnectionRequestTimeout() {
return connectionRequestTimeout;
}
/**
- * Sets the maximum length of time in milliseconds to wait for a connection to be returned from the connection
- * manager. A value of less than 1 indicates no timeout.
+ * Sets the maximum length of time to wait for a connection to be returned from the connection
+ * manager. A value of less than 1 ms indicates no timeout.
*
- * @param timeout maximum length of time in milliseconds to wait for the connection to be established
+ * @param timeout maximum length of time to wait for the connection to be established
*/
- public void setConnectionRequestTimeout(@Duration final long timeout) {
- if (timeout > Integer.MAX_VALUE) {
- throw new IllegalArgumentException("Timeout was too large");
- }
- connectionRequestTimeout = (int) timeout;
+ public void setConnectionRequestTimeout(@Nonnull final Duration timeout) {
+ Constraint.isNotNull(timeout, "Connection request timeout cannot be null");
+ Constraint.isLessThanOrEqual(Integer.MAX_VALUE, timeout.toMillis(), "Connection request timeout too large");
+
+ connectionRequestTimeout = timeout;
}
/**
@@ -1051,16 +1051,16 @@ public class HttpClientBuilder {
requestConfigBuilder.setLocalAddress(socketLocalAddress);
}
- if (socketTimeout >= 0) {
- requestConfigBuilder.setSocketTimeout(socketTimeout);
+ if (!socketTimeout.isNegative()) {
+ requestConfigBuilder.setSocketTimeout((int) socketTimeout.toMillis());
}
- if (connectionTimeout >= 0) {
- requestConfigBuilder.setConnectTimeout(connectionTimeout);
+ if (!connectionTimeout.isNegative()) {
+ requestConfigBuilder.setConnectTimeout((int) connectionTimeout.toMillis());
}
- if (connectionRequestTimeout >= 0) {
- requestConfigBuilder.setConnectionRequestTimeout(connectionRequestTimeout);
+ if (!connectionRequestTimeout.isNegative()) {
+ requestConfigBuilder.setConnectionRequestTimeout((int) connectionRequestTimeout.toMillis());
}
requestConfigBuilder.setStaleConnectionCheckEnabled(connectionStaleCheck);
diff --git a/src/test/java/net/shibboleth/utilities/java/support/httpclient/HttpClientBuilderTest.java b/src/test/java/net/shibboleth/utilities/java/support/httpclient/HttpClientBuilderTest.java
index 34cdfd2..3cf0409 100644
--- a/src/test/java/net/shibboleth/utilities/java/support/httpclient/HttpClientBuilderTest.java
+++ b/src/test/java/net/shibboleth/utilities/java/support/httpclient/HttpClientBuilderTest.java
@@ -18,10 +18,14 @@
package net.shibboleth.utilities.java.support.httpclient;
+import java.time.Duration;
+
import org.apache.http.client.HttpClient;
import org.testng.Assert;
import org.testng.annotations.Test;
+import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
+
public class HttpClientBuilderTest {
// Default timeouts
@@ -29,9 +33,16 @@ public class HttpClientBuilderTest {
final HttpClientBuilder builder = new HttpClientBuilder();
// Check the defaults at the builder level
- Assert.assertEquals(builder.getConnectionTimeout(), 60000);
- Assert.assertEquals(builder.getSocketTimeout(), 60000);
- Assert.assertEquals(builder.getConnectionRequestTimeout(), 60000);
+ Assert.assertEquals(builder.getConnectionTimeout(), Duration.ofSeconds(60));
+ Assert.assertEquals(builder.getSocketTimeout(), Duration.ofSeconds(60));
+ Assert.assertEquals(builder.getConnectionRequestTimeout(), Duration.ofSeconds(60));
+
+ try {
+ builder.setSocketTimeout(Duration.ofMillis(Long.MAX_VALUE));
+ Assert.fail();
+ } catch (final ConstraintViolationException e) {
+
+ }
// Just make sure we can create a client, too
final HttpClient client = builder.buildClient();
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list