Skip to content

MeetingFolderListener never runs — resolveSchemaSlug() probes every candidate getter with method_exists(), which is false for all of them #471

Description

@rubenvdlinde

Verified against origin/development with git show, not a local working tree.

Summary

MeetingFolderListener::resolveSchemaSlug() loops candidate getters through a method_exists() probe with a variable method name:

// lib/Listener/MeetingFolderListener.php:155
if (method_exists($entity, $getter) === false) { continue; }

Every candidate getter on an OCA\OpenRegister\Db\ObjectEntity is served by Entity::__call() and declared as an @method docblock, so the probe is false for every candidate and the loop completes having found nothing.

Measured

Live, against the server's own Entity.php:

subject method_exists is_callable
ObjectEntity::getSchema false true
ObjectEntity::getObject — concrete control true true

Consequence

The row carries no _schemaSlug/_schema/schema key, getSchemaSlug() does not exist, and the probe loop yields nothing — so resolveSchemaSlug() returns '', the !== 'meeting' comparison is always true, and the meeting-folder listener never runs.

Unlike PortalCreateOpenParentGuardListener, which has a Tier-2 detectSchemaBySignature() fallback carrying the guard, there is no second tier here.

SubmissionDeadlineListener.php:194 and BackgroundJob/TranscriptRetentionJob.php:476 use the same variable-name shape and are worth checking in the same pass.

Why this was nearly missed

A literal-name grep for method_exists($x, 'getSchema') cannot see this. The variable-name form is the more dangerous of the two, because a loop over candidate getters that all fail looks exhaustive while checking nothing. Fleet-wide there are 30 such sites across 10 repos.

Credit where it is due

tests/Stubs/Db/ObjectEntity.php is the fleet's reference double — it extends Entity, declares only genuinely concrete methods, and carries a "SIGNATURE PARITY CONTRACT (decidesk#399)" header naming this exact failure mode. Every other repo with a confirmed instance had a double that declared the accessors concretely and inverted the predicate under test.

That contract is why decidesk's count is 1 and pipelinq's is 5. It is not sufficient on its own — this defect survived it, because the parity contract constrains the double, not the probe.

Fix notes

is_callable() is not a membership test on a __call class — measured, it is true for any name and the call then raises BadFunctionCallException. Swapping the probe yields an always-true guard, so the call must be made exception-safe in the same edit.

getObject() is genuinely concrete and always injects the uuid under id, so reading @self.schema from the payload is the more robust route. Note getId() is also magic — do not fall back to it.

Fleet context and the full 18-repo table: fleet-board/findings/method-exists-sweep.md.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions