Aklakan commented on code in PR #2253:
URL: https://github.com/apache/jena/pull/2253#discussion_r1481502151


##########
jena-rdfconnection/src/main/java/org/apache/jena/rdflink/RDFLinkHTTP.java:
##########
@@ -241,77 +258,125 @@ public QueryExec query(Query query) {
 
     @Override
     public QueryExecBuilder newQuery() {
-        return createQExecBuilder();
+        return createQExecBuilder(null);
     }
 
     // Create the QExec
 
     private QueryExec queryExec(Query query, String queryString, QueryType 
queryType) {
-        checkQuery();
-        if ( query == null && queryString == null )
-            throw new InternalErrorException("Both query and query string are 
null");
-        if ( query == null ) {
-            if ( parseCheckQueries )
-                // Don't retain the query.
-                QueryFactory.create(queryString);
+        QueryExecHTTPBuilder builder = createQExecBuilder(queryType);
+        if (queryString != null) {
+            builder = builder.queryString(queryString);
         }
-
-        // Use the query string as provided if possible, otherwise serialize 
the query.
-        String queryStringToSend = ( queryString != null ) ? queryString : 
query.toString();
-        return createQExec(query, queryStringToSend, queryType);
+        if (query != null) {
+            builder = builder.query(query);
+        }
+        return builder.build();
     }
 
     // Create the QExec
 
-    /** Create a builder, configured with the link setup. */
-    private QueryExecHTTPBuilder createQExecBuilder() {
-        return 
QueryExecHTTPBuilder.create().endpoint(svcQuery).httpClient(httpClient);
-    }
-
-    private QueryExec createQExec(Query query, String queryStringToSend, 
QueryType queryType) {
-        QueryExecHTTPBuilder builder = 
createQExecBuilder().queryString(queryStringToSend);
-        QueryType qt = queryType;
-        if ( query != null && qt == null )
-            qt = query.queryType();
-        if ( qt == null )
-            qt = QueryType.UNKNOWN;
-        // Set the accept header - use the most specific method.
-        String requestAcceptHeader = null;
-        switch(qt) {
-            case SELECT :
-                if ( acceptSelectResult != null )
-                    requestAcceptHeader = acceptSelectResult;
-                break;
-            case ASK :
-                if ( acceptAskResult != null )
-                    requestAcceptHeader = acceptAskResult;
-                break;
-            case DESCRIBE :
-            case CONSTRUCT :
-                if ( acceptGraph != null )
-                    requestAcceptHeader = acceptGraph;
-                break;
-            case UNKNOWN:
-                // All-purpose content type.
-                if ( acceptSparqlResults != null )
-                    requestAcceptHeader = acceptSparqlResults;
-                else
-                    // No idea! Set an "anything" and hope.
-                    // (Reasonable chance this is going to end up as HTML 
though.)
-                    requestAcceptHeader = "*/*";
-            default :
-                break;
+    /**
+     * An internal subclass of {@link QueryExecHTTPBuilder} that overrides
+     * {@link #buildX(HttpClient, Query, String, Context)}
+     * in order to have the accept header field derived based on this {@link 
RDFLinkHTTP} configuration.
+     */
+    /* package */ class QueryExecHTTPBuilderOverRDFLinkHTTP
+        extends QueryExecHTTPBuilder
+    {
+        protected QueryType presetQueryType;
+
+        public QueryExecHTTPBuilderOverRDFLinkHTTP(QueryType presetQueryType) {
+            super();
+            this.presetQueryType = presetQueryType;
         }
 
-        // Make sure it was set somehow.
-        if ( requestAcceptHeader == null )
-            throw new JenaConnectionException("No Accept header");
-        if ( requestAcceptHeader != null )
-            builder.acceptHeader(requestAcceptHeader);
-        // Delayed creation so QueryExecution.setTimeout works.
+        @Override
+        protected QueryExecHTTP buildX(HttpClient hClient, Query queryActual, 
String queryStringActual, Context cxt) {
+            checkQuery();
+            // The check below is handled in the base class before this method 
is called.
+            // if ( query == null && queryString == null )
+            //     throw new InternalErrorException("Both query and query 
string are null");
+
+            QueryType finalQueryType = presetQueryType != null
+                    ? presetQueryType
+                    : queryActual != null
+                        ? queryActual.queryType()
+                        : QueryType.UNKNOWN;
+
+            Query parsedQuery = query;
+            if ( queryActual == null ) {
+                if ( parseCheckQueries ) {
+                    // Don't retain the query
+                    // However, use it to derive the queryType
+                    parsedQuery = QueryFactory.create(queryStringActual);
+                    if (QueryType.UNKNOWN.equals(finalQueryType)) {
+                        finalQueryType = parsedQuery.queryType();
+                    }
+                }
+            }
+
+            // Use the query string as provided if possible, otherwise 
serialize the query.
+            String queryStringToSend = ( queryStringActual != null ) ? 
queryStringActual : queryActual.toString();

Review Comment:
   Redundant, handled by the base class.



##########
jena-rdfconnection/src/main/java/org/apache/jena/rdflink/RDFLinkHTTP.java:
##########
@@ -241,77 +258,125 @@ public QueryExec query(Query query) {
 
     @Override
     public QueryExecBuilder newQuery() {
-        return createQExecBuilder();
+        return createQExecBuilder(null);
     }
 
     // Create the QExec
 
     private QueryExec queryExec(Query query, String queryString, QueryType 
queryType) {
-        checkQuery();
-        if ( query == null && queryString == null )
-            throw new InternalErrorException("Both query and query string are 
null");
-        if ( query == null ) {
-            if ( parseCheckQueries )
-                // Don't retain the query.
-                QueryFactory.create(queryString);
+        QueryExecHTTPBuilder builder = createQExecBuilder(queryType);
+        if (queryString != null) {
+            builder = builder.queryString(queryString);
         }
-
-        // Use the query string as provided if possible, otherwise serialize 
the query.
-        String queryStringToSend = ( queryString != null ) ? queryString : 
query.toString();
-        return createQExec(query, queryStringToSend, queryType);
+        if (query != null) {
+            builder = builder.query(query);
+        }
+        return builder.build();
     }
 
     // Create the QExec
 
-    /** Create a builder, configured with the link setup. */
-    private QueryExecHTTPBuilder createQExecBuilder() {
-        return 
QueryExecHTTPBuilder.create().endpoint(svcQuery).httpClient(httpClient);
-    }
-
-    private QueryExec createQExec(Query query, String queryStringToSend, 
QueryType queryType) {
-        QueryExecHTTPBuilder builder = 
createQExecBuilder().queryString(queryStringToSend);
-        QueryType qt = queryType;
-        if ( query != null && qt == null )
-            qt = query.queryType();
-        if ( qt == null )
-            qt = QueryType.UNKNOWN;
-        // Set the accept header - use the most specific method.
-        String requestAcceptHeader = null;
-        switch(qt) {
-            case SELECT :
-                if ( acceptSelectResult != null )
-                    requestAcceptHeader = acceptSelectResult;
-                break;
-            case ASK :
-                if ( acceptAskResult != null )
-                    requestAcceptHeader = acceptAskResult;
-                break;
-            case DESCRIBE :
-            case CONSTRUCT :
-                if ( acceptGraph != null )
-                    requestAcceptHeader = acceptGraph;
-                break;
-            case UNKNOWN:
-                // All-purpose content type.
-                if ( acceptSparqlResults != null )
-                    requestAcceptHeader = acceptSparqlResults;
-                else
-                    // No idea! Set an "anything" and hope.
-                    // (Reasonable chance this is going to end up as HTML 
though.)
-                    requestAcceptHeader = "*/*";
-            default :
-                break;
+    /**
+     * An internal subclass of {@link QueryExecHTTPBuilder} that overrides
+     * {@link #buildX(HttpClient, Query, String, Context)}
+     * in order to have the accept header field derived based on this {@link 
RDFLinkHTTP} configuration.
+     */
+    /* package */ class QueryExecHTTPBuilderOverRDFLinkHTTP
+        extends QueryExecHTTPBuilder
+    {
+        protected QueryType presetQueryType;
+
+        public QueryExecHTTPBuilderOverRDFLinkHTTP(QueryType presetQueryType) {
+            super();
+            this.presetQueryType = presetQueryType;
         }
 
-        // Make sure it was set somehow.
-        if ( requestAcceptHeader == null )
-            throw new JenaConnectionException("No Accept header");
-        if ( requestAcceptHeader != null )
-            builder.acceptHeader(requestAcceptHeader);
-        // Delayed creation so QueryExecution.setTimeout works.
+        @Override
+        protected QueryExecHTTP buildX(HttpClient hClient, Query queryActual, 
String queryStringActual, Context cxt) {
+            checkQuery();
+            // The check below is handled in the base class before this method 
is called.
+            // if ( query == null && queryString == null )
+            //     throw new InternalErrorException("Both query and query 
string are null");
+
+            QueryType finalQueryType = presetQueryType != null
+                    ? presetQueryType
+                    : queryActual != null
+                        ? queryActual.queryType()
+                        : QueryType.UNKNOWN;
+
+            Query parsedQuery = query;
+            if ( queryActual == null ) {
+                if ( parseCheckQueries ) {
+                    // Don't retain the query
+                    // However, use it to derive the queryType
+                    parsedQuery = QueryFactory.create(queryStringActual);
+                    if (QueryType.UNKNOWN.equals(finalQueryType)) {
+                        finalQueryType = parsedQuery.queryType();
+                    }
+                }
+            }
+
+            // Use the query string as provided if possible, otherwise 
serialize the query.
+            String queryStringToSend = ( queryStringActual != null ) ? 
queryStringActual : queryActual.toString();
+
+            // If the appAcceptHeader has not been explicitly set then derive 
it.
+            if (this.appAcceptHeader == null) {
+                String requestAcceptHeader = null;
+                // Use the most specific method.
+                switch(finalQueryType) {
+                    case SELECT :
+                        if ( acceptSelectResult != null )
+                            requestAcceptHeader = acceptSelectResult;
+                        break;
+                    case ASK :
+                        if ( acceptAskResult != null )
+                            requestAcceptHeader = acceptAskResult;
+                        break;
+                    case DESCRIBE :
+                    case CONSTRUCT :
+                        // Best effort to handle construct quads. Requires a 
parsed query.
+                        if (parsedQuery != null && 
parsedQuery.isConstructQuad()) {
+                            if ( acceptDataset != null )
+                                requestAcceptHeader = acceptDataset;
+                        } else {
+                            if ( acceptGraph != null )
+                                requestAcceptHeader = acceptGraph;
+                        }
+                        break;
+                    case UNKNOWN:
+                        // All-purpose content type.
+                        if ( acceptSparqlResults != null )
+                            requestAcceptHeader = acceptSparqlResults;
+                        else
+                            // No idea! Set an "anything" and hope.
+                            // (Reasonable chance this is going to end up as 
HTML though.)
+                            requestAcceptHeader = "*/*";
+                    default :
+                        break;
+                }
+                // Make sure it was set somehow.
+                if ( requestAcceptHeader == null )
+                    throw new JenaConnectionException("No Accept header");
+
+                this.acceptHeader(requestAcceptHeader);
+            }
+
+            // Delayed creation so QueryExecution.setTimeout works.

Review Comment:
   Redundant, handled by the base class.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to