diff --git a/changelog/unreleased/SOLR-8003-fl-glob-raw-transformer.yml b/changelog/unreleased/SOLR-8003-fl-glob-raw-transformer.yml new file mode 100644 index 000000000000..a386e5f89503 --- /dev/null +++ b/changelog/unreleased/SOLR-8003-fl-glob-raw-transformer.yml @@ -0,0 +1,9 @@ +title: > + The fl parameter accepts a glob in front of a raw value transformer, for example fl=*_json:[json], which + writes every returned field matching the glob raw instead of failing with "Error parsing fieldname". +type: added +authors: + - name: Nick Shanin +links: + - name: SOLR-8003 + url: https://issues.apache.org/jira/browse/SOLR-8003 diff --git a/solr/core/src/java/org/apache/solr/response/TextResponseWriter.java b/solr/core/src/java/org/apache/solr/response/TextResponseWriter.java index 065255811cd3..735e85bd240a 100644 --- a/solr/core/src/java/org/apache/solr/response/TextResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/TextResponseWriter.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Set; import org.apache.lucene.document.Document; import org.apache.lucene.index.IndexableField; @@ -31,6 +32,7 @@ import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.util.FastWriter; +import org.apache.solr.common.util.GlobPatternUtil; import org.apache.solr.common.util.TextWriter; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.transform.DocTransformer; @@ -65,6 +67,7 @@ public abstract class TextResponseWriter implements TextWriter { private final TextResponseWriter rawShim; private final Set rawFields; + private final List rawFieldGlobs; private final ReturnFields rawReturnFields; public TextResponseWriter(Writer writer, SolrQueryRequest req, SolrQueryResponse rsp) { @@ -79,14 +82,19 @@ public TextResponseWriter(Writer writer, SolrQueryRequest req, SolrQueryResponse returnFields = rsp.getReturnFields(); if (req.getParams().getBool(CommonParams.OMIT_HEADER, false)) rsp.removeResponseHeader(); DocTransformer rootDocTransformer = returnFields.getTransformer(); - Collection rawFields; - if (rootDocTransformer == null || (rawFields = rootDocTransformer.getRawFields()).isEmpty()) { + Collection rawFields = + rootDocTransformer == null ? Set.of() : rootDocTransformer.getRawFields(); + Collection rawGlobs = + rootDocTransformer == null ? Set.of() : rootDocTransformer.getRawFieldGlobs(); + if (rawFields.isEmpty() && rawGlobs.isEmpty()) { this.rawFields = null; + this.rawFieldGlobs = List.of(); this.rawShim = null; this.rawReturnFields = NO_RAW_FIELDS; } else { this.rawFields = rawFields.size() == 1 ? Set.of(rawFields.iterator().next()) : new HashSet<>(rawFields); + this.rawFieldGlobs = List.copyOf(rawGlobs); this.rawShim = new RawShimTextResponseWriter(this); this.rawReturnFields = returnFields; } @@ -102,6 +110,7 @@ public TextResponseWriter(Writer writer, SolrQueryRequest req, SolrQueryResponse this.doIndent = indent; this.rawShim = null; this.rawFields = null; + this.rawFieldGlobs = List.of(); this.rawReturnFields = null; } @@ -111,7 +120,18 @@ public TextResponseWriter(Writer writer, SolrQueryRequest req, SolrQueryResponse */ @SuppressWarnings("ReferenceEquality") // strict object identity is intentional, see javadoc above protected final boolean shouldWriteRaw(String fname, ReturnFields returnFields) { - return rawReturnFields == returnFields && rawFields.contains(fname); + if (rawReturnFields != returnFields) { + return false; + } + if (rawFields.contains(fname)) { + return true; + } + for (String glob : rawFieldGlobs) { + if (GlobPatternUtil.matches(glob, fname)) { + return true; + } + } + return false; } /** done with this ResponseWriter... make sure any buffers are flushed to writer */ diff --git a/solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java b/solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java index 109afb979cc9..7a481bb5f5f6 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java +++ b/solr/core/src/java/org/apache/solr/response/transform/DocTransformer.java @@ -71,6 +71,14 @@ public Collection getRawFields() { return Set.of(); } + /** + * Like {@link #getRawFields()}, but each entry is a glob pattern (as accepted in {@code fl}); any + * field whose name matches is written raw. + */ + public Collection getRawFieldGlobs() { + return Set.of(); + } + /** * Indicates if this transformer requires access to the underlying index to perform its functions. * diff --git a/solr/core/src/java/org/apache/solr/response/transform/DocTransformers.java b/solr/core/src/java/org/apache/solr/response/transform/DocTransformers.java index 1c70b7d17808..3434d45a5c72 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/DocTransformers.java +++ b/solr/core/src/java/org/apache/solr/response/transform/DocTransformers.java @@ -53,6 +53,14 @@ public Collection getRawFields() { .collect(Collectors.toList()); } + @Override + public Collection getRawFieldGlobs() { + return children.stream() + .map(DocTransformer::getRawFieldGlobs) + .flatMap(Collection::stream) + .collect(Collectors.toList()); + } + public void addTransformer(DocTransformer a) { children.add(a); } diff --git a/solr/core/src/java/org/apache/solr/response/transform/RawValueTransformerFactory.java b/solr/core/src/java/org/apache/solr/response/transform/RawValueTransformerFactory.java index 2c6b540e6ceb..3b3890055e09 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/RawValueTransformerFactory.java +++ b/solr/core/src/java/org/apache/solr/response/transform/RawValueTransformerFactory.java @@ -62,8 +62,6 @@ public boolean mayModifyValue() { } @Override - @SuppressWarnings( - "ReferenceEquality") // QueryResponseWriter identity, not equality, is what matters here public DocTransformer create( String display, SolrParams params, @@ -80,22 +78,7 @@ public DocTransformer create( if (!copy) { renamedFields.put(field, display); } - // When a 'wt' is specified in the transformer, only apply it to the same wt - boolean apply = true; - if (applyToWT != null) { - String qwt = req.getParams().get(CommonParams.WT); - if (qwt == null) { - QueryResponseWriter qw = req.getCore().getQueryResponseWriter(req); - QueryResponseWriter dw = req.getCore().getQueryResponseWriter(applyToWT); - if (qw != dw) { - apply = false; - } - } else { - apply = applyToWT.equals(qwt); - } - } - - if (apply) { + if (appliesTo(req)) { return new RawTransformer(field, display, copy); } @@ -106,6 +89,54 @@ public DocTransformer create( return new RenameFieldTransformer(field, display, copy); } + /** When a 'wt' is specified in the transformer, it only applies to the same wt */ + @SuppressWarnings("ReferenceEquality") // QueryResponseWriter identity is what matters here + private boolean appliesTo(SolrQueryRequest req) { + if (applyToWT == null) { + return true; + } + String qwt = req.getParams().get(CommonParams.WT); + if (qwt == null) { + QueryResponseWriter qw = req.getCore().getQueryResponseWriter(req); + QueryResponseWriter dw = req.getCore().getQueryResponseWriter(applyToWT); + return qw == dw; + } + return applyToWT.equals(qwt); + } + + /** + * Creates the transformer for {@code glob:[name]}: every returned field whose name matches the + * glob is written raw, under its own name. + * + * @return null if this transformer does not apply to the request's response writer + */ + public DocTransformer createForGlob(String glob, SolrQueryRequest req) { + return appliesTo(req) ? new GlobRawTransformer(glob) : null; + } + + static class GlobRawTransformer extends DocTransformer { + final String glob; + + GlobRawTransformer(String glob) { + this.glob = glob; + } + + @Override + public String getName() { + return glob; + } + + @Override + public Collection getRawFieldGlobs() { + return Set.of(glob); + } + + @Override + public void transform(SolrDocument doc, int docid, DocIterationInfo docInfo) { + // values are untouched; the response writer writes the matching fields raw + } + } + static class RawTransformer extends DocTransformer { final String field; final String display; diff --git a/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java b/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java index bf850f4de8b5..fdd3a7c571bd 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java +++ b/solr/core/src/java/org/apache/solr/search/SolrReturnFields.java @@ -38,6 +38,7 @@ import org.apache.solr.response.transform.DocTransformer; import org.apache.solr.response.transform.DocTransformers; import org.apache.solr.response.transform.OriginalScoreAugmenter; +import org.apache.solr.response.transform.RawValueTransformerFactory; import org.apache.solr.response.transform.RenameFieldTransformer; import org.apache.solr.response.transform.ScoreAugmenter; import org.apache.solr.response.transform.TransformerFactory; @@ -314,6 +315,36 @@ private void add( globs.add(field); } continue; + } else if (field != null && ch == ':' && sp.val.startsWith("[", sp.pos + 1)) { + // "glob:[json]" applies a raw value transformer to every matching field + if ("*".equals(field)) { + throw new SolrException( + SolrException.ErrorCode.BAD_REQUEST, + "A raw value transformer cannot follow a bare *. " + + "Use a named glob such as *_json:[json] instead."); + } + sp.pos++; + ModifiableSolrParams augmenterParams = new ModifiableSolrParams(); + sp.pos += + QueryParsing.parseLocalParams( + sp.val.substring(sp.pos), 0, augmenterParams, req.getParams(), "[", ']'); + String augmenterName = augmenterParams.get("type"); + TransformerFactory factory = req.getCore().getTransformerFactory(augmenterName); + if (!(factory instanceof RawValueTransformerFactory rawFactory)) { + throw new SolrException( + SolrException.ErrorCode.BAD_REQUEST, + "A glob can only be combined with a raw value transformer such as [json] or [xml]: " + + field + + ":[" + + augmenterName + + "]"); + } + globs.add(field); + DocTransformer t = rawFactory.createForGlob(field, req); + if (t != null) { + augmenters.addTransformer(t); + } + continue; } else if (ORIGINAL_SCORE_NAME.equals(field) && sp.opt("(") && sp.opt(")")) { // TODO: Remove this in https://issues.apache.org/jira/browse/SOLR-17784 when // originalScore() becomes a true function diff --git a/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java b/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java index 1fff683d95ae..49144667f2f3 100644 --- a/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java +++ b/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java @@ -20,9 +20,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Properties; import java.util.regex.Pattern; +import java.util.stream.Stream; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer; import org.apache.solr.client.solrj.impl.CloudSolrClient; @@ -31,10 +33,12 @@ import org.apache.solr.client.solrj.response.InputStreamResponseParser; import org.apache.solr.cloud.MiniSolrCloudCluster; import org.apache.solr.cloud.SolrCloudTestCase; +import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrInputDocument; 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.Utils; import org.apache.solr.embedded.JettyConfig; import org.apache.solr.embedded.JettySolrRunner; import org.junit.After; @@ -81,7 +85,8 @@ private static void initStandalone() throws Exception { final Path confDir = collDir.resolve("conf"); Files.createDirectories(confDir); Path srcDir = TEST_HOME().resolve("collection1").resolve("conf"); - Files.copy(srcDir.resolve("schema_latest.xml"), confDir.resolve("schema.xml")); + Files.writeString( + confDir.resolve("schema.xml"), schemaWithJsonField(srcDir.resolve("schema_latest.xml"))); Files.copy(srcDir.resolve("solrconfig-minimal.xml"), confDir.resolve("solrconfig.xml")); for (String file : new String[] { @@ -102,7 +107,7 @@ private static void initStandalone() throws Exception { private static void initCloud() throws Exception { final String configName = DEBUG_LABEL + "_config-set"; - final Path configDir = TEST_HOME().resolve("collection1").resolve("conf"); + final Path configDir = copyConfWithJsonField(); final int numNodes = 3; MiniSolrCloudCluster cloud = @@ -119,6 +124,43 @@ private static void initCloud() throws Exception { JSR = cloud.getRandomJetty(random()); } + /** + * The fixture schema defines no field whose name ends in {@code _json}, so a document field such + * as {@code payload_json} cannot be indexed against it. Returns the text of the given schema with + * one such dynamic field added. Only the copies of the schema this test runs against are + * extended; the checked-in fixture is left as it is. + */ + private static String schemaWithJsonField(Path schema) throws Exception { + String text = Files.readString(schema); + String patched = + text.replace( + "", + " \n"); + assertFalse("the fixture schema has no to extend", patched.equals(text)); + return patched; + } + + /** Copies the collection1 conf dir, extending its schema as {@link #schemaWithJsonField}. */ + private static Path copyConfWithJsonField() throws Exception { + Path srcDir = TEST_HOME().resolve("collection1").resolve("conf"); + Path destDir = createTempDir().resolve("conf"); + try (Stream paths = Files.walk(srcDir)) { + for (Path src : paths.toList()) { + // resolve through a string: destDir is on the test filesystem, srcDir is not + Path dest = destDir.resolve(srcDir.relativize(src).toString()); + if (Files.isDirectory(src)) { + Files.createDirectories(dest); + } else if (src.getFileName().toString().equals("schema_latest.xml")) { + Files.writeString(dest, schemaWithJsonField(src)); + } else { + Files.copy(src, dest); + } + } + } + return destDir; + } + @AfterClass public static void afterClass() throws Exception { if (CLIENT != null) { @@ -159,6 +201,10 @@ private static void initIndex() throws Exception { sdoc.addField("links", "{an_array:[4,5,6]}"); sdoc.addField("content_type", "one"); sdoc.addField("content_type", "two"); + // a field whose name ends in _json, matched by the *_json glob in + // testSuffixGlobJsonTransformer; its dynamic field is added by schemaWithJsonField + sdoc.addField("payload_json", "{\"seq\":[1,2]}"); + sdoc.addField("payload_json", "{\"seq\":[3,4]}"); CLIENT.add("collection1", sdoc); } CLIENT.commit("collection1"); @@ -300,6 +346,113 @@ public void testJsonTransformer() throws Exception { strResponse.contains("\"links\":[\"")); } + @Test + public void testGlobJsonTransformer() throws Exception { + QueryRequest req = + new QueryRequest( + new ModifiableSolrParams( + Map.of( + "q", + new String[] {"*:*"}, + "fl", + new String[] {"id,su*:[json],link?:[json]"}, + "wt", + new String[] {"json"}, + "indent", + new String[] {"false"}))); + req.setResponseParser(JSON_STREAM_RESPONSE_PARSER); + NamedList rsp = CLIENT.request(req, "collection1"); + String strResponse = InputStreamResponseParser.consumeResponseToString(rsp); + + assertTrue( + "glob did not write the single valued field raw: " + strResponse, + strResponse.contains("\"subject\":{poffL:[{offL:[{oGUID:\"7")); + assertTrue( + "glob did not write the multiValued field raw: " + strResponse, + Pattern.compile("\"links\":\\[\\{an_array:\\[1,2,3]},\\s*\\{an_array:\\[4,5,6]}]") + .matcher(strResponse) + .find()); + assertFalse( + "field not matching the globs was returned: " + strResponse, + strResponse.contains("\"author\"")); + + // [xml] does not apply to a json response: the matching fields are returned, not raw + req = + new QueryRequest( + new ModifiableSolrParams( + Map.of( + "q", + new String[] {"*:*"}, + "fl", + new String[] {"au*:[xml]"}, + "wt", + new String[] {"json"}, + "indent", + new String[] {"false"}))); + req.setResponseParser(JSON_STREAM_RESPONSE_PARSER); + rsp = CLIENT.request(req, "collection1"); + strResponse = InputStreamResponseParser.consumeResponseToString(rsp); + assertTrue( + "unexpected serialization of XML field value in JSON response: " + strResponse, + strResponse.contains("\"author\":\"some")); + + // a glob cannot be combined with a transformer that is not a raw value transformer + req = + new QueryRequest( + new ModifiableSolrParams( + Map.of("q", new String[] {"*:*"}, "fl", new String[] {"su*:[docid]"}))); + QueryRequest badReq = req; + SolrException e = + expectThrows(SolrException.class, () -> CLIENT.request(badReq, "collection1")); + assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code()); + } + + @Test + public void testSuffixGlobJsonTransformer() throws Exception { + // *_json:[json] writes every field whose name ends in _json raw, here payload_json + QueryRequest req = + new QueryRequest( + new ModifiableSolrParams( + Map.of( + "q", + new String[] {"*:*"}, + "fl", + new String[] {"id,*_json:[json]"}, + "wt", + new String[] {"json"}, + "indent", + new String[] {"false"}))); + req.setResponseParser(JSON_STREAM_RESPONSE_PARSER); + NamedList rsp = CLIENT.request(req, "collection1"); + String strResponse = InputStreamResponseParser.consumeResponseToString(rsp); + + Object parsed = Utils.fromJSONString(strResponse); + List docs = (List) ((Map) ((Map) parsed).get("response")).get("docs"); + int withPayload = 0; + for (Object doc : docs) { + Map fields = (Map) doc; + if (fields.containsKey("payload_json")) { + withPayload++; + assertEquals( + "payload_json was not written raw as a JSON array: " + strResponse, + "[{seq=[1, 2]}, {seq=[3, 4]}]", + String.valueOf(fields.get("payload_json"))); + } + } + assertTrue("no payload_json field was returned: " + strResponse, withPayload > 0); + assertFalse( + "field not matching the glob was returned: " + strResponse, + strResponse.contains("\"subject\"")); + + QueryRequest starReq = + new QueryRequest( + new ModifiableSolrParams( + Map.of("q", new String[] {"*:*"}, "fl", new String[] {"*:[json]"}))); + SolrException e = + expectThrows(SolrException.class, () -> CLIENT.request(starReq, "collection1")); + assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code()); + } + private static final InputStreamResponseParser XML_STREAM_RESPONSE_PARSER = new InputStreamResponseParser("xml"); private static final InputStreamResponseParser JSON_STREAM_RESPONSE_PARSER =