You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by aj...@apache.org on 2016/12/08 14:23:27 UTC

[1/2] jena git commit: Removing the use of one-shot clients

Repository: jena
Updated Branches:
  refs/heads/master fb9c4f9a5 -> ab9580d6a


Removing the use of one-shot clients


Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/ab9580d6
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/ab9580d6
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/ab9580d6

Branch: refs/heads/master
Commit: ab9580d6a77cad457438d1f10edc3dddbbe4e500
Parents: 183bfa4
Author: ajs6f <aj...@virginia.edu>
Authored: Thu Nov 17 14:10:07 2016 -0500
Committer: ajs6f <aj...@virginia.edu>
Committed: Thu Dec 8 09:23:00 2016 -0500

----------------------------------------------------------------------
 .../java/org/apache/jena/riot/web/HttpOp.java   | 78 ++++-------------
 .../java/org/apache/jena/fuseki/ServerCtl.java  |  4 +-
 .../java/org/apache/jena/fuseki/TestQuery.java  | 90 +++++++++++---------
 3 files changed, 66 insertions(+), 106 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jena/blob/ab9580d6/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java b/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java
index 7bf511c..65b6c86 100644
--- a/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java
+++ b/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java
@@ -19,6 +19,7 @@
 package org.apache.jena.riot.web;
 
 import static java.lang.String.format ;
+import static org.apache.jena.ext.com.google.common.base.MoreObjects.firstNonNull;
 
 import java.io.IOException ;
 import java.io.InputStream ;
