[spring-extensions] branch master updated: JSPT-33 - Refactor HttpClient builders and factory beans

Scott Cantor cantor.2 at osu.edu
Wed Mar 6 18:28:50 EST 2019


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch master
in repository spring-extensions.

View the commit online:
http://git.shibboleth.net/view/?p=spring-extensions.git;a=commit;h=41ffe73a8b9103f9e95fbbeffc494e6258993214

The following commit(s) were added to refs/heads/master by this push:
       new  41ffe73   JSPT-33 - Refactor HttpClient builders and factory beans
41ffe73 is described below

commit 41ffe73a8b9103f9e95fbbeffc494e6258993214
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Mar 6 18:28:48 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.
---
 .../factory/FileCachingHttpClientFactoryBean.java  |  88 +++-----
 .../ext/spring/factory/HttpClientFactoryBean.java  | 230 +++------------------
 .../InMemoryCachingHttpClientFactoryBean.java      |  64 +++---
 src/test/resources/logback-test.xml                |   4 +-
 .../shibboleth/ext/spring/resource/newStyle.xml    |   9 +
 .../shibboleth/ext/spring/resource/oldStyle.xml    |   9 +
 6 files changed, 121 insertions(+), 283 deletions(-)

diff --git a/src/main/java/net/shibboleth/ext/spring/factory/FileCachingHttpClientFactoryBean.java b/src/main/java/net/shibboleth/ext/spring/factory/FileCachingHttpClientFactoryBean.java
index 1efd3a5..f461d75 100644
--- a/src/main/java/net/shibboleth/ext/spring/factory/FileCachingHttpClientFactoryBean.java
+++ b/src/main/java/net/shibboleth/ext/spring/factory/FileCachingHttpClientFactoryBean.java
@@ -17,82 +17,52 @@
 
 package net.shibboleth.ext.spring.factory;
 
-import java.util.ArrayList;
-import java.util.List;
+import javax.annotation.Nullable;
 
-import net.shibboleth.utilities.java.support.component.InitializableComponent;
+import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.httpclient.FileCachingHttpClientBuilder;
-import net.shibboleth.utilities.java.support.httpclient.HttpClientBuilder;
 
 import org.apache.http.client.HttpClient;
+import org.springframework.beans.factory.DisposableBean;
+import org.springframework.beans.factory.FactoryBean;
 
 /**
- * Factory bean to accumulate the parameters into a {@link FileCachingHttpClientBuilder} 
- * and to then emit a {@link org.apache.http.client.HttpClient}.
+ * Factory bean version of {@link FileCachingHttpClientBuilder}.
  * 
- * <p>This class will likely either be removed or moved into an implementation package.
- * Use {@link FileCachingHttpClientBuilder} instead.</p>
- * 
- * @deprecated
+ * <p>This is different than the other factories in order to limit non-singleton use
+ * and implement init/destroy. This can't handle prototypes but that makes no sense when you
+ * consider a shared cache in one directory wouldn't work anyway.</p>
  */
