\Stringable always has precedence over \JsonSerializable

Author: thomascorthals

CHANGELOG.md

@@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - JSON update requests correctly handle `Stringable` object set as field value
 
+### Changed
+- JSON update requests give precedence to `Stringable` over `JsonSerializable` for object set as field value to keep behaviour consistent across request formats
+
 
 ## [6.3.5]
 ### Added

docs/documents.md

@@ -238,6 +238,14 @@ Solarium issues JSON formatted update requests by default. If you change this to
   [SOLR-16183](https://issues.apache.org/jira/browse/SOLR-16183). Any child document you index this way will end up as an anonymous nested child.
 - Atomic updates of child documents aren't fully supported because of [SOLR-12677](https://issues.apache.org/jira/browse/SOLR-12677).
 
+If you set a field value to an object that is both `Stringable` and `JsonSerializable`, it will always be cast to a `string` when building the
+request to keep the behaviour consistent between request formats. You can override this by calling `jsonSerialize()` explicitly. Even if an object
+isn't `Stringable`, depending on `jsonSerialize()` being called implicitly is discouraged as it will only work for JSON formatted update requests.
+
+```php
+$doc->reaction = $reaction->jsonSerialize();
+```
+
 ### Atomic updates
 
 You can create atomic updates by using the `setFieldModifier` method. Set a modifier on the field you want to update. The supported modifiers are:

src/QueryType/Update/Query/Document.php

@@ -498,7 +498,7 @@ public function jsonSerialize()
         foreach ($fields as $key => &$value) {
             if ($value instanceof \DateTimeInterface) {
                 $value = $this->getHelper()->formatDate($value);
-            } elseif ($value instanceof \Stringable && !($value instanceof \JsonSerializable)) {
+            } elseif ($value instanceof \Stringable) {
                 $value = (string) $value;
             } elseif (\is_array($value) && is_numeric(array_key_first($value))) {
                 // ensure consecutive indices so it doesn't serialize to an object

tests/QueryType/Update/RequestBuilder/JsonTest.php

@@ -773,6 +773,10 @@ public function __toString(): string
         );
     }
 
+    /**
+     * Test that \Stringable takes precedence over \JsonSerializable for
+     * consistency across request format.
+     */
     public function testBuildAddJsonWithJsonSerializableAndStringableObject()
     {
         $value = new class() implements \JsonSerializable, \Stringable {
@@ -795,6 +799,46 @@ public function __toString(): string
 
         $this->builder->buildAddJson($command, $json);
 
+        $this->assertCount(1, $json);
+        $this->assertJsonStringEqualsJsonString(
+            '{
+                "add": {
+                    "doc": {
+                        "id": 1,
+                        "my_field": "My string value"
+                    }
+                }
+            }',
+            '{'.$json[0].'}'
+        );
+    }
+
+    /**
+     * Test that the \Stringable precedence on an also \JsonSerializable object
+     * can be overridden by explicitly calling jsonSerialize().
+     */
+    public function testBuildAddJsonWithJsonSerializableAndStringableObjectWithExplicitJsonSerialize()
+    {
+        $value = new class() implements \JsonSerializable, \Stringable {
+            public function jsonSerialize(): mixed
+            {
+                return 'My JSON value';
+            }
+
+            public function __toString(): string
+            {
+                return 'My string value';
+            }
+        };
+
+        $command = new AddCommand();
+        $command->addDocument(
+            new Document(['id' => 1, 'my_field' => $value->jsonSerialize()])
+        );
+        $json = [];
+
+        $this->builder->buildAddJson($command, $json);
+
         $this->assertCount(1, $json);
         $this->assertJsonStringEqualsJsonString(
             '{

tests/QueryType/Update/RequestBuilder/XmlTest.php

@@ -542,6 +542,35 @@ public function __toString(): string
         );
     }
 
+    /**
+     * Test that \Stringable takes precedence over \JsonSerializable for
+     * consistency across request format.
+     */
+    public function testBuildAddXmlWithJsonSerializableAndStringableObject()
+    {
+        $value = new class() implements \JsonSerializable, \Stringable {
+            public function jsonSerialize(): mixed
+            {
+                return 'My JSON value';
+            }
+
+            public function __toString(): string
+            {
+                return 'My string value';
+            }
+        };
+
+        $command = new AddCommand();
+        $command->addDocument(
+            new Document(['id' => 1, 'my_field' => $value])
+        );
+
+        $this->assertSame(
+            '<add><doc><field name="id">1</field><field name="my_field">My string value</field></doc></add>',
+            $this->builder->buildAddXml($command)
+        );
+    }
+
     public function testBuildAddXmlWithFieldModifierAndNullValue()
     {
         $doc = new Document();