@@ -95,11 +96,17 @@ public class HttpOp {
     /** System wide HTTP operation counter for log messages */
     static private AtomicLong counter = new AtomicLong(0);
 
+    private static final LaxRedirectStrategy laxRedirectStrategy = new LaxRedirectStrategy();
+
     /**
      * Default HttpClient.
      */
-    static private HttpClient defaultHttpClient = null;
+    static private HttpClient defaultHttpClient = createDefaultHttpClient();
 
+    public static HttpClient createDefaultHttpClient() {
+        return createCachingHttpClient();
+    }
+    
     /**
      * Constant for the default User-Agent header that ARQ will use
      */
@@ -115,8 +122,6 @@ public class HttpOp {
      */
     static private HttpResponseHandler nullHandler = HttpResponseLib.nullResponse;
 
-    private static final LaxRedirectStrategy laxRedirectStrategy = new LaxRedirectStrategy();
-
     /** Capture response as a string (UTF-8 assumed) */
     public static class CaptureString implements HttpCaptureResponse<String> {
         String result;
@@ -141,38 +146,12 @@ public class HttpOp {
      */
     public static class CaptureInput implements HttpCaptureResponse<TypedInputStream> {
         private TypedInputStream stream;
-        
-        private boolean closeClient = false;
-        
-        private CloseableHttpClient client;
-        
-        public void setClient(CloseableHttpClient client) {
-            this.client = client;
-        }
-
-        private static class ClientRetainingTypedInputStream extends TypedInputStream {
-
-            private final CloseableHttpClient retainedClient;
-            
-            public ClientRetainingTypedInputStream(InputStream in, String contentType, CloseableHttpClient client) {
-                super(in, contentType);
-                this.retainedClient = client;
-            }
-
-            @Override
-            public void close() {
-                IO.close(retainedClient);
-                super.close();
-            }
-            
-        }
 
         @Override
         public void handle(String baseIRI, HttpResponse response) throws IOException {
             HttpEntity entity = response.getEntity();
             String ct = (entity.getContentType() == null) ? null : entity.getContentType().getValue();
-            stream = closeClient ? new ClientRetainingTypedInputStream(entity.getContent(), ct, client)
-                    : new TypedInputStream(entity.getContent(), ct);
+            stream = new TypedInputStream(entity.getContent(), ct);
         }
 
         @Override
@@ -193,21 +172,13 @@ public class HttpOp {
     }
 
     /**
-     * <p>
      * Performance can be improved by using a shared HttpClient that uses
      * connection pooling. However, pool management is complicated and can lead
      * to starvation (the system locks-up, especially on Java6; it's JVM
      * sensitive).
-     * </p>
-     * <p>
-     * Set to "null" to create a new HttpClient for each call (default
-     * behaviour, more reliable, but slower when many HTTP operation are
-     * attempted).
-     * <p>
      * See the Apache Http Client documentation for more details.
      * 
-     * @param httpClient
-     *            HTTP Client
+     * @param httpClient HTTP Client
      */
     public static void setDefaultHttpClient(HttpClient httpClient) {
         defaultHttpClient = httpClient;
@@ -217,10 +188,11 @@ public class HttpOp {
      * Create an HttpClient that performs connection pooling. This can be used
      * with {@link #setDefaultHttpClient} or provided in the HttpOp calls.
      */
-    public static HttpClient createPoolingHttpClient() {
+    public static CloseableHttpClient createPoolingHttpClient() {
         String s = System.getProperty("http.maxConnections", "5");
         int max = Integer.parseInt(s);
         return HttpClientBuilder.create()
+            .setRedirectStrategy(laxRedirectStrategy)
             .setMaxConnPerRoute(max)
             .setMaxConnTotal(2*max)
             .build() ;
@@ -234,6 +206,7 @@ public class HttpOp {
         String s = System.getProperty("http.maxConnections", "5");
         int max = Integer.parseInt(s);
         return CachingHttpClientBuilder.create()
+            .setRedirectStrategy(laxRedirectStrategy)
             .setMaxConnPerRoute(max)
             .setMaxConnTotal(2*max)
             .build() ;
@@ -1058,19 +1031,11 @@ public class HttpOp {
     private static void exec(String url, HttpUriRequest request, String acceptHeader, HttpResponseHandler handler, HttpClient httpClient, HttpContext httpContext) {
         // whether we should close the client after request execution
         // only true if we built the client right here
-        boolean closeClient = false;
-        if (httpClient == null) {
-            if (getDefaultHttpClient() == null ) {
-                httpClient = makeOneUseClient();
-                closeClient = true;
-            }
-            else httpClient = getDefaultHttpClient();
-        }
+        httpClient = firstNonNull(httpClient, getDefaultHttpClient());
         // and also only true if the handler won't close the client for us
-        closeClient = closeClient && !(handler instanceof CaptureInput);
         try {
             if (handler == null)
-                // This cleans up.
+                // This cleans up left-behind streams
                 handler = nullHandler;
 
             long id = counter.incrementAndGet();
@@ -1095,23 +1060,12 @@ public class HttpOp {
 				final String contentPayload = readPayload(response.getEntity());
 				throw new HttpException(statusLine.getStatusCode(), statusLine.getReasonPhrase(), contentPayload);
             }
-            // Redirects are followed by HttpClient.
-            if (handler != null)
-                handler.handle(baseURI, response);
-            // the cast below is safe because if closeClient then we built the client in this method 
-            if (closeClient) IO.close((CloseableHttpClient) httpClient);
+            if (handler != null) handler.handle(baseURI, response);
         } catch (IOException ex) {
             throw new HttpException(ex);
         }
     }
 
-    /**
-     * @return a "default" HttpClient for use with one request
-     */
-    private static CloseableHttpClient makeOneUseClient() {
-        return HttpClientBuilder.create().setRedirectStrategy(laxRedirectStrategy).build();
-    }
-
 	public static String readPayload(HttpEntity entity) throws IOException {
         return entity == null ? null : EntityUtils.toString(entity, ContentType.getOrDefault(entity).getCharset());
 	}

http://git-wip-us.apache.org/repos/asf/jena/blob/ab9580d6/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/ServerCtl.java
----------------------------------------------------------------------
diff --git a/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/ServerCtl.java b/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/ServerCtl.java
index 7df203b..8c1407b 100644
--- a/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/ServerCtl.java
+++ b/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/ServerCtl.java
@@ -92,8 +92,6 @@ public class ServerCtl {
     @After       public void ctlAfterTest()          { ServerCtl.ctlAfterTest(); }
      
     */
-    
-    static HttpClient defaultHttpClient = HttpOp.getDefaultHttpClient();
 
     // Note: it is important to cleanly close a PoolingHttpClient across server restarts
     // otherwise the pooled connections remain for the old server. 
@@ -182,7 +180,7 @@ public class ServerCtl {
 
     /** Restore the original setup */
     private static void resetDefaultHttpClient() {
-        setHttpClient(defaultHttpClient);
+        setHttpClient(HttpOp.createDefaultHttpClient());
     }
     
     /** Set the HttpClient - close the old one if appropriate */

http://git-wip-us.apache.org/repos/asf/jena/blob/ab9580d6/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/TestQuery.java
----------------------------------------------------------------------
diff --git a/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/TestQuery.java b/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/TestQuery.java
index f439bca..624c56e 100644
--- a/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/TestQuery.java
+++ b/jena-fuseki2/jena-fuseki-core/src/test/java/org/apache/jena/fuseki/TestQuery.java
@@ -29,12 +29,14 @@ import java.net.HttpURLConnection ;
 import java.net.URL ;
 import java.util.Iterator ;
 
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.jena.atlas.web.AcceptList ;
 import org.apache.jena.atlas.web.MediaType;
 import org.apache.jena.graph.Node ;
 import org.apache.jena.graph.Triple ;
 import org.apache.jena.query.* ;
 import org.apache.jena.rdf.model.Model ;
+import org.apache.jena.riot.web.HttpOp;
 import org.apache.jena.sparql.core.Quad ;
 import org.apache.jena.sparql.core.Var ;
 import org.apache.jena.sparql.engine.binding.Binding ;
@@ -198,60 +200,66 @@ public class TestQuery extends AbstractFusekiTest {
     }
 
     // Conneg tests:
-    // These avoid pooling connection pooling.
-    // It leads to lock up if the list is long (contentTypeTriXxml seems signiifcant)
-    // Hence: ServerCtl.setHttpClient(null) ;
+    // These use independent connection pooling.
+    // Sharing pooling too much leads to lock up if the list is long (contentTypeTriXxml seems significant)
+    // Hence: try (CloseableHttpClient client = HttpOp.createPoolingHttpClient()) { ... qExec.setClient(client); ... } 
     
     
     @Test
-    public void query_construct_conneg() {
-        ServerCtl.setHttpClient(null) ;
-        
-        String query = " CONSTRUCT {?s ?p ?o} WHERE {?s ?p ?o}" ;
-        for (MediaType type: rdfOfferTest.entries()){
-            
-            String contentType = type.toHeaderString();
-            try ( QueryEngineHTTP qExec = (QueryEngineHTTP) QueryExecutionFactory.sparqlService(serviceQuery(), query) ) {
-                qExec.setModelContentType( contentType );
-                Iterator<Triple> iter = qExec.execConstructTriples();
-                assertTrue(iter.hasNext()) ;
-                String x = qExec.getHttpResponseContentType() ;
-                assertEquals( contentType , x ) ;
+    public void query_construct_conneg() throws IOException {
+        try (CloseableHttpClient client = HttpOp.createPoolingHttpClient()) {
+            String query = " CONSTRUCT {?s ?p ?o} WHERE {?s ?p ?o}";
+            for (MediaType type : rdfOfferTest.entries()) {
+
+                String contentType = type.toHeaderString();
+                try (QueryEngineHTTP qExec = (QueryEngineHTTP) QueryExecutionFactory.sparqlService(serviceQuery(),
+                        query)) {
+                    qExec.setModelContentType(contentType);
+                    qExec.setClient(client);
+                    Iterator<Triple> iter = qExec.execConstructTriples();
+                    assertTrue(iter.hasNext());
+                    String x = qExec.getHttpResponseContentType();
+                    assertEquals(contentType, x);
+                }
             }
         }
     }
 
     @Test
-    public void query_construct_quad_conneg() {
-        ServerCtl.setHttpClient(null) ;
-
-        String queryString = " CONSTRUCT { GRAPH ?g {?s ?p ?o} } WHERE { GRAPH ?g {?s ?p ?o}}" ;
-        Query query = QueryFactory.create(queryString, Syntax.syntaxARQ);
-        for (MediaType type: quadsOfferTest.entries()){
-            String contentType = type.toHeaderString();
-            try ( QueryEngineHTTP qExec = (QueryEngineHTTP) QueryExecutionFactory.sparqlService(serviceQuery(), query) ) {
-                qExec.setDatasetContentType( contentType );
-                Iterator<Quad> iter = qExec.execConstructQuads();
-                assertTrue(iter.hasNext()) ;
-                String x = qExec.getHttpResponseContentType() ;
-                assertEquals( contentType , x ) ;
+    public void query_construct_quad_conneg() throws IOException {
+        try (CloseableHttpClient client = HttpOp.createPoolingHttpClient()) {
+            String queryString = " CONSTRUCT { GRAPH ?g {?s ?p ?o} } WHERE { GRAPH ?g {?s ?p ?o}}";
+            Query query = QueryFactory.create(queryString, Syntax.syntaxARQ);
+            for (MediaType type : quadsOfferTest.entries()) {
+                String contentType = type.toHeaderString();
+                try (QueryEngineHTTP qExec = (QueryEngineHTTP) QueryExecutionFactory.sparqlService(serviceQuery(),
+                        query)) {
+                    qExec.setDatasetContentType(contentType);
+                    qExec.setClient(client);
+                    Iterator<Quad> iter = qExec.execConstructQuads();
+                    assertTrue(iter.hasNext());
+                    String x = qExec.getHttpResponseContentType();
+                    assertEquals(contentType, x);
+                }
             }
         }
     }
 
     @Test
-    public void query_describe_conneg() {
-        ServerCtl.setHttpClient(null) ;
-
-        String query = "DESCRIBE ?s WHERE {?s ?p ?o}" ;
-        for (MediaType type: rdfOfferTest.entries()){
-            String contentType = type.toHeaderString();
-            try ( QueryEngineHTTP qExec = (QueryEngineHTTP) QueryExecutionFactory.sparqlService(serviceQuery(), query) ) {
-                qExec.setModelContentType( contentType );
-                Model m = qExec.execDescribe() ;
-                String x = qExec.getHttpResponseContentType() ;
-                assertEquals( contentType , x ) ;
-                assertFalse(m.isEmpty()) ;
+    public void query_describe_conneg() throws IOException {
+        try (CloseableHttpClient client = HttpOp.createPoolingHttpClient()) {
+            String query = "DESCRIBE ?s WHERE {?s ?p ?o}";
+            for (MediaType type : rdfOfferTest.entries()) {
+                String contentType = type.toHeaderString();
+                try (QueryEngineHTTP qExec = (QueryEngineHTTP) QueryExecutionFactory.sparqlService(serviceQuery(),
+                        query)) {
+                    qExec.setModelContentType(contentType);
+                    qExec.setClient(client);
+                    Model m = qExec.execDescribe();
+                    String x = qExec.getHttpResponseContentType();
+                    assertEquals(contentType, x);
+                    assertFalse(m.isEmpty());
+                }
             }
         }
     }


[2/2] jena git commit: JENA-1263: Recommended redirect behavior

Posted by aj...@apache.org.
JENA-1263: Recommended redirect behavior


Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/183bfa49
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/183bfa49
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/183bfa49

Branch: refs/heads/master
Commit: 183bfa4923adec6560aed793bc01d921f44f6d33
Parents: fb9c4f9
Author: ajs6f <aj...@virginia.edu>
Authored: Thu Nov 17 11:59:29 2016 -0500
Committer: ajs6f <aj...@virginia.edu>
Committed: Thu Dec 8 09:23:00 2016 -0500

----------------------------------------------------------------------
 .../src/main/java/org/apache/jena/riot/web/HttpOp.java | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jena/blob/183bfa49/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java b/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java
index 7830f9a..7bf511c 100644
--- a/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java
+++ b/jena-arq/src/main/java/org/apache/jena/riot/web/HttpOp.java
@@ -36,7 +36,7 @@ import org.apache.http.entity.InputStreamEntity ;
 import org.apache.http.entity.StringEntity ;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClientBuilder ;
-import org.apache.http.impl.client.HttpClients;
+import org.apache.http.impl.client.LaxRedirectStrategy;
 import org.apache.http.impl.client.cache.CachingHttpClientBuilder;
 import org.apache.http.message.BasicNameValuePair ;
 import org.apache.http.protocol.HttpContext ;
@@ -115,6 +115,8 @@ public class HttpOp {
      */
     static private HttpResponseHandler nullHandler = HttpResponseLib.nullResponse;
 
+    private static final LaxRedirectStrategy laxRedirectStrategy = new LaxRedirectStrategy();
+
     /** Capture response as a string (UTF-8 assumed) */
     public static class CaptureString implements HttpCaptureResponse<String> {
         String result;
@@ -1059,7 +1061,7 @@ public class HttpOp {
         boolean closeClient = false;
         if (httpClient == null) {
             if (getDefaultHttpClient() == null ) {
-                httpClient = HttpClients.createMinimal();
+                httpClient = makeOneUseClient();
                 closeClient = true;
             }
             else httpClient = getDefaultHttpClient();
@@ -1103,6 +1105,13 @@ public class HttpOp {
         }
     }
 
+    /**
+     * @return a "default" HttpClient for use with one request
+     */
+    private static CloseableHttpClient makeOneUseClient() {
+        return HttpClientBuilder.create().setRedirectStrategy(laxRedirectStrategy).build();
+    }
+
 	public static String readPayload(HttpEntity entity) throws IOException {
         return entity == null ? null : EntityUtils.toString(entity, ContentType.getOrDefault(entity).getCharset());
 	}