Add attribute-based model data accessors

[giuscris]

This pull request introduces significant improvements to the data handling and attribute system in the codebase. The main changes include the introduction of new Getter and Setter attributes, refactoring of the Site model to use these attributes, and simplification of the AbstractCollection class by removing unnecessary trait indirections and directly using utility methods.

Attribute System Enhancements

• Introduced new Getter and Setter attributes in formwork/src/Data/Attributes/Getter.php and formwork/src/Data/Attributes/Setter.php to mark properties and methods for data access and mutation, enabling a more flexible and explicit attribute-based system. [1] [2]
• Refactored the Site model in formwork/src/Cms/Site.php to replace ReadonlyModelProperty with the new Getter and Setter attributes on relevant properties and methods, clarifying intent and improving maintainability. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20] [21] [22] [23]

Data Collection Refactoring

• Simplified AbstractCollection in formwork/src/Data/AbstractCollection.php by removing the use of DataArrayable and the indirection of baseHas, baseGet, baseSet, and baseRemove methods, now directly invoking the corresponding methods from the Arr utility class. [1] [2] [3] [4] [5] [6]
• Added a toArray() method to AbstractCollection for easier conversion to native arrays.

These changes collectively modernize the data access patterns, improve code clarity, and lay the groundwork for more robust and maintainable data models.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate accessor and serialization issues affect model, file, and image behavior.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR introduces attribute-based getters and setters, refactoring model data access and serialization across pages, sites, files, images, and fields.

Changes:

• Adds Getter, Setter, and cached reflection-based accessor discovery.
• Updates data and model traits to use explicit accessors.
• Deprecates legacy property-access patterns.

File summaries

File	Summary	Review findings
formwork/src/Pages/Traits/PageUid.php	Adds UID getter metadata.	No final findings.
formwork/src/Pages/Traits/PageTraversal.php	Adds traversal getter metadata.	Moderate (1): Site overrides lack inherited getter attributes.
formwork/src/Pages/Traits/PageStatus.php	Adds status getter metadata.	No final findings.
formwork/src/Pages/Page.php	Migrates page accessors.	No final findings.
formwork/src/Model/Model.php	Integrates accessor-based model operations and serialization.	Critical (2): serialization can read uninitialized File::$scheme. Moderate (2): backing properties may remain exported. Moderate (3): undefined dynamic calls change exception type.
formwork/src/Model/Attributes/ReadonlyModelProperty.php	Deprecates the legacy attribute.	Moderate (2): file-scope deprecation triggers during class autoload.
formwork/src/Images/Image.php	Migrates image accessors and serialization.	Critical (3): exported color-profile access can throw for GIF/SVG images. Moderate (3): serialization output contract changes. Moderate (1): path export can trigger processing and disk I/O.
formwork/src/Files/File.php	Migrates file accessors and serialization.	No final findings.
formwork/src/Fields/Field.php	Adds field defaults and accessor metadata.	No final findings.
formwork/src/Data/Traits/DataSetter.php	Supports explicit setters.	No final findings.
formwork/src/Data/Traits/DataGetter.php	Supports explicit getters.	No final findings.
formwork/src/Data/Traits/DataArrayable.php	Exports getter-backed data.	No final findings.
formwork/src/Data/Traits/DataAccessors.php	Discovers and caches accessors.	Critical (1): direct reads can access uninitialized lazy properties. Moderate (1): overridden Site methods lose getter attributes.
formwork/src/Data/Attributes/Setter.php	Defines setter metadata.	No final findings.
formwork/src/Data/Attributes/Getter.php	Defines getter metadata.	No final findings.
formwork/src/Cms/Site.php	Migrates site accessors.	No final findings.

Review details

Suppressed comments (4)

formwork/src/Data/Traits/DataAccessors.php:106

