Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
2f0e37d
WIP1 Add Write support
BastianLedererIcinga May 28, 2026
8aceff4
WIP2 adjust models and some fixes
BastianLedererIcinga May 28, 2026
16b0df8
WIP3
BastianLedererIcinga May 28, 2026
eeb409f
fix php < 8.4
BastianLedererIcinga May 28, 2026
cc36ad5
minor improvements
BastianLedererIcinga May 28, 2026
209c665
Test improvement
BastianLedererIcinga May 28, 2026
3b994ca
Adjust docstring indentation
BastianLedererIcinga May 28, 2026
c8ec7c3
Do not apply persist Behaviors if no changes were made
BastianLedererIcinga May 28, 2026
0ac9659
Add test to ensure changed_at is only updated if the row was changed
BastianLedererIcinga May 28, 2026
b4dde84
Fix phpdoc
BastianLedererIcinga May 28, 2026
1db1b3d
WIP
BastianLedererIcinga May 28, 2026
d8dd5d0
Use hashmap for dirty lookups
BastianLedererIcinga May 28, 2026
893a8e9
Add tests for compounf keys and reinserts
BastianLedererIcinga May 28, 2026
1d434cf
Replace ChangedAt Behavior with a simple check for a changed_at column
BastianLedererIcinga May 28, 2026
192fc87
WIP test improvements and (int) cast in persist
BastianLedererIcinga May 29, 2026
616df17
restructure tests
BastianLedererIcinga May 29, 2026
fbf12e4
fix linting errors
BastianLedererIcinga May 29, 2026
d04b33c
`EntityMangertest`: fix codestyle
BastianLedererIcinga May 29, 2026
e34dba8
fix line length
BastianLedererIcinga May 29, 2026
7e59e4b
Dont set changedAt if a model wasn't modified
BastianLedererIcinga May 29, 2026
0f2bbf0
Cache writable columns per model
BastianLedererIcinga May 29, 2026
b7afa06
Fix many 2 many relations
BastianLedererIcinga May 29, 2026
706159c
refactor saveGraph
BastianLedererIcinga May 29, 2026
ccd20a8
Add missing license headers
BastianLedererIcinga May 29, 2026
18762fe
Unset key when a model is deleted
BastianLedererIcinga May 29, 2026
21e4be7
Check if the connection is already in a transaction
BastianLedererIcinga May 29, 2026
adff3f6
fix linting errors
BastianLedererIcinga May 29, 2026
a6daae5
fix linting
BastianLedererIcinga May 29, 2026
221630d
drop redundant guards
BastianLedererIcinga May 29, 2026
823fcaa
adjust docstring
BastianLedererIcinga May 29, 2026
d19bafa
Optimize inserts of many-to-many link rows
BastianLedererIcinga Jun 1, 2026
4b86067
Prevent duplicate junction table entries
BastianLedererIcinga Jun 1, 2026
9de2b04
Support soft deletes on junction tables
BastianLedererIcinga Jun 1, 2026
99bfe40
fix linting errors
BastianLedererIcinga Jun 1, 2026
835f1d7
fix codestyle
BastianLedererIcinga Jun 1, 2026
cf90e02
fix linting errors
BastianLedererIcinga Jun 2, 2026
5b70401
Support deleting related model when saving a model
BastianLedererIcinga Jun 8, 2026
57fbabf
Adjust Incident base class
BastianLedererIcinga Jun 8, 2026
06b9675
`EntityManager`: Adjust creation of timestamps
BastianLedererIcinga Jun 16, 2026
d704a6f
drop unused deleteOnSave
BastianLedererIcinga Jun 16, 2026
85c4e3b
Fix soft deletes on junction tables
BastianLedererIcinga Jun 16, 2026
4d80aa3
refactoring
BastianLedererIcinga Jun 16, 2026
483c6fe
Throw exception on modified PK
BastianLedererIcinga Jun 16, 2026
b662def
refactor modified relations
BastianLedererIcinga Jun 17, 2026
bd67282
refactor createCondition
BastianLedererIcinga Jun 17, 2026
c1eb22b
fix linting errors
BastianLedererIcinga Jun 17, 2026
ee4d6a4
Address Copilot review
BastianLedererIcinga Jun 17, 2026
0a00366
Support soft deletes
BastianLedererIcinga Jun 18, 2026
8d45a34
Fix writes on related models
BastianLedererIcinga Jun 18, 2026
ee2d576
Save modifed parent of deleted model
BastianLedererIcinga Jun 18, 2026
302f598
Adjust soft deletes and clean up code
BastianLedererIcinga Jun 22, 2026
813cf05
Fix return type for implementations of `Common\Model::on()`
BastianLedererIcinga Jun 22, 2026
f1e30ad
`EntitiyManager::saveGraph()`: Adjust indentation
BastianLedererIcinga Jun 23, 2026
857b244
`EntitiyManager`: Adjust `delete()`
BastianLedererIcinga Jun 23, 2026
3f4e1c9
`EntitiyManager::syncSoftDeleteJunction()`: Use `persist` istead `db-…
BastianLedererIcinga Jun 23, 2026
5239b79
`EnitiyManager`: Throw a `RuntimeException` when saving cyclic graphs
BastianLedererIcinga Jun 24, 2026
0578205
`EntityManager`: prefix own functions with `self::` in phpdoc
BastianLedererIcinga Jun 24, 2026
c5e8f9c
Adjust error message for cyclic graphs
BastianLedererIcinga Jun 25, 2026
19da108
Add getter for `changed_at`
BastianLedererIcinga Jun 25, 2026
4c6ceda
Minor refactoring
BastianLedererIcinga Jun 25, 2026
a0c9d05
Adjust docblocks
BastianLedererIcinga Jun 25, 2026
121b62b
remove `changed_at` dependecy from EntitiyManager comment
BastianLedererIcinga Jun 25, 2026
aee9a6e
Throw a `RuntimeException` when the `$isNew` flag is not set
BastianLedererIcinga Jun 26, 2026
dc424d0
`StatefulQuery::markLoaded()`: Guard against infinite recursion
BastianLedererIcinga Jul 1, 2026
b921378
refactor `markDeleted()` to `delete()`
BastianLedererIcinga Jul 1, 2026
bc83d29
Add `StatefulQuery::deleteAll()`
BastianLedererIcinga Jul 1, 2026
86df655
Make writes on ManyToMany relations additive
BastianLedererIcinga Jul 1, 2026
19acad0
fix linting errors
BastianLedererIcinga Jul 1, 2026
6628523
`Model`: Add `getRealColumnMap()`
BastianLedererIcinga Jul 1, 2026
38ae4e2
`EntityManager::delete()`: Adjust stale docstring
BastianLedererIcinga Jul 1, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
761 changes: 761 additions & 0 deletions library/Notifications/Common/EntityManager.php

Large diffs are not rendered by default.

234 changes: 234 additions & 0 deletions library/Notifications/Common/Model.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
<?php

// SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com>
// SPDX-License-Identifier: GPL-3.0-or-later

namespace Icinga\Module\Notifications\Common;

use ipl\Sql\Connection;
use ipl\Sql\ExpressionInterface;

/**
* Base class for all module models that tracks the changes made to a model
*
* Records which properties have changed since the model was loaded, and whether the model has been
* persisted yet, so the {@see EntityManager} can store a model and write only what actually changed.
*
* {@see self::setNew()} must be called explicitly when creating a new instance, it tells the {@see EntityManager}
* whether to insert or update
*/
Comment thread
sukhwinder33445 marked this conversation as resolved.
abstract class Model extends \ipl\Orm\Model

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add constants for deleted and changed_at. They're easier to override than methods.

{
/** @var ?bool Whether this model is newly created and does not yet exist in the database */
private ?bool $isNew = null;

/** @var bool Whether the model is marked for deletion on the next {@see EntityManager::save()} */
private bool $markedForDeletion = false;

/** @var array<string, true> Names of properties modified since the model was loaded */
private array $modifiedProperties = [];

/**
* Whether a getter for a Closure-backed property is currently resolving.
*
* {@see \ipl\Orm\Common\PropertiesWithDefaults::getProperty()} memoizes a resolved Closure by
* calling {@see setProperty()}. Without this guard, that internal write would be misread as a
* user-driven change and (worse) re-enter {@see setProperty()} recursively.
*
* @var bool
*/
private bool $resolvingProperty = false;

/**
* Get whether this entity is newly created and does not yet exist in the database
*
* @return ?bool
*/
public function isNew(): ?bool
{
return $this->isNew;
}

/**
* Set whether this entity is newly created and does not yet exist in the database
*
* @param bool $new
*
* @return $this
*/
public function setNew(bool $new = true): static
{
$this->isNew = $new;

return $this;
}

/**
* Get whether the entity, or the given property, has unsaved modifications
*
* Always returns false for new entities, which carry no change tracking.
*
* @param ?string $property The property to check, or null to check the whole entity
*
* @return bool
*/
public function isModified(?string $property = null): bool
{
if ($property === null) {
return ! empty($this->modifiedProperties);
}

return isset($this->modifiedProperties[$property]);
}

/**
* Get the names of all properties modified since the entity was loaded as a set keyed by name
*
* The keys may be columns or relations.
*
* @return array<string, true>
*/
public function getModifiedProperties(): array
{
return $this->modifiedProperties;
}

/**
* Reset change tracking and accept the current values as the new baseline
*
* @return $this
*/
public function clearModifiedProperties(): static
{
$this->modifiedProperties = [];
$this->markedForDeletion = false;

return $this;
}

/**
* Get whether the model's table uses soft deletes
*
* @return bool
*/
public function isSoftDeletable(): bool
{
return in_array('deleted', $this->getColumns(), true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should utilize getTableColumns

}

/**
* Mark the model for deletion on the next {@see EntityManager::save()} and return it
*
* If the model uses soft deletes this function must set the `deleted` property
*
* @return $this
*/
public function delete(): static
{
$this->markedForDeletion = true;
if ($this->isSoftDeletable()) {
$this->deleted = true;
}

return $this;
}

/**
* Get whether the model is marked for deletion on the next {@see EntityManager::save()}
*
* @return bool
*/
public function isMarkedForDeletion(): bool
{
return $this->markedForDeletion;
}

/**
* Get the model's real columns as a property => column map
*
* "Real" means that the column maps to an actual table column, and can be written to by the {@see EntityManager}
*
* @return array<string, string>
*/
public function getRealColumnMap(): array
{
$columns = [];
foreach ((array) $this->getKeyName() as $key) {
$columns[$key] = $key;
}

foreach ($this->getColumns() as $alias => $column) {
if ($column instanceof ExpressionInterface) {
continue;
}

if (is_int($alias)) {
$columns[$column] = $column;
} else {
$columns[$alias] = $column;
}
}

return $columns;
}
Comment on lines +146 to +173

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rather had a more static approach in mind. Differences in columns compared to the actual table columns manifest usually only as the primary key being missing. Expressions are possible, but really an edge case as at the moment none of our notifications models have any. So here are my suggestions:

  • Rename to getTableColumns: It's more in-line with getTableName and implies the relevance already in the name. You don't have to explain this so the doc comment gets shorter as well.
  • Let it return array_merge((array) $this->getKeyName(), $this->getColumns()) by default. Any concrete model should have to override this to ensure only strings are returned. The strings being keys, is a requirement of the EntityManager and should therefore its business, not the model's.


/**
* Get the column used to store the timestamp of the most recent modification to the row
*
* `changed_at` is the schema-wide convention, the {@see EntityManager} checks whether the column
* exists on the model before stamping it.
*
* @return string
*/
public function getChangedAtColumn(): string
{
return 'changed_at';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just noticed recently that a particular table had no changed_at yet. So isn't it possible that it does not exist elsewhere yet? And even if not, it may only be our schema's convention, remember that I want to see this as prototype for utilization elsewhere. So just like isSoftDeletable, this should be able to identify a missing changed_at and return null.

}

/**
* @param string $key The name of the property, which may be a column or a relation
*/
protected function getProperty(string $key): mixed
{
$wasResolving = $this->resolvingProperty;
$this->resolvingProperty = true;
try {
return parent::getProperty($key);
} finally {
$this->resolvingProperty = $wasResolving;
}
}

/**
* @param string $key The name of the property, which may be a column or a relation
*/
protected function setProperty(string $key, mixed $value): static
{
if (! $this->resolvingProperty && $this->isNew === false && ! isset($this->modifiedProperties[$key])) {
// Resolve the prior value via the trait's iterator, which skips Closure-valued properties.
// This avoids triggering lazy relation loaders just to capture change-tracking state.
$hadValue = false;
$original = null;
foreach ($this as $k => $v) {
if ($k === $key) {
$hadValue = true;
$original = $v;
break;
}
}

if (! $hadValue || $original !== $value) {
$this->modifiedProperties[$key] = true;
}
}

return parent::setProperty($key, $value);
}

public static function on(Connection $db): StatefulQuery

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more and more convinced that this should possibly be a new method: forUpdate(Connection $db): StatefulQuery

It clearly signals the intention and ::on keeps being the read-only case without the additional overhead.

{
return (new StatefulQuery())
->setDb($db)
->setModel(new static());
}
}
81 changes: 81 additions & 0 deletions library/Notifications/Common/StatefulQuery.php
Comment thread
nilmerg marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php

// SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com>
// SPDX-License-Identifier: GPL-3.0-or-later

namespace Icinga\Module\Notifications\Common;

use Generator;
use ipl\Orm\Query;

/**
* Mark models loaded from the db as not new, and can flag every result for deletion
*/
class StatefulQuery extends Query
{
/** @var bool Whether all yielded models should be marked for deletion */
protected bool $deleteAll = false;

/**
* Mark each yielded model as loaded, and for deletion when {@see self::deleteAll()} was set
*
* @inheritDoc
*
* @return Generator
*/
public function yieldResults(): Generator
{
foreach (parent::yieldResults() as $key => $model) {
if ($model instanceof Model) {
Comment thread
sukhwinder33445 marked this conversation as resolved.
$this->markLoaded($model);
if ($this->deleteAll) {
$model->delete();
}
}

yield $key => $model;
}
Comment thread
Copilot marked this conversation as resolved.
}

/**
* Mark each model as deleted when yielded by {@see self::yieldResults()}
*
* This only affects the root models themselves, not their eager-loaded relations
*
* @return $this
*/
public function deleteAll(): static
{
$this->deleteAll = true;

return $this;
}

/**
* Recursively mark the given model and its eagerly-loaded related models as loaded
*
* @param Model $model
*
* @return void
*/
private function markLoaded(Model $model): void
Comment thread
nilmerg marked this conversation as resolved.
{
if ($model->isNew() !== null) {
return;
}

$model->setNew(false);

foreach ($model as $value) {
if ($value instanceof Model) {
$this->markLoaded($value);
} elseif (is_array($value)) {
foreach ($value as $item) {
if ($item instanceof Model) {
$this->markLoaded($item);
}
}
}
}
}
}
2 changes: 1 addition & 1 deletion library/Notifications/Model/AvailableChannelType.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace Icinga\Module\Notifications\Model;

use ipl\Orm\Model;
use Icinga\Module\Notifications\Common\Model;
use ipl\Orm\Query;
use ipl\Orm\Relations;

Expand Down
2 changes: 1 addition & 1 deletion library/Notifications/Model/BrowserSession.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
namespace Icinga\Module\Notifications\Model;

use DateTime;
use Icinga\Module\Notifications\Common\Model;
use ipl\Orm\Behavior\MillisecondTimestamp;
use ipl\Orm\Behaviors;
use ipl\Orm\Model;

/**
* @property string $php_session_id
Expand Down
2 changes: 1 addition & 1 deletion library/Notifications/Model/Channel.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
namespace Icinga\Module\Notifications\Model;

use DateTime;
use Icinga\Module\Notifications\Common\Model;
use ipl\Orm\Behavior\BoolCast;
use ipl\Orm\Behavior\MillisecondTimestamp;
use ipl\Orm\Behaviors;
use ipl\Orm\Model;
use ipl\Orm\Query;
use ipl\Orm\Relations;
use ipl\Web\Widget\Icon;
Expand Down
6 changes: 3 additions & 3 deletions library/Notifications/Model/Contact.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
namespace Icinga\Module\Notifications\Model;

use DateTime;
use Icinga\Module\Notifications\Common\Model;
use ipl\Orm\Behavior\BoolCast;
use ipl\Orm\Behavior\MillisecondTimestamp;
use ipl\Orm\Behaviors;
use ipl\Orm\Model;
use ipl\Orm\Query;
use ipl\Orm\Relations;

Expand Down Expand Up @@ -87,7 +87,7 @@ public function createRelations(Relations $relations): void
->setCandidateKey('default_channel_id');

$relations->belongsToMany('incident', Incident::class)
->through('incident_contact')
->through(IncidentContact::class)
->setJoinType('LEFT');

$relations->hasMany('incident_contact', IncidentContact::class);
Expand All @@ -101,7 +101,7 @@ public function createRelations(Relations $relations): void
$relations->hasMany('contactgroup_member', ContactgroupMember::class);

$relations->belongsToMany('contactgroup', Contactgroup::class)
->through('contactgroup_member')
->through(ContactgroupMember::class)
->setJoinType('LEFT');
}
}
2 changes: 1 addition & 1 deletion library/Notifications/Model/ContactAddress.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
namespace Icinga\Module\Notifications\Model;

use DateTime;
use Icinga\Module\Notifications\Common\Model;
use ipl\Orm\Behavior\BoolCast;
use ipl\Orm\Behavior\MillisecondTimestamp;
use ipl\Orm\Behaviors;
use ipl\Orm\Model;
use ipl\Orm\Query;
use ipl\Orm\Relations;

Expand Down
Loading
Loading