Merge "REST: Replace sitelink-title-does-not-exist error"

wikimedia · Sep 13, 2024 · 7625558 · 7625558
2 parents 50888fc + e9ab502
commit 7625558
Show file tree

Hide file tree

Showing 12 changed files with 38 additions and 81 deletions.
diff --git a/repo/rest-api/specs/global/examples.json b/repo/rest-api/specs/global/examples.json
@@ -69,17 +69,11 @@
 			"context": { "parameter": "{path_parameter}" }
 		}
 	},
-	"TitleDoesNotExistOnGivenSiteExample": {
+	"ReferencedResourceNotFoundExample": {
 		"value": {
-			"code": "title-does-not-exist",
-			"message": "Page with title {title} does not exist on the given site"
-		}
-	},
-	"TitleDoesNotExistOnGivenSiteWithSiteContextExample": {
-		"value": {
-			"code": "title-does-not-exist",
-			"message": "Page with title {title} does not exist on the given site",
-			"context": { "site": "{site_id}" }
+			"code": "referenced-resource-not-found",
+			"message": "The referenced resource does not exist",
+			"context": { "path":  "{json_pointer}" }
 		}
 	},
 	"CannotModifyReadOnlyValue": {

diff --git a/repo/rest-api/specs/global/responses.json b/repo/rest-api/specs/global/responses.json
@@ -160,8 +160,8 @@
 				"examples": {
 					"value-too-long": { "$ref": "./examples.json#/ValueTooLongExample" },
 					"statement-group-property-id-mismatch": { "$ref": "./examples.json#/StatementGroupPropertyIdMismatch" },
-					"title-does-not-exist": {
-						"$ref": "./examples.json#/TitleDoesNotExistOnGivenSiteWithSiteContextExample"
+					"referenced-resource-not-found": {
+						"$ref": "./examples.json#/ReferencedResourceNotFoundExample"
 					},
 					"invalid-value": { "$ref": "./examples.json#/InvalidValueExample" },
 					"missing-field": { "$ref": "./examples.json#/MissingFieldExample" },
@@ -461,7 +461,7 @@
 					"invalid-value": { "$ref": "./examples.json#/InvalidValueExample" },
 					"missing-field": { "$ref": "./examples.json#/MissingFieldExample" },
 					"value-too-long": { "$ref": "./examples.json#/ValueTooLongExample" },
-					"title-does-not-exist": { "$ref": "./examples.json#/TitleDoesNotExistOnGivenSiteExample" }
+					"referenced-resource-not-found": { "$ref": "./examples.json#/ReferencedResourceNotFoundExample" }
 				}
 			}
 		},

diff --git a/...c/Application/UseCaseRequestValidation/ItemSerializationRequestValidatingDeserializer.php b/...c/Application/UseCaseRequestValidation/ItemSerializationRequestValidatingDeserializer.php
@@ -201,12 +201,7 @@ private function handleSitelinksValidationErrors( ValidationError $validationErr
 				$badgeIndex = Utils::getIndexOfValueInSerialization( $badge, $serialization[$siteId()][ 'badges' ] );
 				throw UseCaseError::newInvalidValue( "/item/sitelinks/{$siteId()}/badges/$badgeIndex" );
 			case SitelinkValidator::CODE_TITLE_NOT_FOUND:
-				$title = $serialization[$siteId()]['title'];
-				throw new UseCaseError(
-					UseCaseError::SITELINK_TITLE_NOT_FOUND,
-					"Page with title $title does not exist on the given site",
-					[ UseCaseError::CONTEXT_SITE_ID => $siteId() ]
-				);
+				throw UseCaseError::newReferencedResourceNotFound( "/item/sitelinks/{$siteId()}/title" );
 			case SitelinkValidator::CODE_SITELINK_CONFLICT:
 				$conflictingItemId = $context[ SitelinkValidator::CONTEXT_CONFLICTING_ITEM_ID ];
 				throw UseCaseError::newDataPolicyViolation(

diff --git a/...pi/src/Application/UseCaseRequestValidation/SitelinkEditRequestValidatingDeserializer.php b/...pi/src/Application/UseCaseRequestValidation/SitelinkEditRequestValidatingDeserializer.php
@@ -39,10 +39,7 @@ public function validateAndDeserialize( SitelinkEditRequest $request ): SiteLink
 					$badgeIndex = Utils::getIndexOfValueInSerialization( $badge, $request->getSitelink()[ 'badges' ] );
 					throw UseCaseError::newInvalidValue( "/sitelink/badges/$badgeIndex" );
 				case SitelinkValidator::CODE_TITLE_NOT_FOUND:
-					throw new UseCaseError(
-						UseCaseError::SITELINK_TITLE_NOT_FOUND,
-						"Page with title {$request->getSitelink()['title']} does not exist on the given site"
-					);
+					throw UseCaseError::newReferencedResourceNotFound( '/sitelink/title' );
 				case SitelinkValidator::CODE_SITELINK_CONFLICT:
 					$conflictingItemId = (string)$validationError->getContext()[ SitelinkValidator::CONTEXT_CONFLICTING_ITEM_ID ];
 					throw UseCaseError::newDataPolicyViolation(

diff --git a/repo/rest-api/src/Application/UseCases/UseCaseError.php b/repo/rest-api/src/Application/UseCases/UseCaseError.php
@@ -39,8 +39,8 @@ class UseCaseError extends UseCaseException {
 	public const POLICY_VIOLATION_LABEL_DESCRIPTION_SAME_VALUE = 'label-description-same-value';
 	public const POLICY_VIOLATION_SITELINK_CONFLICT = 'sitelink-conflict';
 	public const PROPERTY_STATEMENT_ID_MISMATCH = 'property-statement-id-mismatch';
+	public const REFERENCED_RESOURCE_NOT_FOUND = 'referenced-resource-not-found';
 	public const RESOURCE_NOT_FOUND = 'resource-not-found';
-	public const SITELINK_TITLE_NOT_FOUND = 'title-does-not-exist';
 	public const STATEMENT_GROUP_PROPERTY_ID_MISMATCH = 'statement-group-property-id-mismatch';
 	public const UNEXPECTED_ERROR = 'unexpected-error';
 	public const VALUE_TOO_LONG = 'value-too-long';
@@ -98,8 +98,8 @@ class UseCaseError extends UseCaseException {
 		self::PERMISSION_DENIED => [ self::CONTEXT_REASON ],
 		self::PERMISSION_DENIED_UNKNOWN_REASON => [],
 		self::PROPERTY_STATEMENT_ID_MISMATCH => [ self::CONTEXT_PROPERTY_ID, self::CONTEXT_STATEMENT_ID ],
+		self::REFERENCED_RESOURCE_NOT_FOUND => [ self::CONTEXT_PATH ],
 		self::RESOURCE_NOT_FOUND => [ self::CONTEXT_RESOURCE_TYPE ],
-		self::SITELINK_TITLE_NOT_FOUND => [],
 		self::STATEMENT_GROUP_PROPERTY_ID_MISMATCH => [
 			self::CONTEXT_PATH,
 			self::CONTEXT_STATEMENT_GROUP_PROPERTY_ID,
@@ -109,14 +109,6 @@ class UseCaseError extends UseCaseException {
 		self::VALUE_TOO_LONG => [ self::CONTEXT_PATH, self::CONTEXT_LIMIT ],
 	];
 
-	/**
-	 * Depending on the use case and whether it's operating on a single resource or a list, errors may include path information in the
-	 * context.
-	 */
-	private const ADDITIONAL_PATH_CONTEXT = [
-		self::SITELINK_TITLE_NOT_FOUND => [ self::CONTEXT_SITE_ID ],
-	];
-
 	private string $errorCode;
 	private string $errorMessage;
 	private array $errorContext;
@@ -134,7 +126,7 @@ public function __construct( string $code, string $message, array $context = []
 		$contextKeys = array_keys( $context );
 		$unexpectedContext = array_values( array_diff(
 			$contextKeys,
-			array_merge( self::EXPECTED_CONTEXT_KEYS[$code], self::ADDITIONAL_PATH_CONTEXT[$code] ?? [] )
+			self::EXPECTED_CONTEXT_KEYS[$code]
 		) );
 		if ( $unexpectedContext ) {
 			throw new LogicException( "Error context for '$code' should not contain keys: " . json_encode( $unexpectedContext ) );
@@ -238,6 +230,14 @@ public static function newPatchResultReferencedResourceNotFound( string $path, s
 		);
 	}
 
+	public static function newReferencedResourceNotFound( string $path ): self {
+		return new self(
+			self::REFERENCED_RESOURCE_NOT_FOUND,
+			'The referenced resource does not exist',
+			[ self::CONTEXT_PATH => $path ]
+		);
+	}
+
 	public function getErrorCode(): string {
 		return $this->errorCode;
 	}

diff --git a/repo/rest-api/src/RouteHandlers/ErrorResponseToHttpStatus.php b/repo/rest-api/src/RouteHandlers/ErrorResponseToHttpStatus.php
@@ -20,7 +20,7 @@ class ErrorResponseToHttpStatus {
 		UseCaseError::ITEM_STATEMENT_ID_MISMATCH => 400,
 		UseCaseError::MISSING_FIELD => 400,
 		UseCaseError::PROPERTY_STATEMENT_ID_MISMATCH => 400,
-		UseCaseError::SITELINK_TITLE_NOT_FOUND => 400,
+		UseCaseError::REFERENCED_RESOURCE_NOT_FOUND => 400,
 		UseCaseError::STATEMENT_GROUP_PROPERTY_ID_MISMATCH => 400,
 		UseCaseError::VALUE_TOO_LONG => 400,
 

diff --git a/repo/rest-api/src/RouteHandlers/openapi.json b/repo/rest-api/src/RouteHandlers/openapi.json
@@ -3866,8 +3866,8 @@
               "statement-group-property-id-mismatch": {
                 "$ref": "#/components/examples/StatementGroupPropertyIdMismatch"
               },
-              "title-does-not-exist": {
-                "$ref": "#/components/examples/TitleDoesNotExistOnGivenSiteWithSiteContextExample"
+              "referenced-resource-not-found": {
+                "$ref": "#/components/examples/ReferencedResourceNotFoundExample"
               },
               "invalid-value": {
                 "$ref": "#/components/examples/InvalidValueExample"
@@ -4325,8 +4325,8 @@
               "value-too-long": {
                 "$ref": "#/components/examples/ValueTooLongExample"
               },
-              "title-does-not-exist": {
-                "$ref": "#/components/examples/TitleDoesNotExistOnGivenSiteExample"
+              "referenced-resource-not-found": {
+                "$ref": "#/components/examples/ReferencedResourceNotFoundExample"
               }
             }
           }
@@ -6345,18 +6345,12 @@
           }
         }
       },
-      "TitleDoesNotExistOnGivenSiteExample": {
+      "ReferencedResourceNotFoundExample": {
         "value": {
-          "code": "title-does-not-exist",
-          "message": "Page with title {title} does not exist on the given site"
-        }
-      },
-      "TitleDoesNotExistOnGivenSiteWithSiteContextExample": {
-        "value": {
-          "code": "title-does-not-exist",
-          "message": "Page with title {title} does not exist on the given site",
+          "code": "referenced-resource-not-found",
+          "message": "The referenced resource does not exist",
           "context": {
-            "site": "{site_id}"
+            "path": "{json_pointer}"
           }
         }
       },

diff --git a/repo/rest-api/tests/mocha/api-testing/CreateItemTest.js b/repo/rest-api/tests/mocha/api-testing/CreateItemTest.js
@@ -661,11 +661,9 @@ describe( newCreateItemRequestBuilder().getRouteDescription(), () => {
 				sitelinks: { [ localWikiId ]: { title } }
 			} ).makeRequest();
 
-			assertValidError( response, 400, 'title-does-not-exist', { site_id: localWikiId } );
-			assert.strictEqual(
-				response.body.message,
-				`Page with title ${title} does not exist on the given site`
-			);
+			const context = { path: `/item/sitelinks/${localWikiId}/title` };
+			assertValidError( response, 400, 'referenced-resource-not-found', context );
+			assert.strictEqual( response.body.message, 'The referenced resource does not exist' );
 		} );
 	} );
 

diff --git a/repo/rest-api/tests/mocha/api-testing/SetSitelinkTest.js b/repo/rest-api/tests/mocha/api-testing/SetSitelinkTest.js
@@ -374,11 +374,8 @@ describe( newSetSitelinkRequestBuilder().getRouteDescription(), () => {
 			const sitelink = { title: utils.title( 'does-not-exist-' ) };
 			const response = await newSetSitelinkRequestBuilder( testItemId, siteId, sitelink ).makeRequest();
 
-			assertValidError( response, 400, 'title-does-not-exist' );
-			assert.strictEqual(
-				response.body.message,
-				`Page with title ${sitelink.title} does not exist on the given site`
-			);
+			assertValidError( response, 400, 'referenced-resource-not-found', { path: '/sitelink/title' } );
+			assert.strictEqual( response.body.message, 'The referenced resource does not exist' );
 		} );
 	} );
 

diff --git a/...plication/UseCaseRequestValidation/ItemSerializationRequestValidatingDeserializerTest.php b/...plication/UseCaseRequestValidation/ItemSerializationRequestValidatingDeserializerTest.php
@@ -555,11 +555,7 @@ public static function sitelinksValidationErrorProvider(): Generator {
 			new ValidationError( SitelinkValidator::CODE_TITLE_NOT_FOUND, [
 				SitelinkValidator::CONTEXT_SITE_ID => $site,
 			] ),
-			new UseCaseError(
-				UseCaseError::SITELINK_TITLE_NOT_FOUND,
-				'Page with title Whatever does not exist on the given site',
-				[ UseCaseError::CONTEXT_SITE_ID => $site ]
-			),
+			UseCaseError::newReferencedResourceNotFound( "/item/sitelinks/$site/title" ),
 			[ 'sitelinks' => [ $site => [ 'title' => 'Whatever' ] ] ],
 		];
 		yield SitelinkValidator::CODE_SITELINK_CONFLICT => [

diff --git a/...it/Application/UseCaseRequestValidation/SitelinkEditRequestValidatingDeserializerTest.php b/...it/Application/UseCaseRequestValidation/SitelinkEditRequestValidatingDeserializerTest.php
@@ -122,10 +122,7 @@ public function sitelinkValidationErrorProvider(): \Generator {
 		];
 
 		yield 'title not found' => [
-			new UseCaseError(
-				UseCaseError::SITELINK_TITLE_NOT_FOUND,
-				'Page with title Irgendein_titel does not exist on the given site'
-			),
+			UseCaseError::newReferencedResourceNotFound( '/sitelink/title' ),
 			new ValidationError(
 				SitelinkValidator::CODE_TITLE_NOT_FOUND,
 				[ SitelinkValidator::CONTEXT_SITE_ID => 'dewiki' ]

diff --git a/repo/rest-api/tests/phpunit/Application/UseCases/UseCaseErrorTest.php b/repo/rest-api/tests/phpunit/Application/UseCases/UseCaseErrorTest.php
@@ -39,17 +39,6 @@ public function provideValidUseCaseErrorData(): Generator {
 			"Invalid path parameter: 'property_id'",
 			[ UseCaseError::CONTEXT_PARAMETER => 'property_id' ],
 		];
-
-		yield 'valid error with additional path context' => [
-			UseCaseError::SITELINK_TITLE_NOT_FOUND,
-			'Page with title Test_article does not exist on the given site',
-			[ UseCaseError::CONTEXT_SITE_ID => 'enwiki' ],
-		];
-
-		yield 'valid error without additional path context' => [
-			UseCaseError::SITELINK_TITLE_NOT_FOUND,
-			'Page with title Test_article does not exist on the given site',
-		];
 	}
 
 	/**
@@ -75,8 +64,8 @@ public function provideInvalidUseCaseErrorData(): Generator {
 		];
 
 		yield 'wrong path context field name' => [
-			UseCaseError::SITELINK_TITLE_NOT_FOUND,
-			'Page with title Test_article does not exist on the given site',
+			UseCaseError::REFERENCED_RESOURCE_NOT_FOUND,
+			'The referenced resource does not exist',
 			[ UseCaseError::CONTEXT_TITLE => 'Test_article' ],
 		];
 	}