Problem/Motivation
_subscription_manager_sync_remote_subscriptions() exists to self-heal the case where a remote subscription contract exists but the local subscription entity is missing (for example a webhook that never fired). It runs on hook_user_insert() and when cron drains the subscription_manager_remote_sync queue populated at login.
The function only ever queries the connector named in default_connector. On a site with more than one connector, a missed webhook on any non-default connector is never reconciled: the user's remote contract is active, but no local entity is created, no roles are granted, and every login re-enqueues a sync job that checks the wrong service. This contradicts the multi-connector design of the rest of the module, where syncPlans() already iterates all installed connector definitions.
Steps to reproduce
- Install connectors A and B; set
default_connectorto A. - Create a remote subscription on connector B for an email address with no local subscription entity (simulating a missed webhook).
- Register or log in as that user and run cron.
- No local subscription is created; connector B was never consulted.
Proposed resolution
Iterate all installed connector definitions, mirroring SubscriptionManagerService::syncPlans():
- Keep the cheap local short-circuit (an active local subscription skips all remote calls).
- For each installed connector, call
loadRemoteSubscriptionsByUser()and sync any results viasyncRemoteSubscriptionToLocal(), wrapping each connector in its own try/catch so one provider's outage doesn't block reconciliation against the others. - Optionally stop after the first connector that yields results. This short-circuit is only valid if the single-active-subscription invariant from #3616778: Define and enforce single-active-subscription semantics (loadLocalSubscription() returns an arbitrary first match) is adopted (its Option A); if concurrent subscriptions are allowed there, every connector must always be consulted.
Remaining tasks
- Sequence against #3616880: Move _subscription_manager_sync_remote_subscriptions() into SubscriptionManagerService and convert the cron queue drain to a QueueWorker plugin, which moves this routine out of the
.modulefile intoSubscriptionManagerService::syncRemoteSubscriptions()with injected dependencies and a QueueWorker plugin. Landing that refactor first gives this fix a clean, testable home; alternatively both can be done in one branch there. - Patch, plus a kernel test with two test connectors (one returning a remote subscription, one not; assert both are consulted and the entity is created with the correct
connector_plugin_id). - Change record if the stop-after-first behavior is adopted.
User interface changes
None.
API changes
None if implemented in the internal helper. If #3616880: Move _subscription_manager_sync_remote_subscriptions() into SubscriptionManagerService and convert the cron queue drain to a QueueWorker plugin lands first, the change lands in the public SubscriptionManagerService::syncRemoteSubscriptions() method instead, still with no signature change.
Data model changes
None.
Issue fork subscription_manager-3616770
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
colanComment #3
colanComment #4
colanComment #6
colanMR opened, implementing the proposed resolution on top of the #3616880: Move _subscription_manager_sync_remote_subscriptions() into SubscriptionManagerService and convert the cron queue drain to a QueueWorker plugin refactor (the change lands in
SubscriptionManagerService::syncRemoteSubscriptions(), no signature change):hook_user_insert()stays fail-open.MultiConnectorSyncTest): the reproduce case — a contract on a NON-default connector is now healed with the correctconnector_plugin_idrecorded, with both connectors provably consulted (RED before the fix); iteration stops at the first yielding connector; an outage on the default connector does not block healing via another; and a failure with nothing synced rethrows after still consulting the remaining connectors. The test connectors gained a consultation counter and a simulated-outage flag to drive these.Change record for the stop-after-first behavior drafted alongside, as the summary requested.
Comment #8
colan