Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 27 additions & 30 deletions lib/Listener/AutomationCleanupListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
namespace OCA\OpenBuild\Listener;

use OCA\OpenBuild\Service\AutomationCompilerService;
use OCA\OpenBuild\Service\ListenerSlugContract;
use OCA\OpenBuild\Service\ObjectSchemaSlugResolver;
use OCA\OpenRegister\Event\ObjectDeletedEvent;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
Expand All @@ -59,12 +61,16 @@ class AutomationCleanupListener implements IEventListener
*
* @param LoggerInterface $logger PSR logger for diagnostics.
* @param AutomationCompilerService $compiler Owns the artifact-removal logic.
* @param ObjectSchemaSlugResolver $slugs Resolves the event's schema id to a slug.
* @param ListenerSlugContract $contract Gates the corrected comparison.
*
* @return void
*/
public function __construct(
private readonly LoggerInterface $logger,
private readonly AutomationCompilerService $compiler,
private readonly ObjectSchemaSlugResolver $slugs,
private readonly ListenerSlugContract $contract,
) {
}//end __construct()

Expand All @@ -83,8 +89,27 @@ public function handle(Event $event): void
}

$entity = $event->getObject();
$schema = $this->extractSchemaSlug(entity: $entity);
if ($schema !== AutomationCompilerService::AUTOMATION_SCHEMA) {

// GATED ON PURPOSE — see ListenerSlugContract.
//
// extractSchemaSlug() returned the schema's numeric id and compared it
// to the slug 'automation', so this cleanup has never once run and
// every deleted automation has left its compiled artifacts behind. The
// comparison below is correct; enabling it starts DELETING those
// artifacts on automation delete, which is the desired behaviour but is
// still a behaviour change on a path that has never executed.
if ($this->contract->isEnabled() === false) {
return;
}

// The register is checked as well as the schema: `automation` is not a
// unique slug on this instance (two schemas carry it), so matching on
// the schema slug alone would delete artifacts for another app's rows.
if ($this->slugs->isOpenBuildSchema(
entity: $entity,
schemaSlug: AutomationCompilerService::AUTOMATION_SCHEMA
) === false
) {
return;
}

Expand All @@ -105,34 +130,6 @@ public function handle(Event $event): void
}
}//end handle()

/**
* Read the schema slug from the ObjectEntity (defensive — supports both
* direct `getSchemaSlug()` and the `@self.schema` projection, mirroring
* {@see ProductionVersionGuardListener::extractSchemaSlug()}).
*
* @param object $entity The ObjectEntity instance.
*
* @return string Schema slug or empty string when unresolved.
*/
private function extractSchemaSlug(object $entity): string
{
if (method_exists($entity, 'getSchemaSlug') === true) {
$slug = $entity->getSchemaSlug();
if (is_string($slug) === true && $slug !== '') {
return $slug;
}
}

if (method_exists($entity, 'jsonSerialize') === true) {
$serialised = $entity->jsonSerialize();
if (is_array($serialised) === true && isset($serialised['@self']['schema']) === true) {
return (string) $serialised['@self']['schema'];
}
}

return '';
}//end extractSchemaSlug()

/**
* Read the object payload (post-`@self`) from the ObjectEntity.
*
Expand Down
60 changes: 29 additions & 31 deletions lib/Listener/ProductionVersionGuardListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
namespace OCA\OpenBuild\Listener;

use OCA\OpenBuild\Service\ApplicationVersionService;
use OCA\OpenBuild\Service\ListenerSlugContract;
use OCA\OpenBuild\Service\ObjectSchemaSlugResolver;
use OCA\OpenRegister\Event\ObjectCreatingEvent;
use OCA\OpenRegister\Event\ObjectUpdatingEvent;
use OCP\EventDispatcher\Event;
Expand All @@ -57,14 +59,18 @@ class ProductionVersionGuardListener implements IEventListener
/**
* Constructor.
*
* @param LoggerInterface $logger PSR logger for diagnostics
* @param ApplicationVersionService $service The cross-row guard owner
* @param LoggerInterface $logger PSR logger for diagnostics
* @param ApplicationVersionService $service The cross-row guard owner
* @param ObjectSchemaSlugResolver $slugs Resolves the event's schema id to a slug
* @param ListenerSlugContract $contract Gates the corrected comparison
*
* @return void
*/
public function __construct(
private readonly LoggerInterface $logger,
private readonly ApplicationVersionService $service,
private readonly ObjectSchemaSlugResolver $slugs,
private readonly ListenerSlugContract $contract,
) {
}//end __construct()

