From f4547f6a3b34380511df8ef668af59a410aea281 Mon Sep 17 00:00:00 2001 From: Shalin Shekhar Mangar Date: Fri, 21 Dec 2012 13:01:08 +0000 Subject: [PATCH] SOLR-4134: Standard (XML) request writer cannot "set" multiple values into multivalued field with partial updates. (Luis Cappa Banda, Will Butler, shalin) git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1424906 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 3 + .../apache/solr/handler/loader/XMLLoader.java | 36 ++++++++-- .../solr/client/solrj/util/ClientUtils.java | 72 +++++++++++-------- .../solr/client/solrj/SolrExampleTests.java | 36 +++++++++- 4 files changed, 108 insertions(+), 39 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 5aea33ab2f7..5fba325b5ea 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -372,6 +372,9 @@ Bug Fixes * SOLR-4213: Directories that are not shutdown until DirectoryFactory#close do not have close listeners called on them. (Mark Miller) +* SOLR-4134: Standard (XML) request writer cannot "set" multiple values into + multivalued field with partial updates. (Luis Cappa Banda, Will Butler, shalin) + Other Changes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java b/solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java index 1f7a389dabf..a4ed0597062 100644 --- a/solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java +++ b/solr/core/src/java/org/apache/solr/handler/loader/XMLLoader.java @@ -59,7 +59,9 @@ import javax.xml.parsers.SAXParserFactory; import java.io.ByteArrayInputStream; import java.io.InputStream; import java.io.IOException; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; @@ -379,8 +381,9 @@ public class XMLLoader extends ContentStreamLoader { float boost = 1.0f; boolean isNull = false; String update = null; - - while (true) { + Map>> updateMap = null; + boolean complete = false; + while (!complete) { int event = parser.next(); switch (event) { // Add everything to the text @@ -392,13 +395,24 @@ public class XMLLoader extends ContentStreamLoader { case XMLStreamConstants.END_ELEMENT: if ("doc".equals(parser.getLocalName())) { - return doc; + complete = true; + break; } else if ("field".equals(parser.getLocalName())) { Object v = isNull ? null : text.toString(); if (update != null) { - Map extendedValue = new HashMap(1); - extendedValue.put(update, v); - v = extendedValue; + if (updateMap == null) updateMap = new HashMap>>(); + Map> extendedValues = updateMap.get(name); + if (extendedValues == null) { + extendedValues = new HashMap>(1); + updateMap.put(name, extendedValues); + } + List values = extendedValues.get(update); + if (values == null) { + values = new ArrayList(); + extendedValues.put(update, values); + } + values.add(v); + break; } doc.addField(name, v, boost); boost = 1.0f; @@ -434,5 +448,15 @@ public class XMLLoader extends ContentStreamLoader { break; } } + + if (updateMap != null) { + for (Map.Entry>> entry : updateMap.entrySet()) { + name = entry.getKey(); + Map> value = entry.getValue(); + doc.addField(name, value, 1.0f); + } + } + + return doc; } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java b/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java index 8eafe72102a..15b9db2157e 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java @@ -104,43 +104,55 @@ public class ClientUtils // currently only supports a single value for (Entry entry : ((Map)v).entrySet()) { update = entry.getKey().toString(); - Object fieldVal = entry.getValue(); - v = fieldVal; + v = entry.getValue(); + if (v instanceof Collection) { + Collection values = (Collection) v; + for (Object value : values) { + writeVal(writer, boost, name, value, update); + boost = 1.0f; + } + } else { + writeVal(writer, boost, name, v, update); + boost = 1.0f; + } } + } else { + writeVal(writer, boost, name, v, update); + // only write the boost for the first multi-valued field + // otherwise, the used boost is the product of all the boost values + boost = 1.0f; } - - if (v instanceof Date) { - v = DateUtil.getThreadLocalDateFormat().format( (Date)v ); - } else if (v instanceof byte[]) { - byte[] bytes = (byte[]) v; - v = Base64.byteArrayToBase64(bytes, 0,bytes.length); - } else if (v instanceof ByteBuffer) { - ByteBuffer bytes = (ByteBuffer) v; - v = Base64.byteArrayToBase64(bytes.array(), bytes.position(),bytes.limit() - bytes.position()); - } - - if (update == null) { - if( boost != 1.0f ) { - XML.writeXML(writer, "field", v.toString(), "name", name, "boost", boost ); - } else if (v != null) { - XML.writeXML(writer, "field", v.toString(), "name", name ); - } - } else { - if( boost != 1.0f ) { - XML.writeXML(writer, "field", v.toString(), "name", name, "boost", boost, "update", update); - } else if (v != null) { - XML.writeXML(writer, "field", v.toString(), "name", name, "update", update); - } - } - - // only write the boost for the first multi-valued field - // otherwise, the used boost is the product of all the boost values - boost = 1.0f; } } writer.write(""); } + private static void writeVal(Writer writer, float boost, String name, Object v, String update) throws IOException { + if (v instanceof Date) { + v = DateUtil.getThreadLocalDateFormat().format( (Date)v ); + } else if (v instanceof byte[]) { + byte[] bytes = (byte[]) v; + v = Base64.byteArrayToBase64(bytes, 0, bytes.length); + } else if (v instanceof ByteBuffer) { + ByteBuffer bytes = (ByteBuffer) v; + v = Base64.byteArrayToBase64(bytes.array(), bytes.position(),bytes.limit() - bytes.position()); + } + + if (update == null) { + if( boost != 1.0f ) { + XML.writeXML(writer, "field", v.toString(), "name", name, "boost", boost); + } else if (v != null) { + XML.writeXML(writer, "field", v.toString(), "name", name ); + } + } else { + if( boost != 1.0f ) { + XML.writeXML(writer, "field", v.toString(), "name", name, "boost", boost, "update", update); + } else if (v != null) { + XML.writeXML(writer, "field", v.toString(), "name", name, "update", update); + } + } + } + public static String toXML( SolrInputDocument doc ) { diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java index 77f1893b677..ce35863c1d2 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java @@ -19,14 +19,16 @@ package org.apache.solr.client.solrj; import java.io.IOException; -import java.io.StringWriter; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Random; import java.util.concurrent.atomic.AtomicInteger; + +import com.google.common.collect.Lists; import junit.framework.Assert; import org.apache.lucene.util._TestUtil; @@ -35,7 +37,6 @@ import org.apache.solr.client.solrj.impl.BinaryResponseParser; import org.apache.solr.client.solrj.impl.ConcurrentUpdateSolrServer; import org.apache.solr.client.solrj.impl.HttpSolrServer; import org.apache.solr.client.solrj.impl.XMLResponseParser; -import org.apache.solr.client.solrj.request.DirectXmlRequest; import org.apache.solr.client.solrj.request.LukeRequest; import org.apache.solr.client.solrj.request.QueryRequest; import org.apache.solr.client.solrj.response.FieldStatsInfo; @@ -53,7 +54,6 @@ import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrInputDocument; -import org.apache.solr.common.util.XML; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.params.AnalysisParams; import org.apache.solr.common.params.CommonParams; @@ -1380,6 +1380,36 @@ abstract public class SolrExampleTests extends SolrJettyTestBase assertEquals("price was not updated?", 200.0f, resp.getResults().get(0).getFirstValue("price_f")); assertEquals("no name?", "gadget", resp.getResults().get(0).getFirstValue("name")); } + + public void testUpdateMultiValuedField() throws Exception { + SolrServer solrServer = getSolrServer(); + SolrInputDocument doc = new SolrInputDocument(); + doc.addField("id", "123"); + solrServer.add(doc); + solrServer.commit(true, true); + QueryResponse response = solrServer.query(new SolrQuery("id:123")); + assertEquals("Failed to add doc to cloud server", 1, response.getResults().getNumFound()); + + Map> operation = new HashMap>(); + operation.put("set", Lists.asList("first", "second", new String[]{"third"})); + doc.addField("multi_ss", operation); + solrServer.add(doc); + solrServer.commit(true, true); + response = solrServer.query(new SolrQuery("id:123")); + assertTrue("Multi-valued field did not return a collection", response.getResults().get(0).get("multi_ss") instanceof List); + List values = (List) response.getResults().get(0).get("multi_ss"); + assertEquals("Field values was not updated with all values via atomic update", 3, values.size()); + + operation.clear(); + operation.put("add", Lists.asList("fourth", new String[]{"fifth"})); + doc.removeField("multi_ss"); + doc.addField("multi_ss", operation); + solrServer.add(doc); + solrServer.commit(true, true); + response = solrServer.query(new SolrQuery("id:123")); + values = (List) response.getResults().get(0).get("multi_ss"); + assertEquals("Field values was not updated with all values via atomic update", 5, values.size()); + } @Test public void testQueryWithParams() throws SolrServerException {