diff --git a/changelog/unreleased/solr-18410.yml b/changelog/unreleased/solr-18410.yml new file mode 100644 index 00000000000..7ef53e52860 --- /dev/null +++ b/changelog/unreleased/solr-18410.yml @@ -0,0 +1,8 @@ +title: CrossDC Common multivalued params and per-entry params don't survive the serialization round-trip + +type: fixed +authors: + - name: Andrzej Bialecki +links: + - name: SOLR-18410 + url: https://issues.apache.org/jira/browse/SOLR-18410 diff --git a/solr/modules/cross-dc/src/java/org/apache/solr/crossdc/common/MirroredSolrRequestSerializer.java b/solr/modules/cross-dc/src/java/org/apache/solr/crossdc/common/MirroredSolrRequestSerializer.java index 6476dbd9302..dc3d9c185a0 100644 --- a/solr/modules/cross-dc/src/java/org/apache/solr/crossdc/common/MirroredSolrRequestSerializer.java +++ b/solr/modules/cross-dc/src/java/org/apache/solr/crossdc/common/MirroredSolrRequestSerializer.java @@ -29,14 +29,16 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrResponse; import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.CollectionParams; import org.apache.solr.common.params.CoreAdminParams; -import org.apache.solr.common.params.MapSolrParams; import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.CollectionUtil; import org.apache.solr.common.util.ContentStream; import org.apache.solr.common.util.JavaBinCodec; +import org.apache.solr.common.util.NamedList; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -84,7 +86,7 @@ public MirroredSolrRequest deserialize(String topic, byte[] data) { Long.parseLong(String.valueOf(requestMap.getOrDefault("submitTimeNanos", "-1"))); SolrParams params; if (requestMap.get("params") != null) { - params = new MapSolrParams((Map) requestMap.get("params")); + params = new NamedList<>((Map) requestMap.get("params")).toSolrParams(); } else { params = new ModifiableSolrParams(); } @@ -93,7 +95,18 @@ public MirroredSolrRequest deserialize(String topic, byte[] data) { UpdateRequest updateRequest = (UpdateRequest) request; List docs = (List) requestMap.get("docs"); if (docs != null) { - updateRequest.add(docs); + List> docsParams = + (List>) requestMap.get("docsParams"); + if (docsParams.size() != docs.size()) { + throw new RuntimeException("docs and docsParams size mismatch"); + } + for (int i = 0; i < docs.size(); i++) { + Map docParams = docsParams.get(i); + updateRequest.add( + (SolrInputDocument) docs.get(i), + docParams == null ? null : (Integer) docParams.get(UpdateRequest.COMMIT_WITHIN), + docParams == null ? null : (Boolean) docParams.get(UpdateRequest.OVERWRITE)); + } } else { updateRequest.add("id", "1"); // TODO huh? updateRequest.getDocumentsMap().clear(); @@ -101,7 +114,18 @@ public MirroredSolrRequest deserialize(String topic, byte[] data) { List deletes = (List) requestMap.get("deletes"); if (deletes != null) { - updateRequest.deleteById(deletes); + List> deletesParams = + (List>) requestMap.get("deletesParams"); + if (deletesParams.size() != deletes.size()) { + throw new RuntimeException("deletes and deletesParams size mismatch"); + } + for (int i = 0; i < deletes.size(); i++) { + Map deleteParams = deletesParams.get(i); + updateRequest.deleteById( + deletes.get(i), + deleteParams == null ? null : (String) deleteParams.get(ShardParams._ROUTE_), + deleteParams == null ? null : (Long) deleteParams.get(UpdateRequest.VER)); + } } List deletesQuery = (List) requestMap.get("deleteQuery"); @@ -185,8 +209,16 @@ public byte[] serialize(String topic, MirroredSolrRequest request) { map.put("params", solrRequest.getParams()); map.put("type", request.getType().toString()); if (solrRequest instanceof UpdateRequest update) { - map.put("docs", update.getDocuments()); - map.put("deletes", update.getDeleteById()); + Map> docsMap = update.getDocumentsMap(); + if (docsMap != null && !docsMap.isEmpty()) { + map.put("docs", docsMap.keySet()); + map.put("docsParams", update.getDocumentsMap().values()); + } + Map> deletes = update.getDeleteByIdMap(); + if (deletes != null && !deletes.isEmpty()) { + map.put("deletes", deletes.keySet()); + map.put("deletesParams", deletes.values()); + } map.put("deleteQuery", update.getDeleteQuery()); } else if (solrRequest instanceof MirroredSolrRequest.MirroredConfigSetRequest config) { map.put("method", config.getMethod().toString()); diff --git a/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/common/MirroredSolrRequestSerializerTest.java b/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/common/MirroredSolrRequestSerializerTest.java index e0afa71cd33..0b211b8bc32 100644 --- a/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/common/MirroredSolrRequestSerializerTest.java +++ b/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/common/MirroredSolrRequestSerializerTest.java @@ -17,10 +17,15 @@ package org.apache.solr.crossdc.common; import java.util.Arrays; +import java.util.List; +import java.util.Map; import org.apache.lucene.tests.util.TestUtil; import org.apache.solr.SolrTestCase; import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.ShardParams; +import org.apache.solr.common.params.SolrParams; import org.junit.Test; public class MirroredSolrRequestSerializerTest extends SolrTestCase { @@ -55,4 +60,110 @@ public void testSerializationBufferOptimization() { assertEquals(fieldValue, deserValue); } } + + @Test + public void testMultivaluedParamsRoundTrip() { + MirroredSolrRequestSerializer serializer = new MirroredSolrRequestSerializer(); + UpdateRequest req = new UpdateRequest(); + SolrInputDocument doc = new SolrInputDocument(); + doc.setField("id", "1"); + req.add(doc); + + ModifiableSolrParams params = new ModifiableSolrParams(); + params.set("q", "single-value"); + params.add("fq", "a", "b", "c"); + req.setParams(params); + + MirroredSolrRequest mirroredRequest = new MirroredSolrRequest<>(req); + byte[] data = serializer.serialize("test", mirroredRequest); + MirroredSolrRequest deserialized = serializer.deserialize("test", data); + + SolrParams deserializedParams = deserialized.getSolrRequest().getParams(); + assertEquals("single-value", deserializedParams.get("q")); + assertArrayEquals(new String[] {"a", "b", "c"}, deserializedParams.getParams("fq")); + } + + @Test + public void testDocsParamsRoundTrip() { + MirroredSolrRequestSerializer serializer = new MirroredSolrRequestSerializer(); + UpdateRequest req = new UpdateRequest(); + SolrInputDocument doc1 = new SolrInputDocument(); + doc1.setField("id", "1"); + SolrInputDocument doc2 = new SolrInputDocument(); + doc2.setField("id", "2"); + req.add(doc1, 5000, true); + req.add(doc2, 1000, false); + + MirroredSolrRequest mirroredRequest = new MirroredSolrRequest<>(req); + byte[] data = serializer.serialize("test", mirroredRequest); + MirroredSolrRequest deserialized = serializer.deserialize("test", data); + + UpdateRequest deserializedReq = (UpdateRequest) deserialized.getSolrRequest(); + Map> docsMap = deserializedReq.getDocumentsMap(); + assertEquals(2, docsMap.size()); + for (Map.Entry> entry : docsMap.entrySet()) { + String id = (String) entry.getKey().getFieldValue("id"); + Map docParams = entry.getValue(); + if ("1".equals(id)) { + assertEquals(5000, docParams.get(UpdateRequest.COMMIT_WITHIN)); + assertEquals(Boolean.TRUE, docParams.get(UpdateRequest.OVERWRITE)); + } else if ("2".equals(id)) { + assertEquals(1000, docParams.get(UpdateRequest.COMMIT_WITHIN)); + assertEquals(Boolean.FALSE, docParams.get(UpdateRequest.OVERWRITE)); + } else { + fail("Unexpected document id: " + id); + } + } + } + + @Test + public void testDeletesParamsRoundTrip() { + MirroredSolrRequestSerializer serializer = new MirroredSolrRequestSerializer(); + UpdateRequest req = new UpdateRequest(); + req.deleteById("1", "shard1", 100L); + req.deleteById("2", "shard2", 200L); + + MirroredSolrRequest mirroredRequest = new MirroredSolrRequest<>(req); + byte[] data = serializer.serialize("test", mirroredRequest); + MirroredSolrRequest deserialized = serializer.deserialize("test", data); + + UpdateRequest deserializedReq = (UpdateRequest) deserialized.getSolrRequest(); + Map> deleteByIdMap = deserializedReq.getDeleteByIdMap(); + assertEquals(2, deleteByIdMap.size()); + Map params1 = deleteByIdMap.get("1"); + assertEquals("shard1", params1.get(ShardParams._ROUTE_)); + assertEquals(100L, params1.get(UpdateRequest.VER)); + Map params2 = deleteByIdMap.get("2"); + assertEquals("shard2", params2.get(ShardParams._ROUTE_)); + assertEquals(200L, params2.get(UpdateRequest.VER)); + } + + @Test + public void testBothDocsAndDeletesParamsRoundTrip() { + MirroredSolrRequestSerializer serializer = new MirroredSolrRequestSerializer(); + UpdateRequest req = new UpdateRequest(); + SolrInputDocument doc = new SolrInputDocument(); + doc.setField("id", "1"); + req.add(doc, 2000, true); + req.deleteById("2", "shard1", 50L); + req.deleteByQuery("field:value"); + + MirroredSolrRequest mirroredRequest = new MirroredSolrRequest<>(req); + byte[] data = serializer.serialize("test", mirroredRequest); + MirroredSolrRequest deserialized = serializer.deserialize("test", data); + + UpdateRequest deserializedReq = (UpdateRequest) deserialized.getSolrRequest(); + Map> docsMap = deserializedReq.getDocumentsMap(); + assertEquals(1, docsMap.size()); + Map docParams = docsMap.values().iterator().next(); + assertEquals(2000, docParams.get(UpdateRequest.COMMIT_WITHIN)); + assertEquals(Boolean.TRUE, docParams.get(UpdateRequest.OVERWRITE)); + + Map> deleteByIdMap = deserializedReq.getDeleteByIdMap(); + Map deleteParams = deleteByIdMap.get("2"); + assertEquals("shard1", deleteParams.get(ShardParams._ROUTE_)); + assertEquals(50L, deleteParams.get(UpdateRequest.VER)); + + assertEquals(List.of("field:value"), deserializedReq.getDeleteQuery()); + } }