Merge "REST: Fix JsonPatcher return type"

wikimedia · Oct 26, 2023 · c6e15b4 · c6e15b4
2 parents 8748887 + 52b98e1
commit c6e15b4
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 4 deletions.
diff --git a/repo/rest-api/src/Domain/Services/JsonPatcher.php b/repo/rest-api/src/Domain/Services/JsonPatcher.php
@@ -12,10 +12,13 @@
 interface JsonPatcher {
 
 	/**
+	 * @return mixed usually this will be an array, but the type may change if the whole
+	 *               root element is modified by the client
+	 *
 	 * @throws InvalidArgumentException for an invalid patch
 	 * @throws PatchPathException if a path target provided in the patch does not exist
 	 * @throws PatchTestConditionFailedException if a "test" op in the patch fails
 	 */
-	public function patch( array $target, array $patch ): array;
+	public function patch( array $target, array $patch );
 
 }
diff --git a/repo/rest-api/src/Infrastructure/JsonDiffJsonPatcher.php b/repo/rest-api/src/Infrastructure/JsonDiffJsonPatcher.php
@@ -19,7 +19,7 @@ class JsonDiffJsonPatcher implements JsonPatcher {
 	/**
 	 * @inheritDoc
 	 */
-	public function patch( array $target, array $patch ): array {
+	public function patch( array $target, array $patch ) {
 		try {
 			$patchDocument = JsonPatch::import( $patch );
 		} catch ( Exception $e ) {
@@ -41,7 +41,7 @@ public function patch( array $target, array $patch ): array {
 		}
 
 		// TODO investigate. Casting to array is necessary if $target starts out as an empty array.
-		return (array)$target;
+		return is_object( $target ) ? (array)$target : $target;
 	}
 
 }
diff --git a/repo/rest-api/tests/phpunit/Infrastructure/JsonDiffJsonPatcherTest.php b/repo/rest-api/tests/phpunit/Infrastructure/JsonDiffJsonPatcherTest.php
@@ -21,8 +21,12 @@ class JsonDiffJsonPatcherTest extends TestCase {
 
 	/**
 	 * @dataProvider validPatchProvider
+	 *
+	 * @param array $target
+	 * @param array $patch
+	 * @param mixed $expected
 	 */
-	public function testPatch( array $target, array $patch, array $expected ): void {
+	public function testPatch( array $target, array $patch, $expected ): void {
 		$result = ( new JsonDiffJsonPatcher() )->patch( $target, $patch );
 
 		$this->assertEquals( $expected, $result );
@@ -52,6 +56,18 @@ public static function validPatchProvider(): Generator {
 			[ [ 'op' => 'test', 'path' => '/baz', 'value' => 42 ] ],
 			[ 'foo' => 'bar', 'baz' => 42 ],
 		];
+
+		yield 'add a key/value pair to an empty array' => [
+			[],
+			[ [ 'op' => 'add', 'path' => '/foo', 'value' => 'new value' ] ],
+			[ 'foo' => 'new value' ],
+		];
+
+		yield 'patch results in a string' => [
+			[ 'foo' => 'bar', 'baz' => 42 ],
+			[ [ 'op' => 'replace', 'path' => '', 'value' => 'replaced value' ] ],
+			'replaced value',
+		];
 	}
 
 	/**