Skip to content

Commit

Permalink
REST: Use resource-not-found instead of statement-not-found
Browse files Browse the repository at this point in the history
Bug: T373881
Change-Id: I8221b44c29a812e514ca2a2c92780a0c11260ffa
  • Loading branch information
jakobw committed Sep 5, 2024
1 parent d7b4786 commit 9d8e034
Show file tree
Hide file tree
Showing 36 changed files with 137 additions and 186 deletions.
6 changes: 0 additions & 6 deletions repo/rest-api/specs/global/examples.json
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,6 @@
}
}
},
"StatementNotFoundExample": {
"value": {
"code": "statement-not-found",
"message": "Could not find a statement with the ID: {statement_id}"
}
},
"ResourceNotFoundExample": {
"value": {
"code": "resource-not-found",
Expand Down
16 changes: 8 additions & 8 deletions repo/rest-api/specs/global/responses.json
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,14 @@
}
}
},
"StatementNotFound": {
"description": "The specified Statement was not found",
"ResourceNotFound": {
"description": "The specified resource was not found",
"content": {
"application/json": {
"schema": { "$ref": "./response-parts.json#/Error" },
"examples": {
"statement-not-found": {
"$ref": "./examples.json#/StatementNotFoundExample"
"resource-not-found": {
"$ref": "./examples.json#/ResourceNotFoundExample"
}
}
}
Expand All @@ -181,8 +181,8 @@
"item-not-found": {
"$ref": "./examples.json#/ItemNotFoundExample"
},
"statement-not-found": {
"$ref": "./examples.json#/StatementNotFoundExample"
"resource-not-found": {
"$ref": "./examples.json#/ResourceNotFoundExample"
}
}
}
Expand All @@ -205,8 +205,8 @@
"property-not-found": {
"$ref": "./examples.json#/PropertyNotFoundExample"
},
"statement-not-found": {
"$ref": "./examples.json#/StatementNotFoundExample"
"resource-not-found": {
"$ref": "./examples.json#/ResourceNotFoundExample"
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions repo/rest-api/specs/resources/statements/single.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"200": { "$ref": "../../global/responses.json#/Statement" },
"304": { "$ref": "../../global/responses.json#/NotModified" },
"400": { "$ref": "../../global/responses.json#/InvalidRetrieveStatementInput" },
"404": { "$ref": "../../global/responses.json#/StatementNotFound" },
"404": { "$ref": "../../global/responses.json#/ResourceNotFound" },
"412": { "$ref": "../../global/responses.json#/PreconditionFailedError" },
"500": { "$ref": "../../global/responses.json#/UnexpectedError" }
}
Expand All @@ -38,7 +38,7 @@
"responses": {
"200": { "$ref": "../../global/responses.json#/Statement" },
"400": { "$ref": "../../global/responses.json#/InvalidReplaceStatementInput" },
"404": { "$ref": "../../global/responses.json#/StatementNotFound" },
"404": { "$ref": "../../global/responses.json#/ResourceNotFound" },
"412": { "$ref": "../../global/responses.json#/PreconditionFailedError" },
"500": { "$ref": "../../global/responses.json#/UnexpectedError" }
}
Expand All @@ -58,7 +58,7 @@
"responses": {
"200": { "$ref": "../../global/responses.json#/Statement" },
"400": { "$ref": "../../global/responses.json#/InvalidPatch" },
"404": { "$ref": "../../global/responses.json#/StatementNotFound" },
"404": { "$ref": "../../global/responses.json#/ResourceNotFound" },
"409": { "$ref": "../../global/responses.json#/CannotApplyStatementPatch" },
"412": { "$ref": "../../global/responses.json#/PreconditionFailedError" },
"422": { "$ref": "../../global/responses.json#/InvalidPatchedStatement" },
Expand All @@ -80,7 +80,7 @@
"responses": {
"200": { "$ref": "../../global/responses.json#/StatementDeleted" },
"400": { "$ref": "../../global/responses.json#/InvalidRemoveStatementInput" },
"404": { "$ref": "../../global/responses.json#/StatementNotFound" },
"404": { "$ref": "../../global/responses.json#/ResourceNotFound" },
"412": { "$ref": "../../global/responses.json#/PreconditionFailedError" },
"500": { "$ref": "../../global/responses.json#/UnexpectedError" }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@ public function execute( StatementGuid $statementId ): array {
$metaDataResult = $this->metadataRetriever->getLatestRevisionMetadata( $statementId );

if ( !$metaDataResult->subjectExists() ) {
throw new UseCaseError(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
throw UseCaseError::newResourceNotFound( 'statement' );
}

if ( $metaDataResult->isRedirect() ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ public function execute( GetStatementRequest $request ): GetStatementResponse {

$statement = $this->statementRetriever->getStatement( $statementId );
if ( !$statement ) {
throw new UseCaseError(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
throw UseCaseError::newResourceNotFound( 'statement' );
}

return new GetStatementResponse( $statement, $lastModified, $revisionId );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,7 @@ public function execute( PatchStatementRequest $request ): PatchStatementRespons
$statementToPatch = $this->statementRetriever->getStatement( $statementId );

if ( !$statementToPatch ) {
throw new UseCaseError(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
throw UseCaseError::newResourceNotFound( 'statement' );
}

$this->assertUserIsAuthorized->checkEditPermissions( $statementId->getEntityId(), $editMetadata->getUser() );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ public function execute( RemoveStatementRequest $request ): void {

$statementToRemove = $this->statementRetriever->getStatementWriteModel( $deserializedRequest->getStatementId() );
if ( !$statementToRemove ) {
throw new UseCaseError(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: {$deserializedRequest->getStatementId()}"
);
throw UseCaseError::newResourceNotFound( 'statement' );
}

$editMetadata = new EditMetadata(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,7 @@ public function execute( ReplaceStatementRequest $request ): ReplaceStatementRes
)
);
} catch ( StatementNotFoundException $e ) {
throw new UseCaseError(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
throw UseCaseError::newResourceNotFound( 'statement' );
} catch ( PropertyChangedException $e ) {
throw new UseCaseError(
UseCaseError::CANNOT_MODIFY_READ_ONLY_VALUE,
Expand Down
3 changes: 0 additions & 3 deletions repo/rest-api/src/Application/UseCases/UseCaseError.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,10 @@ class UseCaseError extends UseCaseException {
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 STATEMENT_NOT_FOUND = 'statement-not-found';
public const UNEXPECTED_ERROR = 'unexpected-error';
public const VALUE_TOO_LONG = 'value-too-long';

public const CONTEXT_ACTUAL_VALUE = 'actual_value';
public const CONTEXT_ALIAS = 'alias';
public const CONTEXT_CONFLICTING_ITEM_ID = 'conflicting_item_id';
public const CONTEXT_CONFLICTING_PROPERTY_ID = 'conflicting_property_id';
public const CONTEXT_DESCRIPTION = 'description';
Expand Down Expand Up @@ -111,7 +109,6 @@ class UseCaseError extends UseCaseException {
self::CONTEXT_STATEMENT_GROUP_PROPERTY_ID,
self::CONTEXT_STATEMENT_PROPERTY_ID,
],
self::STATEMENT_NOT_FOUND => [],
self::UNEXPECTED_ERROR => [],
self::VALUE_TOO_LONG => [ self::CONTEXT_PATH, self::CONTEXT_LIMIT ],
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class ErrorResponseToHttpStatus {
UseCaseError::ITEM_NOT_FOUND => 404,
UseCaseError::PROPERTY_NOT_FOUND => 404,
UseCaseError::RESOURCE_NOT_FOUND => 404,
UseCaseError::STATEMENT_NOT_FOUND => 404,

// 409 errors:
UseCaseError::ITEM_REDIRECTED => 409,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,8 @@ public function runUseCase( string $itemId, string $statementId ): Response {
} catch ( UseCaseError $e ) {
return $this->responseFactory->newErrorResponseFromException( $e );
} catch ( ItemRedirect $e ) {
return $this->responseFactory->newErrorResponse(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
return $this->responseFactory
->newErrorResponseFromException( UseCaseError::newResourceNotFound( 'statement' ) );
}
}

Expand Down
6 changes: 2 additions & 4 deletions repo/rest-api/src/RouteHandlers/GetStatementRouteHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,8 @@ public function runUseCase( string $statementId ): Response {
} catch ( UseCaseError $e ) {
return $this->responseFactory->newErrorResponseFromException( $e );
} catch ( ItemRedirect $e ) {
return $this->responseFactory->newErrorResponse(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
return $this->responseFactory
->newErrorResponseFromException( UseCaseError::newResourceNotFound( 'statement' ) );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ public function run( Handler $routeHandler, callable $runNext ): Response {
private function isStatementNotFoundResponse( Response $response ): bool {
$responseBody = json_decode( (string)$response->getBody(), true );
return $response->getStatusCode() === 404 &&
is_array( $responseBody ) && $responseBody['code'] === UseCaseError::STATEMENT_NOT_FOUND;
is_array( $responseBody ) &&
$responseBody['code'] === UseCaseError::RESOURCE_NOT_FOUND &&
$responseBody['context']['resource_type'] === 'statement';
}

private function getStatementSubject( ?string $subjectId, string $statementId ): ?StatementListProvidingEntity {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,8 @@ public function runUseCase( string $itemId, string $statementId ): Response {
} catch ( UseCaseError $e ) {
return $this->responseFactory->newErrorResponseFromException( $e );
} catch ( ItemRedirect $e ) {
return $this->responseFactory->newErrorResponse(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
return $this->responseFactory
->newErrorResponseFromException( UseCaseError::newResourceNotFound( 'statement' ) );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,8 @@ public function runUseCase( string $statementId ): Response {
} catch ( UseCaseError $e ) {
return $this->responseFactory->newErrorResponseFromException( $e );
} catch ( ItemRedirect $e ) {
return $this->responseFactory->newErrorResponse(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
return $this->responseFactory
->newErrorResponseFromException( UseCaseError::newResourceNotFound( 'statement' ) );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,8 @@ public function runUseCase( string $itemId, string $statementId ): Response {
} catch ( UseCaseError $exception ) {
return $this->responseFactory->newErrorResponseFromException( $exception );
} catch ( ItemRedirect $e ) {
return $this->responseFactory->newErrorResponse(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
return $this->responseFactory
->newErrorResponseFromException( UseCaseError::newResourceNotFound( 'statement' ) );
}

return $this->newSuccessHttpResponse();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,8 @@ public function runUseCase( string $statementId ): Response {
}

private function respondStatementNotFound( string $statementId ): Response {
return $this->responseFactory->newErrorResponse(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
return $this->responseFactory
->newErrorResponseFromException( UseCaseError::newResourceNotFound( 'statement' ) );
}

public function validate( Validator $restValidator ): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,8 @@ public function runUseCase( string $itemId, string $statementId ): Response {
} catch ( UseCaseError $e ) {
return $this->responseFactory->newErrorResponseFromException( $e );
} catch ( ItemRedirect $e ) {
return $this->responseFactory->newErrorResponse(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
return $this->responseFactory
->newErrorResponseFromException( UseCaseError::newResourceNotFound( 'statement' ) );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,8 @@ public function runUseCase( string $statementId ): Response {
} catch ( UseCaseError $e ) {
return $this->responseFactory->newErrorResponseFromException( $e );
} catch ( ItemRedirect $e ) {
return $this->responseFactory->newErrorResponse(
UseCaseError::STATEMENT_NOT_FOUND,
"Could not find a statement with the ID: $statementId"
);
return $this->responseFactory
->newErrorResponseFromException( UseCaseError::newResourceNotFound( 'statement' ) );
}
}

Expand Down
30 changes: 12 additions & 18 deletions repo/rest-api/src/RouteHandlers/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -2667,7 +2667,7 @@
"$ref": "#/components/responses/InvalidRetrieveStatementInput"
},
"404": {
"$ref": "#/components/responses/StatementNotFound"
"$ref": "#/components/responses/ResourceNotFound"
},
"412": {
"$ref": "#/components/responses/PreconditionFailedError"
Expand Down Expand Up @@ -2709,7 +2709,7 @@
"$ref": "#/components/responses/InvalidReplaceStatementInput"
},
"404": {
"$ref": "#/components/responses/StatementNotFound"
"$ref": "#/components/responses/ResourceNotFound"
},
"412": {
"$ref": "#/components/responses/PreconditionFailedError"
Expand Down Expand Up @@ -2751,7 +2751,7 @@
"$ref": "#/components/responses/InvalidPatch"
},
"404": {
"$ref": "#/components/responses/StatementNotFound"
"$ref": "#/components/responses/ResourceNotFound"
},
"409": {
"$ref": "#/components/responses/CannotApplyStatementPatch"
Expand Down Expand Up @@ -2799,7 +2799,7 @@
"$ref": "#/components/responses/InvalidRemoveStatementInput"
},
"404": {
"$ref": "#/components/responses/StatementNotFound"
"$ref": "#/components/responses/ResourceNotFound"
},
"412": {
"$ref": "#/components/responses/PreconditionFailedError"
Expand Down Expand Up @@ -3864,16 +3864,16 @@
}
}
},
"StatementNotFound": {
"description": "The specified Statement was not found",
"ResourceNotFound": {
"description": "The specified resource was not found",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/responses/ItemNotFound/content/application~1json/schema"
},
"examples": {
"statement-not-found": {
"$ref": "#/components/examples/StatementNotFoundExample"
"resource-not-found": {
"$ref": "#/components/examples/ResourceNotFoundExample"
}
}
}
Expand All @@ -3898,8 +3898,8 @@
"item-not-found": {
"$ref": "#/components/examples/ItemNotFoundExample"
},
"statement-not-found": {
"$ref": "#/components/examples/StatementNotFoundExample"
"resource-not-found": {
"$ref": "#/components/examples/ResourceNotFoundExample"
}
}
}
Expand All @@ -3924,8 +3924,8 @@
"property-not-found": {
"$ref": "#/components/examples/PropertyNotFoundExample"
},
"statement-not-found": {
"$ref": "#/components/examples/StatementNotFoundExample"
"resource-not-found": {
"$ref": "#/components/examples/ResourceNotFoundExample"
}
}
}
Expand Down Expand Up @@ -6698,12 +6698,6 @@
}
}
},
"StatementNotFoundExample": {
"value": {
"code": "statement-not-found",
"message": "Could not find a statement with the ID: {statement_id}"
}
},
"ResourceNotFoundExample": {
"value": {
"code": "resource-not-found",
Expand Down
Loading

0 comments on commit 9d8e034

Please sign in to comment.