Merge pull request #6 from silverstripe-big-o/ignore-empty-fields

Ignore empty fields

code/solr/SolrIndex.php

@@ -104,7 +104,11 @@ abstract class SolrIndex extends SearchIndex {
 
 		if (is_array($value)) foreach($value as $sub) {
 			/* Solr requires dates in the form 1995-12-31T23:59:59Z */
-			if ($type == 'tdate') $sub = gmdate('Y-m-d\TH:i:s\Z', strtotime($sub));
+			if ($type == 'tdate') {
+				if(!$sub) continue;
+				$sub = gmdate('Y-m-d\TH:i:s\Z', strtotime($sub));
+			}
+
 			/* Solr requires numbers to be valid if presented, not just empty */
 			if (($type == 'tint' || $type == 'tfloat' || $type == 'tdouble') && !is_numeric($sub)) continue;
 			
@@ -113,7 +117,11 @@ abstract class SolrIndex extends SearchIndex {
 
 		else {
 			/* Solr requires dates in the form 1995-12-31T23:59:59Z */
-			if ($type == 'tdate') $value = gmdate('Y-m-d\TH:i:s\Z', strtotime($value));
+			if ($type == 'tdate') {
+				if(!$value) return;
+				$value = gmdate('Y-m-d\TH:i:s\Z', strtotime($value));
+			}
+
 			/* Solr requires numbers to be valid if presented, not just empty */
 			if (($type == 'tint' || $type == 'tfloat' || $type == 'tdouble') && !is_numeric($value)) return;
 
@@ -140,17 +148,22 @@ abstract class SolrIndex extends SearchIndex {
 			if ($field['base'] == $base) $this->_addField($doc, $object, $field);
 		}
 
-		Solr::service(get_class($this))->addDocument($doc);
+		$this->getService()->addDocument($doc);
+
+		return $doc;
 	}
 
 	function add($object) {
 		$class = get_class($object);
+		$docs = array();
 
 		foreach ($this->getClasses() as $searchclass => $options) {
 			if ($searchclass == $class || ($options['include_children'] && is_subclass_of($class, $searchclass))) {
-				$this->_addAs($object, $searchclass, $options);
+				$docs[] = $this->_addAs($object, $searchclass, $options);
 			}
 		}
+
+		return $docs;
 	}
 
 	function canAdd($class) {
@@ -163,11 +176,11 @@ abstract class SolrIndex extends SearchIndex {
 
 	function delete($base, $id, $state) {
 		$documentID = $this->getDocumentIDForState($base, $id, $state);
-		Solr::service(get_class($this))->deleteById($documentID);
+		$this->getService()->deleteById($documentID);
 	}
 
 	function commit() {
-		Solr::service(get_class($this))->commit(false, false, false);
+		$this->getService()->commit(false, false, false);
 	}
 
 	/**

docs/Solr.md

@@ -89,4 +89,18 @@ to Solr when saving/publishing in SilverStripe,
 which is useful when debugging front-end queries, 
 see `thirdparty/fulltextsearch/server/silverstripe-solr-test.xml`.
 
-	java -Durl=http://localhost:8983/solr/MyIndex/update/ -Dtype=text/xml -jar post.jar silverstripe-solr-test.xml
+	java -Durl=http://localhost:8983/solr/MyIndex/update/ -Dtype=text/xml -jar post.jar silverstripe-solr-test.xml
+
+## FAQ
+
+### How do I use date ranges where dates might not be defined?
+
+The Solr index updater only includes dates with values,
+so the field might not exist in all your index entries.
+A simple bounded range query (`<field>:[* TO <date>]`) will fail in this case.
+In order to query the field, reverse the search conditions and exclude the ranges you don't want:
+
+	// Wrong: Filter will ignore all empty field values
+	$myQuery->filter(<field>, new SearchQuery_Range('*', <date>));
+	// Better: Exclude the opposite range
+	$myQuery->exclude(<field>, new SearchQuery_Range(<date>, '*'));

tests/SearchUpdaterTest.php

@@ -3,7 +3,8 @@
 class SearchUpdaterTest_Container extends DataObject {
 	static $db = array(
 		'Field1' => 'Varchar',
-		'Field2' => 'Varchar'
+		'Field2' => 'Varchar',
+		'MyDate' => 'Date',
 	);
 
 	static $has_one = array(

tests/SolrIndexTest.php

@@ -26,12 +26,37 @@ class SolrIndexTest extends SapphireTest {
 		);
 	}
 
+	function testIndexExcludesNullValues() {
+		$serviceMock = $this->getServiceMock();
+		$index = new SolrIndexTest_FakeIndex();
+		$index->setService($serviceMock);		
+		$obj = new SearchUpdaterTest_Container();
+
+		$obj->Field1 = 'Field1 val';
+		$obj->Field2 = null;
+		$obj->MyDate = null;
+		$docs = $index->add($obj);
+		$value = $docs[0]->getField('SearchUpdaterTest_Container_Field1');
+		$this->assertEquals('Field1 val', $value['value'], 'Writes non-NULL string fields');
+		$value = $docs[0]->getField('SearchUpdaterTest_Container_Field2');
+		$this->assertFalse($value, 'Ignores string fields if they are NULL');
+		$value = $docs[0]->getField('SearchUpdaterTest_Container_MyDate');
+		$this->assertFalse($value, 'Ignores date fields if they are NULL');
+
+		$obj->MyDate = '2010-12-30';
+		$docs = $index->add($obj);
+		$value = $docs[0]->getField('SearchUpdaterTest_Container_MyDate');
+		$this->assertEquals('2010-12-30T00:00:00Z', $value['value'], 'Writes non-NULL dates');
+	}
+
 	protected function getServiceMock() {
 		$serviceMock = Phockito::mock('SolrService');
 		$fakeResponse = new Apache_Solr_Response(new Apache_Solr_HttpTransport_Response(null, null, null));
+
 		Phockito::when($serviceMock)
-			->search(anything(), anything(), anything(), anything(), anything())
+			->_sendRawPost(anything(), anything(), anything(), anything())
 			->return($fakeResponse);
+
 		return $serviceMock;
 	}
 
@@ -42,6 +67,7 @@ class SolrIndexTest_FakeIndex extends SolrIndex {
 		$this->addClass('SearchUpdaterTest_Container');
 
 		$this->addFilterField('Field1');
+		$this->addFilterField('MyDate', 'Date');
 		$this->addFilterField('HasOneObject.Field1');
 		$this->addFilterField('HasManyObjects.Field1');
 	}