You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by cp...@apache.org on 2021/10/05 08:13:00 UTC
[solr] branch main updated: SOLR-15480: Make Tuple copy constructor,
clone and merge consistent w.r.t. markers (EOF, EXCEPTION),
field names and labels. (#243)
This is an automated email from the ASF dual-hosted git repository.
cpoerschke pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new bb23033 SOLR-15480: Make Tuple copy constructor, clone and merge consistent w.r.t. markers (EOF, EXCEPTION), field names and labels. (#243)
bb23033 is described below
commit bb2303357ee931adb0ae2ec9a339a3727501f139
Author: John Durham <jd...@gmail.com>
AuthorDate: Tue Oct 5 03:12:55 2021 -0500
SOLR-15480: Make Tuple copy constructor, clone and merge consistent w.r.t. markers (EOF, EXCEPTION), field names and labels. (#243)
Co-authored-by: Christine Poerschke <cp...@apache.org>
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/client/solrj/io/Tuple.java | 65 ++++--
.../org/apache/solr/client/solrj/io/TupleTest.java | 223 +++++++++++++++++++++
3 files changed, 277 insertions(+), 13 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0200899..6d6819a 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -365,6 +365,8 @@ Other Changes
* SOLR-13138: Remove deprecated LegacyBM25SimilarityFactory class. (Christine Poerschke, janhoy)
+* SOLR-15480: Make Tuple copy constructor, clone and merge consistent w.r.t. markers (EOF, EXCEPTION), field names and labels. (John Durham, Mike Drob, Christine Poerschke)
+
Bug Fixes
---------------------
* SOLR-14546: Fix for a relatively hard to hit issue in OverseerTaskProcessor that could lead to out of order execution
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/Tuple.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/Tuple.java
index 7cd5e2a..1d51829 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/Tuple.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/Tuple.java
@@ -16,6 +16,9 @@
*/
package org.apache.solr.client.solrj.io;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.params.StreamParams;
+
import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
@@ -25,9 +28,7 @@ import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-
-import org.apache.solr.common.MapWriter;
-import org.apache.solr.common.params.StreamParams;
+import java.util.stream.Collectors;
/**
* A simple abstraction of a record containing key/value pairs.
@@ -87,10 +88,20 @@ public class Tuple implements Cloneable, MapWriter {
* @param fields map containing keys and values to be copied to this tuple
*/
public Tuple(Map<String, ?> fields) {
- // TODO Use bulk putAll operation that will properly size the map
- // https://issues.apache.org/jira/browse/SOLR-15480
- for (Map.Entry<String, ?> entry : fields.entrySet()) {
- put(entry.getKey(), entry.getValue());
+ putAll(fields);
+ }
+
+ /**
+ * A copy constructor
+ * @param original Tuple that will be copied
+ */
+ public Tuple(Tuple original) {
+ this.putAll(original.fields);
+ if (original.fieldNames != null) {
+ this.fieldNames = new ArrayList<>(original.fieldNames);
+ }
+ if (original.fieldLabels != null) {
+ this.fieldLabels = new HashMap<>(original.fieldLabels);
}
}
@@ -107,6 +118,16 @@ public class Tuple implements Cloneable, MapWriter {
}
}
+ public void putAll(Map<String, ?> fields) {
+ this.fields.putAll(fields);
+ if (fields.containsKey(StreamParams.EOF)) {
+ EOF = true;
+ }
+ if (fields.containsKey(StreamParams.EXCEPTION)) {
+ EXCEPTION = true;
+ }
+ }
+
public void remove(String key) {
this.fields.remove(key);
}
@@ -275,15 +296,33 @@ public class Tuple implements Cloneable, MapWriter {
}
public Tuple clone() {
- Tuple clone = new Tuple();
- clone.fields.putAll(fields);
- // TODO This doesn't copy EOF/Exception https://issues.apache.org/jira/browse/SOLR-15480
+ Tuple clone = new Tuple(this);
return clone;
}
-
+
+ /**
+ * The other tuples fields and fieldLabels will be putAll'd directly to this's fields and fieldLabels while
+ * other's fieldNames will be added such that duplicates aren't present.
+ * @param other Tuple to be merged into this.
+ */
public void merge(Tuple other) {
- // TODO This doesn't copy EOF/Exception https://issues.apache.org/jira/browse/SOLR-15480
- fields.putAll(other.getFields());
+ this.putAll(other.getFields());
+ if (other.fieldNames != null) {
+ if (this.fieldNames != null) {
+ this.fieldNames.addAll(other.fieldNames.stream()
+ .filter(otherFieldName -> !this.fieldNames.contains(otherFieldName))
+ .collect(Collectors.toList()));
+ } else {
+ this.fieldNames = new ArrayList<>(other.fieldNames);
+ }
+ }
+ if (other.fieldLabels != null) {
+ if (this.fieldLabels != null) {
+ this.fieldLabels.putAll(other.fieldLabels);
+ } else {
+ this.fieldLabels = new HashMap<>(other.fieldLabels);
+ }
+ }
}
@Override
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/TupleTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/TupleTest.java
new file mode 100644
index 0000000..9600b5d
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/TupleTest.java
@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.io;
+
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.common.MapWriter.EntryWriter;
+import org.apache.solr.common.params.StreamParams;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+public class TupleTest extends SolrTestCase {
+
+ @Test
+ public void putAllSetsEOFMarker() {
+ final Map<String, Object> fields = new HashMap<>();
+ fields.put("field-one", new Object());
+ fields.put("field-two", new Object());
+ fields.put(StreamParams.EOF, true);
+
+ final Tuple tuple = new Tuple();
+ tuple.putAll(fields);
+
+ assertTrue(tuple.EOF);
+ }
+
+ @Test
+ public void putAllSetsEXCEPTIONMarker() {
+ final Map<String, Object> fields = new HashMap<>();
+ fields.put("field-one", new Object());
+ fields.put("field-two", new Object());
+ fields.put(StreamParams.EXCEPTION, "exception");
+
+ final Tuple tuple = new Tuple();
+ tuple.putAll(fields);
+
+ assertTrue(tuple.EXCEPTION);
+ }
+
+ @Test
+ public void copyTest() {
+ final Map<String, Object> fields = new HashMap<>();
+ fields.put("field-one", new Object());
+ fields.put("field-two", new Object());
+ fields.put(StreamParams.EXCEPTION, "exception");
+ fields.put(StreamParams.EOF, true);
+ final Tuple original = new Tuple();
+ original.putAll(fields);
+ original.setFieldNames(new ArrayList<>(Arrays.asList("field-one", "field-two")));
+ original.setFieldLabels(new HashMap<>(Map.ofEntries(
+ Map.entry("field-one", "field one"),
+ Map.entry("field-two", "field two")
+ )));
+
+ final Tuple copy = new Tuple(original);
+
+ assertEquals(original.getFields().entrySet().size(), copy.getFields().entrySet().size());
+ assertEquals(original.getFieldNames().size(), copy.getFieldNames().size());
+ assertEquals(original.getFieldLabels().entrySet().size(), copy.getFieldLabels().entrySet().size());
+ assertEquals(original.EOF, copy.EOF);
+ assertEquals(original.EXCEPTION, copy.EXCEPTION);
+ }
+
+ @Test
+ public void cloneTest() {
+ final Map<String, Object> fields = new HashMap<>();
+ fields.put("field-one", new Object());
+ fields.put("field-two", new Object());
+ fields.put(StreamParams.EXCEPTION, "exception");
+ fields.put(StreamParams.EOF, true);
+ final Tuple original = new Tuple();
+ original.putAll(fields);
+ original.setFieldNames(new ArrayList<>(Arrays.asList("field-one", "field-two")));
+ original.setFieldLabels(new HashMap<>(Map.ofEntries(
+ Map.entry("field-one", "field one"),
+ Map.entry("field-two", "field two")
+ )));
+
+ final Tuple clone = original.clone();
+
+ assertEquals(original.getFields().entrySet().size(), clone.getFields().entrySet().size());
+ assertEquals(original.getFieldNames().size(), clone.getFieldNames().size());
+ assertEquals(original.getFieldLabels().entrySet().size(), clone.getFieldLabels().entrySet().size());
+ assertEquals(original.EOF, clone.EOF);
+ assertEquals(original.EXCEPTION, clone.EXCEPTION);
+ }
+
+ @Test
+ public void mergeTest() {
+ final Map<String, Object> commonFields = new HashMap<>();
+ commonFields.put("field-one", new Object());
+ commonFields.put("field-two", new Object());
+ commonFields.put("field-three", new Object());
+ commonFields.put(StreamParams.EXCEPTION, "exception");
+ commonFields.put(StreamParams.EOF, true);
+
+ final Tuple tupleOne = new Tuple();
+ tupleOne.putAll(commonFields);
+ tupleOne.setFieldNames(new ArrayList<>(Arrays.asList("field-one-name", "field-two-name", "field-three-name")));
+ tupleOne.setFieldLabels(new HashMap<>(Map.ofEntries(
+ Map.entry("field-one-name", "field-one"),
+ Map.entry("field-two-name", "field-two"),
+ Map.entry("field-three-name", "field-three")
+ )));
+
+ final Tuple tupleTwo = new Tuple();
+ tupleTwo.putAll(commonFields);
+ tupleTwo.put("field-four", new Object());
+ tupleTwo.put("new-field-two", new Object());
+ tupleTwo.setFieldNames(new ArrayList<>(Arrays.asList("field-one-name", "field-two-name", "field-four-name")));
+ tupleTwo.setFieldLabels(new HashMap<>(Map.ofEntries(
+ Map.entry("field-one-name", "field-one"),
+ Map.entry("field-two-name", "new-field-two"),
+ Map.entry("field-four-name", "field-four")
+ )));
+
+ tupleOne.merge(tupleTwo);
+
+ assertEquals(7, tupleOne.getFields().size());
+ assertEquals(4, tupleOne.getFieldNames().size()); // fieldNames should contain no duplicates
+ assertEquals(4, tupleOne.getFieldLabels().size());
+ assertEquals("new-field-two", tupleOne.getFieldLabels().get("field-two-name"));
+ assertTrue(tupleOne.EOF);
+ assertTrue(tupleOne.EXCEPTION);
+ }
+
+ @Test
+ public void writeMapTest() throws IOException {
+ final Map<String, Object> commonFields = new HashMap<>();
+ commonFields.put("field a", "1");
+ commonFields.put("field b", "2");
+ commonFields.put("field c", "3");
+
+ final Tuple tupleOne = new Tuple(commonFields);
+ // label all fields
+ tupleOne.setFieldLabels(new HashMap<>(Map.ofEntries(
+ Map.entry("field-one", "field a"),
+ Map.entry("field-two", "field b"),
+ Map.entry("field-three", "field c")
+ )));
+ // then choose a subset for serialisation
+ tupleOne.setFieldNames(new ArrayList<>(Arrays.asList("field-two", "field-three")));
+ {
+ final TupleEntryWriter writer = new TupleEntryWriter();
+ tupleOne.writeMap(writer);
+ assertEquals(2, writer.tuple.getFields().size());
+ assertEquals("2", writer.tuple.get("field b")); // field-two
+ assertEquals("3", writer.tuple.get("field c")); // field-three
+ }
+
+ final Tuple tupleTwo = new Tuple(commonFields);
+ tupleTwo.put("field d", "4");
+ // label most fields (3 of 4)
+ tupleTwo.setFieldLabels(new HashMap<>(Map.ofEntries(
+ Map.entry("field-one", "field b"),
+ Map.entry("field-two", "field a"),
+ Map.entry("field-four", "field d")
+ )));
+ // then choose a subset for serialisation
+ tupleTwo.setFieldNames(new ArrayList<>(Arrays.asList("field-two", "field-four")));
+ {
+ final TupleEntryWriter writer = new TupleEntryWriter();
+ tupleTwo.writeMap(writer);
+ assertEquals(2, writer.tuple.getFields().size());
+ assertEquals("1", writer.tuple.get("field a")); // field-two
+ assertEquals("4", writer.tuple.get("field d")); // field-four
+ }
+
+ // clone and merge
+ final Tuple tupleThree = tupleOne.clone();
+ tupleThree.merge(tupleTwo);
+ assertEquals(4, tupleThree.getFieldLabels().size());
+ assertEquals(3, tupleThree.getFieldNames().size());
+ // serialise merged tuples
+ {
+ final TupleEntryWriter writer = new TupleEntryWriter();
+ tupleThree.writeMap(writer);
+ assertEquals(3, writer.tuple.getFields().size());
+ assertEquals("1", writer.tuple.get("field a")); // field-two label in tupleTwo replaced field-two label from tupleOne
+ assertEquals("3", writer.tuple.get("field c")); // field-three label from tupleOne
+ assertEquals("4", writer.tuple.get("field d")); // field-four label from tupleTwo
+ }
+
+ tupleThree.setFieldNames(null);
+ // full serialisation
+ {
+ final TupleEntryWriter writer = new TupleEntryWriter();
+ tupleThree.writeMap(writer);
+ assertEquals(4, writer.tuple.getFields().size());
+ assertEquals("1", writer.tuple.get("field a"));
+ assertEquals("2", writer.tuple.get("field b"));
+ assertEquals("3", writer.tuple.get("field c"));
+ assertEquals("4", writer.tuple.get("field d"));
+ }
+ }
+
+ private static final class TupleEntryWriter implements EntryWriter {
+ final Tuple tuple = new Tuple();
+ @Override
+ public EntryWriter put(CharSequence k, Object v) throws IOException {
+ tuple.put(k.toString(), v);
+ return this;
+ }
+ }
+}