Skip to content

Commit

Permalink
Fix bug with empty array for multiValued field (#1025)
Browse files Browse the repository at this point in the history
* Fix bug with empty array for multiValued field

* Explicitly mention multivalues need numeric indices

* Test for empty strings

* Use 0 to test for empty values

Co-authored-by: Markus Kalkbrenner <git@kalki.de>
  • Loading branch information
thomascorthals and mkalkbrenner authored Jul 22, 2022
1 parent 67a9d61 commit fe00f6a
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

### Fixed
- An empty array for a multiValued field was wrongly interpreted as an empty child document by the Update request builder in 6.2.5

### Changed

Expand Down
2 changes: 1 addition & 1 deletion docs/documents.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ htmlFooter();

### Multivalue fields

If you set field values by property, array entry or by using the `setField` method you need to supply an array of values for a multivalue field. Any existing field values will be overwritten.
If you set field values by property, array entry or by using the `setField` method you need to supply a *numerically indexed* array of values for a multivalue field. Any existing field values will be overwritten.

If you want to add an extra value to an existing field, without overwriting, you should use the `addField` method. If you use this method on a field with a single value it will automatically be converted into a multivalue field, preserving the current value. You will need to call this method once for each value you want to add, it doesn't support arrays. You can also use this method for creating a new field, so you don't need to use a special method for the first field value.

Expand Down
6 changes: 3 additions & 3 deletions src/QueryType/Update/Query/Document.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public function __construct(array $fields = [], array $boosts = [], array $modif
* object, by field name.
*
* If you supply NULL as the value the field will be removed
* If you supply an array of values a multivalue field will be created.
* If you supply a numerically indexed array of values a multivalue field will be created.
* In all cases any existing (multi)value or child document(s) will be overwritten.
*
* @param string $name
Expand Down Expand Up @@ -228,8 +228,8 @@ public function addField(string $key, $value, ?float $boost = null, ?string $mod
/**
* Set a field value.
*
* If you supply NULL as the value the field will be removed
* If you supply an array of values a multivalue field will be created.
* If you supply NULL as the value and no modifier the field will be removed
* If you supply a numerically indexed array of values a multivalue field will be created.
* In all cases any existing (multi)value or child document(s) will be overwritten.
*
* @param string $key
Expand Down
4 changes: 3 additions & 1 deletion src/QueryType/Update/RequestBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,9 @@ private function buildFieldsXml(string $key, ?float $boost, $value, ?string $mod
}

if (\is_array($value)) {
if (is_numeric(array_key_first($value))) {
if (empty($value)) {
return '';
} elseif (is_numeric(array_key_first($value))) {
$nestedXml = '';

foreach ($value as $multival) {
Expand Down
8 changes: 7 additions & 1 deletion tests/Integration/AbstractTechproductsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1279,7 +1279,7 @@ public function testUpdate()
$select = self::$client->createSelect();
$select->setQuery('cat:solarium-test');
$select->addSort('id', $select::SORT_ASC);
$select->setFields('id,name,price');
$select->setFields('id,name,price,content');

// add, but don't commit
$update = self::$client->createUpdate();
Expand All @@ -1288,11 +1288,13 @@ public function testUpdate()
$doc1->setField('name', 'Solarium Test 1');
$doc1->setField('cat', 'solarium-test');
$doc1->setField('price', 3.14);
$doc1->setField('content', ['foo', 'bar']);
$doc2 = $update->createDocument();
$doc2->setField('id', 'solarium-test-2');
$doc2->setField('name', 'Solarium Test 2');
$doc2->setField('cat', 'solarium-test');
$doc2->setField('price', 42.0);
$doc2->setField('content', []);
$update->addDocuments([$doc1, $doc2]);
self::$client->update($update);
$result = self::$client->select($select);
Expand All @@ -1309,6 +1311,10 @@ public function testUpdate()
'id' => 'solarium-test-1',
'name' => 'Solarium Test 1',
'price' => 3.14,
'content' => [
'foo',
'bar',
],
], $iterator->current()->getFields());
$iterator->next();
$this->assertSame([
Expand Down
37 changes: 35 additions & 2 deletions tests/QueryType/Update/RequestBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ public function testBuildAddXmlWithBooleanValues()
public function testBuildAddXmlWithEmptyValues()
{
$command = new AddCommand();
$command->addDocument(new Document(['id' => 1, 'empty_string' => '', 'array_of_empty_string' => [''], 'null' => null]));
$command->addDocument(new Document(['id' => 0, 'empty_string' => '', 'array_of_empty_string' => [''], 'null' => null]));

// Empty values must be added to the document as empty fields, NULL values are skipped.
$this->assertSame(
'<add><doc><field name="id">1</field><field name="empty_string"></field><field name="array_of_empty_string"></field></doc></add>',
'<add><doc><field name="id">0</field><field name="empty_string"></field><field name="array_of_empty_string"></field></doc></add>',
$this->builder->buildAddXml($command)
);
}
Expand Down Expand Up @@ -146,6 +146,39 @@ public function testBuildAddXmlMultivalueField()
);
}

public function testBuildAddXmlMultivalueFieldWithEmptyArray()
{
$command = new AddCommand();
$command->addDocument(new Document(['id' => [1, 2, 3], 'text' => []]));

$this->assertSame(
'<add>'.
'<doc>'.
'<field name="id">1</field>'.
'<field name="id">2</field>'.
'<field name="id">3</field>'.
'</doc>'.
'</add>',
$this->builder->buildAddXml($command)
);
}

public function testBuildAddXmlWithEmptyStrings()
{
$command = new AddCommand();
$command->addDocument(new Document(['id' => '', 'text' => ['']]));

$this->assertSame(
'<add>'.
'<doc>'.
'<field name="id"></field>'.
'<field name="text"></field>'.
'</doc>'.
'</add>',
$this->builder->buildAddXml($command)
);
}

public function testBuildAddXmlWithSingleNestedDocument()
{
$command = new AddCommand();
Expand Down

0 comments on commit fe00f6a

Please sign in to comment.