6.x - Allow using concrete properties for entity fields. by ADmad · Pull Request #19313 · cakephp/cakephp

ADmad · 2026-03-05T13:52:13Z

Copilot

Pull request overview

This PR aims to let Cake\ORM\Entity use concrete (declared/typed) PHP properties as entity fields, while continuing to support dynamically-set fields.

Changes:

Refactors EntityTrait to split storage between declared properties and a new dynamicFields array.
Updates Entity construction to always go through patch() (removing the old “direct fields assignment” fast-path).
Adds a new entity fixture (UserWithProps) and a large dedicated test suite for entities using concrete properties.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.

File	Description
`src/Datasource/EntityTrait.php`	Introduces `dynamicFields` + `propertyFields`, adds property existence checks, and rewires patch/get/unset/errors/debug behavior to support concrete properties.
`src/ORM/Entity.php`	Removes the `markClean && !useSetters` early return so construction uses `patch()` consistently.
`tests/test_app/TestApp/Model/Entity/UserWithProps.php`	Adds a test entity class with declared typed properties.
`tests/TestCase/ORM/EntityWithConcretePropertiesTest.php`	Adds extensive test coverage targeting concrete-property entities (largely mirroring `EntityTest`).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ADmad · 2026-03-05T16:24:03Z

+        };
+        $entity->things = ['a', 'b'];
+        $entity->things[] = 'c';
+        $this->assertEquals(['a', 'b'], $entity->things);


This is a caveat of using properties through magic __get. Indirect modification of arrays won't work.

Returning direct reference to the property can work but it would fail if the field has our method based getter or PHP's get property hook.

Isn't this long standing behavior as well? I remember it always working this way because of the limitations you listed.

Oh, guess I never tried such indirect modification for arrays hence didn't remember. So it's not a regression and now we have test case for it :)

markstory · 2026-03-07T03:49:00Z

+        'dynamicFields',
+        'propertyFields',
+        'original',
+        'originalFields',
+        'hidden',
+        'virtual',
+        'dirty',
+        'new',
+        'errors',
+        'invalid',
+        'patchable',
+        'registryAlias',
+        'hasBeenVisited',
+        'requireFieldPresence',
+        'restrictedProperties',


Is there a way we could automate this list matching all of the properties defined on the trait? I could see us making a mistake here in the future.

That would need some type of reflection which I have avoided in general when implementing this feature. I can instead add a test case which would report a failure if this list goes out of sync with the trait properties.

Added EntityWithConcretePropertiesTest::testRestrictedPropertiesMatchesTraitProperties().

I had actually missed adding accessors to the list which was caught by the test :)

I had actually missed adding accessors to the list which was caught by the test :)

Huzzah! Even the 🤖 s missed this.

markstory · 2026-03-07T03:54:13Z

+        };
+        $entity->things = ['a', 'b'];
+        $entity->things[] = 'c';
+        $this->assertEquals(['a', 'b'], $entity->things);


Isn't this long standing behavior as well? I remember it always working this way because of the limitations you listed.

ADmad · 2026-03-07T07:05:23Z

The stan failure is because rector wants nullable properties to be initialized as null, but Entity::has() treats uninitialized vs set to null differently.

Update tests

It ensures EntityTrait::$restrictProperties stays in sync with the trait's properties

EntityInterface::has() treats properties set to null different than uninitialized properties

This allows using isset() instead of in_array()

…operty errors

- Convert $originalFields to keyed map for O(1) isset() lookups - Simplify setOriginalField() using array_combine and += merge - Swap branches in setHidden/setVirtual/setDirty for common path first - Use truthy checks instead of === true/false comparisons - Hoist $asOriginal in patch() before loop - Pass $this->propertyFields directly in clean() - Improve patch() phpdoc param type

ADmad added this to the 6.0 milestone Mar 5, 2026

ADmad force-pushed the 6.0-entity-props branch from e8f7c95 to aeca15b Compare March 5, 2026 13:53

ADmad requested a review from Copilot March 5, 2026 13:55

Copilot started reviewing on behalf of ADmad March 5, 2026 13:56 View session

ADmad force-pushed the 6.0-entity-props branch from aeca15b to f39ab04 Compare March 5, 2026 14:02

Copilot AI reviewed Mar 5, 2026

View reviewed changes

ADmad force-pushed the 6.0-entity-props branch from c607910 to cdb13e9 Compare March 5, 2026 16:11

ADmad commented Mar 5, 2026

View reviewed changes

ADmad marked this pull request as ready for review March 5, 2026 16:58

markstory reviewed Mar 7, 2026

View reviewed changes

ADmad force-pushed the 6.0-entity-props branch from c1edac2 to 9c91aa8 Compare March 10, 2026 18:17

ADmad added enhancement labels Mar 10, 2026

ADmad changed the title ~~Allow using concrete properties for entity fields.~~ 6.x - Allow using concrete properties for entity fields. Mar 10, 2026

ADmad added 15 commits April 4, 2026 18:33

Update Entity to use actual class properties for field values

4d5b277

Allow setting any non existent property as dynamic field

d42062b

Add tests for entites with concrete properties

2f21dfc

Fix setting null for properties

2842263

Update tests

Change EntityTrait::$restrictedProperties to static

f5d116e

Add test

005619c

Drop use of Entity class

1e2576b

Update as per copilot review

eff19bc

Add test case.

39d5125

It ensures EntityTrait::$restrictProperties stays in sync with the trait's properties

Remove rector rule which forces defaulting nullable properties to null.

be49e8c

EntityInterface::has() treats properties set to null different than uninitialized properties

Fix rector reported issue

22779b1

Change Entity::$propertyFields to a keyed array.

a5e4f93

This allows using isset() instead of in_array()

Optimize restrictedProperties lookup using keyed array with isset()

1839735

Improve error handling in isModified() to only catch uninitialized pr…

c7e017d

…operty errors

Properly skip resetting unmodified non-null values.

ee3b8b8

ADmad force-pushed the 6.0-entity-props branch from eb6446a to ee3b8b8 Compare April 4, 2026 13:08

Merge branch '6.x' into 6.0-entity-props

b24f510

ADmad mentioned this pull request Apr 4, 2026

RFC – 6.0 – Concrete Entity Properties with Property Hook Support #19380

Open

ADmad mentioned this pull request Apr 5, 2026

6.0 - Entity props with hooks #19381

Open

ADmad marked this pull request as draft April 11, 2026 05:43

Conversation

ADmad commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ADmad Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ADmad commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ADmad commented Mar 5, 2026 •

edited

Loading

ADmad Mar 7, 2026 •

edited

Loading