Adds beforeSerialize and afterSerialize events#156
Adds beforeSerialize and afterSerialize events#156cnizzardini wants to merge 9 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds new CakePHP event hooks around serialization for MixerApi’s CollectionView, HAL, and JSON-LD serializers so consumers can observe/adjust data pre-encode and react post-encode.
Changes:
- Dispatch
beforeSerializeandafterSerializeevents from the CollectionView, HalView, and JsonLdView serializers. - Add serializer accessors/mutators (
setData(),getRequest(),getPaginatorHelper()) to support event handlers. - Extend plugin test suites and READMEs to cover/document the new events.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/json-ld-view/src/JsonSerializer.php | Dispatches JSON-LD before/after serialize events; adds setters/getters for event handlers. |
| plugins/json-ld-view/tests/TestCase/JsonSerializerTest.php | Adds assertions that the new JSON-LD events fire. |
| plugins/json-ld-view/README.md | Documents JSON-LD serialization events with example listeners. |
| plugins/hal-view/src/JsonSerializer.php | Dispatches HAL before/after serialize events; adds setters/getters for event handlers. |
| plugins/hal-view/tests/TestCase/JsonSerializerTest.php | Adds assertions that the new HAL events fire. |
| plugins/hal-view/README.md | Documents HAL serialization events with example listeners. |
| plugins/collection-view/src/Serializer.php | Dispatches CollectionView before/after serialize events (JSON/XML), includes type metadata. |
| plugins/collection-view/tests/TestCase/SerializerTest.php | Adds assertions that the new CollectionView events fire for JSON and XML. |
| plugins/collection-view/README.md | Documents CollectionView serialization events with example listeners. |
Comments suppressed due to low confidence (1)
plugins/collection-view/src/Serializer.php:30
- The
$dataproperty is declaredmixed, but the docblock still says@var array. Please update the docblock to match the actual type (or tighten the property type back toarrayif only arrays are supported).
* serialized data
*
* @var array
*/
private mixed $data;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| public function test_as_json(): void | ||
| { | ||
| EventManager::instance()->trackEvents(true)->setEventList(new EventList()); |
There was a problem hiding this comment.
EventManager::trackEvents() in CakePHP returns a boolean flag, so chaining ->setEventList(...) will call a method on a bool and fail. Split this into two statements (enable tracking, then set the EventList) before asserting events fired.
| EventManager::instance()->trackEvents(true)->setEventList(new EventList()); | |
| $eventManager = EventManager::instance(); | |
| $eventManager->trackEvents(true); | |
| $eventManager->setEventList(new EventList()); |
|
|
||
| public function test_as_xml(): void | ||
| { | ||
| EventManager::instance()->trackEvents(true)->setEventList(new EventList()); |
There was a problem hiding this comment.
EventManager::trackEvents() returns a boolean flag and isn't chainable; ->setEventList(...) will fail at runtime here as well. Use separate calls to enable tracking and then set the EventList.
| EventManager::instance()->trackEvents(true)->setEventList(new EventList()); | |
| $eventManager = EventManager::instance(); | |
| $eventManager->trackEvents(true); | |
| $eventManager->setEventList(new EventList()); |
|
|
||
| public function test_collection(): void | ||
| { | ||
| EventManager::instance()->trackEvents(true)->setEventList(new EventList()); |
There was a problem hiding this comment.
EventManager::trackEvents() isn't chainable (it returns a boolean flag), so ->setEventList(...) will be invoked on a bool and cause a failure. Use separate calls to enable tracking and set the EventList.
| EventManager::instance()->trackEvents(true)->setEventList(new EventList()); | |
| $eventManager = EventManager::instance(); | |
| $eventManager->trackEvents(true); | |
| $eventManager->setEventList(new EventList()); |
| */ | ||
| public function test_collection(string $resultType): void | ||
| { | ||
| EventManager::instance()->trackEvents(true)->setEventList(new EventList()); |
There was a problem hiding this comment.
EventManager::trackEvents() returns a boolean flag, so chaining ->setEventList(...) will fail at runtime. Use separate calls to enable tracking and to set the EventList.
| EventManager::instance()->trackEvents(true)->setEventList(new EventList()); | |
| $eventManager = EventManager::instance(); | |
| $eventManager->trackEvents(true); | |
| $eventManager->setEventList(new EventList()); |
| public function setData(mixed $data): void | ||
| { | ||
| $this->data = $data; |
There was a problem hiding this comment.
setData() accepts mixed (and $data is stored as mixed), but getData() is still declared as ?array. If listeners set non-array data (or if the serializer is constructed with non-array data), getData() will throw a return type TypeError. Align the getData() return type (and its docblock) with the actual $data type, or constrain setData()/$data to arrays only.
| use \MixerApi\JsonLdView\JsonSerializer; | ||
|
|
||
| EventManager::instance()->on(JsonSerializer::BEFORE_SERIALIZE_EVENT, function (Event $event) { | ||
| /** @var Serializer $serializer */ |
There was a problem hiding this comment.
The docblock says @var Serializer $serializer, but the example uses JsonSerializer and there is no Serializer type imported. Update the annotation to JsonSerializer (or fully qualify it) to avoid confusion.
| /** @var Serializer $serializer */ | |
| /** @var JsonSerializer $serializer */ |
| EventManager::instance()->on(JsonSerializer::AFTER_SERIALIZE_EVENT, function (Event $event, string $data) { | ||
| // whatever you want to do here | ||
| }) |
There was a problem hiding this comment.
This afterSerialize listener example is missing a terminating semicolon after the EventManager::instance()->on(...) call, so the snippet is invalid PHP.
| EventManager::instance()->on(Serializer::BEFORE_SERIALIZE_EVENT, function (Event $event, string $type) { | ||
| /** @var Serializer $serializer */ | ||
| $serializer = $event->getSubject(); | ||
| $data = $serializer->getData(); // modify if you want or other stuff | ||
| $serializer->setData($data); | ||
| }) | ||
| ``` |
There was a problem hiding this comment.
The example listener registration is missing a terminating semicolon after the EventManager::instance()->on(...) call ();), so the snippet is invalid PHP as written.
| EventManager::instance()->on(JsonSerializer::BEFORE_SERIALIZE_EVENT, function (Event $event) { | ||
| /** @var Serializer $serializer */ | ||
| $serializer = $event->getSubject(); | ||
| $data = $serializer->getData(); // modify if you want or other stuff | ||
| $serializer->setData($data); | ||
| }) | ||
| ``` |
There was a problem hiding this comment.
The example listener registration is missing a terminating semicolon after the EventManager::instance()->on(...) call ();), so it’s not valid PHP as written.
| EventManager::instance()->on(JsonSerializer::BEFORE_SERIALIZE_EVENT, function (Event $event) { | ||
| /** @var Serializer $serializer */ | ||
| $serializer = $event->getSubject(); | ||
| $data = $serializer->getData(); // modify if you want or other stuff | ||
| $serializer->setData($data); | ||
| }) | ||
| ``` |
There was a problem hiding this comment.
The example listener registration is missing a terminating semicolon after the EventManager::instance()->on(...) call ();), so it’s not valid PHP as written.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Adds the following new events: