diff --git a/components/ILIAS/Authentication/Authentication.php b/components/ILIAS/Authentication/Authentication.php index 6b52a8a2bb31..590117220d69 100644 --- a/components/ILIAS/Authentication/Authentication.php +++ b/components/ILIAS/Authentication/Authentication.php @@ -32,31 +32,23 @@ public function init( array | \ArrayAccess &$pull, array | \ArrayAccess &$internal, ): void { - // currently this is will be a session storage because we cannot store - // data on the client, see https://mantis.ilias.de/view.php?id=38503. - // @todo: this should be implemented by some proper key-value storage (or service). + $implement[KeyValueStorage\TransientStoragePort::class] = static fn() => + $internal[Authentication\KeyValueStorage\SessionStoragePort::class]; + $implement[UI\Storage::class] = static fn() => - new class () implements UI\Storage { - public function offsetExists(mixed $offset): bool - { - return \ilSession::has($offset); - } - public function offsetGet(mixed $offset): mixed - { - return \ilSession::get($offset); - } - public function offsetSet(mixed $offset, mixed $value): void - { - if (!is_string($offset)) { - throw new \InvalidArgumentException('Offset needs to be of type string.'); - } - \ilSession::set($offset, $value); - } - public function offsetUnset(mixed $offset): void - { - \ilSession::clear($offset); - } - }; + new Authentication\KeyValueStorage\UiStorageAdapter( + $use[KeyValueStorage\Factory::class]->transient()->requestCached( + new KeyValueStorage\StorageNamespace('ui.storage') + ) + ); + + $contribute[KeyValueStorage\StorageProvider::class] = static fn() => + new KeyValueStorage\Implementation\StorageProviderBridge( + KeyValueStorage\StorageBackend::TRANSIENT, + $use[KeyValueStorage\TransientStoragePort::class], + $pull[KeyValueStorage\Implementation\NamespacedStorageFactory::class], + $pull[KeyValueStorage\Implementation\RequestScopeCache::class], + ); $contribute[\ILIAS\Setup\Agent::class] = static fn() => new \ilAuthenticationSetupAgent( @@ -69,5 +61,8 @@ public function offsetUnset(mixed $offset): void new Component\Resource\ComponentJS($this, 'js/dist/SessionReminder.min.js'); $contribute[User\Settings\UserSettings::class] = fn() => new Authentication\UserSettings\Settings(); + + $internal[Authentication\KeyValueStorage\SessionStoragePort::class] = static fn() => + new Authentication\KeyValueStorage\SessionStoragePort(); } } diff --git a/components/ILIAS/Authentication/README.md b/components/ILIAS/Authentication/README.md index 0f78aacdf66a..f6abc18bacd9 100644 --- a/components/ILIAS/Authentication/README.md +++ b/components/ILIAS/Authentication/README.md @@ -51,3 +51,63 @@ state of a client, identified by its session ID. (regardless of user type). - Note that `isAuthenticated()` returns `true` for both logged-in and "Anonymous" users. + +## KeyValueStorage Contribution + +Authentication contributes the **transient** backend for +[`ILIAS\KeyValueStorage`](../KeyValueStorage/README.md): + +| Role | Class | +|---|---| +| `TransientStoragePort` | `Authentication\KeyValueStorage\SessionStoragePort` | +| `StorageProvider` | `StorageProviderBridge` with `StorageBackend::TRANSIENT` | +| `UI\Storage` | `Authentication\KeyValueStorage\UiStorageAdapter` (session-backed, request-cached) | + +Session data is the natural persistence mechanism for state that MUST NOT survive +logout. See [Design Decisions](#design-decisions) for how namespaces are laid out in +the session. + +## Design Decisions + +Significant architecture decisions for this component are recorded as lightweight +[Architecture Decision Records](https://github.com/joelparkerhenderson/architecture-decision-record) +(Michael Nygard's *Context / Decision / Consequences* format). Records are +append-only: supersede rather than rewrite. + +### ADR 0001 — Flat Session Keys for Transient KeyValueStorage + +**Status:** Accepted. + +**Context.** `SessionStoragePort` implements `TransientStoragePort` on top of +`ilSession`. The port MUST support namespace-scoped `clear()` (via +`clearNamespace()`), but `ilSession` only exposes single-key `get` / `set` / `has` / +`clear` — there is no API to list or clear keys by prefix. + +An alternative nested layout was considered — storing all entries under one session +root as `$_SESSION['__ilias_kv_storage__'][namespace][key]`. That would make +`clearNamespace()` an O(1) subtree drop without scanning the session, but every +`write` / `remove` would read-modify-write the **entire** root bucket. With ILIAS +session handling (all key/value pairs loaded at request start and written back at +request end), concurrent requests — e.g. parallel AJAX calls from one page — can +overwrite each other's in-flight changes to the same root array. Per-key updates via +`ilSession::set()` avoid that class of lost-update races. + +**Decision.** Use flat, per-entry session keys: + +```php +$_SESSION['__ilias_kv_storage__.{namespace}.{key}'] = encoded_value +``` + +`has`, `read`, `write`, and `remove` each call `ilSession` for exactly one key. +`clearNamespace()` is the sole exception: it iterates `$_SESSION` for keys matching +the namespace prefix and clears each match through `ilSession::clear()`. + +**Consequences.** + +- **+** Per-key writes are atomic at the session layer and less prone to concurrent lost-update races than a shared nested bucket. +- **+** `write` / `read` / `remove` use `ilSession` consistently. +- **−** `clearNamespace()` must scan session keys and accesses `$_SESSION` directly, because `ilSession` offers no prefix-based clearing. +- **−** Namespace clearing is O(n) in the number of session variables (typically acceptable — `clear()` is infrequent). + +**Revisit when** `ilSession` gains prefix-based clearing, or a session API appears that +supports safe partial updates of a structured bucket under concurrent requests. diff --git a/components/ILIAS/Authentication/src/KeyValueStorage/SessionStoragePort.php b/components/ILIAS/Authentication/src/KeyValueStorage/SessionStoragePort.php new file mode 100644 index 000000000000..aa5825572ca4 --- /dev/null +++ b/components/ILIAS/Authentication/src/KeyValueStorage/SessionStoragePort.php @@ -0,0 +1,76 @@ +buildSessionKey($namespace, $key)); + } + + public function read(StorageNamespace $namespace, string $key): ?string + { + $value = \ilSession::get($this->buildSessionKey($namespace, $key)); + + return \is_string($value) ? $value : null; + } + + public function write(StorageNamespace $namespace, string $key, string $value): void + { + \ilSession::set($this->buildSessionKey($namespace, $key), $value); + } + + public function remove(StorageNamespace $namespace, string $key): void + { + \ilSession::clear($this->buildSessionKey($namespace, $key)); + } + + public function clearNamespace(StorageNamespace $namespace): void + { + $prefix = self::SESSION_ROOT . '.' . $namespace->value() . '.'; + $session = $_SESSION ?? []; + + foreach (\array_keys($session) as $session_key) { + if (!\is_string($session_key) || !\str_starts_with($session_key, $prefix)) { + continue; + } + + \ilSession::clear($session_key); + } + } + + private function buildSessionKey(StorageNamespace $namespace, string $key): string + { + return self::SESSION_ROOT . '.' . $namespace->value() . '.' . $key; + } +} diff --git a/components/ILIAS/Authentication/src/KeyValueStorage/UiStorageAdapter.php b/components/ILIAS/Authentication/src/KeyValueStorage/UiStorageAdapter.php new file mode 100644 index 000000000000..c5afba37cd03 --- /dev/null +++ b/components/ILIAS/Authentication/src/KeyValueStorage/UiStorageAdapter.php @@ -0,0 +1,63 @@ +storage->has($this->assertStringOffset($offset)); + } + + public function offsetGet(mixed $offset): mixed + { + return $this->storage->get($this->assertStringOffset($offset)); + } + + public function offsetSet(mixed $offset, mixed $value): void + { + $this->storage->set($this->assertStringOffset($offset), $value); + } + + public function offsetUnset(mixed $offset): void + { + $this->storage->delete($this->assertStringOffset($offset)); + } + + private function assertStringOffset(mixed $offset): string + { + if (!\is_string($offset)) { + throw new \InvalidArgumentException('Offset needs to be of type string.'); + } + + return $offset; + } +} diff --git a/components/ILIAS/Authentication/tests/KeyValueStorage/SessionStoragePortTest.php b/components/ILIAS/Authentication/tests/KeyValueStorage/SessionStoragePortTest.php new file mode 100644 index 000000000000..8b64f3c84123 --- /dev/null +++ b/components/ILIAS/Authentication/tests/KeyValueStorage/SessionStoragePortTest.php @@ -0,0 +1,96 @@ +port = new SessionStoragePort(); + } + + protected function tearDown(): void + { + $_SESSION = []; + } + + public function testWriteUsesExpectedSessionKey(): void + { + $namespace = new StorageNamespace('ui.table'); + + $this->port->write($namespace, 'sort_column', 'encoded'); + + self::assertSame( + 'encoded', + $_SESSION['__ilias_kv_storage__.ui.table.sort_column'] ?? null + ); + } + + public function testWriteAndReadValue(): void + { + $namespace = new StorageNamespace('ui.table'); + + $this->port->write($namespace, 'sort_column', 'encoded'); + + self::assertTrue($this->port->has($namespace, 'sort_column')); + self::assertSame('encoded', $this->port->read($namespace, 'sort_column')); + } + + public function testReadReturnsNullForMissingKey(): void + { + self::assertNull( + $this->port->read(new StorageNamespace('ui.table'), 'missing') + ); + } + + public function testRemoveDeletesValue(): void + { + $namespace = new StorageNamespace('ui.table'); + $this->port->write($namespace, 'sort_column', 'encoded'); + + $this->port->remove($namespace, 'sort_column'); + + self::assertFalse($this->port->has($namespace, 'sort_column')); + self::assertArrayNotHasKey('__ilias_kv_storage__.ui.table.sort_column', $_SESSION); + } + + public function testClearNamespaceRemovesOnlyMatchingKeys(): void + { + $namespace = new StorageNamespace('ui.table'); + $other_namespace = new StorageNamespace('other.namespace'); + + $this->port->write($namespace, 'sort_column', 'encoded'); + $this->port->write($other_namespace, 'key', 'value'); + + $this->port->clearNamespace($namespace); + + self::assertFalse($this->port->has($namespace, 'sort_column')); + self::assertTrue($this->port->has($other_namespace, 'key')); + self::assertSame('value', $this->port->read($other_namespace, 'key')); + } +} diff --git a/components/ILIAS/Authentication/tests/KeyValueStorage/UiStorageAdapterTest.php b/components/ILIAS/Authentication/tests/KeyValueStorage/UiStorageAdapterTest.php new file mode 100644 index 000000000000..e727d0f2ebb7 --- /dev/null +++ b/components/ILIAS/Authentication/tests/KeyValueStorage/UiStorageAdapterTest.php @@ -0,0 +1,116 @@ +storage = new RecordingStorage(); + $this->adapter = new UiStorageAdapter($this->storage); + } + + public function testOffsetExistsDelegatesToHas(): void + { + $this->storage->has_result = true; + + self::assertTrue($this->adapter->offsetExists('view_state')); + self::assertSame('view_state', $this->storage->has_key); + } + + public function testOffsetGetReturnsStoredValue(): void + { + $this->storage->get_result = ['sort' => 'title']; + + self::assertSame(['sort' => 'title'], $this->adapter->offsetGet('view_state')); + self::assertSame('view_state', $this->storage->get_key); + } + + public function testOffsetSetWritesKeyAndValue(): void + { + $this->adapter->offsetSet('view_state', ['page' => 2]); + + self::assertSame('view_state', $this->storage->set_key); + self::assertSame(['page' => 2], $this->storage->set_value); + } + + public function testOffsetUnsetDeletesKey(): void + { + $this->adapter->offsetUnset('view_state'); + + self::assertSame('view_state', $this->storage->deleted_key); + } + + public function testOffsetExistsRejectsNonStringOffset(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Offset needs to be of type string.'); + + $this->adapter->offsetExists(42); + } +} + +final class RecordingStorage implements Storage +{ + public bool $has_result = false; + public mixed $get_result = null; + public ?string $has_key = null; + public ?string $get_key = null; + public ?string $set_key = null; + public mixed $set_value = null; + public ?string $deleted_key = null; + + public function has(string $key): bool + { + $this->has_key = $key; + + return $this->has_result; + } + + public function get(string $key, mixed $default = null): mixed + { + $this->get_key = $key; + + return $this->get_result; + } + + public function set(string $key, mixed $value): void + { + $this->set_key = $key; + $this->set_value = $value; + } + + public function delete(string $key): void + { + $this->deleted_key = $key; + } + + public function clear(): void + { + } +} diff --git a/components/ILIAS/Database/Database.php b/components/ILIAS/Database/Database.php index 4ff651bcd6b7..4df38c0d3970 100644 --- a/components/ILIAS/Database/Database.php +++ b/components/ILIAS/Database/Database.php @@ -36,9 +36,30 @@ public function init( array | \ArrayAccess &$pull, array | \ArrayAccess &$internal, ): void { + $implement[KeyValueStorage\PersistentStoragePort::class] = static fn() => + new Database\KeyValueStorage\DatabaseStoragePort( + $internal[Database\KeyValueStorage\DicDatabaseConnection::class] + ); + + $contribute[KeyValueStorage\StorageProvider::class] = static fn() => + new KeyValueStorage\Implementation\StorageProviderBridge( + KeyValueStorage\StorageBackend::PERSISTENT, + $use[KeyValueStorage\PersistentStoragePort::class], + $pull[KeyValueStorage\Implementation\NamespacedStorageFactory::class], + $pull[KeyValueStorage\Implementation\RequestScopeCache::class], + ); + $contribute[Agent::class] = static fn(): \ilDatabaseSetupAgent => new \ilDatabaseSetupAgent( $pull[Factory::class] ); + + $contribute[Agent::class] = static fn() => + new Database\KeyValueStorage\Setup\Agent( + $pull[Factory::class] + ); + + $internal[Database\KeyValueStorage\DicDatabaseConnection::class] = static fn() => + new Database\KeyValueStorage\DicDatabaseConnection(); } } diff --git a/components/ILIAS/Database/README.md b/components/ILIAS/Database/README.md index 15ef9c9d62a4..0496fd38bf91 100755 --- a/components/ILIAS/Database/README.md +++ b/components/ILIAS/Database/README.md @@ -389,3 +389,101 @@ $q = "SELECT * FROM " . $DIC->database()->quoteIdentifier('select'); ... ``` + +## KeyValueStorage Contribution + +Database contributes the **persistent** backend for +[`ILIAS\KeyValueStorage`](../KeyValueStorage/README.md): + +| Role | Class | +|---|---| +| `PersistentStoragePort` | `Database\KeyValueStorage\DatabaseStoragePort` | +| `StorageProvider` | `StorageProviderBridge` with `StorageBackend::PERSISTENT` | +| Setup / Schema | `Database\KeyValueStorage\Setup\DBUpdateSteps` (`il_kv_storage` table) | + +The port stores **opaque encoded strings** only — JSON encoding and key validation +are handled by KeyValueStorage (`NamespacedStorage`). Database owns the table schema +and setup steps. See [Design Decisions](#design-decisions) for schema and connection +choices. + +## Design Decisions + +Significant architecture decisions for this component are recorded as lightweight +[Architecture Decision Records](https://github.com/joelparkerhenderson/architecture-decision-record) +(Michael Nygard's *Context / Decision / Consequences* format). Records are +append-only: supersede rather than rewrite. + +### ADR 0001 — Composite-Key Table for Persistent KeyValueStorage + +**Status:** Accepted. + +**Context.** KeyValueStorage needs a durable backend whose contents survive session +boundaries until changed or cleared. Consumers isolate data by `StorageNamespace`; +within a namespace they address individual keys. The port MUST support +`clearNamespace()` without scanning unrelated namespaces. Column lengths MUST stay +within utf8mb4 InnoDB primary-key limits on supported MySQL and MariaDB versions. + +**Decision.** Use one greenfield table `il_kv_storage` with a composite primary key +on `(namespace, keyword)`: + +| Column | Type | Length | Role | +|---|---|---|---| +| `namespace` | `text` | 128 | Consumer isolation | +| `keyword` | `text` | 255 | Entry key within namespace | +| `value` | `clob` | — | Opaque encoded payload | + +Column names follow ILIAS conventions (`il_` table prefix) and avoid SQL reserved +words. The combined +key length (128 + 255 = 383 characters, 1532 bytes under utf8mb4) fits the 3072-byte +InnoDB index limit on MySQL 5.7+/8.x and MariaDB 10.3+. + +The lengths chosen here mirror the KeyValueStorage validation limits +(`StorageNamespace::MAX_LENGTH` and `KeyValidator::MAX_LENGTH`) at the time the table +was introduced. The migration step (`DBUpdateSteps`) intentionally **does not import +those constants**: a database update step describes a fixed historical change and must +remain stable even if the validation limits are tightened or relaxed later. The +constants are referenced here, in the design record, rather than in the migration code. +Keeping the two in sync for *new* steps is a deliberate, reviewable act. + +`write()` uses `replace` (upsert). `clearNamespace()` deletes by `namespace` only. +`has()` uses `SELECT EXISTS(...)` rather than reading the `value` column. + +**Consequences.** + +- **+** Namespace clearing is a single indexed `DELETE` — no full-table scan. +- **+** Per-entry reads and writes target one row; safe under concurrent requests. +- **+** The migration is self-contained; changing a KeyValueStorage constant cannot + retroactively alter an already-shipped schema step. +- **−** Schema length and validation limits are kept in sync by convention, not by a + shared symbol; a new migration step is required to widen a column. +- **−** No subject column — per-user persistent data MUST encode the user (or other + subject) in namespace or keyword; see KeyValueStorage README. +- **−** `value` is a CLOB; very large payloads are possible but not the intended use. + +**Revisit when** a dedicated subject column or secondary indexes (e.g. by subject for +bulk deletion) become a cross-cutting requirement. + +### ADR 0002 — Lazy Database Connection in DatabaseStoragePort + +**Status:** Accepted. + +**Context.** `DatabaseStoragePort` is registered during component wiring and MAY be +instantiated during build phases (`composer du`) where the global `$DIC` and +`$DIC->database()` are not yet available. Resolving the connection in `__construct` +would fail or couple the port to bootstrap order. + +**Decision.** Inject a `DatabaseConnection` abstraction (`DicDatabaseConnection` +resolves `$DIC->database()` on demand). Each port method calls +`$this->database_connection->get()` when it first needs the database — not in the +constructor. + +**Consequences.** + +- **+** The port can be constructed during autoload/build without a live database. +- **+** Keeps `$DIC` access in one small adapter; the port itself stays testable with + a stub connection. +- **−** Every port operation resolves the connection reference (cheap once `$DIC` is + up; connection pooling is handled by ILIAS). + +**Revisit when** the component wiring mechanism guarantees a database connection at +construction time for all registered services. diff --git a/components/ILIAS/Database/src/KeyValueStorage/DatabaseConnection.php b/components/ILIAS/Database/src/KeyValueStorage/DatabaseConnection.php new file mode 100644 index 000000000000..e9c09bfcb44d --- /dev/null +++ b/components/ILIAS/Database/src/KeyValueStorage/DatabaseConnection.php @@ -0,0 +1,29 @@ +database_connection->get(); + $namespace_value = $namespace->value(); + $result = $db->query( + 'SELECT EXISTS(SELECT 1 FROM ' . self::TABLE . + ' WHERE namespace = ' . $db->quote($namespace_value, \ilDBConstants::T_TEXT) . + ' AND keyword = ' . $db->quote($key, \ilDBConstants::T_TEXT) . ') AS row_exists' + ); + $row = $db->fetchAssoc($result); + + return (bool) ($row['row_exists'] ?? false); + } + + public function read(StorageNamespace $namespace, string $key): ?string + { + $db = $this->database_connection->get(); + $namespace_value = $namespace->value(); + $result = $db->query( + 'SELECT value FROM ' . self::TABLE . + ' WHERE namespace = ' . $db->quote($namespace_value, \ilDBConstants::T_TEXT) . + ' AND keyword = ' . $db->quote($key, \ilDBConstants::T_TEXT) + ); + $row = $db->fetchAssoc($result); + + if ($row === null) { + return null; + } + + return (string) $row['value']; + } + + public function write(StorageNamespace $namespace, string $key, string $value): void + { + $this->database_connection->get()->replace( + self::TABLE, + [ + 'namespace' => [\ilDBConstants::T_TEXT, $namespace->value()], + 'keyword' => [\ilDBConstants::T_TEXT, $key], + ], + [ + 'value' => [\ilDBConstants::T_CLOB, $value], + ] + ); + } + + public function remove(StorageNamespace $namespace, string $key): void + { + $db = $this->database_connection->get(); + $namespace_value = $namespace->value(); + $db->manipulate( + 'DELETE FROM ' . self::TABLE . + ' WHERE namespace = ' . $db->quote($namespace_value, \ilDBConstants::T_TEXT) . + ' AND keyword = ' . $db->quote($key, \ilDBConstants::T_TEXT) + ); + } + + public function clearNamespace(StorageNamespace $namespace): void + { + $db = $this->database_connection->get(); + $db->manipulate( + 'DELETE FROM ' . self::TABLE . + ' WHERE namespace = ' . $db->quote($namespace->value(), \ilDBConstants::T_TEXT) + ); + } +} diff --git a/components/ILIAS/Database/src/KeyValueStorage/DicDatabaseConnection.php b/components/ILIAS/Database/src/KeyValueStorage/DicDatabaseConnection.php new file mode 100644 index 000000000000..f94df274378e --- /dev/null +++ b/components/ILIAS/Database/src/KeyValueStorage/DicDatabaseConnection.php @@ -0,0 +1,34 @@ +database(); + } +} diff --git a/components/ILIAS/Database/src/KeyValueStorage/Setup/Agent.php b/components/ILIAS/Database/src/KeyValueStorage/Setup/Agent.php new file mode 100644 index 000000000000..1eda05d6215b --- /dev/null +++ b/components/ILIAS/Database/src/KeyValueStorage/Setup/Agent.php @@ -0,0 +1,41 @@ +database = $db; + } + + public function step_1(): void + { + if ($this->database->tableExists(self::TABLE)) { + return; + } + + $this->database->createTable(self::TABLE, [ + 'namespace' => [ + 'type' => ilDBConstants::T_TEXT, + 'length' => self::NAMESPACE_LENGTH, + 'notnull' => true, + ], + 'keyword' => [ + 'type' => ilDBConstants::T_TEXT, + 'length' => self::KEYWORD_LENGTH, + 'notnull' => true, + ], + 'value' => [ + 'type' => ilDBConstants::T_CLOB, + 'notnull' => false, + ], + ]); + + $this->database->addPrimaryKey(self::TABLE, ['namespace', 'keyword']); + } +} diff --git a/components/ILIAS/Database/tests/KeyValueStorage/DatabaseStoragePortTest.php b/components/ILIAS/Database/tests/KeyValueStorage/DatabaseStoragePortTest.php new file mode 100644 index 000000000000..a99d0f0a751d --- /dev/null +++ b/components/ILIAS/Database/tests/KeyValueStorage/DatabaseStoragePortTest.php @@ -0,0 +1,190 @@ +createMock(DatabaseConnection::class); + $database_connection->expects(self::never()) + ->method('get'); + + new DatabaseStoragePort($database_connection); + } + + public function testWriteResolvesDatabaseConnectionOnce(): void + { + $db = $this->createMock(\ilDBInterface::class); + $db->expects(self::once()) + ->method('replace') + ->with( + 'il_kv_storage', + [ + 'namespace' => [\ilDBConstants::T_TEXT, 'ui.table'], + 'keyword' => [\ilDBConstants::T_TEXT, 'sort_column'], + ], + [ + 'value' => [\ilDBConstants::T_CLOB, 'encoded'], + ] + ); + + $database_connection = $this->createMock(DatabaseConnection::class); + $database_connection->expects(self::once()) + ->method('get') + ->willReturn($db); + + $port = new DatabaseStoragePort($database_connection); + $port->write(new StorageNamespace('ui.table'), 'sort_column', 'encoded'); + } + + public function testReadReturnsStoredValue(): void + { + $db = $this->createMock(\ilDBInterface::class); + $db->expects(self::once()) + ->method('query') + ->with( + "SELECT value FROM il_kv_storage WHERE namespace = 'ui.table' AND keyword = 'sort_column'" + ) + ->willReturn($this->createStub(\ilDBStatement::class)); + $db->expects(self::once()) + ->method('fetchAssoc') + ->willReturn(['value' => 'encoded']); + $db->method('quote')->willReturnCallback(static fn(string $value): string => "'" . $value . "'"); + + $port = $this->createPort($db); + + self::assertSame('encoded', $port->read(new StorageNamespace('ui.table'), 'sort_column')); + } + + public function testReadReturnsNullWhenRowMissing(): void + { + $db = $this->createMock(\ilDBInterface::class); + $db->expects(self::once()) + ->method('query') + ->willReturn($this->createStub(\ilDBStatement::class)); + $db->expects(self::once()) + ->method('fetchAssoc') + ->willReturn(null); + + $port = $this->createPort($db); + + self::assertNull($port->read(new StorageNamespace('ui.table'), 'missing')); + } + + public function testHasUsesExistsQuery(): void + { + $db = $this->createMock(\ilDBInterface::class); + $db->expects(self::once()) + ->method('query') + ->with( + "SELECT EXISTS(SELECT 1 FROM il_kv_storage WHERE namespace = 'ui.table' " + . "AND keyword = 'sort_column') AS row_exists" + ) + ->willReturn($this->createStub(\ilDBStatement::class)); + $db->expects(self::once()) + ->method('fetchAssoc') + ->willReturn(['row_exists' => 1]); + $db->method('quote')->willReturnCallback(static fn(string $value): string => "'" . $value . "'"); + + $port = $this->createPort($db); + + self::assertTrue($port->has(new StorageNamespace('ui.table'), 'sort_column')); + } + + public function testHasReturnsFalseWhenExistsIsZero(): void + { + $db = $this->createMock(\ilDBInterface::class); + $db->expects(self::once()) + ->method('query') + ->willReturn($this->createStub(\ilDBStatement::class)); + $db->expects(self::once()) + ->method('fetchAssoc') + ->willReturn(['row_exists' => 0]); + $db->method('quote')->willReturnCallback(static fn(string $value): string => "'" . $value . "'"); + + $port = $this->createPort($db); + + self::assertFalse($port->has(new StorageNamespace('ui.table'), 'missing')); + } + + public function testHasReturnsFalseWhenResultRowMissing(): void + { + $db = $this->createMock(\ilDBInterface::class); + $db->expects(self::once()) + ->method('query') + ->willReturn($this->createStub(\ilDBStatement::class)); + $db->expects(self::once()) + ->method('fetchAssoc') + ->willReturn(null); + $db->method('quote')->willReturnCallback(static fn(string $value): string => "'" . $value . "'"); + + $port = $this->createPort($db); + + self::assertFalse($port->has(new StorageNamespace('ui.table'), 'missing')); + } + + public function testRemoveDeletesByNamespaceAndKey(): void + { + $db = $this->createMock(\ilDBInterface::class); + $db->expects(self::once()) + ->method('manipulate') + ->with( + "DELETE FROM il_kv_storage WHERE namespace = 'ui.table' AND keyword = 'sort_column'" + ); + $db->method('quote')->willReturnCallback(static fn(string $value): string => "'" . $value . "'"); + + $port = $this->createPort($db); + $port->remove(new StorageNamespace('ui.table'), 'sort_column'); + } + + public function testClearNamespaceDeletesByNamespace(): void + { + $db = $this->createMock(\ilDBInterface::class); + $db->expects(self::once()) + ->method('manipulate') + ->with( + 'DELETE FROM il_kv_storage WHERE namespace = ' . + "'ui.table'" + ); + + $db->method('quote')->willReturnCallback(static fn(string $value): string => "'" . $value . "'"); + + $port = $this->createPort($db); + $port->clearNamespace(new StorageNamespace('ui.table')); + } + + private function createPort(MockObject&\ilDBInterface $db): DatabaseStoragePort + { + $database_connection = $this->createMock(DatabaseConnection::class); + $database_connection->expects(self::once()) + ->method('get') + ->willReturn($db); + + return new DatabaseStoragePort($database_connection); + } +} diff --git a/components/ILIAS/KeyValueStorage/KeyValueStorage.php b/components/ILIAS/KeyValueStorage/KeyValueStorage.php new file mode 100644 index 000000000000..bd94447d1f79 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/KeyValueStorage.php @@ -0,0 +1,70 @@ + + $internal[KeyValueStorage\Implementation\Factory::class]; + + $provide[KeyValueStorage\Implementation\NamespacedStorageFactory::class] = static fn() => + $internal[KeyValueStorage\Implementation\NamespacedStorageFactory::class]; + + $provide[KeyValueStorage\Implementation\RequestScopeCache::class] = static fn() => + $internal[KeyValueStorage\Implementation\RequestScopeCache::class]; + + $internal[KeyValueStorage\Implementation\Factory::class] = static fn() => + new KeyValueStorage\Implementation\Factory( + $seek[KeyValueStorage\StorageProvider::class] + ); + + $internal[KeyValueStorage\Implementation\NamespacedStorageFactory::class] = static fn() => + new KeyValueStorage\Implementation\NamespacedStorageFactory( + $internal[KeyValueStorage\KeyValidator::class], + $internal[KeyValueStorage\ValueCodec::class], + ); + + $internal[KeyValueStorage\Implementation\RequestScopeCache::class] = static fn() => + new KeyValueStorage\Implementation\RequestScopeCache(); + + $internal[KeyValueStorage\KeyValidator::class] = static fn() => + new KeyValueStorage\KeyValidator(); + + $internal[KeyValueStorage\ValueCodec::class] = static fn() => + new KeyValueStorage\ValueCodec(); + } +} diff --git a/components/ILIAS/KeyValueStorage/README.md b/components/ILIAS/KeyValueStorage/README.md new file mode 100644 index 000000000000..5cc9a1926305 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/README.md @@ -0,0 +1,756 @@ +# KeyValueStorage + +KeyValueStorage provides a unified **namespace-scoped key-value API** for +application state that MUST be retained until explicitly changed or cleared. + +It is **not** a cache. Use `ILIAS\Cache` for derived, evictable performance data. + +The component offers two storage lifetimes (**transient** and **persistent**), +public consumer interfaces, and ports for backend contributors. It does **not** +implement persistence itself — that is contributed by other components. + +The key words “MUST”, “MUST NOT”, “REQUIRED”, “SHALL”, “SHALL NOT”, “SHOULD”, +“SHOULD NOT”, “RECOMMENDED”, “MAY”, and “OPTIONAL” in this document are to be +interpreted as described in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt). + +**Table of Contents** +* [General](#general) + * [Audience](#audience) +* [Quick Start: Consumers](#quick-start-consumers) +* [Quick Start: Backend Providers](#quick-start-backend-providers) +* [Concepts](#concepts) + * [Backends](#backends) + * [Namespaces and Keys](#namespaces-and-keys) + * [Per-User and Per-Subject Data](#per-user-and-per-subject-data) + * [Value Encoding](#value-encoding) + * [Request-Scoped Caching](#request-scoped-caching) +* [Choosing the Right Storage](#choosing-the-right-storage) + * [KeyValueStorage vs Cache](#keyvaluestorage-vs-cache) + * [KeyValueStorage vs ilSetting](#keyvaluestorage-vs-ilsetting) +* [Public API Reference](#public-api-reference) + * [Factory](#factory) + * [Storages](#storages) + * [Storage](#storage) + * [StorageNamespace](#storagenamespace) + * [Defined Services](#defined-services) + * [Other Public Types](#other-public-types) +* [Backend Provider Reference](#backend-provider-reference) + * [StoragePort Contract](#storageport-contract) + * [StorageProvider Contribution](#storageprovider-contribution) + * [Layer Stack](#layer-stack) +* [Component Integration](#component-integration) + * [Runtime Sequence](#runtime-sequence) + * [Class Structure](#class-structure) +* [Component Layout](#component-layout) +* [Error Handling](#error-handling) +* [PSR Alignment](#psr-alignment) +* [Design Decisions](#design-decisions) + * [ADR 0001 — Storage Lifetime Is a Curated, Internal Taxonomy](#adr-0001--storage-lifetime-is-a-curated-internal-taxonomy) +* [Tests](#tests) + +## General + +Consumers obtain a `Storage` instance from the contributed `Factory` by choosing a +lifetime (`transient()` or `persistent()`) and a namespace, then read or write +values through a PSR-16-shaped API. + +Backend providers implement a storage port in their component, contribute a +`StorageProvider`, and own any setup or schema required by their persistence +mechanism. + +### Audience + +| Role | Start Here | +|---|---| +| **Consumer** (feature/component code storing state) | [Quick Start: Consumers](#quick-start-consumers) | +| **Backend Provider** (component owning persistence) | [Quick Start: Backend Providers](#quick-start-backend-providers) | + +## Quick Start: Consumers + +Obtain a `Storage` instance from the contributed `Factory` by selecting a lifetime +and a namespace, then use the PSR-16-shaped API: + +```php +use ILIAS\KeyValueStorage\Factory; +use ILIAS\KeyValueStorage\Storage; +use ILIAS\KeyValueStorage\StorageNamespace; + +/** @var Factory $factory */ // $use[ILIAS\KeyValueStorage\Factory::class] + +$storage = $factory->transient()->storage( + new StorageNamespace('my_component.view_state') +); + +$storage->set('sort_column', 'title'); +$storage->set('filters', ['status' => 'open', 'limit' => 10]); + +if ($storage->has('sort_column')) { + $column = $storage->get('sort_column', 'id'); +} + +$storage->delete('sort_column'); +$storage->clear(); // all keys in this namespace only +``` + +**Choose the Lifetime** + +| Need | Accessor | +|---|---| +| State tied to the current user session | `$factory->transient()` | +| State that survives session boundaries | `$factory->persistent()` | + +Both accessors return a `Storages` object exposing `storage()` and `requestCached()`. +The underlying backend identifier is an implementation detail and not part of the +consumer API. + +Consumers SHOULD choose **one namespace per feature area** — see +[Namespaces and Keys](#namespaces-and-keys). Use a stable, unique identifier +(e.g. `my_component.view_state`). + +**Storable Values** — scalars, arrays, `null`, and objects implementing +`JsonSerializable`. See [Value Encoding](#value-encoding). + +**Opt-In Request Cache** — `storage()` talks to the backend on every operation. +If a value is read repeatedly within a single HTTP request, request the explicitly +decorated variant instead, which memoizes reads and writes through for the duration +of the request: + +```php +$storage = $factory->persistent()->requestCached( + new StorageNamespace('my_component.view_state') +); +``` + +See [Request-Scoped Caching](#request-scoped-caching). This is **not** a +cross-request cache — use `ILIAS\Cache` for that. + +If the requested lifetime has no contributing backend, `Factory::transient()` and +`Factory::persistent()` MUST raise `StorageNotAvailableException`. + +## Quick Start: Backend Providers + +To supply a backend: + +1. **Implement** `TransientStoragePort` or `PersistentStoragePort` in your component + (transport opaque encoded strings; see [StoragePort Contract](#storageport-contract)). +2. **`$implement`** the port interface pointing to your implementation. +3. **`$contribute`** a `StorageProvider` using `StorageProviderBridge`. +4. **`$pull`** `NamespacedStorageFactory` and `RequestScopeCache` from KeyValueStorage. +5. Own **setup/schema** for your persistence mechanism in your component. + +**Transient Backend** + +```php +// MyComponent.php — init() + +$implement[KeyValueStorage\TransientStoragePort::class] = static fn() => + $internal[MyComponent\KeyValueStorage\MyTransientStoragePort::class]; + +$contribute[KeyValueStorage\StorageProvider::class] = static fn() => + new KeyValueStorage\Implementation\StorageProviderBridge( + KeyValueStorage\StorageBackend::TRANSIENT, + $use[KeyValueStorage\TransientStoragePort::class], + $pull[KeyValueStorage\Implementation\NamespacedStorageFactory::class], + $pull[KeyValueStorage\Implementation\RequestScopeCache::class], + ); + +$internal[MyComponent\KeyValueStorage\MyTransientStoragePort::class] = static fn() => + new MyComponent\KeyValueStorage\MyTransientStoragePort(/* dependencies */); +``` + +**Persistent Backend** + +```php +$implement[KeyValueStorage\PersistentStoragePort::class] = static fn() => + $internal[MyComponent\KeyValueStorage\MyPersistentStoragePort::class]; + +$contribute[KeyValueStorage\StorageProvider::class] = static fn() => + new KeyValueStorage\Implementation\StorageProviderBridge( + KeyValueStorage\StorageBackend::PERSISTENT, + $use[KeyValueStorage\PersistentStoragePort::class], + $pull[KeyValueStorage\Implementation\NamespacedStorageFactory::class], + $pull[KeyValueStorage\Implementation\RequestScopeCache::class], + ); + +$internal[MyComponent\KeyValueStorage\MyPersistentStoragePort::class] = static fn() => + new MyComponent\KeyValueStorage\MyPersistentStoragePort(/* dependencies */); +``` + +Each contributed `StorageProvider` SHOULD return a **distinct** `StorageBackend` +(the internal lifetime identifier used for resolution). If two providers share a +backend, the last contribution wins — see +[Backends](#backends). Providers MUST NOT reimplement validation, encoding, or +request caching — the bridge applies `NamespacedStorage` on every call and +`RequestScopedStorage` only when the consumer asks for it via `requestCached()`. + +Full details: [Backend Provider Reference](#backend-provider-reference). + +## Concepts + +### Backends + +| Lifetime | Consumer Accessor | Port | Internal Backend | Typical Lifetime | +|---|---|---|---|---| +| Transient | `Factory::transient()` | `TransientStoragePort` | `StorageBackend::TRANSIENT` | Current user session | +| Persistent | `Factory::persistent()` | `PersistentStoragePort` | `StorageBackend::PERSISTENT` | Until changed or cleared | + +KeyValueStorage defines ports and seeks `StorageProvider` contributions. Concrete +persistence lives in contributing components. `StorageBackend` is `@internal`: it +wires providers to lifetimes and is never referenced by consumers. Implementation +details and contributor-specific design decisions are documented in the contributing +components (e.g. transient/session layout in +[`Authentication/README.md`](../Authentication/README.md#adr-0001--flat-session-keys-for-transient-keyvaluestorage), +persistent/database schema in +[`Database/README.md`](../Database/README.md#adr-0001--composite-key-table-for-persistent-keyvaluestorage)). + +**Multiple Providers for the Same Lifetime** — each lifetime is served by exactly +one provider. The factory indexes contributions by their backend, so if more than +one provider is contributed for the same lifetime, the **last contribution wins** +(deterministic by contribution order). This makes deliberate overrides possible +(a component replacing the default persistent backend), but contributing two +*unrelated* providers for the same lifetime is a configuration error and SHOULD be +avoided — there is no merging or fallback between backends of the same lifetime. + +### Namespaces and Keys + +**Namespaces** isolate keys between consumers within the same backend. + +```php +new StorageNamespace('my_component.view_state'); +``` + +Format: dot-separated lowercase identifier, starting with a letter. + +``` +my_component.view_state +ui.table +export.job +``` + +Invalid: empty string, uppercase, leading digits (`1invalid`), hyphens, more than +`StorageNamespace::MAX_LENGTH` (128) characters. + +For the persistent backend, the combined namespace and keyword length MUST fit within +`StorageIdentifierLimits::MAX_UTF8MB4_INNODB_PRIMARY_KEY_CHARACTERS` (768 characters +under utf8mb4 on modern InnoDB; 128 + 255 = 383 today). + +**Keys** follow [PSR-16](https://www.php-fig.org/psr/psr-16/) rules (enforced by +`KeyValidator` on every `Storage` operation): + +- Non-empty string, at most `KeyValidator::MAX_LENGTH` (255) characters +- MUST NOT contain `{}()/\@:` (PSR-16 reserved characters) + +`clear()` removes all keys for the **current namespace only**. + +### Per-User and Per-Subject Data + +KeyValueStorage does **not** attach a subject (user, role, object) to stored values. +The transient backend is implicitly scoped to the current user session, but the +**persistent backend is global**. Consumers that persist values for a specific +subject — e.g. a user's UI preferences — MUST encode that subject into the +namespace or key themselves. + +```php +// Subject in the KEY — one namespace holds keys for many users: +$storage = $factory->persistent()->storage( + new StorageNamespace('my_component.user_prefs') +); +$storage->set("u{$user_id}.collapsed_panels", $panels); + +// Subject in the NAMESPACE — clear() then wipes exactly one user at once: +$storage = $factory->persistent()->storage( + new StorageNamespace("my_component.user_prefs.u{$user_id}") +); +$storage->clear(); // removes only this user's preferences +``` + +Encode the subject in the **key** when a single namespace serves many subjects, and +in the **namespace** when you want `clear()` to remove all of one subject's keys at +once. Namespace segments MUST start with a letter, so numeric ids MUST be prefixed +(e.g. `u{$user_id}`). Mind the length limits (namespace 128, key 255 characters) when +composing identifiers from subject ids. + +### Value Encoding + +Consumers pass native PHP values to `Storage`. Before data reaches a port, +`NamespacedStorage` encodes via `ValueCodec`: + +| Type | Supported | +|---|---| +| `null`, `bool`, `int`, `float`, `string` | yes | +| `array` (nested; string or int keys) | yes | +| `JsonSerializable` | yes — output of `jsonSerialize()` MUST be JSON-compatible | +| other `object` / `resource` | **no** — `InvalidArgumentException` on `set` | + +Encoding uses `json_encode()`; decoding uses `json_decode(..., true)`. PHP +`serialize()` / `unserialize()` MUST NOT be used — no object instantiation on +read. Invalid stored JSON MUST raise `InvalidStoragePayloadException`. + +Example stored payload for an array: + +```json +{"sort":"title","direction":"asc","count":3} +``` + +Ports see **opaque encoded strings** only. + +### Request-Scoped Caching + +Request-scoped caching is an **opt-in decorator**, not implicit. `Storages::storage()` +returns storage that talks to the backend on every operation. `Storages::requestCached()` +returns the same storage wrapped in `RequestScopedStorage` (write-through, request-local), +analogous to a first-level / in-request cache in modern frameworks (e.g. Doctrine's +first-level cache or Symfony's in-memory array adapter). + +Repeated reads within one HTTP request then avoid extra port access. Cache scope: +`{backend}:{namespace}` (e.g. `transient:ui.table`). + +Cross-request caching is **out of scope** — use `ILIAS\Cache`. + +| Operation | Behaviour (Request-Cached Variant) | +|---|---| +| `get` | Cache miss: read port, populate cache. Hit: return cached value. | +| `set` | Write to port and update cache. | +| `delete` | Delete from port and remove from cache. | +| `clear` | Clear port namespace and drop cache bucket. | + +```php +$storages = $factory->transient(); // or $factory->persistent() + +// Plain: every read/write hits the backend. +$storage = $storages->storage($namespace); + +// Decorated: reads memoized, writes written through, for this request only. +$cached = $storages->requestCached($namespace); +``` + +## Choosing the Right Storage + +### KeyValueStorage vs Cache + +| Concern | KeyValueStorage | Cache (`ILIAS\Cache`) | +|---|---|---| +| Purpose | Application state | Performance optimisation | +| Durability | Retained until changed | May be evicted anytime | +| TTL | Backend-specific | First-class | +| Typical data | Wizard state, UI preferences | Query results, language strings | + +Consumers MUST use KeyValueStorage for application state and MUST NOT treat it +as a cache. Use `ILIAS\Cache` when data MAY be discarded without functional impact. + +### KeyValueStorage vs ilSetting + +| Concern | KeyValueStorage | `ilSetting` | +|---|---|---| +| Purpose | Component-owned runtime state | Installation and module configuration | +| Data shape | JSON-encoded PHP values | Scalar strings | +| Identification | Namespace + key | Module + keyword | +| Typical use | Developer-controlled state | Legacy configuration API | + +Consumers SHOULD use KeyValueStorage for runtime state, structured values, and +transient scope. Use `ilSetting` for existing module configuration accessed via +the `ilSetting` API (module + keyword). + +There is **no plan to replace `ilSetting`**. Consumers MUST NOT migrate existing +`ilSetting` entries unless a functional redesign explicitly moves configuration +to runtime state. + +## Public API Reference + +### Factory + +```php +interface Factory +{ + public function transient(): Storages; + + public function persistent(): Storages; +} +``` + +Defined via `$define`, implemented by KeyValueStorage. Consumers `$use` this interface. + +| Method | Returns | +|---|---| +| `transient()` | `Storages` for the session-scoped lifetime | +| `persistent()` | `Storages` for the durable lifetime | + +Both raise `StorageNotAvailableException` if no backend contributed the lifetime. + +### Storages + +```php +interface Storages +{ + public function storage(StorageNamespace $namespace): Storage; + + public function requestCached(StorageNamespace $namespace): Storage; +} +``` + +Returned by `Factory::transient()` / `Factory::persistent()`. + +| Method | Returns | +|---|---| +| `storage()` | Storage that reads/writes the backend on every operation | +| `requestCached()` | Storage decorated with an in-request (first-level) cache | + +### Storage + +Namespace-scoped consumer interface (PSR-16 operation shape, not a cache): + +| Method | Description | +|---|---| +| `has(string $key): bool` | Key exists in this namespace | +| `get(string $key, mixed $default = null): mixed` | Read value or default | +| `set(string $key, mixed $value): void` | Write value | +| `delete(string $key): void` | Remove one key | +| `clear(): void` | Remove all keys in this namespace | + +All key operations validate via `KeyValidator` and encode/decode via `ValueCodec`. + +### StorageNamespace + +Value object with `value(): string` and `Stringable`. Validated on construction. +Maximum length: `StorageNamespace::MAX_LENGTH` (128). + +### KeyValidator + +Validates storage keys on every `Storage` operation. Maximum length: +`KeyValidator::MAX_LENGTH` (255). Enforces PSR-16 reserved-character rules. + +### Defined Services + +| Interface | Role | +|---|---| +| `ILIAS\KeyValueStorage\Factory` | Consumer entry point (`$define`) | +| `ILIAS\KeyValueStorage\TransientStoragePort` | Implemented by transient contributor (`$define`) | +| `ILIAS\KeyValueStorage\PersistentStoragePort` | Implemented by persistent contributor (`$define`) | + +### Other Public Types + +| Type | Role | +|---|---| +| `Storages` | Lifetime accessor returned by `Factory` (`storage()`, `requestCached()`) | +| `StorageBackend` | `@internal` `TRANSIENT` \| `PERSISTENT` enum (backend wiring only) | +| `StorageProvider` | Contribution interface for backends (`extends Storages`) | +| `StoragePort` | Low-level port (opaque strings) | +| `KeyValidator` | PSR-16 key rules and length limit (for port authors sharing validation) | +| `StorageIdentifierLimits` | Shared utf8mb4 InnoDB primary-key character ceiling (768) | +| `ValueCodec` | JSON encode/decode (used internally by `NamespacedStorage`) | + +## Backend Provider Reference + +### StoragePort Contract + +Port implementations MUST transport **opaque encoded strings**. They MUST NOT +encode or decode PHP values — `NamespacedStorage` handles that. + +```php +interface StoragePort +{ + public function has(StorageNamespace $namespace, string $key): bool; + + public function read(StorageNamespace $namespace, string $key): ?string; + + public function write(StorageNamespace $namespace, string $key, string $value): void; + + public function remove(StorageNamespace $namespace, string $key): void; + + public function clearNamespace(StorageNamespace $namespace): void; +} +``` + +`TransientStoragePort` and `PersistentStoragePort` extend `StoragePort` as marker +interfaces — choose the one matching your backend's lifetime semantics. + +**Implementor Guidelines** + +- Data MUST be isolated by `StorageNamespace` — MUST NOT mix namespaces in storage keys. +- Stored strings MUST be treated as opaque; ports MUST NOT parse JSON. +- `read()` MUST return `null` when the key is absent. +- `clearNamespace()` MUST remove all keys for one namespace only. +- Ports MUST NOT call `$DIC` — inject dependencies via constructor. + +### StorageProvider Contribution + +Contributors register via `StorageProviderBridge`, which: + +1. Binds your port to a `StorageBackend` value (the internal lifetime identifier). +2. Wraps `NamespacedStorage` (validation + encoding) for `storage()`. +3. Additionally wraps `RequestScopedStorage` (request cache using shared + `RequestScopeCache`) for `requestCached()`. + +**Provided by KeyValueStorage (`$provide`, Pulled by Contributors)** + +| Class | Role | +|---|---| +| `Implementation\NamespacedStorageFactory` | Creates `NamespacedStorage` for a namespace + port | +| `Implementation\RequestScopeCache` | Shared in-request cache buckets (one per request) | + +**Contributor Checklist** + +- [ ] Port implements `TransientStoragePort` or `PersistentStoragePort` +- [ ] `$implement[Port]` wired to port instance +- [ ] `$contribute[StorageProvider]` returns `StorageProviderBridge` with correct backend +- [ ] `$pull` both `NamespacedStorageFactory` and `RequestScopeCache` +- [ ] Distinct `StorageBackend` per contribution (a duplicate silently overrides — last wins) +- [ ] Setup/schema owned by contributing component (if persistent storage needs tables) + +### Layer Stack + +```mermaid +flowchart TB + Factory["Factory (transient/persistent)"] + Provider["StorageProviderBridge (Storages)"] + RQS["RequestScopedStorage (only via requestCached)"] + NS["NamespacedStorage"] + Port["StoragePort"] + Impl["Port implementation"] + + Factory --> Provider + Provider -->|"requestCached()"| RQS + Provider -->|"storage()"| NS + RQS --> NS + NS --> Port + Port --> Impl +``` + +| Layer | Responsibility | +|---|---| +| `Implementation\Factory` | Resolve `StorageProvider` (a `Storages`) by lifetime (O(1) lookup) | +| `StorageProviderBridge` | Wire port → public `StorageProvider`; optionally add the request cache | +| `RequestScopedStorage` | Request-local write-through cache (opt-in decorator) | +| `NamespacedStorage` | Key validation, JSON encoding, namespace delegation | +| Your port | Opaque string persistence | + +## Component Integration + +KeyValueStorage wiring (`KeyValueStorage.php`): + +| Mechanism | Entry | +|---|---| +| `$define` | `Factory`, `TransientStoragePort`, `PersistentStoragePort` | +| `$implement` | `Factory` → `Implementation\Factory` | +| `$provide` | `NamespacedStorageFactory`, `RequestScopeCache` | +| `$seek` | `StorageProvider[]` (from contributors) | + +```mermaid +flowchart TB + subgraph Consumer["Consumer component"] + C["Application code"] + end + + subgraph KVS["KeyValueStorage"] + D1["$define: Factory, ports"] + I1["$implement: Factory"] + P1["$provide: NamespacedStorageFactory, RequestScopeCache"] + S1["$seek: StorageProvider[]"] + end + + subgraph Contributor["Backend contributor"] + I2["$implement: TransientStoragePort or PersistentStoragePort"] + C1["$contribute: StorageProviderBridge"] + PortImpl["Port implementation"] + I2 --> PortImpl + PortImpl --> C1 + end + + C -->|"use Factory"| I1 + C1 -->|"seek StorageProvider"| S1 + C1 -->|"pull provided classes"| P1 + C1 -->|"use port"| I2 +``` + +### Runtime Sequence + +```mermaid +sequenceDiagram + actor Consumer + participant Factory + participant Bridge as StorageProviderBridge + participant RQS as RequestScopedStorage + participant NS as NamespacedStorage + participant Port as StoragePort + + Consumer->>Factory: transient() (or persistent()) + Factory->>Bridge: resolve lifetime + Bridge-->>Consumer: Storages + Consumer->>Bridge: requestCached(namespace) + Bridge-->>Consumer: RequestScopedStorage (wrapping NamespacedStorage) + + Consumer->>RQS: set(key, value) + RQS->>NS: set(key, value) + NS->>NS: validate key, encode JSON + NS->>Port: write(namespace, key, encoded) + RQS->>RQS: update cache + + Consumer->>RQS: get(key) + RQS->>RQS: cache hit → return +``` + +### Class Structure + +```mermaid +classDiagram + class Factory { + <> + +transient() Storages + +persistent() Storages + } + + class Storages { + <> + +storage(namespace) Storage + +requestCached(namespace) Storage + } + + class Storage { + <> + +has(key) bool + +get(key, default) mixed + +set(key, value) void + +delete(key) void + +clear() void + } + + class StorageProvider { + <> + +backend() StorageBackend + } + + class StoragePort { + <> + +has(namespace, key) bool + +read(namespace, key) string|null + +write(namespace, key, value) void + +remove(namespace, key) void + +clearNamespace(namespace) void + } + + class StorageBackend { + <> + TRANSIENT + PERSISTENT + } + + StorageProvider --|> Storages + TransientStoragePort --|> StoragePort + PersistentStoragePort --|> StoragePort + StorageProviderBridge ..|> StorageProvider + RequestScopedStorage ..|> Storage + RequestScopedStorage --> NamespacedStorage + NamespacedStorage ..|> Storage + NamespacedStorage --> StoragePort + Factory ..> Storages : returns + Factory ..> StorageProvider : seeks +``` + +## Component Layout + +``` +components/ILIAS/KeyValueStorage/ +├── KeyValueStorage.php +├── README.md +├── maintenance.json +├── src/ +│ ├── Factory.php +│ ├── Storages.php +│ ├── Storage.php +│ ├── StorageProvider.php +│ ├── StoragePort.php +│ ├── TransientStoragePort.php +│ ├── PersistentStoragePort.php +│ ├── StorageBackend.php +│ ├── StorageNamespace.php +│ ├── StorageIdentifierLimits.php +│ ├── KeyValidator.php +│ ├── ValueCodec.php +│ ├── Exception/ +│ │ ├── StorageNotAvailableException.php +│ │ └── InvalidStoragePayloadException.php +│ └── Implementation/ +│ ├── Factory.php +│ ├── NamespacedStorage.php +│ ├── NamespacedStorageFactory.php +│ ├── RequestScopeCache.php +│ ├── RequestScopedStorage.php +│ └── StorageProviderBridge.php +└── tests/ +``` + +## Error Handling + +| Situation | Exception | When | +|---|---|---| +| Backend not contributed | `StorageNotAvailableException` | `Factory::transient()` / `Factory::persistent()` | +| Invalid namespace (format or length) | `\InvalidArgumentException` | `new StorageNamespace(…)` | +| Invalid key (format or length) | `\InvalidArgumentException` | Any `Storage` operation | +| Non-JSON stored value | `InvalidStoragePayloadException` | `ValueCodec::decode()` | +| Non-JSON-serializable value | `\InvalidArgumentException` | `Storage::set()` / encode | + +`StorageNotAvailableException` extends `\LogicException` — missing integration +wiring, not an end-user error. + +## PSR Alignment + +The `Storage` interface follows the operation shape of +[PSR-16 (Simple Cache)](https://www.php-fig.org/psr/psr-16/). Key validation +uses PSR-16 reserved-character rules via `KeyValidator`. + +This component **does not** implement `Psr\SimpleCache\CacheInterface` to avoid +overlap with `ILIAS\Cache` and to keep TTL/eviction out of application-state storage. + +## Design Decisions + +Significant architecture decisions are recorded as lightweight +[Architecture Decision Records](https://github.com/joelparkerhenderson/architecture-decision-record) +(Michael Nygard's *Context / Decision / Consequences* format, as adopted by many +applications and frameworks). Records are append-only: supersede rather than rewrite. + +### ADR 0001 — Storage Lifetime Is a Curated, Internal Taxonomy + +**Status:** Accepted. + +**Context.** The component offers exactly two storage lifetimes (transient and +persistent). Consumers SHOULD NOT have to understand or depend on that taxonomy to +read and write state, whereas backend providers (an SPI audience) MUST declare which +lifetime they fulfil. The set of lifetimes is small and stable, and each lifetime is +already represented by a marker port (`TransientStoragePort` / `PersistentStoragePort`). + +**Decision.** Model the lifetime as a string enum `StorageBackend` owned by +KeyValueStorage. The enum is marked `@internal`: consumers select a lifetime through +the named accessors `Factory::transient()` / `Factory::persistent()` and never +reference the enum. Backend providers MAY reference it when contributing a +`StorageProviderBridge`. We deliberately accept that the lifetime is expressed both by +the marker port and by the enum value. + +**Consequences.** + +- **+** The consumer API stays minimal and discoverable; no enum knowledge is required, and the only consumer-visible vocabulary is "transient" vs "persistent". +- **+** The taxonomy is curated and centralised — its semantics are owned by a single component rather than scattered across contributors. +- **−** Adding a new lifetime requires editing KeyValueStorage. This is acceptable: lifetimes are a rare, deliberate domain decision, not an open extension point. +- **−** The lifetime is encoded twice (marker port + enum); contributors MUST keep the two consistent. + +**Revisit when** a third lifetime or externally-defined backend lifetimes become +necessary. The alternative is to split the SPI into dedicated +`TransientStorageProvider` / `PersistentStorageProvider` interfaces resolved by type, +removing both the enum and `StorageProvider::backend()` from the contribution surface. + +## Tests + +PHPUnit tests in `components/ILIAS/KeyValueStorage/tests/`: + +- `FactoryTest` — lifetime resolution (`transient()`/`persistent()`), missing backend, plain vs request-cached delegation +- `StorageProviderBridgeTest` — plain vs request-cached storage, write-through, backend +- `StorageNamespaceTest`, `KeyValidatorTest`, `StorageIdentifierLimitsTest` — validation rules +- `ValueCodecTest` — JSON encoding, payload errors, `JsonSerializable` +- `NamespacedStorageTest` — port interaction, encoding +- `RequestScopedStorageTest` — request cache semantics +- `InvalidStoragePayloadExceptionTest`, `StorageNotAvailableExceptionTest` + +Run: + +```bash +phpunit components/ILIAS/KeyValueStorage/tests/ +``` diff --git a/components/ILIAS/KeyValueStorage/maintenance.json b/components/ILIAS/KeyValueStorage/maintenance.json new file mode 100644 index 000000000000..29f49c3b1e2e --- /dev/null +++ b/components/ILIAS/KeyValueStorage/maintenance.json @@ -0,0 +1,14 @@ +{ + "maintenance_model": "Classic", + "first_maintainer": "mjansen(8784)", + "second_maintainer": "", + "implicit_maintainers": [], + "coordinator": [ + "" + ], + "tester": "", + "testcase_writer": "", + "path": "components/ILIAS/KeyValueStorage", + "belong_to_component": "KeyValueStorage", + "used_in_components": [] +} diff --git a/components/ILIAS/KeyValueStorage/src/Exception/InvalidStoragePayloadException.php b/components/ILIAS/KeyValueStorage/src/Exception/InvalidStoragePayloadException.php new file mode 100644 index 000000000000..47c30010d3ea --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/Exception/InvalidStoragePayloadException.php @@ -0,0 +1,37 @@ +value . '".' + ); + } +} diff --git a/components/ILIAS/KeyValueStorage/src/Factory.php b/components/ILIAS/KeyValueStorage/src/Factory.php new file mode 100644 index 000000000000..650f6107df23 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/Factory.php @@ -0,0 +1,44 @@ + */ + private array $providers_by_backend; + + /** + * @param list $providers + */ + public function __construct(array $providers) + { + $providers_by_backend = []; + foreach ($providers as $provider) { + $providers_by_backend[$provider->backend()->value] = $provider; + } + + $this->providers_by_backend = $providers_by_backend; + } + + public function transient(): Storages + { + return $this->provider(StorageBackend::TRANSIENT); + } + + public function persistent(): Storages + { + return $this->provider(StorageBackend::PERSISTENT); + } + + private function provider(StorageBackend $backend): StorageProvider + { + return $this->providers_by_backend[$backend->value] + ?? throw new StorageNotAvailableException($backend); + } +} diff --git a/components/ILIAS/KeyValueStorage/src/Implementation/NamespacedStorage.php b/components/ILIAS/KeyValueStorage/src/Implementation/NamespacedStorage.php new file mode 100644 index 000000000000..f0d77ae150d9 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/Implementation/NamespacedStorage.php @@ -0,0 +1,79 @@ +key_validator->validate($key); + + return $this->port->has($this->namespace, $key); + } + + public function get(string $key, mixed $default = null): mixed + { + $this->key_validator->validate($key); + + $encoded = $this->port->read($this->namespace, $key); + if ($encoded === null) { + return $default; + } + + return $this->value_codec->decode($encoded); + } + + public function set(string $key, mixed $value): void + { + $this->key_validator->validate($key); + + $this->port->write($this->namespace, $key, $this->value_codec->encode($value)); + } + + public function delete(string $key): void + { + $this->key_validator->validate($key); + + $this->port->remove($this->namespace, $key); + } + + public function clear(): void + { + $this->port->clearNamespace($this->namespace); + } +} diff --git a/components/ILIAS/KeyValueStorage/src/Implementation/NamespacedStorageFactory.php b/components/ILIAS/KeyValueStorage/src/Implementation/NamespacedStorageFactory.php new file mode 100644 index 000000000000..3e77f4ec57eb --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/Implementation/NamespacedStorageFactory.php @@ -0,0 +1,44 @@ +key_validator, $this->value_codec); + } +} diff --git a/components/ILIAS/KeyValueStorage/src/Implementation/RequestScopeCache.php b/components/ILIAS/KeyValueStorage/src/Implementation/RequestScopeCache.php new file mode 100644 index 000000000000..d37545dc663a --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/Implementation/RequestScopeCache.php @@ -0,0 +1,56 @@ +> */ + private array $values = []; + + public function has(string $scope_key, string $key): bool + { + return isset($this->values[$scope_key]) + && \array_key_exists($key, $this->values[$scope_key]); + } + + public function get(string $scope_key, string $key): mixed + { + return $this->values[$scope_key][$key]; + } + + public function set(string $scope_key, string $key, mixed $value): void + { + $this->values[$scope_key][$key] = $value; + } + + public function forget(string $scope_key, string $key): void + { + unset($this->values[$scope_key][$key]); + } + + public function clearScope(string $scope_key): void + { + unset($this->values[$scope_key]); + } +} diff --git a/components/ILIAS/KeyValueStorage/src/Implementation/RequestScopedStorage.php b/components/ILIAS/KeyValueStorage/src/Implementation/RequestScopedStorage.php new file mode 100644 index 000000000000..9172e4a91981 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/Implementation/RequestScopedStorage.php @@ -0,0 +1,86 @@ +cache_miss = new \stdClass(); + } + + public function has(string $key): bool + { + if ($this->request_scope_cache->has($this->scope_key, $key)) { + return true; + } + + return $this->inner->has($key); + } + + public function get(string $key, mixed $default = null): mixed + { + if ($this->request_scope_cache->has($this->scope_key, $key)) { + return $this->request_scope_cache->get($this->scope_key, $key); + } + + $value = $this->inner->get($key, $this->cache_miss); + if ($value === $this->cache_miss) { + return $default; + } + + $this->request_scope_cache->set($this->scope_key, $key, $value); + + return $value; + } + + public function set(string $key, mixed $value): void + { + $this->inner->set($key, $value); + $this->request_scope_cache->set($this->scope_key, $key, $value); + } + + public function delete(string $key): void + { + $this->inner->delete($key); + $this->request_scope_cache->forget($this->scope_key, $key); + } + + public function clear(): void + { + $this->inner->clear(); + $this->request_scope_cache->clearScope($this->scope_key); + } +} diff --git a/components/ILIAS/KeyValueStorage/src/Implementation/StorageProviderBridge.php b/components/ILIAS/KeyValueStorage/src/Implementation/StorageProviderBridge.php new file mode 100644 index 000000000000..7e8145568de8 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/Implementation/StorageProviderBridge.php @@ -0,0 +1,63 @@ +scope_prefix = $backend->value . ':'; + } + + public function backend(): StorageBackend + { + return $this->backend; + } + + public function storage(StorageNamespace $namespace): Storage + { + return $this->storage_factory->create($namespace, $this->port); + } + + public function requestCached(StorageNamespace $namespace): Storage + { + return new RequestScopedStorage( + $this->storage($namespace), + $this->scope_prefix . $namespace->value(), + $this->request_scope_cache, + ); + } +} diff --git a/components/ILIAS/KeyValueStorage/src/KeyValidator.php b/components/ILIAS/KeyValueStorage/src/KeyValidator.php new file mode 100644 index 000000000000..0d060d1b3474 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/KeyValidator.php @@ -0,0 +1,64 @@ + self::MAX_LENGTH) { + throw new \InvalidArgumentException( + 'Storage key must not exceed ' . self::MAX_LENGTH . ' characters, got ' + . \mb_strlen($key, 'UTF-8') . '.' + ); + } + + if (\strpbrk($key, self::RESERVED_CHARACTERS) !== false) { + throw new \InvalidArgumentException( + 'Storage key must not contain reserved characters "' . self::RESERVED_CHARACTERS . '".' + ); + } + } +} diff --git a/components/ILIAS/KeyValueStorage/src/PersistentStoragePort.php b/components/ILIAS/KeyValueStorage/src/PersistentStoragePort.php new file mode 100644 index 000000000000..43535db07567 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/PersistentStoragePort.php @@ -0,0 +1,28 @@ + self::MAX_LENGTH) { + throw new \InvalidArgumentException( + 'Storage namespace must not exceed ' . self::MAX_LENGTH . ' characters, got ' + . \strlen($value) . '.' + ); + } + + if (!\preg_match('/^[a-z][a-z0-9_]*(\.[a-z][a-z0-9_]*)*$/', $value)) { + throw new \InvalidArgumentException( + 'Storage namespace must be a dot-separated lowercase identifier, got "' . $value . '".' + ); + } + } + + public function value(): string + { + return $this->value; + } + + public function __toString(): string + { + return $this->value; + } +} diff --git a/components/ILIAS/KeyValueStorage/src/StoragePort.php b/components/ILIAS/KeyValueStorage/src/StoragePort.php new file mode 100644 index 000000000000..fde8690cc694 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/src/StoragePort.php @@ -0,0 +1,39 @@ +assertEncodableValue($value); + + try { + return \json_encode($value, JSON_THROW_ON_ERROR); + } catch (\JsonException $exception) { + throw InvalidStoragePayloadException::fromJsonException($exception); + } + } + + public function decode(string $value): mixed + { + try { + $decoded = \json_decode($value, true, 512, JSON_THROW_ON_ERROR); + } catch (\JsonException $exception) { + throw InvalidStoragePayloadException::fromJsonException($exception); + } + + $this->assertStorableDecodedValue($decoded); + + return $decoded; + } + + private function assertEncodableValue(mixed $value): void + { + if ($value === null || \is_bool($value) || \is_int($value) || \is_float($value) || \is_string($value)) { + return; + } + + if (\is_array($value)) { + foreach ($value as $key => $item) { + if (!\is_int($key) && !\is_string($key)) { + throw new \InvalidArgumentException('Array keys must be integers or strings.'); + } + + $this->assertEncodableValue($item); + } + + return; + } + + if ($value instanceof \JsonSerializable) { + $this->assertEncodableValue($value->jsonSerialize()); + + return; + } + + throw new \InvalidArgumentException( + 'Only JSON-serializable values can be stored. Supported: null, boolean, integer, float, string, ' + . 'array, and objects implementing JsonSerializable. Got ' . \gettype($value) . '.' + ); + } + + private function assertStorableDecodedValue(mixed $value): void + { + if ($value === null || \is_bool($value) || \is_int($value) || \is_float($value) || \is_string($value)) { + return; + } + + if (\is_array($value)) { + foreach ($value as $key => $item) { + if (!\is_int($key) && !\is_string($key)) { + throw InvalidStoragePayloadException::fromInvalidStructure( + 'Decoded array keys must be integers or strings.' + ); + } + + $this->assertStorableDecodedValue($item); + } + + return; + } + + throw InvalidStoragePayloadException::fromInvalidStructure( + 'Decoded value contains unsupported type ' . \gettype($value) . '.' + ); + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/FactoryTest.php b/components/ILIAS/KeyValueStorage/tests/FactoryTest.php new file mode 100644 index 000000000000..21fe885f9954 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/FactoryTest.php @@ -0,0 +1,178 @@ +transient()->storage(new StorageNamespace('ui.state')); + + self::assertSame($plain, $storage); + self::assertSame('ui.state', $provider->last_storage_namespace); + self::assertNull($provider->last_request_cached_namespace); + } + + public function testPersistentReturnsPlainStorageFromPersistentProvider(): void + { + $plain = new StubStorage(); + $provider = new RecordingProvider(StorageBackend::PERSISTENT, $plain, new StubStorage()); + $factory = new Factory([$provider]); + + $storage = $factory->persistent()->storage(new StorageNamespace('export.job')); + + self::assertSame($plain, $storage); + self::assertSame('export.job', $provider->last_storage_namespace); + } + + public function testTransientRequestCachedDelegatesToProvider(): void + { + $cached = new StubStorage(); + $provider = new RecordingProvider(StorageBackend::TRANSIENT, new StubStorage(), $cached); + $factory = new Factory([$provider]); + + $storage = $factory->transient()->requestCached(new StorageNamespace('ui.state')); + + self::assertSame($cached, $storage); + self::assertSame('ui.state', $provider->last_request_cached_namespace); + self::assertNull($provider->last_storage_namespace); + } + + public function testSelectsProviderByLifetime(): void + { + $transient_storage = new StubStorage(); + $persistent_storage = new StubStorage(); + $factory = new Factory([ + new RecordingProvider(StorageBackend::TRANSIENT, $transient_storage, new StubStorage()), + new RecordingProvider(StorageBackend::PERSISTENT, $persistent_storage, new StubStorage()), + ]); + + $namespace = new StorageNamespace('export.job'); + + self::assertSame($transient_storage, $factory->transient()->storage($namespace)); + self::assertSame($persistent_storage, $factory->persistent()->storage($namespace)); + } + + public function testUsesLastProviderWhenBackendIsRegisteredTwice(): void + { + $first = new StubStorage(); + $second = new StubStorage(); + $factory = new Factory([ + new RecordingProvider(StorageBackend::TRANSIENT, $first, new StubStorage()), + new RecordingProvider(StorageBackend::TRANSIENT, $second, new StubStorage()), + ]); + + self::assertSame($second, $factory->transient()->storage(new StorageNamespace('ui.state'))); + } + + public function testTransientThrowsWhenBackendIsNotContributed(): void + { + $factory = new Factory([ + new RecordingProvider(StorageBackend::PERSISTENT, new StubStorage(), new StubStorage()), + ]); + + $this->expectException(StorageNotAvailableException::class); + $this->expectExceptionMessage('No storage provider is registered for backend "transient".'); + + $factory->transient(); + } + + public function testPersistentThrowsWhenBackendIsNotContributed(): void + { + $factory = new Factory([ + new RecordingProvider(StorageBackend::TRANSIENT, new StubStorage(), new StubStorage()), + ]); + + $this->expectException(StorageNotAvailableException::class); + $this->expectExceptionMessage('No storage provider is registered for backend "persistent".'); + + $factory->persistent(); + } +} + +final class StubStorage implements Storage +{ + public function has(string $key): bool + { + return false; + } + + public function get(string $key, mixed $default = null): mixed + { + return $default; + } + + public function set(string $key, mixed $value): void + { + } + + public function delete(string $key): void + { + } + + public function clear(): void + { + } +} + +final class RecordingProvider implements StorageProvider +{ + public ?string $last_storage_namespace = null; + public ?string $last_request_cached_namespace = null; + + public function __construct( + private readonly StorageBackend $storage_backend, + private readonly Storage $plain_storage, + private readonly Storage $request_cached_storage, + ) { + } + + public function backend(): StorageBackend + { + return $this->storage_backend; + } + + public function storage(StorageNamespace $namespace): Storage + { + $this->last_storage_namespace = $namespace->value(); + + return $this->plain_storage; + } + + public function requestCached(StorageNamespace $namespace): Storage + { + $this->last_request_cached_namespace = $namespace->value(); + + return $this->request_cached_storage; + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/InvalidStoragePayloadExceptionTest.php b/components/ILIAS/KeyValueStorage/tests/InvalidStoragePayloadExceptionTest.php new file mode 100644 index 000000000000..e7af73631747 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/InvalidStoragePayloadExceptionTest.php @@ -0,0 +1,45 @@ +getMessage()); + self::assertSame($json_exception, $exception->getPrevious()); + } + + public function testFromInvalidStructureUsesProvidedMessage(): void + { + $exception = InvalidStoragePayloadException::fromInvalidStructure('Decoded value contains unsupported type object.'); + + self::assertSame('Decoded value contains unsupported type object.', $exception->getMessage()); + self::assertNull($exception->getPrevious()); + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/KeyValidatorTest.php b/components/ILIAS/KeyValueStorage/tests/KeyValidatorTest.php new file mode 100644 index 000000000000..6d354b7c2746 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/KeyValidatorTest.php @@ -0,0 +1,125 @@ +validator = new KeyValidator(); + } + + #[DataProvider('validKeyProvider')] + public function testAcceptsValidKey(string $key): void + { + $this->validator->validate($key); + + self::assertNotSame('', $key); + } + + /** + * @return array + */ + public static function validKeyProvider(): array + { + return [ + 'simple' => ['sort_column'], + 'dotted' => ['ui.table.sort'], + 'with_digits' => ['column2'], + ]; + } + + public function testRejectsEmptyKey(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Storage key must be a non-empty string.'); + + $this->validator->validate(''); + } + + public function testAcceptsKeyAtMaxLength(): void + { + $key = \str_repeat('k', KeyValidator::MAX_LENGTH); + + $this->validator->validate($key); + + self::assertSame(KeyValidator::MAX_LENGTH, \mb_strlen($key, 'UTF-8')); + } + + public function testRejectsKeyExceedingMaxLength(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage( + 'Storage key must not exceed ' . KeyValidator::MAX_LENGTH . ' characters, got ' + . (KeyValidator::MAX_LENGTH + 1) . '.' + ); + + $this->validator->validate(\str_repeat('k', KeyValidator::MAX_LENGTH + 1)); + } + + public function testRejectsKeyExceedingMaxLengthWithMultibyteCharacters(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage( + 'Storage key must not exceed ' . KeyValidator::MAX_LENGTH . ' characters, got ' + . (KeyValidator::MAX_LENGTH + 1) . '.' + ); + + $this->validator->validate(\str_repeat('é', KeyValidator::MAX_LENGTH + 1)); + } + + #[DataProvider('reservedCharacterProvider')] + public function testRejectsReservedCharacters(string $key, string $character): void + { + try { + $this->validator->validate($key); + self::fail('Expected InvalidArgumentException for reserved character ' . $character . '.'); + } catch (\InvalidArgumentException $exception) { + self::assertSame( + 'Storage key must not contain reserved characters "{}()/\@:".', + $exception->getMessage() + ); + } + } + + /** + * @return array + */ + public static function reservedCharacterProvider(): array + { + return [ + 'brace open' => ['key{part', '{'], + 'brace close' => ['key}part', '}'], + 'parenthesis open' => ['key(part', '('], + 'parenthesis close' => ['key)part', ')'], + 'slash' => ['key/part', '/'], + 'backslash' => ['key\\part', '\\'], + 'at sign' => ['key@part', '@'], + 'colon' => ['key:part', ':'], + ]; + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/NamespacedStorageTest.php b/components/ILIAS/KeyValueStorage/tests/NamespacedStorageTest.php new file mode 100644 index 000000000000..89dcd963114b --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/NamespacedStorageTest.php @@ -0,0 +1,163 @@ +port = new RecordingStoragePort(); + $this->storage = new NamespacedStorage( + new StorageNamespace('ui.table'), + $this->port, + new KeyValidator(), + new ValueCodec(), + ); + } + + public function testSetEncodesValueBeforeWritingToPort(): void + { + $this->storage->set('sort_column', 'title'); + + self::assertSame(['sort_column' => '"title"'], $this->port->written_values); + } + + public function testSetAndGetValue(): void + { + $this->storage->set('sort_column', 'title'); + + self::assertTrue($this->storage->has('sort_column')); + self::assertSame('title', $this->storage->get('sort_column')); + self::assertSame(1, $this->port->read_count); + } + + public function testGetReturnsDefaultWhenMissing(): void + { + self::assertSame('id', $this->storage->get('sort_column', 'id')); + self::assertSame(1, $this->port->read_count); + } + + public function testDeleteRemovesValue(): void + { + $this->storage->set('sort_column', 'title'); + + $this->storage->delete('sort_column'); + + self::assertFalse($this->storage->has('sort_column')); + self::assertSame(1, $this->port->remove_count); + } + + public function testClearRemovesNamespaceOnly(): void + { + $this->storage->set('sort_column', 'title'); + $this->port->write(new StorageNamespace('other.namespace'), 'key', '"value"'); + + $this->storage->clear(); + + self::assertFalse($this->storage->has('sort_column')); + self::assertSame('"value"', $this->port->read(new StorageNamespace('other.namespace'), 'key')); + self::assertSame(1, $this->port->clear_namespace_count); + } + + public function testRejectsInvalidKeyOnSet(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Storage key must not contain reserved characters'); + + $this->storage->set('invalid/key', 'value'); + } + + public function testRejectsInvalidKeyOnGet(): void + { + $this->expectException(\InvalidArgumentException::class); + + $this->storage->get('invalid/key'); + } + + public function testRejectsKeyExceedingMaxLengthOnSet(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage( + 'Storage key must not exceed ' . KeyValidator::MAX_LENGTH . ' characters, got ' + . (KeyValidator::MAX_LENGTH + 1) . '.' + ); + + $this->storage->set(\str_repeat('k', KeyValidator::MAX_LENGTH + 1), 'value'); + } +} + +final class RecordingStoragePort implements StoragePort +{ + /** @var array */ + public array $written_values = []; + + public int $read_count = 0; + + public int $remove_count = 0; + + public int $clear_namespace_count = 0; + + /** @var array> */ + private array $data = []; + + public function has(StorageNamespace $namespace, string $key): bool + { + return \array_key_exists($key, $this->data[$namespace->value()] ?? []); + } + + public function read(StorageNamespace $namespace, string $key): ?string + { + ++$this->read_count; + + return $this->data[$namespace->value()][$key] ?? null; + } + + public function write(StorageNamespace $namespace, string $key, string $value): void + { + if ($namespace->value() === 'ui.table') { + $this->written_values[$key] = $value; + } + + $this->data[$namespace->value()][$key] = $value; + } + + public function remove(StorageNamespace $namespace, string $key): void + { + ++$this->remove_count; + unset($this->data[$namespace->value()][$key]); + } + + public function clearNamespace(StorageNamespace $namespace): void + { + ++$this->clear_namespace_count; + unset($this->data[$namespace->value()]); + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/RequestScopedStorageTest.php b/components/ILIAS/KeyValueStorage/tests/RequestScopedStorageTest.php new file mode 100644 index 000000000000..ab8b952ec552 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/RequestScopedStorageTest.php @@ -0,0 +1,181 @@ + 'title']); + $storage = new RequestScopedStorage($inner, 'transient:reads_once', new RequestScopeCache()); + + self::assertSame('title', $storage->get('sort_column', 'id')); + self::assertSame('title', $storage->get('sort_column', 'id')); + + self::assertSame(1, $inner->read_count); + self::assertSame(0, $inner->has_count); + } + + public function testHasUsesCacheAfterFirstGet(): void + { + $inner = new RequestScopedStorageTestStorage(['sort_column' => 'title']); + $storage = new RequestScopedStorage($inner, 'transient:has_cache', new RequestScopeCache()); + + $storage->get('sort_column'); + $inner->has_count = 0; + + self::assertTrue($storage->has('sort_column')); + self::assertSame(0, $inner->has_count); + } + + public function testSetUpdatesCacheWithoutExtraRead(): void + { + $inner = new RequestScopedStorageTestStorage(); + $storage = new RequestScopedStorage($inner, 'transient:write_through', new RequestScopeCache()); + + $storage->set('sort_column', 'title'); + $inner->read_count = 0; + + self::assertSame('title', $storage->get('sort_column')); + self::assertSame(0, $inner->read_count); + } + + public function testGetReturnsDefaultWithoutCachingMiss(): void + { + $inner = new RequestScopedStorageTestStorage(); + $storage = new RequestScopedStorage($inner, 'transient:default', new RequestScopeCache()); + + self::assertSame('fallback', $storage->get('missing', 'fallback')); + self::assertSame(1, $inner->read_count); + self::assertFalse($storage->has('missing')); + } + + public function testDeleteRemovesCachedValue(): void + { + $inner = new RequestScopedStorageTestStorage(); + $storage = new RequestScopedStorage($inner, 'transient:delete', new RequestScopeCache()); + + $storage->set('sort_column', 'title'); + $storage->delete('sort_column'); + + self::assertFalse($storage->has('sort_column')); + self::assertSame(1, $inner->delete_count); + } + + public function testClearDropsCachedNamespace(): void + { + $inner = new RequestScopedStorageTestStorage(); + $storage = new RequestScopedStorage($inner, 'transient:clear', new RequestScopeCache()); + + $storage->set('sort_column', 'title'); + $storage->clear(); + + self::assertFalse($storage->has('sort_column')); + self::assertSame([], $inner->values); + self::assertSame(1, $inner->clear_count); + } + + public function testSeparateScopesDoNotShareCache(): void + { + $inner = new RequestScopedStorageTestStorage(['sort_column' => 'title']); + $request_scope_cache = new RequestScopeCache(); + $first = new RequestScopedStorage($inner, 'transient:scope_a', $request_scope_cache); + $second = new RequestScopedStorage($inner, 'transient:scope_b', $request_scope_cache); + + self::assertSame('title', $first->get('sort_column')); + $inner->read_count = 0; + + self::assertSame('title', $second->get('sort_column')); + self::assertSame(1, $inner->read_count); + } + + public function testCachesNullValue(): void + { + $inner = new RequestScopedStorageTestStorage(); + $storage = new RequestScopedStorage($inner, 'transient:null', new RequestScopeCache()); + + $storage->set('nullable', null); + + self::assertTrue($storage->has('nullable')); + self::assertNull($storage->get('nullable')); + self::assertSame(0, $inner->read_count); + } +} + +final class RequestScopedStorageTestStorage implements Storage +{ + public int $read_count = 0; + + public int $has_count = 0; + + public int $delete_count = 0; + + public int $clear_count = 0; + + /** @var array */ + public array $values; + + /** @param array $values */ + public function __construct(array $values = []) + { + $this->values = $values; + } + + public function has(string $key): bool + { + ++$this->has_count; + + return \array_key_exists($key, $this->values); + } + + public function get(string $key, mixed $default = null): mixed + { + ++$this->read_count; + + if (!\array_key_exists($key, $this->values)) { + return $default; + } + + return $this->values[$key]; + } + + public function set(string $key, mixed $value): void + { + $this->values[$key] = $value; + } + + public function delete(string $key): void + { + ++$this->delete_count; + unset($this->values[$key]); + } + + public function clear(): void + { + ++$this->clear_count; + $this->values = []; + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/StorageBackendTest.php b/components/ILIAS/KeyValueStorage/tests/StorageBackendTest.php new file mode 100644 index 000000000000..6a5ab0d714c1 --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/StorageBackendTest.php @@ -0,0 +1,37 @@ +value); + } + + public function testPersistentBackendValue(): void + { + self::assertSame('persistent', StorageBackend::PERSISTENT->value); + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/StorageNamespaceTest.php b/components/ILIAS/KeyValueStorage/tests/StorageNamespaceTest.php new file mode 100644 index 000000000000..541054ec23df --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/StorageNamespaceTest.php @@ -0,0 +1,94 @@ +value()); + self::assertSame('ui.table.sort', (string) $namespace); + } + + public function testAcceptsSingleSegmentNamespace(): void + { + $namespace = new StorageNamespace('ui'); + + self::assertSame('ui', $namespace->value()); + } + + public function testRejectsEmptyNamespace(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Storage namespace must not be empty.'); + + new StorageNamespace(''); + } + + public function testRejectsUppercaseNamespace(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Storage namespace must be a dot-separated lowercase identifier'); + + new StorageNamespace('UI.Table'); + } + + public function testRejectsLeadingDigit(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Storage namespace must be a dot-separated lowercase identifier'); + + new StorageNamespace('1invalid'); + } + + public function testRejectsHyphenatedNamespace(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Storage namespace must be a dot-separated lowercase identifier'); + + new StorageNamespace('ui-table'); + } + + public function testAcceptsNamespaceAtMaxLength(): void + { + $namespace_value = 'a' . \str_repeat('b', StorageNamespace::MAX_LENGTH - 1); + + $namespace = new StorageNamespace($namespace_value); + + self::assertSame(StorageNamespace::MAX_LENGTH, \strlen($namespace->value())); + } + + public function testRejectsNamespaceExceedingMaxLength(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage( + 'Storage namespace must not exceed ' . StorageNamespace::MAX_LENGTH . ' characters, got ' + . (StorageNamespace::MAX_LENGTH + 1) . '.' + ); + + new StorageNamespace('a' . \str_repeat('b', StorageNamespace::MAX_LENGTH)); + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/StorageNotAvailableExceptionTest.php b/components/ILIAS/KeyValueStorage/tests/StorageNotAvailableExceptionTest.php new file mode 100644 index 000000000000..64171ab480ad --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/StorageNotAvailableExceptionTest.php @@ -0,0 +1,54 @@ +getMessage()); + } + + /** + * @return array + */ + public static function backendMessageProvider(): array + { + return [ + 'transient' => [ + StorageBackend::TRANSIENT, + 'No storage provider is registered for backend "transient".', + ], + 'persistent' => [ + StorageBackend::PERSISTENT, + 'No storage provider is registered for backend "persistent".', + ], + ]; + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/StorageProviderBridgeTest.php b/components/ILIAS/KeyValueStorage/tests/StorageProviderBridgeTest.php new file mode 100644 index 000000000000..7b65aec407ab --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/StorageProviderBridgeTest.php @@ -0,0 +1,134 @@ +port = new BridgeRecordingStoragePort(); + $this->bridge = new StorageProviderBridge( + StorageBackend::PERSISTENT, + $this->port, + new NamespacedStorageFactory(new KeyValidator(), new ValueCodec()), + new RequestScopeCache(), + ); + } + + public function testBackendReturnsConfiguredBackend(): void + { + self::assertSame(StorageBackend::PERSISTENT, $this->bridge->backend()); + } + + public function testPlainStorageReadsBackendOnEveryGet(): void + { + $storage = $this->bridge->storage(new StorageNamespace('export.job')); + $storage->set('state', 'running'); + + self::assertSame('running', $storage->get('state')); + self::assertSame('running', $storage->get('state')); + self::assertSame(2, $this->port->read_count); + } + + public function testRequestCachedStorageMemoizesReadsWithinRequest(): void + { + $storage = $this->bridge->requestCached(new StorageNamespace('export.job')); + $storage->set('state', 'running'); + + self::assertSame('running', $storage->get('state')); + self::assertSame('running', $storage->get('state')); + self::assertSame(0, $this->port->read_count); + } + + public function testRequestCachedStorageStillWritesThroughToBackend(): void + { + $storage = $this->bridge->requestCached(new StorageNamespace('export.job')); + + $storage->set('state', 'running'); + + self::assertSame(['state' => '"running"'], $this->port->writes_for('export.job')); + } + + public function testPlainAndRequestCachedShareTheSameBackendNamespace(): void + { + $namespace = new StorageNamespace('export.job'); + $this->bridge->storage($namespace)->set('state', 'queued'); + + $cached = $this->bridge->requestCached($namespace); + + self::assertSame('queued', $cached->get('state')); + self::assertSame(1, $this->port->read_count); + } +} + +final class BridgeRecordingStoragePort implements StoragePort +{ + public int $read_count = 0; + + /** @var array> */ + private array $data = []; + + public function has(StorageNamespace $namespace, string $key): bool + { + return \array_key_exists($key, $this->data[$namespace->value()] ?? []); + } + + public function read(StorageNamespace $namespace, string $key): ?string + { + ++$this->read_count; + + return $this->data[$namespace->value()][$key] ?? null; + } + + public function write(StorageNamespace $namespace, string $key, string $value): void + { + $this->data[$namespace->value()][$key] = $value; + } + + public function remove(StorageNamespace $namespace, string $key): void + { + unset($this->data[$namespace->value()][$key]); + } + + public function clearNamespace(StorageNamespace $namespace): void + { + unset($this->data[$namespace->value()]); + } + + /** @return array */ + public function writes_for(string $namespace): array + { + return $this->data[$namespace] ?? []; + } +} diff --git a/components/ILIAS/KeyValueStorage/tests/ValueCodecTest.php b/components/ILIAS/KeyValueStorage/tests/ValueCodecTest.php new file mode 100644 index 000000000000..e2825519b46d --- /dev/null +++ b/components/ILIAS/KeyValueStorage/tests/ValueCodecTest.php @@ -0,0 +1,174 @@ +codec = new ValueCodec(); + } + + #[DataProvider('roundTripValueProvider')] + public function testRoundTripsValue(mixed $value): void + { + self::assertSame($value, $this->codec->decode($this->codec->encode($value))); + } + + /** + * @return array + */ + public static function roundTripValueProvider(): array + { + return [ + 'string' => ['title'], + 'integer' => [42], + 'float' => [3.14], + 'boolean true' => [true], + 'boolean false' => [false], + 'null' => [null], + 'array' => [['sort' => 'title', 'direction' => 'asc', 'count' => 3, 'enabled' => true]], + 'nested array' => [['filters' => ['status' => 'open', 'limit' => 10], 'page' => 1]], + 'list array' => [[1, 2, 3]], + ]; + } + + public function testEncodeProducesPlainJsonForScalar(): void + { + self::assertSame('"title"', $this->codec->encode('title')); + } + + public function testEncodeProducesPlainJsonForArray(): void + { + self::assertSame( + '{"sort":"title","count":3}', + $this->codec->encode(['sort' => 'title', 'count' => 3]) + ); + } + + public function testEncodesJsonSerializableObjectViaJsonEncode(): void + { + $value = new JsonSerializableStorageValue(['sort' => 'title']); + + self::assertSame('{"sort":"title"}', $this->codec->encode($value)); + self::assertSame(['sort' => 'title'], $this->codec->decode($this->codec->encode($value))); + } + + public function testEncodesJsonSerializableObjectInNestedArray(): void + { + $value = [ + 'state' => new JsonSerializableStorageValue(['page' => 2]), + 'enabled' => true, + ]; + + self::assertSame( + '{"state":{"page":2},"enabled":true}', + $this->codec->encode($value) + ); + } + + public function testRejectsObjectsWithoutJsonSerializableOnEncode(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Only JSON-serializable values can be stored'); + + $this->codec->encode(new \stdClass()); + } + + public function testRejectsNestedObjectWithoutJsonSerializableOnEncode(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Only JSON-serializable values can be stored'); + + $this->codec->encode(['item' => new \stdClass()]); + } + + public function testRejectsResourceOnEncode(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Only JSON-serializable values can be stored'); + + $this->codec->encode(\fopen('php://memory', 'rb')); + } + + public function testRejectsJsonSerializableThatReturnsObjectOnEncode(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Only JSON-serializable values can be stored'); + + $this->codec->encode(new JsonSerializableReturningObject()); + } + + public function testThrowsInvalidStoragePayloadExceptionForMalformedJson(): void + { + $this->expectException(InvalidStoragePayloadException::class); + $this->expectExceptionMessage('Stored value is not valid JSON.'); + + $this->codec->decode('{invalid'); + } + + public function testWrapsJsonExceptionAsPrevious(): void + { + try { + $this->codec->decode('{invalid'); + self::fail('Expected InvalidStoragePayloadException.'); + } catch (InvalidStoragePayloadException $exception) { + self::assertSame('Stored value is not valid JSON.', $exception->getMessage()); + self::assertInstanceOf(\JsonException::class, $exception->getPrevious()); + } + } + + public function testDecodeAcceptsTopLevelScalarJson(): void + { + self::assertSame('title', $this->codec->decode('"title"')); + self::assertSame(42, $this->codec->decode('42')); + self::assertTrue($this->codec->decode('true')); + self::assertNull($this->codec->decode('null')); + } +} + +final readonly class JsonSerializableStorageValue implements \JsonSerializable +{ + /** @param array $data */ + public function __construct(private array $data) + { + } + + public function jsonSerialize(): array + { + return $this->data; + } +} + +final readonly class JsonSerializableReturningObject implements \JsonSerializable +{ + public function jsonSerialize(): \stdClass + { + return new \stdClass(); + } +}