Extract a reusable EntityFactory - #13
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 120 138 +18
===========================================
Files 12 13 +1
Lines 307 349 +42
===========================================
+ Hits 307 349 +42 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors entity creation/hydration and collection metadata handling by introducing a new EntityFactory (plus #[NotPersistable]) and migrating mappers/tests away from ad-hoc dynamic property access and Collection “extras”.
Changes:
- Add
EntityFactoryand#[NotPersistable]to standardize entity instantiation, property access, and persistence extraction. - Refactor
AbstractMapper/InMemoryMapperand style integration tests to useEntityFactory(constructor injection) instead ofsetStyle/entityNamespace. - Replace
Collection“extras” with dedicated typed collection classes/properties (Typed,Filtered, newComposite) and update tests accordingly.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Styles/Sakila/SakilaIntegrationTest.php | Switches InMemoryMapper setup to inject an EntityFactory configured with style + entity namespace. |
| tests/Styles/Plural/PluralIntegrationTest.php | Same: inject EntityFactory into mapper for entity resolution. |
| tests/Styles/NorthWind/NorthWindIntegrationTest.php | Same: inject EntityFactory into mapper for entity resolution. |
| tests/Styles/CakePHP/CakePHPIntegrationTest.php | Same: inject EntityFactory into mapper for entity resolution. |
| tests/Stubs/TypedEntity.php | Adds a typed stub entity for EntityFactory behavior tests. |
| tests/Stubs/EntityWithExcluded.php | Adds a stub entity using #[NotPersistable] for extraction tests. |
| tests/InMemoryMapper.php | Refactors entity creation/hydration/flush to go through EntityFactory. |
| tests/EntityFactoryTest.php | Adds unit tests for EntityFactory behaviors (creation, constructor disabling, get/set, extraction, hydrate, style). |
| tests/Collections/TypedTest.php | Updates assertions to use Typed::getType() and class-based assertions. |
| tests/Collections/MixedTest.php | Removes tests for deleted Mix collection. |
| tests/Collections/FilteredTest.php | Updates to Filtered::getFilters() and adds coverage for isIdentifierOnly(). |
| tests/Collections/CompositeTest.php | Adds coverage for new Composite collection class and its compositions. |
| tests/Collections/CollectionTest.php | Removes tests for deleted Collection::extra/getExtra/have API. |
| tests/AbstractMapperTest.php | Updates test to verify style is sourced from injected EntityFactory. |
| src/NotPersistable.php | Introduces attribute used to exclude properties during persistence extraction. |
| src/EntityFactory.php | Introduces factory for entity creation, property get/set, extraction, and hydration. |
| src/Collections/Typed.php | Replaces extras-based type storage with a private property + accessor. |
| src/Collections/Typable.php | Removes unused interface. |
| src/Collections/Mixable.php | Removes unused interface. |
| src/Collections/Mix.php | Removes Mix collection implementation (replaced by Composite). |
| src/Collections/Filtered.php | Replaces extras-based filters with a private list + accessor + IDENTIFIER_ONLY support. |
| src/Collections/Filterable.php | Removes unused interface. |
| src/Collections/Composite.php | Adds new Composite collection class with compositions metadata. |
| src/Collections/Collection.php | Removes extras API and forces chained nodes after the first to be plain Collection instances. |
| src/AbstractMapper.php | Injects EntityFactory and derives style from it (removes setStyle). |
| phpstan.neon.dist | Updates ignore regex to remove Mix and include Composite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
fd403b7 to
51326e6
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors entity creation/hydration and collection metadata handling by introducing an EntityFactory, wiring mapper style through it, and replacing “extras” on collections with explicit typed properties (e.g., filters/type/compositions).
Changes:
- Introduces
EntityFactory(+NotPersistableattribute) and updatesAbstractMapper/InMemoryMapperto use it for entity creation, property access, and persistence extraction. - Removes generic
Collection“extras” and replaces priorMix/Filtered/Typedextra-based APIs with dedicated classes/properties (Composite,Filtered::getFilters(),Typed::getType(), etc.). - Updates and adds PHPUnit coverage (new
EntityFactoryTest, newCompositeTest, integration test setup updates).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Styles/Sakila/SakilaIntegrationTest.php | Injects EntityFactory into InMemoryMapper to configure style + entity namespace for typed entities. |
| tests/Styles/Plural/PluralIntegrationTest.php | Same as above for Plural style integration. |
| tests/Styles/NorthWind/NorthWindIntegrationTest.php | Same as above for NorthWind style integration. |
| tests/Styles/CakePHP/CakePHPIntegrationTest.php | Same as above for CakePHP style integration. |
| tests/Stubs/TypedEntity.php | Adds typed-entity stub for EntityFactory constructor/property tests. |
| tests/Stubs/EntityWithExcluded.php | Adds stub using #[NotPersistable] to validate extraction exclusions. |
| tests/Stubs/EdgeCaseEntity.php | Adds edge-case stub for static/uninitialized/non-public property extraction behavior. |
| tests/InMemoryMapper.php | Switches entity creation/property access/persistence extraction to EntityFactory. |
| tests/EntityFactoryTest.php | New unit tests covering EntityFactory creation, set/get, extraction, hydration, and attributes. |
| tests/Collections/TypedTest.php | Updates tests to assert typed accessors (getType()) and correct subclass instances. |
| tests/Collections/MixedTest.php | Removes tests for the deleted Mix collection. |
| tests/Collections/FilteredTest.php | Updates tests to use typed filter accessors and adds isIdentifierOnly() coverage. |
| tests/Collections/CompositeTest.php | New tests for the new Composite collection behavior. |
| tests/Collections/CollectionTest.php | Updates tests for new parent/next access patterns and removes “extras” tests. |
| tests/AbstractMapperTest.php | Updates style configuration test to assert style is sourced from EntityFactory. |
| src/NotPersistable.php | Adds attribute to mark entity properties as excluded from persistence extraction. |
| src/EntityFactory.php | Adds central factory for entity instantiation, set/get, hydration, and property extraction. |
| src/Collections/Typed.php | Replaces extras-based type storage with a private $type + accessor; updates factory methods. |
| src/Collections/Typable.php | Removes old interface tied to extras-based typing. |
| src/Collections/Mixable.php | Removes old interface tied to extras-based mixins. |
| src/Collections/Mix.php | Removes old Mix collection (replaced by Composite). |
| src/Collections/Filtered.php | Replaces extras-based filters with private $filters, accessor, and isIdentifierOnly(). |
| src/Collections/Filterable.php | Removes old interface tied to extras-based filtering. |
| src/Collections/Composite.php | Adds new Composite collection storing compositions via a typed property + accessor. |
| src/Collections/Collection.php | Removes extras system; adjusts parent/next API; small chaining/stacking refactor. |
| src/AbstractMapper.php | Injects EntityFactory and derives mapper style from it; removes setStyle(). |
| phpstan.neon.dist | Updates ignored-error patterns to reflect collection renames/removals and new stubs. |
Comments suppressed due to low confidence (1)
tests/InMemoryMapper.php:118
- When assigning a new primary key for newly persisted entities, this still writes directly via
$entity->{$pk} = ...instead of going throughEntityFactory::set(). That bypasses the new typed/non-public property handling and can fail (or create deprecated dynamic properties) if the PK property isn’t a public dynamic field. Consider using the factory to set the PK consistently with the rest of the mapper logic.
if (!isset($row[$pk])) {
++$this->lastInsertId;
$entity->{$pk} = $this->lastInsertId;
$row[$pk] = $this->lastInsertId;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Remove generic $extras key-value bag from Collection in favor of typed properties on Filtered ($filters), Composite ($compositions), and Typed ($type) subclasses - Rename Mix to Composite, align internal terminology (mixin → composition) - Delete Filterable, Mixable, Typable interfaces — instanceof checks replace string-key lookups - Extract EntityFactory from Relational into Data with cached reflection, immutable construction, and #[NotPersistable] attribute support - Make EntityFactory an immutable constructor dependency on AbstractMapper, removing setStyle() and mutable entityNamespace/disableEntityConstructor - Add Filtered::IDENTIFIER_ONLY constant replacing the '*' magic string - Add getParent() to Collection, remove getParentName()/getNextName() convenience methods in favor of null-safe chains - Fix LSB leaking in Collection::__call for chained nodes - Add dedicated tests for EntityFactory, Filtered::isIdentifierOnly(), and property edge cases (static, uninitialized, NotPersistable)
properties on Filtered ($filters), Composite ($compositions), and
Typed ($type) subclasses
string-key lookups
immutable construction, and #[NotPersistable] attribute support
removing setStyle() and mutable entityNamespace/disableEntityConstructor
convenience methods in favor of null-safe chains
and property edge cases (static, uninitialized, NotPersistable)