Expand Down Expand Up @@ -96,8 +102,27 @@ public function handle(Event $event): void
return;
}

$schema = $this->extractSchemaSlug(entity: $entity);
if ($schema !== ApplicationVersionService::APPLICATION_SCHEMA) {
// GATED ON PURPOSE — read before flipping the flag.
//
// This guard has never once executed: extractSchemaSlug() returned the
// schema's numeric id and compared it to the slug 'application', so the
// `!==` was always true and this method always returned here. The
// comparison below is now correct, but making it correct CHANGES
// BEHAVIOUR: this is a fail-closed validation guard, so waking it
// starts REJECTING production-version writes that succeed today.
//
// Enabling it is therefore a rollout decision, not a bug fix, and it is
// deliberately off by default. Enable with:
// occ config:app:set openbuild listener_slug_contract --value=yes.
if ($this->contract->isEnabled() === false) {
return;
}

if ($this->slugs->isOpenBuildSchema(
entity: $entity,
schemaSlug: ApplicationVersionService::APPLICATION_SCHEMA
) === false
) {
return;
}

Expand Down Expand Up @@ -143,33 +168,6 @@ public function handle(Event $event): void
}//end try
}//end handle()

/**
* Read the schema slug from the ObjectEntity (defensive — supports
* both direct `getSchemaSlug()` and the `@self.schema` projection).
*
* @param object $entity The ObjectEntity instance
*
* @return string Schema slug or empty string when unresolved
*/
private function extractSchemaSlug(object $entity): string
{
if (method_exists($entity, 'getSchemaSlug') === true) {
$slug = $entity->getSchemaSlug();
if (is_string($slug) === true && $slug !== '') {
return $slug;
}
}

if (method_exists($entity, 'jsonSerialize') === true) {
$serialised = $entity->jsonSerialize();
if (is_array($serialised) === true && isset($serialised['@self']['schema']) === true) {
return (string) $serialised['@self']['schema'];
}
}

return '';
}//end extractSchemaSlug()

/**
* Read the object payload (post-`@self`) from the ObjectEntity.
*
Expand Down
81 changes: 81 additions & 0 deletions lib/Service/ListenerSlugContract.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php

/**
* Feature gate for openbuild's corrected listener schema matching.
*
* OpenBuild's OpenRegister listeners compared a schema **id** against a schema
* **slug** literal, so their handler bodies had never run once. Correcting the
* comparison is a one-line change, but it is not a behaviour-neutral one: the
* listeners it wakes include a fail-closed validation guard that starts
* REJECTING writes which succeed today, a bulk approval-chain initialiser, and
* a document generator. None of them have ever executed in production, so none
* of them have ever been exercised against real data.
*
* This gate exists so the fix can ship, be reviewed and be tested without
* silently switching all of that on in one deploy. It mirrors the approach
* openregister#2248 took for the sibling `ObjectTransitionedEvent` defect,
* which is likewise shipped behind a default-off flag.
*
* Default: OFF. Enable per instance, after reviewing each handler body:
* occ config:app:set openbuild listener_slug_contract --value=yes
*
* @category Service
* @package OCA\OpenBuild\Service
*
* @author Conduction Development Team <info@conduction.nl>
* @copyright 2026 Conduction B.V.
* @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
*
* SPDX-License-Identifier: EUPL-1.2
* SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
*
* @version GIT: <git-id>
*
* @link https://openbuild.nl
*/

declare(strict_types=1);

namespace OCA\OpenBuild\Service;

use OCP\IAppConfig;

/**
* Reports whether the corrected listener schema matching is enabled.
*/
class ListenerSlugContract
{

/**
* The app id the flag is stored under.
*
* @var string
*/
private const APP_ID = 'openbuild';

/**
* The config key holding the flag.
*
* @var string
*/
private const CONFIG_KEY = 'listener_slug_contract';

/**
* Constructor.
*
* @param IAppConfig $appConfig Nextcloud app configuration.
*/
public function __construct(private readonly IAppConfig $appConfig)
{
}//end __construct()

/**
* Whether the corrected slug comparison should be honoured.
*
* @return bool True when the contract is enabled for this instance.
*/
public function isEnabled(): bool
{
return $this->appConfig->getValueBool(self::APP_ID, self::CONFIG_KEY, false);
}//end isEnabled()
}//end class
Loading
Loading