task: #3629746: Move the Graph API client to the base module and add a typed API exception

Orkut Murat Yılmazrequested to merge
issue/mapillary-3629746:3629746-move-the-graph into 1.0.x

Closes #3629746

What this does

  • Moves MapillaryApiClient, MapillaryApiCallEvent and MissingAccessTokenException from mapillary_field to the base module. The client is now the mapillary.api_client service.
  • Adds MapillaryApiException (extends \RuntimeException), thrown for every failed request. It carries the HTTP status, the Graph API error code and Retry-After, and answers isNotFound(), isBadId() and isServiceWide(). MissingAccessTokenException extends it.
  • Adds typed methods that check the ID format before the ID becomes part of the path: getImage(), getImages(), getSequenceImageIds(), getMapFeature(), getDetections(). An input such as me or 123/detections is rejected without a request. get() stays for other paths.
  • Adds checkToken(). ApiSettingsForm uses it instead of building the request itself.
  • Dispatches MapillaryApiCallEvent once, in a finally block, also when the token is missing.
  • The client no longer logs. The callers that run unattended (queue workers, thumbnail refresher, Drush) already log the failures they handle, the forms and widgets show them, and Mapillary Logs records each failed request. A failed request was logged up to three times before.
  • MapillaryThumbnailRefresher uses the exception methods instead of unwrapping Guzzle exceptions.

Backwards compatibility

  • mapillary_field.api_client is a deprecated alias of mapillary.api_client.
  • The old class names in mapillary_field keep working, also in catch blocks and type hints. Neither of these triggers the autoloader, so each new class file registers the old name with class_alias(). The old files only trigger a deprecation and load the new class, for code that uses an old name first.
  • The constructor still accepts the old signature (factory, config, logger, dispatcher), with a deprecation, so code that builds the client by hand and a container cached before drush updb keep working.
  • MapillaryThumbnailRefresher::isServiceWideFailure() is deprecated in favour of MapillaryApiException::isServiceWide().
  • The event name mapillary_field.api_call does not change.
  • An empty post_update rebuilds the container. No schema or config change.

All deprecations are removed in 1.0.0-beta1.

Also in this MR

  • The ID patterns use the D modifier, so "123\n" is rejected too.
  • PHPStan level 6 warnings in the touched importer files are fixed (ContentEntityInterface return types).
  • CHANGELOG and AGENTS.md are updated.

Testing

  • Unit tests for the client and the exception: every status and Graph API error code, Retry-After in seconds and as an HTTP date, invalid IDs sending no request, the event dispatched once on every path including a missing token, the old constructor signature.
  • Legacy tests for the old class names (catch, type hints, old name used first) and the old service ID.
  • Kernel tests: a failed request reaches the system log once; a missing token is logged by Mapillary Logs; the image and sequence import forms send no request for me, 123/detections or abc/def.
  • The full PHPUnit suites of mapillary, mapillary_field, mapillary_logs, mapillary_detection, mapillary_map_feature, mapillary_leaflet (without FunctionalJavascript) and mapillary_entity pass locally on PostgreSQL.
  • Manual check against the live Graph API: an unknown ID gives HTTP 400 with code 100 (isBadId()), an invalid token gives HTTP 401 with code 190 (isServiceWide()).

Follow-ups

  • Moving the image and sequence importer to mapillary_entity. After that, mapillary_detection and mapillary_map_feature no longer need mapillary_field.
  • Queue and cascade failure policy, which builds on isBadId() and isServiceWide().

Merge request reports