-public class FileCachingHttpClientFactoryBean extends HttpClientFactoryBean {
-    
-    /** List of HttpClients produced by this factory, used to invoke their destroy() 
-     * when this factory instances is destroy()-ed. */
-    private List<HttpClient> clientRefs;
-
-    /** Constructor. */
-    public FileCachingHttpClientFactoryBean() {
-        clientRefs = new ArrayList<>();
-    }
+public class FileCachingHttpClientFactoryBean extends FileCachingHttpClientBuilder
+        implements FactoryBean<HttpClient>, DisposableBean {
 
-    /**
-     * Set the cache directory path.
-     * 
-     * @param cacheDirectory The cacheDirectory to set.
-     */
-    public void setCacheDirectory(final String cacheDirectory) {
-        ((FileCachingHttpClientBuilder)getHttpClientBuilder()).setCacheDirectory(cacheDirectory);
-    }
+    /** Our captive client in singleton cases. */
+    @Nullable private HttpClient singletonInstance;
 
-    /**
-     * Set the maximum number of cached responses.
-     * 
-     * @param maxCacheEntries The maxCacheEntries to set.
-     */
-    public void setMaxCacheEntries(final int maxCacheEntries) {
-        ((FileCachingHttpClientBuilder)getHttpClientBuilder()).setMaxCacheEntries(maxCacheEntries);
+    /** {@inheritDoc} */
+    public boolean isSingleton() {
+        return true;
     }
-
-    /**
-     * Set the maximum response body size, in bytes, that will be eligible for caching.
-     * 
-     * @param maxCacheEntrySize The maxCacheEntrySize to set.
-     */
-    public void setMaxCacheEntrySize(final long maxCacheEntrySize) {
-        ((FileCachingHttpClientBuilder)getHttpClientBuilder()).setMaxCacheEntrySize(maxCacheEntrySize);
+    
+    /** {@inheritDoc} */
+    public Class<HttpClient> getObjectType() {
+        return HttpClient.class;
     }
-
+    
     /** {@inheritDoc} */
-    @Override
-    protected HttpClientBuilder createHttpClientBuilder() {
-        return new FileCachingHttpClientBuilder();
+    public void destroy() {
+        ComponentSupport.destroy(singletonInstance);
     }
 
     /** {@inheritDoc} */
-    @Override
-    protected HttpClient doCreateInstance() throws Exception {
-        final HttpClient client = super.doCreateInstance();
-        synchronized(this) {
-            if (client instanceof InitializableComponent) {
-                final InitializableComponent component = (InitializableComponent) client;
-                if (!component.isInitialized()) {
-                   component.initialize(); 
-                }
-            }
-            clientRefs.add(client);
+    public HttpClient getObject() throws Exception {
+        if (singletonInstance == null) {
+            final HttpClient theBean = buildClient();
+            ComponentSupport.initialize(theBean);
+            singletonInstance = theBean;
         }
-        return client;
+        
+        return singletonInstance;
     }
     
 }
\ No newline at end of file
diff --git a/src/main/java/net/shibboleth/ext/spring/factory/HttpClientFactoryBean.java b/src/main/java/net/shibboleth/ext/spring/factory/HttpClientFactoryBean.java
index 3025d76..5f6c487 100644
--- a/src/main/java/net/shibboleth/ext/spring/factory/HttpClientFactoryBean.java
+++ b/src/main/java/net/shibboleth/ext/spring/factory/HttpClientFactoryBean.java
@@ -19,225 +19,59 @@ package net.shibboleth.ext.spring.factory;
 
 import javax.annotation.Nullable;
 
-import net.shibboleth.utilities.java.support.annotation.Duration;
 import net.shibboleth.utilities.java.support.httpclient.HttpClientBuilder;
 
 import org.apache.http.client.HttpClient;
-import org.apache.http.conn.socket.LayeredConnectionSocketFactory;
+import org.springframework.beans.factory.FactoryBean;
 
 /**
- * Factory bean to accumulate the parameters into a {@link HttpClientBuilder} and to then emit a {@link HttpClient}.
- * 
- * <p>This class will likely either be removed or moved into an implementation package.
- * Use {@link HttpClientBuilder} instead.</p>
- * 
- * @deprecated
+ * Factory bean version of {@link HttpClientBuilder}.
  */
-public class HttpClientFactoryBean extends AbstractComponentAwareFactoryBean<HttpClient> {
+public class HttpClientFactoryBean extends HttpClientBuilder implements FactoryBean<HttpClient> {
 
-    /** Our captive builder. */
-    private final HttpClientBuilder builder;
+    /** Singleton flag. */
+    private boolean singleton;
     
-    /**
-     * Connection timeout.<br/>
-     * We need this field to ensure that Spring does the conversion.
-     */
-    @Duration private long connectionTimeout;
-    
-    /**
-     * Connection request timeout.<br/>
-     * We need this field to ensure that Spring does the conversion.
-     */
-    @Duration private long connectionRequestTimeout;
-
-    /**
-     * Socket timeout.<br/>
-     * We need this field to ensure that Spring does the conversion.
-     */
-    @Duration private long socketTimeout;
+    /** Our captive client in singleton cases. */
+    @Nullable private HttpClient singletonInstance;
 
-    /**
-     * Constructor.
-     *
-     */
+    /** Constructor. */
     public HttpClientFactoryBean() {
-        builder = createHttpClientBuilder();
-    }
-
-    /** {@inheritDoc} */
-    @Override public Class<HttpClient> getObjectType() {
-
-        return HttpClient.class;
-    }
-    
-    /**
-     * Sets the max total simultaneous connections allowed by the pooling connection manager.
-     * 
-     * @param max the max total connection
-     */
-    public void setMaxConnectionsTotal(final int max) {
-        builder.setMaxConnectionsTotal(max);
-    }
-    
-    /**
-     * Sets the max simultaneous connections per route allowed by the pooling connection manager.
-     * 
-     * @param max the max connections per route
-     */
-    public void setMaxConnectionsPerRoute(final int max) {
-        builder.setMaxConnectionsPerRoute(max);
-    }
-
-    /**
-     * 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.
-     * 
-     * @param timeout maximum length of time in milliseconds to wait for the connection to be established
-     */
-    @Duration public void setConnectionTimeout(@Duration final long timeout) {
-        connectionTimeout = timeout;
-        if (timeout > Integer.MAX_VALUE) {
-            throw new IllegalArgumentException("Timeout was too large");
-        }
-        builder.setConnectionTimeout((int) timeout);
-    }
-    
-    /**
-     * 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.
-     * 
-     * @param timeout maximum length of time in milliseconds to wait for a connection from the connection manager
-     */
-    @Duration public void setConnectionRequestTimeout(@Duration final long timeout) {
-        connectionRequestTimeout = timeout;
-        if (timeout > Integer.MAX_VALUE) {
-            throw new IllegalArgumentException("Timeout was too large");
-        }
-        builder.setConnectionRequestTimeout((int) timeout);
-    }
-    
-    /**
-     * Sets the maximum period inactivity between two consecutive data packets in milliseconds. A value of less
-     * than 1 indicates no timeout.
-     * 
-     * @param timeout maximum length of time in milliseconds between two consecutive data packets
-     */
-    @Duration public void setSocketTimeout(@Duration final long timeout) {
-        socketTimeout = timeout;
-        if (timeout > Integer.MAX_VALUE) {
-            throw new IllegalArgumentException("Timeout was too large");
-        }
-        builder.setSocketTimeout((int) timeout);
-    }
-    
-    /**
-     * Set the TLS socket factory to use.
-     * 
-     * @param factory the new socket factory, may be null
-     */
-    public void setTLSSocketFactory(@Nullable final LayeredConnectionSocketFactory factory) {
-        builder.setTLSSocketFactory(factory);
+        singleton = true;
     }
     
     /**
-     * Sets whether the responder's SSL/TLS certificate should be ignored.
+     * Set if a singleton should be created, or a new object on each request
+     * otherwise. Default is {@code true} (a singleton).
      * 
-     * @param disregard whether the responder's SSL/TLS certificate should be ignored
+     * @param flag flag to set
      */
-    public void setConnectionDisregardTLSCertificate(final boolean disregard) {
-        builder.setConnectionDisregardTLSCertificate(disregard);
+    public void setSingleton(final boolean flag) {
+        singleton = flag;
     }
 
-    /**
-     * Sets the hostname of the default proxy used when making connection. A null indicates no default proxy.
-     * 
-     * @param host hostname of the default proxy used when making connection
-     */
-    public void setConnectionProxyHost(@Nullable final String host) {
-        builder.setConnectionProxyHost(host);
-    }
-
-    /**
-     * Sets the port of the default proxy used when making connection.
-     * 
-     * @param port port of the default proxy used when making connection; must be greater than 0 and less than 65536
-     */
-    public void setConnectionProxyPort(final int port) {
-        builder.setConnectionProxyPort(port);
-    }
-
-    /**
-     * Sets the username to use when authenticating to the proxy.
-     * 
-     * @param usename username to use when authenticating to the proxy; may be null
-     */
-    public void setConnectionProxyUsername(@Nullable final String usename) {
-        builder.setConnectionProxyUsername(usename);
-    }
-
-    /**
-     * Sets the password used when authenticating to the proxy.
-     * 
-     * @param password password used when authenticating to the proxy; may be null
-     */
-    public void setConnectionProxyPassword(@Nullable final String password) {
-        builder.setConnectionProxyPassword(password);
-    }
-
-    /**
-     * Sets the user agent to be used when talking to the server. may not be null in which case the default will be
-     * used.
-     * 
-     * @param agent what to set
-     */
-    public void setUserAgent(@Nullable final String agent) {
-        builder.setUserAgent(agent);
-    }
-    
-    /**
-     * Sets whether to disable content compression.
-     * 
-     * @param disable whether to disable content compression
-     * 
-     * @since 5.4.0
-     */
-    public void setDisableContentCompression(final boolean disable) {
-        builder.setDisableContentCompression(disable);
+    /** {@inheritDoc} */
+    public boolean isSingleton() {
+        return singleton;
     }
     
-    /**
-     * Sets whether to disable cookie management.
-     * 
-     * @param disable whether to disable cookie management
-     * 
-     * @since 5.4.0
-     */
-    public void setDisableCookieManagement(final boolean disable) {
-        builder.setDisableCookieManagement(disable);
-    }
-
-    /**
-     * Create and return the instance of {@link HttpClientBuilder} to use.  
-     * Subclasses may override to build a specialized subclass.
-     * 
-     * @return a new builder instance
-     */
-    protected HttpClientBuilder createHttpClientBuilder() {
-        return new HttpClientBuilder();
+    /** {@inheritDoc} */
+    public Class<HttpClient> getObjectType() {
+        return HttpClient.class;
     }
     
-    /**
-     * Get the instance of {@link HttpClientBuilder} to use.
-     * 
-     * @return the existing builder instance in use
-     */
-    protected HttpClientBuilder getHttpClientBuilder() {
-        return builder;
-    }
-
     /** {@inheritDoc} */
-    @Override protected HttpClient doCreateInstance() throws Exception {
-        return builder.buildClient();
+    public synchronized HttpClient getObject() throws Exception {
+        if (isSingleton()) {
+            if (singletonInstance != null) {
+                return singletonInstance;
+            }
+            
+            singletonInstance = buildClient();
+            return singletonInstance;
+        } else {
+            return buildClient();
+        }
     }
     
-}
+}
\ No newline at end of file
diff --git a/src/main/java/net/shibboleth/ext/spring/factory/InMemoryCachingHttpClientFactoryBean.java b/src/main/java/net/shibboleth/ext/spring/factory/InMemoryCachingHttpClientFactoryBean.java
index 2bc552d..6641385 100644
--- a/src/main/java/net/shibboleth/ext/spring/factory/InMemoryCachingHttpClientFactoryBean.java
+++ b/src/main/java/net/shibboleth/ext/spring/factory/InMemoryCachingHttpClientFactoryBean.java
@@ -17,46 +17,62 @@
 
 package net.shibboleth.ext.spring.factory;
 
-import net.shibboleth.utilities.java.support.httpclient.HttpClientBuilder;
+import javax.annotation.Nullable;
+
+import org.apache.http.client.HttpClient;
+import org.springframework.beans.factory.FactoryBean;
+
 import net.shibboleth.utilities.java.support.httpclient.InMemoryCachingHttpClientBuilder;
 
 /**
- * Factory bean to accumulate the parameters into a {@link InMemoryCachingHttpClientBuilder} and to then emit a
- * {@link org.apache.http.client.HttpClient}.
- * 
- * <p>This class will likely either be removed or moved into an implementation package.
- * Use {@link InMemoryCachingHttpClientBuilder} instead.</p>
- * 
- * @deprecated
+ * Factory bean version of {@link InMemoryCachingHttpClientBuilder}.
  */
-public class InMemoryCachingHttpClientFactoryBean extends HttpClientFactoryBean {
+public class InMemoryCachingHttpClientFactoryBean extends InMemoryCachingHttpClientBuilder
+    implements FactoryBean<HttpClient> {
+
+    /** Singleton flag. */
+    private boolean singleton;
+    
+    /** Our captive client in singleton cases. */
+    @Nullable private HttpClient singletonInstance;
 
     /** Constructor. */
     public InMemoryCachingHttpClientFactoryBean() {
-        
+        singleton = true;
     }
-
+    
     /**
-     * Set the maximum number of cached responses.
+     * Set if a singleton should be created, or a new object on each request
+     * otherwise. Default is {@code true} (a singleton).
      * 
-     * @param maxCacheEntries The maxCacheEntries to set.
+     * @param flag flag to set
      */
-    public void setMaxCacheEntries(final int maxCacheEntries) {
-        ((InMemoryCachingHttpClientBuilder) getHttpClientBuilder()).setMaxCacheEntries(maxCacheEntries);
+    public void setSingleton(final boolean flag) {
+        singleton = flag;
     }
 
-    /**
-     * Set the maximum response body size, in bytes, that will be eligible for caching.
-     * 
-     * @param maxCacheEntrySize The maxCacheEntrySize to set.
-     */
-    public void setMaxCacheEntrySize(final long maxCacheEntrySize) {
-        ((InMemoryCachingHttpClientBuilder) getHttpClientBuilder()).setMaxCacheEntrySize(maxCacheEntrySize);
+    /** {@inheritDoc} */
+    public boolean isSingleton() {
+        return singleton;
     }
 
     /** {@inheritDoc} */
-    @Override protected HttpClientBuilder createHttpClientBuilder() {
-        return new InMemoryCachingHttpClientBuilder();
+    public Class<HttpClient> getObjectType() {
+        return HttpClient.class;
+    }
+    
+    /** {@inheritDoc} */
+    public synchronized HttpClient getObject() throws Exception {
+        if (isSingleton()) {
+            if (singletonInstance != null) {
+                return singletonInstance;
+            }
+            
+            singletonInstance = buildClient();
+            return singletonInstance;
+        } else {
+            return buildClient();
+        }
     }
 
 }
\ No newline at end of file
diff --git a/src/test/resources/logback-test.xml b/src/test/resources/logback-test.xml
index f92b9e4..a3f1c9a 100644
--- a/src/test/resources/logback-test.xml
+++ b/src/test/resources/logback-test.xml
@@ -1,8 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <configuration>
 
-    <logger name="net.shibboleth.ext.spring" level="DEBUG"/>
-    <logger name="net.shibboleth.utilities" level="DEBUG"/>
+    <logger name="net.shibboleth.ext.spring" level="INFO"/>
+    <logger name="net.shibboleth.utilities" level="INFO"/>
 
     <appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
         <encoder class="ch.qos.logback.classic.encoder.PatternLayoutEncoder">
diff --git a/src/test/resources/net/shibboleth/ext/spring/resource/newStyle.xml b/src/test/resources/net/shibboleth/ext/spring/resource/newStyle.xml
index 3c9629c..003d1e2 100644
--- a/src/test/resources/net/shibboleth/ext/spring/resource/newStyle.xml
+++ b/src/test/resources/net/shibboleth/ext/spring/resource/newStyle.xml
@@ -6,6 +6,15 @@
                            http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context.xsd
                            http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util.xsd">
                            
+    <!-- This bean MUST be called "conversionService" to work properly. -->
+    <bean id="conversionService" class="org.springframework.context.support.ConversionServiceFactoryBean">
+        <property name="converters">
+            <set>
+                <bean class="net.shibboleth.ext.spring.config.StringToDurationConverter" />
+            </set>
+        </property>
+    </bean>
+                           
     <bean id="shibboleth.NonCachingHttpClient"
           lazy-init="true"
           class="net.shibboleth.ext.spring.factory.HttpClientFactoryBean"
diff --git a/src/test/resources/net/shibboleth/ext/spring/resource/oldStyle.xml b/src/test/resources/net/shibboleth/ext/spring/resource/oldStyle.xml
index 1173881..11f3158 100644
--- a/src/test/resources/net/shibboleth/ext/spring/resource/oldStyle.xml
+++ b/src/test/resources/net/shibboleth/ext/spring/resource/oldStyle.xml
@@ -6,6 +6,15 @@
                            http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context.xsd
                            http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util.xsd">
                            
+    <!-- This bean MUST be called "conversionService" to work properly. -->
+    <bean id="conversionService" class="org.springframework.context.support.ConversionServiceFactoryBean">
+        <property name="converters">
+            <set>
+                <bean class="net.shibboleth.ext.spring.config.StringToDurationConverter" />
+            </set>
+        </property>
+    </bean>
+                           
     <bean id="shibboleth.NonCachingHttpClient"
           lazy-init="true"
           class="net.shibboleth.ext.spring.factory.HttpClientFactoryBean"

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list