From a73f175156e4bc6d0ce88daa16438896b366291f Mon Sep 17 00:00:00 2001 From: Sanjay Dutt Date: Tue, 15 Oct 2024 16:17:27 +0530 Subject: [PATCH 1/3] SchemaDesigner API switch to CloudSolrClient --- .../org/apache/solr/core/CoreContainer.java | 2 +- .../SchemaDesignerConfigSetHelper.java | 98 +++++++++++-------- .../solr/handler/sql/TestSQLHandler.java | 16 +-- 3 files changed, 64 insertions(+), 52 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index a92c73ef496..02781d37a24 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -831,7 +831,7 @@ private void loadInternal() { solrClientProvider = new HttpSolrClientProvider(cfg.getUpdateShardHandlerConfig(), solrMetricsContext); updateShardHandler.initializeMetrics(solrMetricsContext, "updateShardHandler"); - solrClientCache = new SolrClientCache(updateShardHandler.getDefaultHttpClient()); + solrClientCache = new SolrClientCache(solrClientProvider.getSolrClient()); Map cachesConfig = cfg.getCachesConfig(); if (cachesConfig.isEmpty()) { diff --git a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java index 90eb3eb0c1c..50b373fd3f2 100644 --- a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java +++ b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java @@ -57,20 +57,17 @@ import java.util.zip.ZipOutputStream; import org.apache.commons.io.FilenameUtils; import org.apache.commons.io.file.PathUtils; -import org.apache.http.HttpResponse; -import org.apache.http.HttpStatus; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpPost; import org.apache.http.client.utils.URIBuilder; import org.apache.http.client.utils.URLEncodedUtils; -import org.apache.http.entity.ByteArrayEntity; -import org.apache.http.util.EntityUtils; import org.apache.lucene.util.IOSupplier; +import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrResponse; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.impl.CloudLegacySolrClient; +import org.apache.solr.client.solrj.impl.BinaryResponseParser; import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.InputStreamResponseParser; import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.GenericSolrRequest; import org.apache.solr.client.solrj.request.schema.FieldTypeDefinition; import org.apache.solr.client.solrj.request.schema.SchemaRequest; import org.apache.solr.client.solrj.response.schema.SchemaResponse; @@ -85,6 +82,8 @@ import org.apache.solr.common.cloud.ZkMaintenanceUtils; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CommonParams; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.IOUtils; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.Utils; @@ -128,43 +127,27 @@ class SchemaDesignerConfigSetHelper implements SchemaDesignerConstants { Map analyzeField(String configSet, String fieldName, String fieldText) throws IOException { final String mutableId = getMutableId(configSet); - final URI uri; - try { - uri = - collectionApiEndpoint(mutableId, "analysis", "field") - .setParameter(CommonParams.WT, CommonParams.JSON) - .setParameter("analysis.showmatch", "true") - .setParameter("analysis.fieldname", fieldName) - .setParameter("analysis.fieldvalue", "POST") - .build(); - } catch (URISyntaxException e) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e); - } - - Map analysis = Collections.emptyMap(); - HttpPost httpPost = new HttpPost(uri); - httpPost.setHeader("Content-Type", "text/plain"); - httpPost.setEntity(new ByteArrayEntity(fieldText.getBytes(StandardCharsets.UTF_8))); + var solrParams = new ModifiableSolrParams(); + solrParams.add("analysis.showmatch", "true"); + solrParams.add("analysis.fieldname", fieldName); + solrParams.add("analysis.fieldvalue", "POST"); + var request = new GenericSolrRequest(SolrRequest.METHOD.POST, "/analysis/field", solrParams); + request.withContent(fieldText.getBytes(StandardCharsets.UTF_8), "text/plain"); + request.setRequiresCollection(true); + request.setResponseParser(new InputStreamResponseParser(null)); + InputStream is = null; try { - HttpResponse resp = ((CloudLegacySolrClient) cloudClient()).getHttpClient().execute(httpPost); - int statusCode = resp.getStatusLine().getStatusCode(); - if (statusCode != HttpStatus.SC_OK) { - throw new SolrException( - SolrException.ErrorCode.getErrorCode(statusCode), - EntityUtils.toString(resp.getEntity(), StandardCharsets.UTF_8)); - } - + var resp = request.process(cloudClient(), mutableId).getResponse(); + is = (InputStream) resp.get("stream"); Map response = (Map) - fromJSONString(EntityUtils.toString(resp.getEntity(), StandardCharsets.UTF_8)); - if (response != null) { - analysis = (Map) response.get("analysis"); - } + fromJSONString(new String(is.readAllBytes(), StandardCharsets.UTF_8)); + return (Map) response.get("analysis"); + } catch (SolrServerException e) { + throw new RuntimeException(e); } finally { - httpPost.releaseConnection(); + IOUtils.closeQuietly(is); } - - return analysis; } List listCollectionsForConfig(String configSet) { @@ -533,11 +516,31 @@ List getStoredSampleDocs(final String configSet) throws IOExc collectionApiEndpoint(BLOB_STORE_ID, "blob", configSet + "_sample") .setParameter(CommonParams.WT, "filestream") .build(); + } catch (URISyntaxException e) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e); } + final var params = new ModifiableSolrParams(); + var request = + new GenericSolrRequest(SolrRequest.METHOD.GET, "/blob/" + configSet + "_sample", params); + request.setRequiresCollection(true); + request.setResponseParser(new InputStreamResponseParser("filestream")); + InputStream inputStream = null; + try { + var resp = request.process(cloudClient(), BLOB_STORE_ID).getResponse(); + inputStream = (InputStream) resp.get("stream"); + var bytes = inputStream.readAllBytes(); + if (bytes.length > 0) { + return (List) Utils.fromJavabin(bytes); + } else return Collections.emptyList(); + } catch (SolrServerException e) { + throw new IOException("Failed to lookup stored docs for " + configSet + " due to: " + e); + } finally { + assert inputStream != null; + inputStream.close(); + } - HttpGet httpGet = new HttpGet(uri); + /*HttpGet httpGet = new HttpGet(uri); try { HttpResponse entity = ((CloudLegacySolrClient) cloudClient()).getHttpClient().execute(httpGet); @@ -558,7 +561,7 @@ List getStoredSampleDocs(final String configSet) throws IOExc } finally { httpGet.releaseConnection(); } - return docs != null ? docs : Collections.emptyList(); + return docs != null ? docs : Collections.emptyList();*/ } void storeSampleDocs(final String configSet, List docs) throws IOException { @@ -575,7 +578,16 @@ static byte[] readAllBytes(IOSupplier hasStream) throws IOException protected void postDataToBlobStore(CloudSolrClient cloudClient, String blobName, byte[] bytes) throws IOException { - final URI uri; + var request = new GenericSolrRequest(SolrRequest.METHOD.POST, "/blob/" + blobName); + request.withContent(bytes, BinaryResponseParser.BINARY_CONTENT_TYPE); + request.setRequiresCollection(true); + + try { + request.process(cloudClient, BLOB_STORE_ID); + } catch (Exception e) { + // throw new IOException(e); + } + /*final URI uri; try { uri = collectionApiEndpoint(BLOB_STORE_ID, "blob", blobName).build(); } catch (URISyntaxException e) { @@ -595,7 +607,7 @@ protected void postDataToBlobStore(CloudSolrClient cloudClient, String blobName, } } finally { httpPost.releaseConnection(); - } + }*/ } private String getBaseUrl(final String collection) { diff --git a/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java b/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java index 94e997998ea..5cc5b907871 100644 --- a/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java +++ b/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java @@ -2994,8 +2994,8 @@ public void testSelectEmptyField() throws Exception { // notafield_i matches a dynamic field pattern but has no docs, so don't allow this expectThrows( IOException.class, () -> expectResults("SELECT id, stringx, notafield_i FROM $ALIAS", 5)); - expectThrows( - IOException.class, () -> expectResults("SELECT id, stringx, notstored FROM $ALIAS", 5)); + // expectThrows(IOException.class, () -> expectResults("SELECT id, stringx, notstored FROM + // $ALIAS", 5)); } @Test @@ -3059,12 +3059,12 @@ public void testMultiValuedFieldHandling() throws Exception { 1); // can't sort by a mv field - expectThrows( - IOException.class, - () -> - expectResults( - "SELECT stringxmv FROM $ALIAS WHERE stringxmv IS NOT NULL ORDER BY stringxmv ASC", - 0)); + /*expectThrows( + IOException.class, + () -> + expectResults( + "SELECT stringxmv FROM $ALIAS WHERE stringxmv IS NOT NULL ORDER BY stringxmv ASC", + 0));*/ // even id's have these fields, odd's are null ... expectListInResults("0", "stringsx", stringsx, -1, 5); From 86cf9fc6784cd9eebd1732b9cd5ce06cab15afee Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 28 Nov 2024 00:47:25 -0500 Subject: [PATCH 2/3] AnalyzeField: use NamedList, simpler. Don't need InputStreamParser getStoredSampleDocs: simplify postDataToBlobStore: don't swallow exception collectionApiEndpoint: unused delete commented code --- .../handler/designer/SchemaDesignerAPI.java | 2 +- .../SchemaDesignerConfigSetHelper.java | 102 ++---------------- .../TestSchemaDesignerConfigSetHelper.java | 11 +- 3 files changed, 13 insertions(+), 102 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java index f34793aa744..03efab8f985 100644 --- a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java +++ b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java @@ -354,7 +354,7 @@ public void getSampleValue(SolrQueryRequest req, SolrQueryResponse rsp) throws I } if (textValue != null) { - Map analysis = configSetHelper.analyzeField(configSet, fieldName, textValue); + var analysis = configSetHelper.analyzeField(configSet, fieldName, textValue); rsp.getValues().addAll(Map.of(idField, docId, fieldName, textValue, "analysis", analysis)); } } diff --git a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java index 50b373fd3f2..7aadc38caaf 100644 --- a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java +++ b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java @@ -18,7 +18,6 @@ package org.apache.solr.handler.designer; import static org.apache.solr.common.params.CommonParams.VERSION_FIELD; -import static org.apache.solr.common.util.Utils.fromJSONString; import static org.apache.solr.common.util.Utils.toJavabin; import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME; import static org.apache.solr.handler.designer.SchemaDesignerAPI.getConfigSetZkPath; @@ -31,8 +30,6 @@ import java.io.IOException; import java.io.InputStream; import java.lang.invoke.MethodHandles; -import java.net.URI; -import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.nio.file.FileVisitResult; import java.nio.file.Files; @@ -57,8 +54,6 @@ import java.util.zip.ZipOutputStream; import org.apache.commons.io.FilenameUtils; import org.apache.commons.io.file.PathUtils; -import org.apache.http.client.utils.URIBuilder; -import org.apache.http.client.utils.URLEncodedUtils; import org.apache.lucene.util.IOSupplier; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrResponse; @@ -74,6 +69,7 @@ import org.apache.solr.cloud.ZkConfigSetService; import org.apache.solr.cloud.ZkSolrResourceLoader; import org.apache.solr.common.SolrException; +import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.DocCollection; @@ -81,7 +77,6 @@ import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.cloud.ZkMaintenanceUtils; import org.apache.solr.common.cloud.ZkStateReader; -import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.IOUtils; import org.apache.solr.common.util.NamedList; @@ -124,7 +119,7 @@ class SchemaDesignerConfigSetHelper implements SchemaDesignerConstants { } @SuppressWarnings("unchecked") - Map analyzeField(String configSet, String fieldName, String fieldText) + NamedList analyzeField(String configSet, String fieldName, String fieldText) throws IOException { final String mutableId = getMutableId(configSet); var solrParams = new ModifiableSolrParams(); @@ -134,19 +129,11 @@ Map analyzeField(String configSet, String fieldName, String fiel var request = new GenericSolrRequest(SolrRequest.METHOD.POST, "/analysis/field", solrParams); request.withContent(fieldText.getBytes(StandardCharsets.UTF_8), "text/plain"); request.setRequiresCollection(true); - request.setResponseParser(new InputStreamResponseParser(null)); - InputStream is = null; try { var resp = request.process(cloudClient(), mutableId).getResponse(); - is = (InputStream) resp.get("stream"); - Map response = - (Map) - fromJSONString(new String(is.readAllBytes(), StandardCharsets.UTF_8)); - return (Map) response.get("analysis"); + return (NamedList) resp.get("analysis"); } catch (SolrServerException e) { - throw new RuntimeException(e); - } finally { - IOUtils.closeQuietly(is); + throw new SolrException(ErrorCode.SERVER_ERROR, e); } } @@ -508,21 +495,7 @@ void deleteStoredSampleDocs(String configSet) { @SuppressWarnings("unchecked") List getStoredSampleDocs(final String configSet) throws IOException { - List docs = null; - - final URI uri; - try { - uri = - collectionApiEndpoint(BLOB_STORE_ID, "blob", configSet + "_sample") - .setParameter(CommonParams.WT, "filestream") - .build(); - - } catch (URISyntaxException e) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e); - } - final var params = new ModifiableSolrParams(); - var request = - new GenericSolrRequest(SolrRequest.METHOD.GET, "/blob/" + configSet + "_sample", params); + var request = new GenericSolrRequest(SolrRequest.METHOD.GET, "/blob/" + configSet + "_sample"); request.setRequiresCollection(true); request.setResponseParser(new InputStreamResponseParser("filestream")); InputStream inputStream = null; @@ -536,32 +509,8 @@ List getStoredSampleDocs(final String configSet) throws IOExc } catch (SolrServerException e) { throw new IOException("Failed to lookup stored docs for " + configSet + " due to: " + e); } finally { - assert inputStream != null; - inputStream.close(); - } - - /*HttpGet httpGet = new HttpGet(uri); - try { - HttpResponse entity = - ((CloudLegacySolrClient) cloudClient()).getHttpClient().execute(httpGet); - int statusCode = entity.getStatusLine().getStatusCode(); - if (statusCode == HttpStatus.SC_OK) { - byte[] bytes = readAllBytes(() -> entity.getEntity().getContent()); - if (bytes.length > 0) { - docs = (List) Utils.fromJavabin(bytes); - } - } else if (statusCode != HttpStatus.SC_NOT_FOUND) { - byte[] bytes = readAllBytes(() -> entity.getEntity().getContent()); - throw new IOException( - "Failed to lookup stored docs for " - + configSet - + " due to: " - + new String(bytes, StandardCharsets.UTF_8)); - } // else not found is ok - } finally { - httpGet.releaseConnection(); + IOUtils.closeQuietly(inputStream); } - return docs != null ? docs : Collections.emptyList();*/ } void storeSampleDocs(final String configSet, List docs) throws IOException { @@ -581,33 +530,11 @@ protected void postDataToBlobStore(CloudSolrClient cloudClient, String blobName, var request = new GenericSolrRequest(SolrRequest.METHOD.POST, "/blob/" + blobName); request.withContent(bytes, BinaryResponseParser.BINARY_CONTENT_TYPE); request.setRequiresCollection(true); - try { request.process(cloudClient, BLOB_STORE_ID); - } catch (Exception e) { - // throw new IOException(e); - } - /*final URI uri; - try { - uri = collectionApiEndpoint(BLOB_STORE_ID, "blob", blobName).build(); - } catch (URISyntaxException e) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e); + } catch (SolrServerException e) { + throw new SolrException(ErrorCode.SERVER_ERROR, e); } - - HttpPost httpPost = new HttpPost(uri); - try { - httpPost.setHeader("Content-Type", "application/octet-stream"); - httpPost.setEntity(new ByteArrayEntity(bytes)); - HttpResponse resp = ((CloudLegacySolrClient) cloudClient).getHttpClient().execute(httpPost); - int statusCode = resp.getStatusLine().getStatusCode(); - if (statusCode != HttpStatus.SC_OK) { - throw new SolrException( - SolrException.ErrorCode.getErrorCode(statusCode), - EntityUtils.toString(resp.getEntity(), StandardCharsets.UTF_8)); - } - } finally { - httpPost.releaseConnection(); - }*/ } private String getBaseUrl(final String collection) { @@ -633,19 +560,6 @@ private String getBaseUrl(final String collection) { return baseUrl; } - private URIBuilder collectionApiEndpoint( - final String collection, final String... morePathSegments) throws URISyntaxException { - URI baseUrl = new URI(getBaseUrl(collection)); - // build up a list of path segments including any path in the base URL, collection, and - // additional segments provided by caller - List path = new ArrayList<>(URLEncodedUtils.parsePathSegments(baseUrl.getPath())); - path.add(collection); - if (morePathSegments != null && morePathSegments.length > 0) { - path.addAll(Arrays.asList(morePathSegments)); - } - return new URIBuilder(baseUrl).setPathSegments(path); - } - protected String getManagedSchemaZkPath(final String configSet) { return getConfigSetZkPath(configSet, DEFAULT_MANAGED_SCHEMA_RESOURCE_NAME); } diff --git a/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java b/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java index af6412da947..461bdb198b2 100644 --- a/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java +++ b/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerConfigSetHelper.java @@ -35,6 +35,7 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrConfig; @@ -280,15 +281,11 @@ public void testAnalyzeField() throws Exception { helper.addSchemaObject(configSet, Collections.singletonMap("add-field", addField)); assertEquals("title", addedFieldName); - Map analysis = + NamedList analysis = helper.analyzeField(configSet, "title", "The Pillars of the Earth"); - Map title = - (Map) ((Map) analysis.get("field_names")).get("title"); - assertNotNull(title); - List index = (List) title.get("index"); - assertNotNull(index); - assertFalse(index.isEmpty()); + var indexNl = (NamedList) analysis.findRecursive("field_names", "title", "index"); + assertTrue(indexNl.size() > 0); } @Test From ae9c5bb739fc3cd062beaf6bfbd61b64316d8cd8 Mon Sep 17 00:00:00 2001 From: iamsanjay Date: Tue, 3 Dec 2024 20:20:49 +0530 Subject: [PATCH 3/3] revert failing test changes --- .../apache/solr/handler/sql/TestSQLHandler.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java b/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java index 5cc5b907871..94e997998ea 100644 --- a/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java +++ b/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java @@ -2994,8 +2994,8 @@ public void testSelectEmptyField() throws Exception { // notafield_i matches a dynamic field pattern but has no docs, so don't allow this expectThrows( IOException.class, () -> expectResults("SELECT id, stringx, notafield_i FROM $ALIAS", 5)); - // expectThrows(IOException.class, () -> expectResults("SELECT id, stringx, notstored FROM - // $ALIAS", 5)); + expectThrows( + IOException.class, () -> expectResults("SELECT id, stringx, notstored FROM $ALIAS", 5)); } @Test @@ -3059,12 +3059,12 @@ public void testMultiValuedFieldHandling() throws Exception { 1); // can't sort by a mv field - /*expectThrows( - IOException.class, - () -> - expectResults( - "SELECT stringxmv FROM $ALIAS WHERE stringxmv IS NOT NULL ORDER BY stringxmv ASC", - 0));*/ + expectThrows( + IOException.class, + () -> + expectResults( + "SELECT stringxmv FROM $ALIAS WHERE stringxmv IS NOT NULL ORDER BY stringxmv ASC", + 0)); // even id's have these fields, odd's are null ... expectListInResults("0", "stringsx", stringsx, -1, 5);