Fix: Indexing with atomic-update can remove all other attributes from…

… the index.

Fix: Reindexing solr-objects with specific attributes (atomic-update) will remove
all indexed attributes on an object if it does not provide this index or if the
field is multivalued and the value is empty. (fixes #171)

CHANGES.rst

@@ -4,6 +4,11 @@ Changelog
 5.0.4 (unreleased)
 ------------------
 
+- Fix: Reindexing solr-objects with specific attributes (atomic-update) will remove
+  all indexed attributes on an object if it does not provide this index or if the
+  field is multivalued and the value is empty. (fixes #171)
+  [elioschmutz]
+
 - Use unittest instead unittest2 in test_browser_suggest.py to fix tests.
   [elioschmutz]
 

src/collective/solr/indexer.py

@@ -199,7 +199,7 @@ def index(self, obj, attributes=None):
                     attributes.add(uniqueKey)
 
             data, missing = self.getData(obj, attributes=attributes)
-            if not data:
+            if not data or not set(data.keys()) - set([uniqueKey]):
                 return          # don't index with no data...
             prepareData(data)
             if data.get(uniqueKey, None) is not None and not missing:

src/collective/solr/solr.py

@@ -257,8 +257,14 @@ def add(self, boost_values=None, atomic_updates=True, **fields):
                 tmpl = tmpl.replace(' update="set"', '')
 
             if isinstance(v, (list, tuple)):  # multi-valued
-                for value in v:
-                    lst.append(tmpl % self.escapeVal(value))
+                if v:
+                    for value in v:
+                        lst.append(tmpl % self.escapeVal(value))
+                else:
+                    # Add an empty element for empty lists/tuples.
+                    # Otherwise all indexes gets removed if you update an empty
+                    # multivalued attribute.
+                    lst.append(tmpl % '')
             else:
                 lst.append(tmpl % self.escapeVal(v))
         lst.append('</doc>')

src/collective/solr/tests/data/schema.xml

@@ -1,6 +1,6 @@
 HTTP/1.1 200 OK
 Content-Type: text/xml; charset=utf-8
-Content-Length: 7086
+Content-Length: 7071
 Server: Jetty(6.1.3)
 
 <?xml version="1.0" encoding="UTF-8"?>
@@ -66,7 +66,7 @@ Server: Jetty(6.1.3)
  <fields>
    <field name="id" type="string" indexed="true" stored="true" required="true"/> 
    <field name="sku" type="textTight" indexed="true" stored="true" omitNorms="true"/>
-   <field name="name" type="text" indexed="true" stored="true" required="true" />
+   <field name="name" type="text" indexed="true" stored="true" />
    <field name="nameSort" type="string" indexed="true" stored="false"/>
    <field name="alphaNameSort" type="alphaOnlySort" indexed="true" stored="false"/>
    <field name="manu" type="text" indexed="true" stored="true" omitNorms="true"/>
@@ -113,4 +113,4 @@ Server: Jetty(6.1.3)
    <copyField source="features" dest="text"/>
    <copyField source="includes" dest="text"/>
    <copyField source="manu" dest="manu_exact"/>
-</schema>
+</schema>

src/collective/solr/tests/test_indexer.py

@@ -182,13 +182,40 @@ def testCommit(self):
         self.assertEqual(str(output), getData('commit_request.txt'))
 
     def testNoIndexingWithoutAllRequiredFields(self):
+        response = getData('dummy_response.txt')
+        # fake add response
+        output = fakehttp(self.mngr.getConnection(), response)
+        # indexing sends data
+        self.proc.index(Foo(text='lorem ipsum'))  # required id is missing
+        self.assertEqual(str(output), '')
+
+    def testNoIndexingWithOnlyUniqueKeyField(self):
         response = getData('dummy_response.txt')
         # fake add response
         output = fakehttp(self.mngr.getConnection(), response)
         # indexing sends data
         self.proc.index(Foo(id='500'))
         self.assertEqual(str(output), '')
 
+    def testNoIndexingWithOnlyUniqueKeyFieldAndNotExistingIndex(self):
+        response = getData('dummy_response.txt')
+        # fake add response
+        output = fakehttp(self.mngr.getConnection(), response)
+        # indexing sends data
+        self.proc.index(Foo(id='500', notexisting='some data'))
+        self.assertEqual(str(output), '')
+
+    def testIndexingEmptyMultiValuedFieldWillAddFieldAsWell(self):
+        response = getData('dummy_response.txt')
+        # fake add response
+        output = fakehttp(self.mngr.getConnection(), response)
+        # indexing sends data
+        self.proc.index(Foo(id='500', features=()))
+        self.assertIn(
+            '<field name="features" update="set"></field>',
+            str(output),
+            "The empty multivalued field have to be in the output neverless")
+
     def testIndexerMethods(self):
         class Bar(Foo):
 

src/collective/solr/tests/test_parser.py

@@ -129,7 +129,7 @@ def testParseConfig(self):
         self.assertEqual(schema['defaultSearchField'], 'text')
         self.assertEqual(schema['uniqueKey'], 'id')
         self.assertEqual(schema['solrQueryParser'].defaultOperator, 'OR')
-        self.assertEqual(schema['requiredFields'], ['id', 'name'])
+        self.assertEqual(schema['requiredFields'], ['id'])
         self.assertEqual(schema['id'].type, 'string')
         self.assertEqual(schema['id'].class_, 'solr.StrField')
         self.assertEqual(schema['id'].required, True)
@@ -151,7 +151,7 @@ def testParseConfig(self):
         self.assertEqual(schema.word.indexed, False)
         fields = schema.values()
         self.assertEqual(len([f for f in fields if
-                              getattr(f, 'required', False)]), 2)
+                              getattr(f, 'required', False)]), 1)
         self.assertEqual(len([f for f in fields if
                               getattr(f, 'multiValued', False)]), 3)
 

src/collective/solr/tests/test_server.py

@@ -2,7 +2,6 @@
 from Acquisition import aq_base
 from Acquisition import aq_parent
 from DateTime import DateTime
-from Missing import MV
 from Products.CMFCore.utils import getToolByName
 from collective.indexing.queue import getQueue
 from collective.indexing.queue import processQueue
@@ -1215,7 +1214,7 @@ def testFlareHasDataForAllMetadataColumns(self):
         self.maintenance.reindex()
         results = solrSearchResults(SearchableText='Welcome')
         self.assertEqual(len(results), 1)
-        self.assertEqual(results[0].get('Subject'), MV)
+        self.assertEqual(results[0].get('Subject'), [''])
         schema = self.search.getManager().getSchema()
         expected = set(list(schema.stored) + ['score'])     # score gets added
         self.assertEqual(set(results[0].keys()), expected)