diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 98a84ed2a..672b1cdec 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -20,7 +20,6 @@ namespace OCA\Stackiq\AppInfo; -use OCA\Decidesk\Event\DecisionConcludedEvent; use OCA\OpenRegister\Contract\ObjectServiceInterface; use OCA\OpenRegister\Event\ObjectCreatedEvent; use OCA\OpenRegister\Event\ObjectUpdatedEvent; @@ -803,13 +802,31 @@ private function registerEventListeners(IRegistrationContext $context): void { // Sync user profile updates into the contactpersoon mirror. $context->registerEventListener(UserProfileUpdatedEvent::class, UserProfileUpdatedEventListener::class); - // Project a concluded decidesk contract-approval Decision onto the - // catalog contract. Only fires when decidesk is installed (it owns the - // DecisionConcludedEvent class); the listener filters by sourceApp and + // Project a concluded contract-approval Decision from the decision app + // onto the catalog contract. The listener filters by sourceApp and // IDOR-checks the decision id before projecting (the In onderhandeling // -> Actief transition is reached only here). Replaces the former HTTP // outcome-callback + daily reconcile poll. - $context->registerEventListener(DecisionConcludedEvent::class, DecisionConcludedListener::class); + // + // BOTH SPELLINGS, by FQN STRING rather than `::class`. That app renamed + // its PSR-4 root from OCA\Decidesk to OCA\Decidiq with no compatibility + // alias, and `::class` on an imported name resolves at COMPILE TIME — so + // this registered a class nothing dispatches any more, the listener + // never fired, and every approved contract stayed in `In onderhandeling`. + // An event with no listener and a listener on no event look identical + // from here: nothing throws, nothing is logged. + // + // Registering a name that does not resolve is harmless, because dispatch + // matches on the concrete event class, but the guard is kept so this + // stays symmetric with ContractApprovalService::isDelegationConfigured() + // on the outbound side. + foreach (ContractApprovalService::DECISION_CONCLUDED_EVENTS as $concludedEvent) { + if (class_exists($concludedEvent) === false) { + continue; + } + + $context->registerEventListener($concludedEvent, DecisionConcludedListener::class); + } }//end registerEventListeners() diff --git a/lib/EventListener/DecisionConcludedListener.php b/lib/EventListener/DecisionConcludedListener.php index 682747bed..9edf775bd 100644 --- a/lib/EventListener/DecisionConcludedListener.php +++ b/lib/EventListener/DecisionConcludedListener.php @@ -30,7 +30,8 @@ namespace OCA\Stackiq\EventListener; -use OCA\Decidesk\Event\DecisionConcludedEvent; +use OCA\Decidesk\Event\DecisionConcludedEvent as DecideskDecisionConcludedEvent; +use OCA\Decidiq\Event\DecisionConcludedEvent as DecidiqDecisionConcludedEvent; use OCA\Stackiq\Service\ContractApprovalService; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; @@ -71,7 +72,19 @@ public function __construct( * @spec openspec/specs/contract-decision-delegation/spec.md */ public function handle(Event $event): void { - if (($event instanceof DecisionConcludedEvent) === false) { + // BOTH SPELLINGS. The decision app renamed its PSR-4 root from + // OCA\Decidesk to OCA\Decidiq with no compatibility alias, so an + // `instanceof` against one name silently rejects the other app's real + // event and this method returns as if the event were somebody else's. + // The two classes publish an identical getter surface (verified against + // decidiq development d72839c), so everything below is unchanged. + // + // `instanceof` against a class that is not installed is simply false — + // it neither autoloads nor errors — which is why naming both here costs + // nothing on an instance that runs only one of them. + if (($event instanceof DecidiqDecisionConcludedEvent) === false + && ($event instanceof DecideskDecisionConcludedEvent) === false + ) { return; } diff --git a/lib/Service/ContractApprovalService.php b/lib/Service/ContractApprovalService.php index 6407130ad..875914e3e 100644 --- a/lib/Service/ContractApprovalService.php +++ b/lib/Service/ContractApprovalService.php @@ -76,6 +76,33 @@ class ContractApprovalService { '\\OCA\\Decidesk\\Event\\DecisionRequestedEvent', ]; + /** + * The fully-qualified conclusion-event class spellings, NEWEST FIRST — what + * {@see \OCA\Stackiq\AppInfo\Application} attaches the inbound listener to. + * + * THE INBOUND HALF OF THE SAME PROBLEM AS THE CONSTANT ABOVE, and it stayed + * broken after that one was fixed. Application.php imported + * `OCA\Decidesk\Event\DecisionConcludedEvent` and registered `::class`, + * which resolves at COMPILE TIME to a string nothing dispatches any more, so + * the listener attached to a name that never fires. Nothing errors: an + * event with no listener and a listener on no event look identical from + * here, and the contract simply never leaves `In onderhandeling`. + * + * Measured 2026-09-09 against decidiq development d72839c: + * OCA\Decidiq\Event\DecisionConcludedEvent EXISTS with the full getter + * surface this app reads (getSourceApp, getSubjectId, getExternalReference, + * getDecisionId, getStatus); OCA\Decidesk\Event\DecisionConcludedEvent is + * MISSING. The old spelling is kept regardless — an instance pinned to a + * release from before that rename still dispatches it, and dropping it here + * re-breaks the integration in the other direction. + * + * @var array + */ + public const DECISION_CONCLUDED_EVENTS = [ + '\\OCA\\Decidiq\\Event\\DecisionConcludedEvent', + '\\OCA\\Decidesk\\Event\\DecisionConcludedEvent', + ]; + /** * This consumer app id, stamped on the request event as `sourceApp` and * used by the conclusion listener to filter inbound events. diff --git a/phpstan.neon b/phpstan.neon index 0cebad817..804b28f31 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -22,6 +22,10 @@ parameters: # class` errors that a bare ignore pattern cannot fix. Analysis-only; # never loaded at runtime or by PHPUnit. - tests/analysis-stubs/decidesk-events.stub.php + # The SAME contract under the namespace that app renamed to. Both + # spellings are in the field, and without this one the analyser proves + # the newer half of the inbound guard dead. + - tests/analysis-stubs/decidiq-events.stub.php ignoreErrors: # OrganizationSyncService's `if ($contactObject !== null)` at the top of diff --git a/psalm.xml b/psalm.xml index 60e83f60c..bd3c1db1f 100644 --- a/psalm.xml +++ b/psalm.xml @@ -40,6 +40,7 @@ what also resolves the isHandled()/getDecisionId() calls on the dispatched event. Analysis-only; never loaded at runtime. --> + diff --git a/tests/Unit/EventListener/DecisionConcludedListenerNamespaceTest.php b/tests/Unit/EventListener/DecisionConcludedListenerNamespaceTest.php new file mode 100644 index 000000000..312e626ef --- /dev/null +++ b/tests/Unit/EventListener/DecisionConcludedListenerNamespaceTest.php @@ -0,0 +1,114 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Tests\Unit\EventListener; + +use OCA\Stackiq\EventListener\DecisionConcludedListener; +use OCA\Stackiq\Service\ContractApprovalService; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; + +require_once __DIR__ . '/decision-conclusion-event-doubles.php'; + +/** + * A cross-app event class name is a runtime lookup this app can only follow. + * + * The decision app renamed its PSR-4 root from `OCA\Decidesk` to `OCA\Decidiq` + * with no compatibility alias. `handle()` tested `instanceof` against the old + * spelling only, so the real event arrived and was rejected as somebody else's: + * the contract stayed in `In onderhandeling`, nothing threw, and nothing was + * logged. An event with no listener looks exactly like a listener on no event. + * + * Both spellings are asserted, because pinning either one alone reproduces the + * outage on the half of the fleet running the other. + */ +class DecisionConcludedListenerNamespaceTest extends TestCase { + /** + * The listener projects an outcome carried by either spelling of the event. + * + * @return void + */ + public function testTheOutcomeIsProjectedUnderEitherFleetNamespace(): void { + $spellings = [ + 'OCA\Decidiq\Event\DecisionConcludedEvent', + 'OCA\Decidesk\Event\DecisionConcludedEvent', + ]; + + foreach ($spellings as $fqcn) { + $projected = []; + + $approvalService = $this->createMock(ContractApprovalService::class); + $approvalService->method('resolveContractForOutcome')->willReturn('contract-1'); + $approvalService->method('projectOutcome')->willReturnCallback( + static function (string $contractUuid, string $outcomeStatus) use (&$projected): void { + $projected[] = [$contractUuid, $outcomeStatus]; + } + ); + + $listener = new DecisionConcludedListener($approvalService, $this->createMock(LoggerInterface::class)); + $listener->handle(new $fqcn('decision-1', 'approved', ContractApprovalService::SOURCE_APP, 'subject-1', 'ext-1')); + + $this->assertSame( + [['contract-1', 'approved']], + $projected, + 'the listener ignored a real conclusion event dispatched as ' . $fqcn + ); + } + }//end testTheOutcomeIsProjectedUnderEitherFleetNamespace() + + /** + * An event raised by a different consumer app is still ignored. + * + * Widening the accepted class names must not widen the sourceApp filter: + * that filter is what stops this app projecting another consumer's decision + * onto its own contracts. + * + * @return void + */ + public function testAnotherConsumersDecisionIsStillIgnored(): void { + $approvalService = $this->createMock(ContractApprovalService::class); + $approvalService->expects($this->never())->method('resolveContractForOutcome'); + + $listener = new DecisionConcludedListener($approvalService, $this->createMock(LoggerInterface::class)); + $listener->handle( + new \OCA\Decidiq\Event\DecisionConcludedEvent('decision-1', 'approved', 'dossiq', 'subject-1', 'ext-1') + ); + + }//end testAnotherConsumersDecisionIsStillIgnored() + + /** + * The registration list carries both spellings, newest first. + * + * Written out rather than read from the constant under test: iterating the + * same list the assertion checks is the shape that cannot fail. + * + * @return void + */ + public function testTheRegistrationListCarriesBothSpellingsNewestFirst(): void { + $this->assertSame( + [ + '\OCA\Decidiq\Event\DecisionConcludedEvent', + '\OCA\Decidesk\Event\DecisionConcludedEvent', + ], + ContractApprovalService::DECISION_CONCLUDED_EVENTS, + 'order is the contract: the current namespace first, the pre-rename one retained' + ); + + }//end testTheRegistrationListCarriesBothSpellingsNewestFirst() +}//end class diff --git a/tests/Unit/EventListener/decision-conclusion-event-doubles.php b/tests/Unit/EventListener/decision-conclusion-event-doubles.php new file mode 100644 index 000000000..123698f41 --- /dev/null +++ b/tests/Unit/EventListener/decision-conclusion-event-doubles.php @@ -0,0 +1,157 @@ + + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace { + // The base class both doubles extend, and the interface the listener under + // test implements. `nextcloud/ocp` ships it as a real, + // self-contained class (its only dependency is the PSR + // StoppableEventInterface, which composer does autoload) but declares NO + // autoload section, so in pure-unit mode — a checkout outside an installed + // Nextcloud root, which is how this suite runs on a developer machine and + // how tests/bootstrap.php deliberately arranges it — nothing resolves it. + // + // Without this the listener cannot be called at all: `handle(Event $event)` + // resolves its parameter type at call time. Skipping instead would make the + // one test that proves the fix unable to fail locally, which is the same as + // not having it. + // + // Guarded: when a real Nextcloud root has booted, the real class is already + // loaded and this does nothing. + $ocpDir = __DIR__ . '/../../../vendor/nextcloud/ocp/OCP/EventDispatcher/'; + if (interface_exists(\OCP\EventDispatcher\IEventListener::class, false) === false + && file_exists($ocpDir . 'IEventListener.php') === true + ) { + require_once $ocpDir . 'IEventListener.php'; + } + + if (class_exists(\OCP\EventDispatcher\Event::class, false) === false + && file_exists($ocpDir . 'Event.php') === true + ) { + require_once $ocpDir . 'Event.php'; + } +} + +namespace OCA\Decidiq\Event { + if (class_exists(DecisionConcludedEvent::class, false) === false) { + /** + * Runtime double for decidiq's DecisionConcludedEvent. + */ + class DecisionConcludedEvent extends \OCP\EventDispatcher\Event { + /** + * Constructor. + * + * @param string $decisionId The concluded Decision id. + * @param string $status The derived outcome status. + * @param string $sourceApp The consumer app that raised the decision. + * @param string|null $subjectId The originating object id. + * @param string $externalReference The consumer's own reference. + */ + public function __construct( + private readonly string $decisionId, + private readonly string $status, + private readonly string $sourceApp, + private readonly ?string $subjectId = null, + private readonly string $externalReference = '', + ) { + parent::__construct(); + } + + /** @return string The decision id. */ + public function getDecisionId(): string { + return $this->decisionId; + } + + /** @return string The status. */ + public function getStatus(): string { + return $this->status; + } + + /** @return string The source app. */ + public function getSourceApp(): string { + return $this->sourceApp; + } + + /** @return string|null The subject id. */ + public function getSubjectId(): ?string { + return $this->subjectId; + } + + /** @return string The external reference. */ + public function getExternalReference(): string { + return $this->externalReference; + } + } + } +} + +namespace OCA\Decidesk\Event { + if (class_exists(DecisionConcludedEvent::class, false) === false) { + /** + * Runtime double for the pre-rename spelling of the same event. + */ + class DecisionConcludedEvent extends \OCP\EventDispatcher\Event { + /** + * Constructor. + * + * @param string $decisionId The concluded Decision id. + * @param string $status The derived outcome status. + * @param string $sourceApp The consumer app that raised the decision. + * @param string|null $subjectId The originating object id. + * @param string $externalReference The consumer's own reference. + */ + public function __construct( + private readonly string $decisionId, + private readonly string $status, + private readonly string $sourceApp, + private readonly ?string $subjectId = null, + private readonly string $externalReference = '', + ) { + parent::__construct(); + } + + /** @return string The decision id. */ + public function getDecisionId(): string { + return $this->decisionId; + } + + /** @return string The status. */ + public function getStatus(): string { + return $this->status; + } + + /** @return string The source app. */ + public function getSourceApp(): string { + return $this->sourceApp; + } + + /** @return string|null The subject id. */ + public function getSubjectId(): ?string { + return $this->subjectId; + } + + /** @return string The external reference. */ + public function getExternalReference(): string { + return $this->externalReference; + } + } + } +} diff --git a/tests/analysis-stubs/decidiq-events.stub.php b/tests/analysis-stubs/decidiq-events.stub.php new file mode 100644 index 000000000..205c370db --- /dev/null +++ b/tests/analysis-stubs/decidiq-events.stub.php @@ -0,0 +1,221 @@ +`, and NEVER loaded at runtime or during PHPUnit. + * + * The sibling of decidesk-events.stub.php, and the reason there are two. + * The decision app renamed its PSR-4 root from `OCA\Decidesk` to `OCA\Decidiq` + * with no compatibility alias, so the class stackiq listens for has two + * spellings in the field at once and this app can only follow, never move it. + * Stackiq's outbound half already carries both + * ({@see \OCA\Stackiq\Service\ContractApprovalService::DECISION_REQUESTED_EVENTS}); + * this stub is what lets the inbound half do the same without the analyser + * proving the newer spelling dead. + * + * Signatures mirror the REAL class at decidiq/lib/Event/DecisionConcludedEvent.php, + * read on 2026-09-09 at development d72839c, not the call site's assumption + * about it. A stub written from the consumer agrees with the consumer by + * construction and therefore cannot fail. + * + * Why this lives in `tests/analysis-stubs/` and NOT in `tests/Stubs/`: + * `tests/bootstrap.php` `require_once`s every file matching + * `tests/Stubs/{,**\/}*.php` BEFORE Nextcloud's app bootstrap, deliberately + * letting those stubs win over the real classes for mock generation. Doing + * that to the decision-app event classes would shadow the REAL events + * dispatched through IEventDispatcher on any instance where that app is + * installed. These declarations must therefore stay out of that glob. + * + * @category Test + * @package OCA\Decidiq\Event + * + * @author Conduction Development Team + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Decidiq\Event; + +use OCP\EventDispatcher\Event; + +/** + * Analysis-only mirror of decidiq's DecisionConcludedEvent. + * + * Dispatched by decidiq when a Decision reaches a terminal outcome; consumed + * by Stackiq's DecisionConcludedListener. + */ +class DecisionConcludedEvent extends Event { + + /** + * Construct the conclusion event. + * + * @param string $decisionId The concluded Decision id. + * @param string $decisionType The Decision type. + * @param string $status Derived outcome status. + * @param string $outcome Raw decision outcome. + * @param bool $signed Whether a signature stage resolved. + * @param string|null $signingReference Signing reference, when signed. + * @param array $signers Resolved signers list. + * @param string|null $decidedAt When the decision concluded. + * @param string $sourceApp Consumer app that raised the decision. + * @param string|null $subjectRegister OpenRegister register of the origin object. + * @param string|null $subjectSchema OpenRegister schema of the origin object. + * @param string|null $subjectId OpenRegister id of the origin object. + * @param string $externalReference Consumer's own reference. + * @param string $correlationId Correlation id from the request event. + */ + public function __construct( + private readonly string $decisionId, + private readonly string $decisionType, + private readonly string $status, + private readonly string $outcome, + private readonly bool $signed, + private readonly ?string $signingReference, + private readonly array $signers, + private readonly ?string $decidedAt, + private readonly string $sourceApp, + private readonly ?string $subjectRegister, + private readonly ?string $subjectSchema, + private readonly ?string $subjectId, + private readonly string $externalReference = '', + private readonly string $correlationId = '', + ) { + parent::__construct(); + }//end __construct() + + /** + * Get the concluded Decision id. + * + * @return string + */ + public function getDecisionId(): string { + return $this->decisionId; + }//end getDecisionId() + + /** + * Get the Decision type. + * + * @return string + */ + public function getDecisionType(): string { + return $this->decisionType; + }//end getDecisionType() + + /** + * Get the derived outcome status. + * + * @return string + */ + public function getStatus(): string { + return $this->status; + }//end getStatus() + + /** + * Get the raw decision outcome. + * + * @return string + */ + public function getOutcome(): string { + return $this->outcome; + }//end getOutcome() + + /** + * Whether a signature stage resolved. + * + * @return bool + */ + public function isSigned(): bool { + return $this->signed; + }//end isSigned() + + /** + * Get the signing reference, when signed. + * + * @return string|null + */ + public function getSigningReference(): ?string { + return $this->signingReference; + }//end getSigningReference() + + /** + * Get the resolved signers list. + * + * @return array + */ + public function getSigners(): array { + return $this->signers; + }//end getSigners() + + /** + * Get when the decision concluded. + * + * @return string|null + */ + public function getDecidedAt(): ?string { + return $this->decidedAt; + }//end getDecidedAt() + + /** + * Get the consumer app that raised the decision. + * + * @return string + */ + public function getSourceApp(): string { + return $this->sourceApp; + }//end getSourceApp() + + /** + * Get the OpenRegister register of the originating object. + * + * @return string|null + */ + public function getSubjectRegister(): ?string { + return $this->subjectRegister; + }//end getSubjectRegister() + + /** + * Get the OpenRegister schema of the originating object. + * + * @return string|null + */ + public function getSubjectSchema(): ?string { + return $this->subjectSchema; + }//end getSubjectSchema() + + /** + * Get the OpenRegister id of the originating object. + * + * @return string|null + */ + public function getSubjectId(): ?string { + return $this->subjectId; + }//end getSubjectId() + + /** + * Get the consumer's own reference. + * + * @return string + */ + public function getExternalReference(): string { + return $this->externalReference; + }//end getExternalReference() + + /** + * Get the correlation id from the request event. + * + * @return string + */ + public function getCorrelationId(): string { + return $this->correlationId; + }//end getCorrelationId() + +}//end class