Model::__isset() is attribute-only while __get() is extension-aware, so `$model->extensionProperty ?? $default` silently returns the default

[LukeTowers]

Summary

Winter\Storm\Database\Model::__isset() resolves attributes only, while __get() resolves through extendableGet(). The two disagree for any property provided by the extension system — addDynamicProperty() values and public properties declared on a Behavior.

The practical consequence is that $model->someExtensionProperty ?? $default always yields $default, silently, even though reading the property directly returns the right value. Plain Extendable objects do not have this problem, so the behaviour is inconsistent within Winter itself.

Reproduction

No plugins required:

// A — plain Extendable (no __isset defined)
$obj = new class extends \Winter\Storm\Extension\Extendable {};
$obj->addDynamicProperty('foo', 'bar');

$obj->foo;              // 'bar'
isset($obj->foo);       // false
$obj->foo ?? 'DEFAULT'; // 'bar'      <-- works

// B — Database\Model
\System\Models\EventLog::extend(function ($m) {
    $m->addDynamicProperty('foo', 'bar');
});
$m = new \System\Models\EventLog;

$m->foo;                     // 'bar'
$m->propertyExists('foo');   // true
isset($m->foo);              // false
$m->foo ?? 'DEFAULT';        // 'DEFAULT'  <-- silently wrong

// C — a property declared on a Behaviour, same result
class DemoBehavior extends \Winter\Storm\Extension\ExtensionBase {
    public $demoProp = 'from behaviour';
}
\System\Models\Parameter::extend(fn ($m) => $m->extendClassWith(DemoBehavior::class));

(new \System\Models\Parameter)->demoProp;        // 'from behaviour'
isset((new \System\Models\Parameter)->demoProp); // false

Why ?? breaks on models but not on Extendable

This is the part that makes it easy to miss. PHP's null-coalescing operator consults __isset() only if the class defines one; with __get() alone it falls through to __get():

class WithGetOnly      { public function __get($n) { return 'bar'; } }
class WithGetAndIsset  { public function __get($n) { return 'bar'; }
                         public function __isset($n) { return false; } }

(new WithGetOnly)->foo ?? 'DEFAULT';     // 'bar'
(new WithGetAndIsset)->foo ?? 'DEFAULT'; // 'DEFAULT'

Extendable defines no __isset (method_exists($obj, '__isset') === false), so it lands in the first case and ?? works. Database\Model defines one, so it lands in the second and ?? breaks.

So the model's __isset() is not merely failing to report extension properties — it is removing working behaviour that exists everywhere else in Winter.

Source

Winter\Storm\Database\Model (src/Database/Model.php, line 810 on both 1.2 and wip/1.3):

public function __get($name)
{
    return $this->extendableGet($name);   // extension-aware
}

public function __isset($key)
{
    return !is_null($this->getAttribute($key));   // Eloquent only
}

Winter\Storm\Halcyon\Model::__isset() (line 1683) has the same shape — attributes and get-mutators only, no extendableGet() — so Halcyon models are presumably affected too, though I have only reproduced this on the database model.

How this shows up in practice

Found in LukeTowers.EasyAudit, where per-model overrides applied through its modelsToTrack config had no effect at all. The plugin registers them with addDynamicProperty() and reads them back as $this->subject?->trackableIgnoredAttributes ?? [], so every override resolved to the default — an audit log recording attributes the config explicitly excluded, and, less visibly, per-model IP/user-agent/URL logging opt-outs being ignored.

It went unnoticed for a long time because models that declare these as real class properties behave correctly; only the dynamically-registered path fails. Fixed plugin-side in LukeTowers/wn-easyaudit-plugin#8 by not coalescing on a property access, but the underlying asymmetry seems worth addressing.

Worth noting the trap is easy to fall back into: the first draft of that plugin fix still ended in $subject->$property ?? $default and reproduced the bug exactly.

Where it can surface silently

__isset() is not only reached through isset():

• empty($model->foo)
• $model->foo ?? $default
• Twig {{ model.foo is defined }} and {{ model.foo|default(...) }}

Each of these currently reports "not set" for a property that reads back fine.

The compatibility question

Making __isset() extension-aware is a behavioural change across every model and behaviour, so it seems like a minor-release item rather than a patch, and I did not want to open a PR before the semantics call is made. The things that seem worth weighing:

1. isset() starts returning true for behaviour-declared properties. Any code branching on isset($model->foo) where some behaviour happens to declare foo changes meaning. On the site this was found on, Winter.Search puts a searchable property on every indexed model, so this is not hypothetical.

2. Eloquent's null semantics. isset() on an Eloquent attribute means "present and not null". Dynamic properties would need to follow the same rule to stay consistent — an explicitly-null dynamic property should presumably report false.

3. Double resolution / lazy loading. extendableGet() falls back to the parent __get(), so a naive implementation that tries attributes and then extendableGet() would resolve attributes twice, and can trigger relation lazy-loading during what looks like a cheap existence check. __isset() should probably consult extensionData directly rather than routing through extendableGet().

4. __unset() has the same asymmetry and would want considering alongside it.

A conservative version — check extensionData['dynamicProperties'] and extension objects directly, without touching the existing attribute path — would fix the reported problem while keeping attribute semantics untouched. But that still flips (1), so it is a judgement call for the maintainers.

Environment

• Winter CMS build 1.2.13
• winter/storm dev-wip/1.3 (e8424705); the same code is present on the 1.2 branch at the same line
• PHP 8.3.24