From b1b566f57bba46cadae33bc8198246fa05609287 Mon Sep 17 00:00:00 2001 From: Cao Manh Dat Date: Tue, 20 Jun 2017 12:46:33 +0700 Subject: [PATCH] SOLR-10406: v2 API error messages list the URL request path as /solr/____v2/... when the original path was /v2/... --- solr/CHANGES.txt | 2 + .../java/org/apache/solr/api/V2HttpCall.java | 27 +++++++++---- .../org/apache/solr/servlet/HttpSolrCall.java | 2 +- .../solr/handler/V2ApiIntegrationTest.java | 38 +++++++++++++++++++ .../client/solrj/impl/HttpSolrClient.java | 12 ++++-- 5 files changed, 68 insertions(+), 13 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 15b86a29b22..b163a7f1fb6 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -149,6 +149,8 @@ New Features * SOLR-9989: Add support for PointFields in FacetModule (JSON Facets) (Cao Manh Dat) +* SOLR-10406: v2 API error messages list the URL request path as /solr/____v2/... when the original path was /v2/... (Cao Manh Dat, noble) + Bug Fixes ---------------------- * SOLR-9262: Connection and read timeouts are being ignored by UpdateShardHandler after SOLR-4509. diff --git a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java index 5e7c0b16e42..49d50e932b8 100644 --- a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java +++ b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java @@ -56,7 +56,6 @@ import org.slf4j.LoggerFactory; import static org.apache.solr.common.params.CommonParams.JSON; import static org.apache.solr.common.params.CommonParams.WT; import static org.apache.solr.servlet.SolrDispatchFilter.Action.ADMIN; -import static org.apache.solr.servlet.SolrDispatchFilter.Action.PASSTHROUGH; import static org.apache.solr.servlet.SolrDispatchFilter.Action.PROCESS; import static org.apache.solr.common.util.PathTrie.getPathSegments; @@ -76,7 +75,7 @@ public class V2HttpCall extends HttpSolrCall { protected void init() throws Exception { String path = this.path; - String fullPath = path = path.substring(7);//strip off '/____v2' + final String fullPath = path = path.substring(7);//strip off '/____v2' try { pieces = getPathSegments(path); if (pieces.size() == 0 || (pieces.size() == 1 && path.endsWith(CommonParams.INTROSPECT))) { @@ -159,7 +158,7 @@ public class V2HttpCall extends HttpSolrCall { log.error("Error in init()", rte); throw rte; } finally { - if (api == null) action = PASSTHROUGH; + if (api == null) action = PROCESS; if (solrReq != null) solrReq.getContext().put(CommonParams.PATH, path); } } @@ -309,16 +308,28 @@ public class V2HttpCall extends HttpSolrCall { @Override protected void handleAdmin(SolrQueryResponse solrResp) { - api.call(this.solrReq, solrResp); + try { + api.call(this.solrReq, solrResp); + } catch (Exception e) { + solrResp.setException(e); + } } @Override protected void execute(SolrQueryResponse rsp) { - try { - api.call(solrReq, rsp); - } catch (RuntimeException e) { - throw e; + SolrCore.preDecorateResponse(solrReq, rsp); + if (api == null) { + rsp.setException(new SolrException(SolrException.ErrorCode.NOT_FOUND, + "Cannot find correspond api for the path : " + solrReq.getContext().get(CommonParams.PATH))); + } else { + try { + api.call(solrReq, rsp); + } catch (Exception e) { + rsp.setException(e); + } } + + SolrCore.postDecorateResponse(handler, solrReq, rsp); } @Override diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java index bf6c55356bb..5542d4c9fbf 100644 --- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java @@ -687,7 +687,7 @@ public class HttpSolrCall { solrReq = new SolrQueryRequestBase(core, solrParams) { }; } - QueryResponseWriter writer = core.getQueryResponseWriter(solrReq); + QueryResponseWriter writer = getResponseWriter(); writeResponse(solrResp, writer, Method.GET); } catch (Exception e) { // This error really does not matter exp = e; diff --git a/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java b/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java index 5a8d4820b38..dac495edcdd 100644 --- a/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java @@ -22,11 +22,15 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import org.apache.solr.client.solrj.ResponseParser; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.impl.BinaryResponseParser; import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.XMLResponseParser; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.V2Request; +import org.apache.solr.client.solrj.response.DelegationTokenResponse; import org.apache.solr.client.solrj.response.V2Response; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.params.ModifiableSolrParams; @@ -57,6 +61,40 @@ public class V2ApiIntegrationTest extends SolrCloudTestCase { assertEquals(0, res.getStatus()); } + private void testException(ResponseParser responseParser, int expectedCode, String path, String payload) throws IOException, SolrServerException { + V2Request v2Request = new V2Request.Builder(path) + .withMethod(SolrRequest.METHOD.POST) + .withPayload(payload) + .build(); + v2Request.setResponseParser(responseParser); + V2Response response = v2Request.process(cluster.getSolrClient()); + assertEquals(getStatus(response), expectedCode); + } + + @Test + public void testException() throws Exception { + String notFoundPath = "/c/" + COLL_NAME + "/abccdef"; + String incorrectPayload = "{rebalance-leaders: {maxAtOnce: abc, maxWaitSeconds: xyz}}"; + testException(new XMLResponseParser(),404, + notFoundPath, incorrectPayload); + testException(new DelegationTokenResponse.JsonMapResponseParser(),404, + notFoundPath, incorrectPayload); + testException(new BinaryResponseParser(),404, + notFoundPath, incorrectPayload); + testException(new XMLResponseParser(), 400, "/c/" + COLL_NAME, incorrectPayload); + testException(new BinaryResponseParser(), 400, "/c/" + COLL_NAME, incorrectPayload); + testException(new DelegationTokenResponse.JsonMapResponseParser(), 400, "/c/" + COLL_NAME, incorrectPayload); + } + + private long getStatus(V2Response response) { + Object header = response.getResponse().get("responseHeader"); + if (header instanceof NamedList) { + return (int) ((NamedList) header).get("status"); + } else { + return (long) ((Map) header).get("status"); + } + } + @Test public void testIntrospect() throws Exception { ModifiableSolrParams params = new ModifiableSolrParams(); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java index 104ab1fe131..fa1ccf1dfd0 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java @@ -228,7 +228,11 @@ public class HttpSolrClient extends SolrClient { throws SolrServerException, IOException { HttpRequestBase method = createMethod(request, collection); setBasicAuthHeader(request, method); - return executeMethod(method, processor); + return executeMethod(method, processor, isV2ApiRequest(request)); + } + + private boolean isV2ApiRequest(final SolrRequest request) { + return request instanceof V2Request || request.getPath().contains("/____v2"); } private void setBasicAuthHeader(SolrRequest request, HttpRequestBase method) throws UnsupportedEncodingException { @@ -268,7 +272,7 @@ public class HttpSolrClient extends SolrClient { ExecutorService pool = ExecutorUtil.newMDCAwareFixedThreadPool(1, new SolrjNamedThreadFactory("httpUriRequest")); try { MDC.put("HttpSolrClient.url", baseUrl); - mrr.future = pool.submit(() -> executeMethod(method, processor)); + mrr.future = pool.submit(() -> executeMethod(method, processor, isV2ApiRequest(request))); } finally { pool.shutdown(); @@ -473,7 +477,7 @@ public class HttpSolrClient extends SolrClient { } - protected NamedList executeMethod(HttpRequestBase method, final ResponseParser processor) throws SolrServerException { + protected NamedList executeMethod(HttpRequestBase method, final ResponseParser processor, final boolean isV2Api) throws SolrServerException { method.addHeader("User-Agent", AGENT); org.apache.http.client.config.RequestConfig.Builder requestConfigBuilder = HttpClientUtil.createDefaultRequestConfigBuilder(); @@ -571,7 +575,7 @@ public class HttpSolrClient extends SolrClient { } catch (Exception e) { throw new RemoteSolrException(baseUrl, httpStatus, e.getMessage(), e); } - if (httpStatus != HttpStatus.SC_OK) { + if (httpStatus != HttpStatus.SC_OK && !isV2Api) { NamedList metadata = null; String reason = null; try {