Page MenuHomePhabricator

🌶️ Change elements of an item statement
Closed, ResolvedPublic13 Estimated Story Points

Description

As a tool developer I want to change elements on the statement so that my tool can improve data in Wikibase instance.
As a gadget developer I want to change parts of the statement so that users of the gadget can correct mistakes on Wikidata

PATCH /entities/items/{item_id}/statements/{statement_id}
PATCH /statements/{statement_id}

See also:

Success case:

  • status code 200 and the new statement state
  • ETag header with the last revision ID
  • Last-Modified header

Error cases:

  • If the statement with the request ID does not exist, API would respond with 404 (error code and message as in POST responses)
  • If the subject item of the statement is a redirect, API would also respond with 404 (error code and message as in POST responses)
  • If applying the requested patch results in a change to the Statement ID, the API should respond with 400 (error code: invalid-operation-change-statement-id, error message: Cannot change the ID of the existing statement)
  • If applying the requested patch results in a change to the mainsnak.property field, the API should respond with 400 (error code: invalid-operation-change-property-of-statement, error message: Cannot change the property of the existing statement) unless the provided property ID matches the the existing statement's property
  • if the provided JSON Patch input is invalid (e.g. not a valid JSON, uses unrecognized operation names) the API should respond with 400 (error code invalid-patch, error message: The provided patch is invalid)
  • if the provided patch cannot be applied to the given statement (e.g. paths referred do not exist on a given statement) the API should respond with 409 (error code cannot-apply-patch, error message The provided patch cannot be applied to the statement {statement_id})
    • if a test operation defined in the provided patch fails the API should respond with 409 (error code patch-test-failed, error message Test operation in the patch provided failed)
  • if applying the provided patch results in an invalid statement (i.e. a statement that cannot be persisted, e.g. changes the value to an incorrect type not matching the property data type any more) the API should respond with 422 (error code: patched-statement-invalid, error message: The patch results in an invalid statement which cannot be stored)

Other remarks:

  • If applying the requested patch results in fields that are not "valid" parts of the statement data, or are not expected to be specified by the client (except guid and property, see above) those are to be silently ignored, i.e. they are not stored by the API, and the API does not respond with error response to such requests

All editing REST API endpoints are meant to include a consistent "metadata" optional parameters

  • mediawiki "metadata" can be specified:
    • "comment" to be included in the edit summary (excluding the "automatic summary" part for now)
    • mediawiki tag(s) for the edit
    • flag edit as made by a bot
  • to help Wikibase solve potential edit conflict ("lost update problem"), the latest revision of an item known by a client when making the request can also be provided using If-Match HTTP header. In case the ETag provided is not the most recent version if the item, the API would response in 412 Precondition Failed response code.
    • As a timestamp-based counterpart, also use If-Unmodified-Since header with a respective logic.
    • 412 response should not include any extra text in response body. The response should also not include ETag and Last-Modified header

Related Objects

StatusSubtypeAssignedTask
ResolvedIfrahkhanyaree_WMDE
ResolvedWMDE-leszek
ResolvedJakob_WMDE
ResolvedOllie.Shotton_WMDE
ResolvedJakob_WMDE
ResolvedSilvan_WMDE
ResolvedSilvan_WMDE
ResolvedSilvan_WMDE
ResolvedOllie.Shotton_WMDE
ResolvedWMDE-leszek
ResolvedSilvan_WMDE
ResolvedSilvan_WMDE
ResolvedJakob_WMDE
ResolvedJakob_WMDE
ResolvedJakob_WMDE
ResolvedJakob_WMDE
ResolvedJakob_WMDE

Event Timeline

Notes from Backlog Refinement:

Investigation steps:

  • investigate current status of JSON-PATCH (is it still state-of-the-art?)
  • investigate PHP tooling support for JSON-PATCH (compared to MERGE-PATCH)
Silvan_WMDE set the point value for this task to 13.
Silvan_WMDE renamed this task from Change elements of an item statement to 🌶️ Change elements of an item statement.Aug 25 2022, 1:43 PM

Notes from task breakdown:

Sub task: T316315: 🌶️ Add PATCH statement endpoints to OpenAPI document

Sub task: T316241: 🌶️ Create a JsonPatchValidator

  • use the swaggest/json-diff library to load the patch document and create validation errors in case it is invalid

Sub task: T316317: 🌶️ Create a StatementPatcher

  • takes as input a validated patch (array of patch operations) plus the statement to be patched
  • returns the patched statement (happy path) or throws exceptions for:
    • InapplicablePatch
    • PatchTestConditionFailed
    • InvalidPatchedSerialization
    • InvalidPatchedStatement

Sub task: T316316: 🌶️ Modify StatementPatcher to throw Exception in case of invalid patch result

  • takes as an input the patched statement
  • returns null in case of success or a validation error in case of any mismatch between a property data type and a snak value type
  • this needs to be extracted from the existing SnakValidatorStatementValidator

Sub task: T316242: 🌶️ Implement the PatchItemStatement use case happy path

  • create a new PatchItemStatement use case, including PatchItemStatementRequest
  • retrieve Item
  • get statement from Item
  • use the newly created StatementPatcher to applay patch
  • replace the statement
  • save using ItemUpdater
  • create a PatchItemStatementSuccessResponse and return it

Sub task: T316320: 🌶️ Create PatchStatementRouteHandler (long route) and implement happy path

  • middlewares: auth, unexpected errors, conditional requests

Sub task: T316318: 🌶️ Create PatchStatementRouteHandler (short route) and implement happy path

  • middlewares: auth, unexpected errors, conditional requests

Sub task: T316243: 🌶️ Add request validation to PatchItemStatement use case

  • validate:
    • item id if present,
    • statement id,
    • metadata,
    • the patch, using newly created JsonPatchValidator
  • use case converts validation error into PatchItemStatementErrorResponse
  • RouteHandler turns use case error into http error response

Sub tasks: T316321: 🌶️ Handle statement not found and statement subject redirect errors

  • add retrieval of Item metadata to the use case
  • use case returns PatchItemStatementErrorResponse
  • RouteHandler turns use case error into http error response

Sub task: T316319: 🌶️ Handle errors caused by patching

  • handle patcher exceptions
  • use StatementSnaksValueTypeValidator to validate the patched statement
  • handle exceptions thrown by StatementList.replace()
  • create PatchItemStatementErrorResponse in case of failure
  • RouteHandler turns use case error into http error response

Sub tasks: T316244: 🌶️ Add OpenAPI schema tests for PatchItemStatement routes

Verified, works great, thank you!
Some further work on error reporting and improving examples is certainly going to be useful but in some other story.

Scenarios verified

  • replacing value: string, item
  • adding, removing a reference
  • adding, removing a qualifier
  • changes resulting in an incomplete, or incorrect statement - no changes saved
  • patch cannot be applied on a particular statement
  • incorrect input: invalid (syntatically) patch, statement ID, item ID, item redirected
  • changes conditionally allowed with If-Match, If-Unmodified-Since headers
  • cannot change ID or property of a statement

JSON patch operations tried out: replace

  • add (incl. array elements)
  • remove (incl. array elements)
  • replace
  • copy
  • move
  • test
WMDE-leszek claimed this task.