Replace constants with methods for identity and model version in aggregates. by gustavofreze · Pull Request #2 · tiny-blocks/building-blocks

gustavofreze · 2026-04-18T22:20:50Z

No description provided.

…on in aggregates.

Copilot

Pull request overview

This PR replaces reflective class-constant contracts in aggregates/entities with explicit protected methods, introduces named factories for core value objects, and adds infrastructure for upcaster chaining and event (de)serialization.

Changes:

Replaced IDENTITY / MODEL_VERSION constants with identityName() / modelVersion() methods across entity/aggregate behaviors and updated tests/docs accordingly.
Made Revision, SequenceNumber, and EventType constructors private and added/expanded semantic factories (initial(), first(), of(), fromString()).
Added Upcasters collection with chain() plus IntermediateEvent object-mapping support (via tiny-blocks/mapper) and new upcast chain tests.

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/Upcast/UpcastersTest.php	Adds coverage for chaining multiple upcasters and pass-through behavior.
tests/Upcast/SingleUpcasterBehaviorTest.php	Updates revision construction to use factories (`Revision::initial()`, `Revision::of()`).
tests/Upcast/IntermediateEventTest.php	Extends coverage to `fromIterable()`, `toArray()`, and `toJson()` mapping/serialization.
tests/Snapshot/SnapshotTest.php	Updates `SequenceNumber` construction to `SequenceNumber::first()`.
tests/Models/ProductV2Upcaster.php	Adds a V2 upcaster fixture for chaining tests.
tests/Models/OrderWithoutIdentityConstant.php	Removes fixture now obsolete after dropping `IDENTITY` constant enforcement.
tests/Models/OrderWithMissingIdentityProperty.php	Migrates fixture to `identityName()` and new `push()` API + revision factories.
tests/Models/Order.php	Migrates fixture to `identityName()` and new `push()` API + revision factories.
tests/Models/CartWithoutIdentityConstant.php	Removes fixture now obsolete after dropping `IDENTITY` constant enforcement.
tests/Models/Cart.php	Migrates fixture to `identityName()`/`modelVersion()` methods + revision factory usage.
tests/Event/SequenceNumberTest.php	Adds factory/visibility assertions and migrates away from public constructor usage.
tests/Event/RevisionTest.php	Adds factory/visibility assertions and migrates away from public constructor usage.
tests/Event/EventTypeTest.php	Adds constructor visibility assertion and migrates invalid-value tests to `fromString()`.
tests/Event/EventRecordsTest.php	Updates record construction to use `Revision::initial()` and `SequenceNumber::first()`.
tests/Event/EventRecordTest.php	Updates record construction to use `Revision::initial()` and `SequenceNumber::first()`.
tests/Entity/EntityBehaviorTest.php	Removes constant-missing test and aligns expectations with `identityName()` contract.
tests/Aggregate/EventSourcingRootBehaviorTest.php	Removes constant-missing test, reflecting new `identityName()` approach.
src/Upcast/Upcasters.php	Introduces `Upcasters` collection with `chain()` to apply upcasters sequentially.
src/Upcast/Upcaster.php	Documentation update pointing users to `Upcasters::chain()`.
src/Upcast/SingleUpcasterBehavior.php	Updates revision bumping to `Revision::of(...)`.
src/Upcast/IntermediateEvent.php	Implements mapper support (`ObjectMapper` + `ObjectMappability`) for reconstitution/serialization.
src/Internal/Exceptions/MissingIdentityConstant.php	Removes obsolete exception after moving away from `IDENTITY` constant.
src/Event/SequenceNumber.php	Makes constructor private and adds `of()` factory.
src/Event/Revision.php	Makes constructor private and adds `initial()`/`of()` factories.
src/Event/EventType.php	Makes constructor private (enforcing use of factories).
src/Entity/EntityBehavior.php	Replaces constant lookup with `identityName()` method contract.
src/Entity/Entity.php	Updates public API documentation to reflect `identityName()` method contract.
src/Aggregate/EventualAggregateRootBehavior.php	Renames `pushEvent()` to `push()` and keeps event recording behavior.
src/Aggregate/EventSourcingRootBehavior.php	Updates `blank()` identity assignment to use `identityName()`.
src/Aggregate/EventSourcingRoot.php	Updates PHPDoc to reflect `identityName()`/MissingIdentityProperty semantics.
src/Aggregate/AggregateRootBehavior.php	Switches model version resolution to `modelVersion()` and reorders helpers.
src/Aggregate/AggregateRoot.php	Updates API documentation to reflect `modelVersion()` method contract.
phpstan.neon.dist	Adds suppression for `Upcasters::chain()` return-type analysis issue.
composer.json	Adds `tiny-blocks/mapper` dependency.
README.md	Updates documentation/examples for `identityName()`/`modelVersion()`, `push()`, upcaster chaining, and mapping.