• ReflectionClass::getMethods() returns the concrete overriding methods, so the #[Getter] attributes declared on PageTraversal::parent(), siblings(), and inclusiveSiblings() are not present on the corresponding Site overrides. The resolver consequently does not register those Site accessors; Site::get()/toArray() fall back to the deprecated implicit property path instead of the methods. Add the attributes to the overrides or explicitly merge attributes from the overridden declarations.

        foreach ($reflectionClass->getMethods() as $method) {
            $name = $method->getName();

            foreach ($method->getAttributes(Getter::class) as $attribute) {
                $attribute = $attribute->newInstance();

formwork/src/Images/Image.php:345

• Model::toArray() now invokes every exported getter, so this accessor runs while serializing every image. GIF and SVG handlers explicitly throw UnsupportedFeatureException from getExifData(), making toArray() fail for valid images without EXIF support; the previous Image::toArray() did not call this method. Exclude this operational getter from export.

    #[Getter]

formwork/src/Images/Image.php:73

• This marks Image::path() as an exported getter, but that method calls process(), which can create directories, write a converted image, and clear the pending transforms. Model::toArray() will now perform image processing and disk I/O merely to serialize an image, and subsequent exported dimensions can describe the post-mutation original rather than the serialized path. Keep the raw path out of this accessor export or preserve a side-effect-free serializer.

    #[Getter]

formwork/src/Pages/Traits/PageTraversal.php:69

• Getter attributes are not inherited by overriding methods, so Site::parent(), siblings(), and inclusiveSiblings() remain unregistered despite this trait annotation. Site access and serialization therefore fall back to deprecated implicit property access and emit deprecation notices; annotate the Site overrides as well.

    #[Getter]

• Files reviewed: 16/16 changed files
• Comments generated: 7
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings affect duplication, accessor export, cache reset, and status semantics.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

formwork/src/Pages/Traits/PageStatus.php:31

• This raw-data lookup bypasses Model::get(), which resolves the published field and applies its value()/custom return() behavior. It also changes an explicit null from the previous get('published', true) semantics into true via ??. Keep the accessor call (or otherwise resolve through the field) so custom/dynamic published fields retain their behavior.
  formwork/src/Pages/Traits/PageTraversal.php:69
• These attributes are attached to the trait implementations, but Site overrides parent(), siblings(), and inclusiveSiblings() with unannotated methods. Reflection sees the overriding Site methods, so those keys are absent from Site's registered getters and fall back to the deprecated implicit-property path in get()/toArray(). Add #[Getter] to each override or make accessor resolution account for overridden methods.

formwork/src/Data/Traits/DataAccessors.php:25

• Page::resetProperties() iterates ReflectionClass($this)->getProperties() and resets every returned property without checking isStatic(). This new static cache is therefore treated as an instance property during Page::reload(); because it is private on Model, the child-scope $this->dataAccessors reset can fail (or create a dynamic property) instead of resetting page state. Filter static properties in that reset loop, or keep this cache outside the model's reflected properties.

    private static array $dataAccessors = [];

formwork/src/Model/Model.php:258

• The implicit-property list excludes accessor method/property names, but not accessor keys. For Page::hasLoaded() the registered key is loaded with export: false while the accessor name is hasLoaded; convertToArray(true) therefore still includes $loaded in $properties, calls get('loaded'), and exports the supposedly non-exported value (while also warning about implicit access). Exclude the registered getter keys as well as their names when building $properties.

        $properties = $includeAllProperties
            ? array_diff(
                array_keys(get_class_vars(static::class)),
                array_column($this->dataGetters(), 'name'),
                ['data', 'dataAccessors']
            )

formwork/src/Pages/Traits/PageStatus.php:31

• ?? true treats an explicit published: null as published. Before this change Arr::get(..., true) returned the present null, so the same unvalidated frontmatter was classified as not published. Preserve the distinction between an absent key and a present null, for example with array_key_exists().

        $published = $this->data['published'] ?? true;

• Files reviewed: 16/16 changed files
• Comments generated: 4
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate behavioral regressions remain in accessor, model, page, field, and traversal handling.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

formwork/src/Fields/Field.php:102

• After remove('formName'), this now calls get() without a fallback and returns null, violating the declared string return type (the previous implementation recomputed the default). Keep the fallback here or make the default non-removable.

        return $this->get('formName');

formwork/src/Fields/Field.php:174

• visible is now seeded in $data, but remove('visible') can still delete it; after that this calls is() with its false default, whereas the old behavior treated a missing visibility value as visible. Preserve the true fallback.

        return $this->is('visible');

formwork/src/Model/Model.php:104

• The old ReadonlyModelProperty markers were removed from the model's scheme and fields properties, but their public scheme() and fields() accessors are not registered with #[Getter]. Calls such as get('scheme')/get('fields') therefore fall through to the newly deprecated implicit-property path (and can produce deprecation warnings during legacy export) instead of using the explicit accessor system. Annotate the intended public accessors or keep these properties explicitly supported.

        if (isset($this->dataGetters()[$key])) {

formwork/src/Pages/Traits/PageStatus.php:31

• This changes the previous get('published', true) semantics: an explicitly null published value is now coalesced to true, so a page that was previously treated as not published becomes published. Retain the getter/default behavior (or distinguish a missing key from a null value).

        $published = $this->data['published'] ?? true;

formwork/src/Pages/Traits/PageTraversal.php:141

• Getter defaults export to true, and Model::convertToArray() invokes every exported getter. That means every Page::toArray() now calls descendants(), which loads the entire page tree via retrievePages(..., recursive: true) even when the caller only wants the page's scalar data. Mark the traversal getters (at least this recursive one) as non-exporting, or provide a separate explicit tree serialization path.

    #[Getter]

formwork/src/Pages/Traits/PageTraversal.php:69

• PHP attributes are not inherited by overriding methods. Site overrides parent(), siblings(), and inclusiveSiblings() without #[Getter], so these keys still use the deprecated implicit path on Site and are not registered by DataAccessors; annotate each override as well.

    #[Getter]

• Files reviewed: 16/16 changed files
• Comments generated: 3
• Review effort level: Lite