-
Notifications
You must be signed in to change notification settings - Fork 10
Description
Improvement description
Background
Three methods on AbstractElement currently use a double-underscore prefix to signal internal use and to avoid naming collisions with dynamically generated getter/setter methods on DataObject classes:
__getDataVersionTimestamp(): ?int__setDataVersionTimestamp(int $timestamp): void__isBasedOnLatestData(): bool
The __ prefix was a workaround (i guess). PHP reserves __ for magic methods, and static analysis tools (PHPStan, PhpStorm) correctly flag these as violations.
Proposed Solution
Introduce a DataVersion value object and replace all three methods with a single getDataVersion() accessor on AbstractElement:
final class DataVersion
{
public function __construct(
public readonly int $modificationDate,
public readonly int $versionCount,
) {}
}AbstractElement exposes:
public function getDataVersion(): ?DataVersion {}
public function setDataVersion(DataVersion $version): void {}Each DAO type implements isBasedOnLatestData() (without __) and fetches the
current DB state to compare against the snapshot (table name differs per type):
// DataObject/AbstractObject/Dao.php
public function isBasedOnLatestData(): bool
{
$data = $this->db->fetchAssociative(
'SELECT modificationDate, versionCount FROM objects WHERE id = ?',
[$this->model->getId()]
);
$version = $this->model->getDataVersion();
return $data !== false
&& $version !== null
&& $data['modificationDate'] == $version->modificationDate
&& $data['versionCount'] == $version->versionCount;
}versionCount is already publicly accessible via getVersionCount(). It does not need to move into DataVersion. DataVersion only wraps what was previously hidden behind __getDataVersionTimestamp().
opendxp/models/Element/AbstractElement.php
Lines 540 to 553 in 39da8f8
| public function __getDataVersionTimestamp(): ?int | |
| { | |
| return $this->__dataVersionTimestamp; | |
| } | |
| public function __setDataVersionTimestamp(int $_dataVersionTimestamp): void | |
| { | |
| $this->__dataVersionTimestamp = $_dataVersionTimestamp; | |
| } | |
| public function __isBasedOnLatestData(): bool | |
| { | |
| return $this->getDao()->__isBasedOnLatestData(); | |
| } |