Copilot · 2026-04-18T22:26:37Z

+    - message: '#Upcasters::chain\(\) should return#'
+      path: src/Upcast/Upcasters.php


phpstan.neon.dist adds an ignore for the Upcasters::chain() return-type error. This reduces static-analysis signal for a real typing issue; it would be better to adjust Upcasters::chain() so PHPStan can infer/guarantee the return type and then drop this suppression.

Suggested change

- message: '#Upcasters::chain should return#'

path: src/Upcast/Upcasters.php

Copilot · 2026-04-18T22:26:37Z

-
        $aggregate = new ReflectionClass(objectOrClass: static::class)->newInstanceWithoutConstructor();
-        new ReflectionProperty($aggregate, static::IDENTITY)->setValue(objectOrValue: $aggregate, value: $identity);
+        new ReflectionProperty($aggregate, $aggregate->identityName())


EventSourcingRootBehavior::blank() uses ReflectionProperty with $aggregate->identityName() directly. If identityName() returns a non-existent property, this will throw a ReflectionException instead of the library’s MissingIdentityProperty (and contradicts the EventSourcingRoot PHPDoc that now advertises MissingIdentityProperty). Consider resolving the property name via $aggregate->getIdentityName() (or explicitly checking property_exists and throwing MissingIdentityProperty) before constructing ReflectionProperty.

Suggested change

new ReflectionProperty($aggregate, $aggregate->identityName())

$identityProperty = $aggregate->identityName();

if (!property_exists($aggregate, $identityProperty)) {

throw new MissingIdentityProperty($identityProperty);

}

new ReflectionProperty($aggregate, $identityProperty)

Copilot · 2026-04-18T22:26:37Z

+        $upcast = static function (IntermediateEvent $carried, Upcaster $upcaster): IntermediateEvent {
+            return $upcaster->upcast(event: $carried);
+        };
+
+        return $this->reduce(accumulator: $upcast, initial: $event);


Upcasters::chain() currently relies on Collection::reduce(), but PHPStan cannot prove the reduced value is an IntermediateEvent (hence the new ignore rule). Instead of suppressing static analysis, consider rewriting chain() with an explicit fold over the collection (or adding a runtime type check before returning) so the declared return type is provably satisfied and the phpstan.neon ignore can be removed.

Suggested change

$upcast = static function (IntermediateEvent $carried, Upcaster $upcaster): IntermediateEvent {

return $upcaster->upcast(event: $carried);

};

return $this->reduce(accumulator: $upcast, initial: $event);

$carried = $event;

foreach ($this as $upcaster) {

$carried = $upcaster->upcast(event: $carried);

}

return $carried;

refactor: Replace constants with methods for identity and model versi…

e210a6f

…on in aggregates.

Copilot AI review requested due to automatic review settings April 18, 2026 22:20

Copilot started reviewing on behalf of gustavofreze April 18, 2026 22:21 View session

github-actions Bot assigned gustavofreze Apr 18, 2026

gustavofreze merged commit 72e9519 into main Apr 18, 2026
8 of 9 checks passed

gustavofreze deleted the feature/develop branch April 18, 2026 22:22

Copilot AI reviewed Apr 18, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace constants with methods for identity and model version in aggregates.#2

Replace constants with methods for identity and model version in aggregates.#2
gustavofreze merged 1 commit intomainfrom
feature/develop

gustavofreze commented Apr 18, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		- message: '#Upcasters::chain\(\) should return#'
		path: src/Upcast/Upcasters.php

-        new ReflectionProperty($aggregate, $aggregate->identityName())
+        $identityProperty = $aggregate->identityName();
+        if (!property_exists($aggregate, $identityProperty)) {
+            throw new MissingIdentityProperty($identityProperty);
+        }
+        new ReflectionProperty($aggregate, $identityProperty)

-        $upcast = static function (IntermediateEvent $carried, Upcaster $upcaster): IntermediateEvent {
-            return $upcaster->upcast(event: $carried);
-        };
-        return $this->reduce(accumulator: $upcast, initial: $event);
+        $carried = $event;
+        foreach ($this as $upcaster) {
+            $carried = $upcaster->upcast(event: $carried);
+        }
+        return $carried;

Conversation

gustavofreze commented Apr 18, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants