From 9f76f8f4a6f5dd24933cb1e2bd7d0108f653f649 Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Wed, 22 May 2024 12:37:36 +0200 Subject: [PATCH 01/15] Move ::write and ::ensure into ContentStorageHandler class --- src/Content/ContentStorageHandler.php | 56 +++++++++++++++++-- .../PlainTextContentStorageHandler.php | 24 -------- .../PlainTextContentStorageHandlerTest.php | 20 +++---- tests/Content/TestContentStorageHandler.php | 14 +---- 4 files changed, 62 insertions(+), 52 deletions(-) diff --git a/src/Content/ContentStorageHandler.php b/src/Content/ContentStorageHandler.php index 75bc2ed22d..a7f0820561 100644 --- a/src/Content/ContentStorageHandler.php +++ b/src/Content/ContentStorageHandler.php @@ -6,6 +6,7 @@ use Kirby\Cms\Language; use Kirby\Cms\ModelWithContent; use Kirby\Cms\Page; +use Kirby\Exception\NotFoundException; /** * Abstract for content storage handlers; @@ -52,7 +53,10 @@ public function all(): Generator * * @param array $fields Content fields */ - abstract public function create(VersionId $versionId, Language $language, array $fields): void; + public function create(VersionId $versionId, Language $language, array $fields): void + { + $this->write($versionId, $language, $fields); + } /** * Deletes an existing version in an idempotent way if it was already deleted @@ -91,6 +95,26 @@ public function dynamicVersions(): array return $versions; } + /** + * Checks if a version/language combination exists and otherwise + * will throw a NotFoundException + * + * @throws \Kirby\Exception\NotFoundException If the version does not exist + */ + public function ensure(VersionId $versionId, Language $language): bool + { + if ($this->exists($versionId, $language) !== true) { + $message = match($this->model->kirby()->multilang()) { + true => 'Version "' . $versionId . ' (' . $language->code() . ')" does not already exist', + false => 'Version "' . $versionId . '" does not already exist', + }; + + throw new NotFoundException($message); + } + + return true; + } + /** * Checks if a version exists */ @@ -104,12 +128,21 @@ abstract public function modified(VersionId $versionId, Language $language): int /** * Moves content from one version-language combination to another */ - abstract public function move( + public function move( VersionId $fromVersionId, Language $fromLanguage, VersionId $toVersionId, Language $toLanguage - ): void; + ): void { + // read the existing fields + $fields = $this->read($fromVersionId, $fromLanguage); + + // create the new version + $this->create($toVersionId, $toLanguage, $fields); + + // clean up the old version + $this->delete($fromVersionId, $fromLanguage); + } /** * Adapts all versions when converting languages @@ -158,7 +191,20 @@ public function touchLanguage(Language $language): void * * @param array $fields Content fields * - * @throws \Kirby\Exception\NotFoundException If the version does not exist + * @throws \Kirby\Exception\Exception If the file cannot be written + */ + public function update(VersionId $versionId, Language $language, array $fields): void + { + $this->ensure($versionId, $language); + $this->write($versionId, $language, $fields); + } + + /** + * Writes the content fields of an existing version + * + * @param array $fields Content fields + * + * @throws \Kirby\Exception\Exception If the content cannot be written */ - abstract public function update(VersionId $versionId, Language $language, array $fields): void; + abstract protected function write(VersionId $versionId, Language $language, array $fields): void; } diff --git a/src/Content/PlainTextContentStorageHandler.php b/src/Content/PlainTextContentStorageHandler.php index 9ce36a2388..84328f8855 100644 --- a/src/Content/PlainTextContentStorageHandler.php +++ b/src/Content/PlainTextContentStorageHandler.php @@ -141,18 +141,6 @@ public function contentFiles(VersionId $versionId): array ]; } - /** - * Creates a new version - * - * @param array $fields Content fields - * - * @throws \Kirby\Exception\Exception If the file cannot be written - */ - public function create(VersionId $versionId, Language $language, array $fields): void - { - $this->write($versionId, $language, $fields); - } - /** * Deletes an existing version in an idempotent way if it was already deleted */ @@ -247,18 +235,6 @@ public function touch(VersionId $versionId, Language $language): void // @codeCoverageIgnoreEnd } - /** - * Updates the content fields of an existing version - * - * @param array $fields Content fields - * - * @throws \Kirby\Exception\Exception If the file cannot be written - */ - public function update(VersionId $versionId, Language $language, array $fields): void - { - $this->write($versionId, $language, $fields); - } - /** * Writes the content fields of an existing version * diff --git a/tests/Content/PlainTextContentStorageHandlerTest.php b/tests/Content/PlainTextContentStorageHandlerTest.php index 078023baa6..f68ec7d3cc 100644 --- a/tests/Content/PlainTextContentStorageHandlerTest.php +++ b/tests/Content/PlainTextContentStorageHandlerTest.php @@ -410,11 +410,11 @@ public function testUpdateChangesMultiLang() 'text' => 'Bar' ]; - Dir::make(static::TMP . '/_changes'); - Data::write(static::TMP . '/_changes/article.en.txt', $fields); + Dir::make($this->model->root() . '/_changes'); + Data::write($this->model->root() . '/_changes/article.en.txt', $fields); $this->storage->update(VersionId::changes(), $this->app->language('en'), $fields); - $this->assertSame($fields, Data::read(static::TMP . '/_changes/article.en.txt')); + $this->assertSame($fields, Data::read($this->model->root() . '/_changes/article.en.txt')); } /** @@ -429,11 +429,11 @@ public function testUpdateChangesSingleLang() 'text' => 'Bar' ]; - Dir::make(static::TMP . '/_changes'); - Data::write(static::TMP . '/_changes/article.txt', $fields); + Dir::make($this->model->root() . '/_changes'); + Data::write($this->model->root() . '/_changes/article.txt', $fields); $this->storage->update(VersionId::changes(), Language::single(), $fields); - $this->assertSame($fields, Data::read(static::TMP . '/_changes/article.txt')); + $this->assertSame($fields, Data::read($this->model->root() . '/_changes/article.txt')); } /** @@ -448,10 +448,10 @@ public function testUpdatePublishedMultiLang() 'text' => 'Bar' ]; - Data::write(static::TMP . '/article.en.txt', $fields); + Data::write($this->model->root() . '/article.en.txt', $fields); $this->storage->update(VersionId::published(), $this->app->language('en'), $fields); - $this->assertSame($fields, Data::read(static::TMP . '/article.en.txt')); + $this->assertSame($fields, Data::read($this->model->root() . '/article.en.txt')); } /** @@ -466,10 +466,10 @@ public function testUpdatePublishedSingleLang() 'text' => 'Bar' ]; - Data::write(static::TMP . '/article.txt', $fields); + Data::write($this->model->root() . '/article.txt', $fields); $this->storage->update(VersionId::published(), Language::single(), $fields); - $this->assertSame($fields, Data::read(static::TMP . '/article.txt')); + $this->assertSame($fields, Data::read($this->model->root() . '/article.txt')); } /** diff --git a/tests/Content/TestContentStorageHandler.php b/tests/Content/TestContentStorageHandler.php index 363cf13315..28da437496 100644 --- a/tests/Content/TestContentStorageHandler.php +++ b/tests/Content/TestContentStorageHandler.php @@ -6,10 +6,6 @@ class TestContentStorageHandler extends ContentStorageHandler { - public function create(VersionId $versionId, Language $language, array $fields): void - { - } - public function delete(VersionId $versionId, Language $language): void { } @@ -24,14 +20,6 @@ public function modified(VersionId $versionId, Language $language): int|null return null; } - public function move( - VersionId $fromVersionId, - Language $fromLanguage, - VersionId $toVersionId, - Language $toLanguage - ): void { - } - public function read(VersionId $versionId, Language $language): array { return []; @@ -41,7 +29,7 @@ public function touch(VersionId $versionId, Language $language): void { } - public function update(VersionId $versionId, Language $language, array $fields): void + public function write(VersionId $versionId, Language $language, array $fields): void { } } From 5b3d9394b9aab4472ab2227471ea0d6bf3197f48 Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Wed, 22 May 2024 12:37:46 +0200 Subject: [PATCH 02/15] Use ContentStorageHandler::ensure in Version class --- src/Content/Version.php | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/Content/Version.php b/src/Content/Version.php index c5395e9293..d053dcc709 100644 --- a/src/Content/Version.php +++ b/src/Content/Version.php @@ -5,7 +5,6 @@ use Kirby\Cms\Language; use Kirby\Cms\ModelWithContent; use Kirby\Exception\InvalidArgumentException; -use Kirby\Exception\NotFoundException; /** * The Version class handles all actions for a single @@ -87,17 +86,8 @@ public function delete(): void */ public function ensure( Language|string $language = 'default' - ): void { - if ($this->exists($language) === true) { - return; - } - - $message = match($this->model->kirby()->multilang()) { - true => 'Version "' . $this->id . ' (' . $language . ')" does not already exist', - false => 'Version "' . $this->id . '" does not already exist', - }; - - throw new NotFoundException($message); + ): bool { + return $this->model->storage()->ensure($this->id, $this->language($language)); } /** From ae4e067ea52e69186e3651ea0bcd3c418eb717b7 Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Wed, 22 May 2024 12:37:56 +0200 Subject: [PATCH 03/15] First draft for a new MemoryContentStorageHandler --- src/Content/MemoryContentStorageHandler.php | 130 ++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 src/Content/MemoryContentStorageHandler.php diff --git a/src/Content/MemoryContentStorageHandler.php b/src/Content/MemoryContentStorageHandler.php new file mode 100644 index 0000000000..5cab9333a3 --- /dev/null +++ b/src/Content/MemoryContentStorageHandler.php @@ -0,0 +1,130 @@ + + * @link https://getkirby.com + * @copyright Bastian Allgeier + * @license https://getkirby.com/license + */ +class MemoryContentStorageHandler extends ContentStorageHandler +{ + /** + * Cache instance, used to store content in memory + */ + protected MemoryCache $cache; + + /** + * Sets up the cache instance + */ + public function __construct(protected ModelWithContent $model) + { + parent::__construct($model); + $this->cache = new MemoryCache(); + } + + /** + * Returns a unique id for a combination + * of the version id, the language code and the model id + */ + protected function cacheId(VersionId $versionId, Language $language): string + { + return $versionId->value() . '/' . $language->code() . '/' . $this->model->id(); + } + + /** + * Deletes an existing version in an idempotent way if it was already deleted + */ + public function delete(VersionId $versionId, Language $language): void + { + $this->cache->remove($this->cacheId($versionId, $language)); + } + + /** + * Checks if a version exists + */ + public function exists(VersionId $versionId, Language $language): bool + { + $data = $this->cache->get($this->cacheId($versionId, $language)); + + // validate the cached data + if ( + isset($data['fields']) === true && + is_array($data['fields']) === true && + isset($data['modified']) === true && + is_int($data['modified']) === true + ) { + return true; + } + + return false; + } + + /** + * Returns the modification timestamp of a version if it exists + */ + public function modified(VersionId $versionId, Language $language): int|null + { + if ($this->exists($versionId, $language) === false) { + return null; + } + + return $this->cache->get($this->cacheId($versionId, $language))['modified']; + } + + /** + * Returns the stored content fields + * + * @return array + * + * @throws \Kirby\Exception\NotFoundException If the version does not exist + */ + public function read(VersionId $versionId, Language $language): array + { + $this->ensure($versionId, $language); + return $this->cache->get($this->cacheId($versionId, $language))['fields']; + } + + /** + * Updates the modification timestamp of an existing version + * + * @throws \Kirby\Exception\NotFoundException If the version does not exist + */ + public function touch(VersionId $versionId, Language $language): void + { + $fields = $this->read($versionId, $language); + $this->write($versionId, $language, $fields); + } + + /** + * Updates the content fields of an existing version + * + * @param array $fields Content fields + * + * @throws \Kirby\Exception\NotFoundException If the version does not exist + */ + public function update(VersionId $versionId, Language $language, array $fields): void + { + $this->ensure($versionId, $language); + $this->write($versionId, $language, $fields); + } + + /** + * Writes the content fields of an existing version + * + * @param array $fields Content fields + */ + protected function write(VersionId $versionId, Language $language, array $fields): void + { + $this->cache->set($this->cacheId($versionId, $language), [ + 'fields' => $fields, + 'modified' => time() + ]); + } +} From 42d5dabab712c85540169f26bad53bb23c2ea346 Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Wed, 22 May 2024 13:49:23 +0200 Subject: [PATCH 04/15] Add unit tests for the MemoryContentStorageHandler --- .../MemoryContentStorageHandlerTest.php | 289 ++++++++++++++++++ 1 file changed, 289 insertions(+) create mode 100644 tests/Content/MemoryContentStorageHandlerTest.php diff --git a/tests/Content/MemoryContentStorageHandlerTest.php b/tests/Content/MemoryContentStorageHandlerTest.php new file mode 100644 index 0000000000..6f63f07cf4 --- /dev/null +++ b/tests/Content/MemoryContentStorageHandlerTest.php @@ -0,0 +1,289 @@ +storage->create($versionId, $language, []); + + $this->assertTrue($this->storage->exists($versionId, $language)); + + $this->storage->delete($versionId, $language); + + $this->assertFalse($this->storage->exists($versionId, $language)); + } + + public function assertCreateAndRead(VersionId $versionId, Language $language): void + { + $fields = [ + 'title' => 'Foo', + 'text' => 'Bar' + ]; + + $this->storage->create($versionId, $language, $fields); + + $this->assertTrue($this->storage->exists($versionId, $language)); + $this->assertSame($fields, $this->storage->read($versionId, $language)); + } + + public function setUpMultiLanguage(): void + { + parent::setUpMultiLanguage(); + + $this->storage = new MemoryContentStorageHandler($this->model); + } + + public function setUpSingleLanguage(): void + { + parent::setUpSingleLanguage(); + + $this->storage = new MemoryContentStorageHandler($this->model); + } + + /** + * @covers ::create + * @covers ::read + */ + public function testCreateAndReadChangesMultiLang() + { + $this->setUpMultiLanguage(); + + $versionId = VersionId::changes(); + $language = $this->app->language('en'); + + $this->assertCreateAndRead($versionId, $language); + } + + /** + * @covers ::create + * @covers ::read + */ + public function testCreateAndReadChangesSingleLang() + { + $this->setUpSingleLanguage(); + + $versionId = VersionId::changes(); + $language = Language::single(); + + $this->assertCreateAndRead($versionId, $language); + } + + /** + * @covers ::create + * @covers ::read + */ + public function testCreateAndReadPublishedMultiLang() + { + $this->setUpMultiLanguage(); + + $versionId = VersionId::published(); + $language = $this->app->language('en'); + + $this->assertCreateAndRead($versionId, $language); + } + + /** + * @covers ::create + * @covers ::read + */ + public function testCreateAndReadPublishedSingleLang() + { + $this->setUpSingleLanguage(); + + $versionId = VersionId::published(); + $language = Language::single(); + + $this->assertCreateAndRead($versionId, $language); + } + + /** + * @covers ::delete + */ + public function testDeleteNonExisting() + { + $this->setUpSingleLanguage(); + + $versionId = VersionId::published(); + $language = Language::single(); + + $this->assertFalse($this->storage->exists($versionId, $language)); + + // test idempotency + $this->storage->delete($versionId, $language); + + $this->assertFalse($this->storage->exists($versionId, $language)); + } + + /** + * @covers ::delete + */ + public function testDeleteChangesMultiLang() + { + $this->setUpMultiLanguage(); + + $versionId = VersionId::changes(); + $language = $this->app->language('en'); + + $this->assertCreateAndDelete($versionId, $language); + } + + /** + * @covers ::delete + */ + public function testDeleteChangesSingleLang() + { + $this->setUpSingleLanguage(); + + $versionId = VersionId::changes(); + $language = Language::single(); + + $this->assertCreateAndDelete($versionId, $language); + } + + /** + * @covers ::delete + */ + public function testDeletePublishedMultiLang() + { + $this->setUpMultiLanguage(); + + $versionId = VersionId::published(); + $language = $this->app->language('en'); + + $this->assertCreateAndDelete($versionId, $language); + } + + /** + * @covers ::delete + */ + public function testDeletePublishedSingleLang() + { + $this->setUpSingleLanguage(); + + $versionId = VersionId::published(); + $language = Language::single(); + + $this->assertCreateAndDelete($versionId, $language); + } + + /** + * @covers ::exists + */ + public function testExistsNoneExistingMultiLanguage() + { + $this->setUpMultiLanguage(); + + $this->assertFalse($this->storage->exists(VersionId::changes(), $this->app->language('en'))); + $this->assertFalse($this->storage->exists(VersionId::changes(), $this->app->language('de'))); + } + + /** + * @covers ::exists + */ + public function testExistsNoneExistingSingleLanguage() + { + $this->setUpSingleLanguage(); + + $this->assertFalse($this->storage->exists(VersionId::changes(), Language::single())); + } + + /** + * @covers ::modified + */ + public function testModifiedNoneExistingMultiLanguage() + { + $this->setUpMultiLanguage(); + + $this->assertNull($this->storage->modified(VersionId::changes(), $this->app->language('en'))); + $this->assertNull($this->storage->modified(VersionId::published(), $this->app->language('en'))); + } + + /** + * @covers ::modified + */ + public function testModifiedNoneExistingSingleLanguage() + { + $this->setUpSingleLanguage(); + + $this->assertNull($this->storage->modified(VersionId::changes(), Language::single())); + $this->assertNull($this->storage->modified(VersionId::published(), Language::single())); + } + + /** + * @covers ::modified + */ + public function testModifiedSomeExistingMultiLanguage() + { + $this->setUpMultiLanguage(); + + $changes = VersionId::changes(); + $language = $this->app->language('en'); + + $this->storage->create($changes, $language, []); + + $this->assertIsInt($this->storage->modified($changes, $language)); + $this->assertNull($this->storage->modified(VersionId::published(), $language)); + } + + /** + * @covers ::modified + */ + public function testModifiedSomeExistingSingleLanguage() + { + $this->setUpSingleLanguage(); + + $changes = VersionId::changes(); + $language = Language::single(); + + $this->storage->create($changes, $language, []); + + $this->assertIsInt($this->storage->modified($changes, $language)); + $this->assertNull($this->storage->modified(VersionId::published(), $language)); + } + + /** + * @covers ::touch + */ + public function testTouchMultiLang() + { + $this->setUpMultiLanguage(); + + $versionId = VersionId::changes(); + $language = $this->app->language('en'); + + $time = time(); + + $this->storage->create($versionId, $language, []); + $this->storage->touch($versionId, $language); + + $this->assertGreaterThanOrEqual($time, $this->storage->modified($versionId, $language)); + } + + /** + * @covers ::touch + */ + public function testTouchSingleLang() + { + $this->setUpSingleLanguage(); + + $versionId = VersionId::changes(); + $language = Language::single(); + + $time = time(); + + $this->storage->create($versionId, $language, []); + $this->storage->touch($versionId, $language); + + $this->assertGreaterThanOrEqual($time, $this->storage->modified($versionId, $language)); + } +} From 3d86a11e6a85e1dd881b72bccbb6a9d96423179d Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Wed, 22 May 2024 15:09:22 +0200 Subject: [PATCH 05/15] Add unit tests --- .../MemoryContentStorageHandlerTest.php | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/Content/MemoryContentStorageHandlerTest.php b/tests/Content/MemoryContentStorageHandlerTest.php index 6f63f07cf4..561fe9a8d7 100644 --- a/tests/Content/MemoryContentStorageHandlerTest.php +++ b/tests/Content/MemoryContentStorageHandlerTest.php @@ -7,11 +7,18 @@ /** * @coversDefaultClass Kirby\Content\MemoryContentStorageHandler * @covers ::__construct + * @covers ::cacheId */ class MemoryContentStorageHandlerTest extends TestCase { protected $storage; + /** + * @covers ::create + * @covers ::delete + * @covers ::exists + * @covers ::write + */ public function assertCreateAndDelete(VersionId $versionId, Language $language): void { $this->storage->create($versionId, $language, []); @@ -23,6 +30,12 @@ public function assertCreateAndDelete(VersionId $versionId, Language $language): $this->assertFalse($this->storage->exists($versionId, $language)); } + /** + * @covers ::create + * @covers ::exists + * @covers ::read + * @covers ::write + */ public function assertCreateAndRead(VersionId $versionId, Language $language): void { $fields = [ @@ -36,6 +49,30 @@ public function assertCreateAndRead(VersionId $versionId, Language $language): v $this->assertSame($fields, $this->storage->read($versionId, $language)); } + /** + * @covers ::create + * @covers ::exists + * @covers ::read + * @covers ::update + * @covers ::write + */ + public function assertCreateAndUpdate(VersionId $versionId, Language $language): void + { + $fields = [ + 'title' => 'Foo', + 'text' => 'Bar' + ]; + + $this->storage->create($versionId, $language, []); + + $this->assertSame([], $this->storage->read($versionId, $language)); + + $this->storage->update($versionId, $language, $fields); + + $this->assertTrue($this->storage->exists($versionId, $language)); + $this->assertSame($fields, $this->storage->read($versionId, $language)); + } + public function setUpMultiLanguage(): void { parent::setUpMultiLanguage(); @@ -108,6 +145,7 @@ public function testCreateAndReadPublishedSingleLang() /** * @covers ::delete + * @covers ::exists */ public function testDeleteNonExisting() { @@ -286,4 +324,30 @@ public function testTouchSingleLang() $this->assertGreaterThanOrEqual($time, $this->storage->modified($versionId, $language)); } + + /** + * @covers ::update + */ + public function testUpdateMultiLang() + { + $this->setUpMultiLanguage(); + + $versionId = VersionId::changes(); + $language = $this->app->language('en'); + + $this->assertCreateAndUpdate($versionId, $language); + } + + /** + * @covers ::update + */ + public function testUpdateSingleLang() + { + $this->setUpSingleLanguage(); + + $versionId = VersionId::changes(); + $language = Language::single(); + + $this->assertCreateAndUpdate($versionId, $language); + } } From 31c40eae68afe8c2336fba4e5523ec0eb58f3ad0 Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Tue, 11 Jun 2024 16:10:05 +0200 Subject: [PATCH 06/15] Fix return type of Version::ensure and ContentStorageHandler::ensure --- src/Content/ContentStorageHandler.php | 18 +++++++++--------- src/Content/Version.php | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Content/ContentStorageHandler.php b/src/Content/ContentStorageHandler.php index a7f0820561..c1084fd7a5 100644 --- a/src/Content/ContentStorageHandler.php +++ b/src/Content/ContentStorageHandler.php @@ -101,18 +101,18 @@ public function dynamicVersions(): array * * @throws \Kirby\Exception\NotFoundException If the version does not exist */ - public function ensure(VersionId $versionId, Language $language): bool + public function ensure(VersionId $versionId, Language $language): void { - if ($this->exists($versionId, $language) !== true) { - $message = match($this->model->kirby()->multilang()) { - true => 'Version "' . $versionId . ' (' . $language->code() . ')" does not already exist', - false => 'Version "' . $versionId . '" does not already exist', - }; - - throw new NotFoundException($message); + if ($this->exists($versionId, $language) === true) { + return; } - return true; + $message = match($this->model->kirby()->multilang()) { + true => 'Version "' . $versionId . ' (' . $language->code() . ')" does not already exist', + false => 'Version "' . $versionId . '" does not already exist', + }; + + throw new NotFoundException($message); } /** diff --git a/src/Content/Version.php b/src/Content/Version.php index d053dcc709..d9ba0d93a8 100644 --- a/src/Content/Version.php +++ b/src/Content/Version.php @@ -86,8 +86,8 @@ public function delete(): void */ public function ensure( Language|string $language = 'default' - ): bool { - return $this->model->storage()->ensure($this->id, $this->language($language)); + ): void { + $this->model->storage()->ensure($this->id, $this->language($language)); } /** From 6690f220691fc3696e25c9f0877c38c870dc3362 Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Thu, 20 Jun 2024 09:07:27 +0200 Subject: [PATCH 07/15] Improve docblock Co-authored-by: Lukas Bestle --- src/Content/ContentStorageHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Content/ContentStorageHandler.php b/src/Content/ContentStorageHandler.php index c1084fd7a5..dcd5f10821 100644 --- a/src/Content/ContentStorageHandler.php +++ b/src/Content/ContentStorageHandler.php @@ -97,7 +97,7 @@ public function dynamicVersions(): array /** * Checks if a version/language combination exists and otherwise - * will throw a NotFoundException + * will throw a `NotFoundException` * * @throws \Kirby\Exception\NotFoundException If the version does not exist */ From e937227b219ad70175cec9893ec00ab588366608 Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Thu, 20 Jun 2024 09:11:50 +0200 Subject: [PATCH 08/15] Remove invalid covers rules --- .../MemoryContentStorageHandlerTest.php | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/tests/Content/MemoryContentStorageHandlerTest.php b/tests/Content/MemoryContentStorageHandlerTest.php index 561fe9a8d7..0351f37fd0 100644 --- a/tests/Content/MemoryContentStorageHandlerTest.php +++ b/tests/Content/MemoryContentStorageHandlerTest.php @@ -13,12 +13,6 @@ class MemoryContentStorageHandlerTest extends TestCase { protected $storage; - /** - * @covers ::create - * @covers ::delete - * @covers ::exists - * @covers ::write - */ public function assertCreateAndDelete(VersionId $versionId, Language $language): void { $this->storage->create($versionId, $language, []); @@ -30,12 +24,6 @@ public function assertCreateAndDelete(VersionId $versionId, Language $language): $this->assertFalse($this->storage->exists($versionId, $language)); } - /** - * @covers ::create - * @covers ::exists - * @covers ::read - * @covers ::write - */ public function assertCreateAndRead(VersionId $versionId, Language $language): void { $fields = [ @@ -49,13 +37,6 @@ public function assertCreateAndRead(VersionId $versionId, Language $language): v $this->assertSame($fields, $this->storage->read($versionId, $language)); } - /** - * @covers ::create - * @covers ::exists - * @covers ::read - * @covers ::update - * @covers ::write - */ public function assertCreateAndUpdate(VersionId $versionId, Language $language): void { $fields = [ From 3f4bb89892b5f63fdf7780f9816a0760fb58067d Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Thu, 20 Jun 2024 09:27:17 +0200 Subject: [PATCH 09/15] Mark ::move as covered by the moveLanguage tests --- tests/Content/ContentStorageHandlerTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Content/ContentStorageHandlerTest.php b/tests/Content/ContentStorageHandlerTest.php index 527529a276..ebc14ce44c 100644 --- a/tests/Content/ContentStorageHandlerTest.php +++ b/tests/Content/ContentStorageHandlerTest.php @@ -274,6 +274,7 @@ public function testDynamicVersionsForUser() } /** + * @covers ::move * @covers ::moveLanguage */ public function testMoveSingleLanguageToMultiLanguage() @@ -305,6 +306,7 @@ public function testMoveSingleLanguageToMultiLanguage() } /** + * @covers ::move * @covers ::moveLanguage */ public function testMoveMultiLanguageToSingleLanguage() From ae3d9a28d2b53fcf6a6e937d4ad89b2c4756944b Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Thu, 20 Jun 2024 09:28:37 +0200 Subject: [PATCH 10/15] Mark ::write as covered by ::create and ::update tests --- tests/Content/MemoryContentStorageHandlerTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Content/MemoryContentStorageHandlerTest.php b/tests/Content/MemoryContentStorageHandlerTest.php index 0351f37fd0..f8375e7af2 100644 --- a/tests/Content/MemoryContentStorageHandlerTest.php +++ b/tests/Content/MemoryContentStorageHandlerTest.php @@ -71,6 +71,7 @@ public function setUpSingleLanguage(): void /** * @covers ::create * @covers ::read + * @covers ::write */ public function testCreateAndReadChangesMultiLang() { @@ -85,6 +86,7 @@ public function testCreateAndReadChangesMultiLang() /** * @covers ::create * @covers ::read + * @covers ::write */ public function testCreateAndReadChangesSingleLang() { @@ -99,6 +101,7 @@ public function testCreateAndReadChangesSingleLang() /** * @covers ::create * @covers ::read + * @covers ::write */ public function testCreateAndReadPublishedMultiLang() { @@ -113,6 +116,7 @@ public function testCreateAndReadPublishedMultiLang() /** * @covers ::create * @covers ::read + * @covers ::write */ public function testCreateAndReadPublishedSingleLang() { @@ -308,6 +312,7 @@ public function testTouchSingleLang() /** * @covers ::update + * @covers ::write */ public function testUpdateMultiLang() { @@ -321,6 +326,7 @@ public function testUpdateMultiLang() /** * @covers ::update + * @covers ::write */ public function testUpdateSingleLang() { From f69786cfc737aca1ffe1daf9ddcec65785446ed9 Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Thu, 20 Jun 2024 09:38:04 +0200 Subject: [PATCH 11/15] Add a positive test for ::exists --- .../MemoryContentStorageHandlerTest.php | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/Content/MemoryContentStorageHandlerTest.php b/tests/Content/MemoryContentStorageHandlerTest.php index f8375e7af2..2cd1e9c9fc 100644 --- a/tests/Content/MemoryContentStorageHandlerTest.php +++ b/tests/Content/MemoryContentStorageHandlerTest.php @@ -199,6 +199,42 @@ public function testDeletePublishedSingleLang() $this->assertCreateAndDelete($versionId, $language); } + /** + * @covers ::exists + */ + public function testExistsMultiLanguage() + { + $this->setUpMultiLanguage(); + + $versionId = VersionId::published(); + + $this->assertFalse($this->storage->exists($versionId, $this->app->language('en'))); + $this->assertFalse($this->storage->exists($versionId, $this->app->language('de'))); + + $this->storage->create($versionId, $this->app->language('en'), []); + $this->storage->create($versionId, $this->app->language('de'), []); + + $this->assertTrue($this->storage->exists($versionId, $this->app->language('en'))); + $this->assertTrue($this->storage->exists($versionId, $this->app->language('de'))); + } + + /** + * @covers ::exists + */ + public function testExistsSingleLanguage() + { + $this->setUpSingleLanguage(); + + $versionId = VersionId::published(); + $language = Language::single(); + + $this->assertFalse($this->storage->exists($versionId, $language)); + + $this->storage->create($versionId, $language, []); + + $this->assertTrue($this->storage->exists($versionId, $language)); + } + /** * @covers ::exists */ From 99bdc4dbd918b805c369a5764ce33756b03bafa1 Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Thu, 20 Jun 2024 10:19:14 +0200 Subject: [PATCH 12/15] Improve ContentStorageHandlerTest class --- tests/Content/ContentStorageHandlerTest.php | 168 ++++++++++++++++---- tests/Content/TestContentStorageHandler.php | 23 ++- 2 files changed, 158 insertions(+), 33 deletions(-) diff --git a/tests/Content/ContentStorageHandlerTest.php b/tests/Content/ContentStorageHandlerTest.php index ebc14ce44c..311a6673f1 100644 --- a/tests/Content/ContentStorageHandlerTest.php +++ b/tests/Content/ContentStorageHandlerTest.php @@ -34,10 +34,18 @@ public function testAllMultiLanguageForFile() $versions = iterator_to_array($handler->all(), false); - // The TestContentStorage handler always returns true - // for every version and language. Thus there should be - // 2 versions for every language. - // + $this->assertCount(0, $versions); + + // create all possible versions + $handler->create(VersionId::published(), $this->app->language('en'), []); + $handler->create(VersionId::published(), $this->app->language('de'), []); + + $handler->create(VersionId::changes(), $this->app->language('en'), []); + $handler->create(VersionId::changes(), $this->app->language('de'), []); + + // count again + $versions = iterator_to_array($handler->all(), false); + // article.en.txt // article.de.txt // _changes/article.en.txt @@ -61,10 +69,15 @@ public function testAllSingleLanguageForFile() $versions = iterator_to_array($handler->all(), false); - // The TestContentStorage handler always returns true - // for every version and language. Thus there should be - // 2 versions in a single language installation. - // + $this->assertCount(0, $versions); + + // create all possible versions + $handler->create(VersionId::published(), Language::single(), []); + $handler->create(VersionId::changes(), Language::single(), []); + + // count again + $versions = iterator_to_array($handler->all(), false); + // article.txt // _changes/article.txt $this->assertCount(2, $versions); @@ -83,6 +96,18 @@ public function testAllMultiLanguageForPage() $versions = iterator_to_array($handler->all(), false); + $this->assertCount(0, $versions); + + // create all possible versions + $handler->create(VersionId::published(), $this->app->language('en'), []); + $handler->create(VersionId::published(), $this->app->language('de'), []); + + $handler->create(VersionId::changes(), $this->app->language('en'), []); + $handler->create(VersionId::changes(), $this->app->language('de'), []); + + // count again + $versions = iterator_to_array($handler->all(), false); + // A page that's not in draft mode can have published and changes versions // and thus should have changes and published for every language $this->assertCount(4, $versions); @@ -101,6 +126,17 @@ public function testAllMultiLanguageForPageDraft() $versions = iterator_to_array($handler->all(), false); + $this->assertCount(0, $versions); + + $handler->create(VersionId::published(), $this->app->language('en'), []); + $handler->create(VersionId::published(), $this->app->language('de'), []); + + $handler->create(VersionId::changes(), $this->app->language('en'), []); + $handler->create(VersionId::changes(), $this->app->language('de'), []); + + // count again + $versions = iterator_to_array($handler->all(), false); + // A draft page has only changes and thus should only have // a changes for every language, but no published versions $this->assertCount(2, $versions); @@ -119,6 +155,15 @@ public function testAllSingleLanguageForPage() $versions = iterator_to_array($handler->all(), false); + $this->assertCount(0, $versions); + + // create all possible versions + $handler->create(VersionId::published(), Language::single(), []); + $handler->create(VersionId::changes(), Language::single(), []); + + // count again + $versions = iterator_to_array($handler->all(), false); + // A page that's not in draft mode can have published and changes versions $this->assertCount(2, $versions); } @@ -136,6 +181,15 @@ public function testAllSingleLanguageForPageDraft() $versions = iterator_to_array($handler->all(), false); + $this->assertCount(0, $versions); + + // create all possible versions + $handler->create(VersionId::published(), Language::single(), []); + $handler->create(VersionId::changes(), Language::single(), []); + + // count again + $versions = iterator_to_array($handler->all(), false); + // A draft page has only changes and thus should only have // a single version in a single language installation $this->assertCount(1, $versions); @@ -148,22 +202,21 @@ public function testDeleteLanguageMultiLanguage() { $this->setUpMultiLanguage(); - // Use the plain text handler, as the abstract class and the test handler do not - // implement the necessary methods to test this. - $handler = new PlainTextContentStorageHandler( + $handler = new TestContentStorageHandler( model: $this->model ); - Data::write($filePublished = $this->model->root() . '/article.de.txt', []); - Data::write($fileChanges = $this->model->root() . '/_changes/article.de.txt', []); + // create two versions for the German language + $handler->create(VersionId::published(), $this->app->language('de'), []); + $handler->create(VersionId::changes(), $this->app->language('de'), []); - $this->assertFileExists($filePublished); - $this->assertFileExists($fileChanges); + $this->assertTrue($handler->exists(VersionId::published(), $this->app->language('de'))); + $this->assertTrue($handler->exists(VersionId::changes(), $this->app->language('de'))); $handler->deleteLanguage($this->app->language('de')); - $this->assertFileDoesNotExist($filePublished); - $this->assertFileDoesNotExist($fileChanges); + $this->assertFalse($handler->exists(VersionId::published(), $this->app->language('de'))); + $this->assertFalse($handler->exists(VersionId::changes(), $this->app->language('de'))); } /** @@ -175,20 +228,23 @@ public function testDeleteLanguageSingleLanguage() // Use the plain text handler, as the abstract class and the test handler do not // implement the necessary methods to test this. - $handler = new PlainTextContentStorageHandler( + $handler = new TestContentStorageHandler( model: $this->model ); - Data::write($filePublished = $this->model->root() . '/article.txt', []); - Data::write($fileChanges = $this->model->root() . '/_changes/article.txt', []); + $language = Language::single(); - $this->assertFileExists($filePublished); - $this->assertFileExists($fileChanges); + // create two versions + $handler->create(VersionId::published(), $language, []); + $handler->create(VersionId::changes(), $language, []); - $handler->deleteLanguage(Language::single()); + $this->assertTrue($handler->exists(VersionId::published(), $language)); + $this->assertTrue($handler->exists(VersionId::changes(), $language)); - $this->assertFileDoesNotExist($filePublished); - $this->assertFileDoesNotExist($fileChanges); + $handler->deleteLanguage($language); + + $this->assertFalse($handler->exists(VersionId::published(), $language)); + $this->assertFalse($handler->exists(VersionId::changes(), $language)); } /** @@ -275,14 +331,67 @@ public function testDynamicVersionsForUser() /** * @covers ::move + */ + public function testMoveMultiLanguage() + { + $this->setUpMultiLanguage(); + + $handler = new TestContentStorageHandler( + model: $this->model + ); + + $en = $this->app->language('en'); + $de = $this->app->language('de'); + + $handler->create(VersionId::published(), $en, []); + + $this->assertTrue($handler->exists(VersionId::published(), $en)); + $this->assertFalse($handler->exists(VersionId::published(), $de)); + + $handler->move( + VersionId::published(), + $en, + VersionId::published(), + $de + ); + + $this->assertFalse($handler->exists(VersionId::published(), $en)); + $this->assertTrue($handler->exists(VersionId::published(), $de)); + } + + public function testMoveSingleLanguage() + { + $this->setUpSingleLanguage(); + + $handler = new TestContentStorageHandler( + model: $this->model + ); + + $handler->create(VersionId::published(), Language::single(), []); + + $this->assertTrue($handler->exists(VersionId::published(), Language::single())); + $this->assertFalse($handler->exists(VersionId::changes(), Language::single())); + + $handler->move( + VersionId::published(), + Language::single(), + VersionId::changes(), + Language::single() + ); + + $this->assertFalse($handler->exists(VersionId::published(), Language::single())); + $this->assertTrue($handler->exists(VersionId::changes(), Language::single())); + } + + /** * @covers ::moveLanguage */ public function testMoveSingleLanguageToMultiLanguage() { $this->setUpMultiLanguage(); - // Use the plain text handler, as the abstract class and the test handler do not - // implement the necessary methods to test this. + // Use the plain text handler, as it offers the most + // realistic, testable results for this test $handler = new PlainTextContentStorageHandler( model: $this->model ); @@ -306,15 +415,14 @@ public function testMoveSingleLanguageToMultiLanguage() } /** - * @covers ::move * @covers ::moveLanguage */ public function testMoveMultiLanguageToSingleLanguage() { $this->setUpMultiLanguage(); - // Use the plain text handler, as the abstract class and the test handler do not - // implement the necessary methods to test this. + // Use the plain text handler, as it offers the most + // realistic, testable results for this test $handler = new PlainTextContentStorageHandler( model: $this->model ); diff --git a/tests/Content/TestContentStorageHandler.php b/tests/Content/TestContentStorageHandler.php index 28da437496..451fb4b38d 100644 --- a/tests/Content/TestContentStorageHandler.php +++ b/tests/Content/TestContentStorageHandler.php @@ -2,34 +2,51 @@ namespace Kirby\Content; +use Exception; use Kirby\Cms\Language; +use Kirby\Cms\ModelWithContent; class TestContentStorageHandler extends ContentStorageHandler { + public array $store = []; + + public function __construct(protected ModelWithContent $model) + { + $this->store = []; + } + public function delete(VersionId $versionId, Language $language): void { + unset($this->store[$this->key($versionId, $language)]); } public function exists(VersionId $versionId, Language $language): bool { - return true; + return isset($this->store[$this->key($versionId, $language)]); + } + + public function key(VersionId $versionId, Language $language): string + { + return $versionId . '/' . $language; } public function modified(VersionId $versionId, Language $language): int|null { - return null; + throw new Exception('Not implemented'); } public function read(VersionId $versionId, Language $language): array { - return []; + return $this->store[$this->key($versionId, $language)] ?? []; } public function touch(VersionId $versionId, Language $language): void { + throw new Exception('Not implemented'); } public function write(VersionId $versionId, Language $language, array $fields): void { + $this->store[$this->key($versionId, $language)] = $fields; } } From 0ea064fee2cc329e7d517884db99a736c691a4fa Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Fri, 21 Jun 2024 10:33:04 +0200 Subject: [PATCH 13/15] Add MemoryCache test for the modified method --- tests/Cache/MemoryCacheTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/Cache/MemoryCacheTest.php b/tests/Cache/MemoryCacheTest.php index 76c44805d3..6b20b19ada 100644 --- a/tests/Cache/MemoryCacheTest.php +++ b/tests/Cache/MemoryCacheTest.php @@ -116,4 +116,18 @@ public function testFlushWithMultipleInstances() $this->assertTrue($cache2->exists('a')); $this->assertTrue($cache2->exists('b')); } + + /** + * @covers ::modified + */ + public function testModified() + { + $cache = new MemoryCache(); + + $time = time(); + + $cache->set('a', 'A basic value'); + + $this->assertGreaterThanOrEqual($time, $cache->modified('a')); + } } From 8e49e5d0711508cd1560ef499c9523f240f9031b Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Fri, 21 Jun 2024 10:43:55 +0200 Subject: [PATCH 14/15] Simply MemoryContentStorageHandler by using more MemoryCache methods --- src/Content/MemoryContentStorageHandler.php | 23 ++++----------------- tests/Content/mocks.php | 19 +++++++++++++++++ tests/bootstrap.php | 1 + 3 files changed, 24 insertions(+), 19 deletions(-) create mode 100644 tests/Content/mocks.php diff --git a/src/Content/MemoryContentStorageHandler.php b/src/Content/MemoryContentStorageHandler.php index 5cab9333a3..e4616477e0 100644 --- a/src/Content/MemoryContentStorageHandler.php +++ b/src/Content/MemoryContentStorageHandler.php @@ -51,19 +51,7 @@ public function delete(VersionId $versionId, Language $language): void */ public function exists(VersionId $versionId, Language $language): bool { - $data = $this->cache->get($this->cacheId($versionId, $language)); - - // validate the cached data - if ( - isset($data['fields']) === true && - is_array($data['fields']) === true && - isset($data['modified']) === true && - is_int($data['modified']) === true - ) { - return true; - } - - return false; + return $this->cache->exists($this->cacheId($versionId, $language)); } /** @@ -75,7 +63,7 @@ public function modified(VersionId $versionId, Language $language): int|null return null; } - return $this->cache->get($this->cacheId($versionId, $language))['modified']; + return $this->cache->modified($this->cacheId($versionId, $language)); } /** @@ -88,7 +76,7 @@ public function modified(VersionId $versionId, Language $language): int|null public function read(VersionId $versionId, Language $language): array { $this->ensure($versionId, $language); - return $this->cache->get($this->cacheId($versionId, $language))['fields']; + return $this->cache->get($this->cacheId($versionId, $language)); } /** @@ -122,9 +110,6 @@ public function update(VersionId $versionId, Language $language, array $fields): */ protected function write(VersionId $versionId, Language $language, array $fields): void { - $this->cache->set($this->cacheId($versionId, $language), [ - 'fields' => $fields, - 'modified' => time() - ]); + $this->cache->set($this->cacheId($versionId, $language), $fields); } } diff --git a/tests/Content/mocks.php b/tests/Content/mocks.php new file mode 100644 index 0000000000..36a1f324b3 --- /dev/null +++ b/tests/Content/mocks.php @@ -0,0 +1,19 @@ + Date: Mon, 24 Jun 2024 10:35:16 +0200 Subject: [PATCH 15/15] Remove redundant ::update method --- src/Content/MemoryContentStorageHandler.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/Content/MemoryContentStorageHandler.php b/src/Content/MemoryContentStorageHandler.php index e4616477e0..668f8d7bed 100644 --- a/src/Content/MemoryContentStorageHandler.php +++ b/src/Content/MemoryContentStorageHandler.php @@ -90,19 +90,6 @@ public function touch(VersionId $versionId, Language $language): void $this->write($versionId, $language, $fields); } - /** - * Updates the content fields of an existing version - * - * @param array $fields Content fields - * - * @throws \Kirby\Exception\NotFoundException If the version does not exist - */ - public function update(VersionId $versionId, Language $language, array $fields): void - { - $this->ensure($versionId, $language); - $this->write($versionId, $language, $fields); - } - /** * Writes the content fields of an existing version *