Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for old locks #6816

Open
wants to merge 8 commits into
base: v5/develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions panel/src/components/Views/PreviewView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,16 @@
</header>
<iframe v-if="hasChanges" ref="changes" :src="src.changes"></iframe>
<k-empty v-else>
{{ $t("lock.unsaved.empty") }}
<k-button icon="edit" variant="filled" :link="back">
{{ $t("edit") }}
</k-button>
<template v-if="lock.isLegacy">
This content is locked by our old lock system. <br />
Changes cannot be previewed.
</template>
<template v-else>
{{ $t("lock.unsaved.empty") }}
<k-button icon="edit" variant="filled" :link="back">
{{ $t("edit") }}
</k-button>
</template>
</k-empty>
</section>
</main>
Expand Down Expand Up @@ -223,6 +229,8 @@ export default {
flex-grow: 1;
justify-content: center;
flex-direction: column;
text-align: center;
padding-inline: var(--spacing-3);
gap: var(--spacing-6);
--button-color-text: var(--color-text);
}
Expand Down
28 changes: 28 additions & 0 deletions src/Api/Controller/Changes.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
namespace Kirby\Api\Controller;

use Kirby\Cms\ModelWithContent;
use Kirby\Content\Lock;
use Kirby\Content\VersionId;
use Kirby\Filesystem\F;
use Kirby\Form\Form;

/**
Expand All @@ -18,13 +20,31 @@
*/
class Changes
{
/**
* Cleans up legacy lock files. The `discard`, `publish` and `save` actions
* are perfect for this cleanup job. They will be stopped early if
* the lock is still active and otherwise, we can use them to clean
* up outdated .lock files to keep the content folders clean. This
* can be removed as soon as old .lock files should no longer be around.
*
* @todo Remove in 6.0.0
*/
protected static function cleanup(ModelWithContent $model): void
{
F::remove(Lock::legacyFile($model));
}

/**
* Discards unsaved changes by deleting the changes version
*/
public static function discard(ModelWithContent $model): array
{
$model->version(VersionId::changes())->delete('current');

// Removes the old .lock file when it is no longer needed
// @todo Remove in 6.0.0
static::cleanup($model);

return [
'status' => 'ok'
];
Expand All @@ -41,6 +61,10 @@ public static function publish(ModelWithContent $model, array $input): array
input: $input
);

// Removes the old .lock file when it is no longer needed
// @todo Remove in 6.0.0
static::cleanup($model);

// get the changes version
$changes = $model->version(VersionId::changes());

Expand Down Expand Up @@ -77,6 +101,10 @@ public static function save(ModelWithContent $model, array $input): array
$changes = $model->version(VersionId::changes());
$latest = $model->version(VersionId::latest());

// Removes the old .lock file when it is no longer needed
// @todo Remove in 6.0.0
static::cleanup($model);
bastianallgeier marked this conversation as resolved.
Show resolved Hide resolved

// combine the new field changes with the
// last published state
$changes->save(
Expand Down
65 changes: 64 additions & 1 deletion src/Content/Lock.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
use Kirby\Cms\App;
use Kirby\Cms\Language;
use Kirby\Cms\Languages;
use Kirby\Cms\ModelWithContent;
use Kirby\Cms\User;
use Kirby\Data\Data;
use Kirby\Toolkit\Str;

/**
Expand All @@ -27,6 +29,7 @@ class Lock
public function __construct(
protected User|null $user = null,
protected int|null $modified = null,
protected bool $legacy = false
) {
}

Expand All @@ -39,6 +42,11 @@ public static function for(
Version $version,
Language|string $language = 'default'
): static {

if ($legacy = static::legacy($version->model())) {
return $legacy;
}

// wildcard to search for a lock in any language
// the first locked one will be preferred
if ($language === '*') {
Expand Down Expand Up @@ -87,13 +95,21 @@ public function isActive(): bool
}

/**
* Check if content locking is enabled at all
* Checks if content locking is enabled at all
*/
public static function isEnabled(): bool
{
return App::instance()->option('content.locking', true) !== false;
}

/**
* Checks if the lock is coming from an old .lock file
*/
public function isLegacy(): bool
{
return $this->legacy;
}

/**
* Checks if the lock is actually locked
*/
Expand Down Expand Up @@ -124,6 +140,52 @@ public function isLocked(): bool
return true;
}

/**
* Looks for old .lock files and tries to create a
* usable lock instance from them
*/
public static function legacy(ModelWithContent $model): static|null
{
$kirby = $model->kirby();
$file = static::legacyFile($model);
$id = '/' . $model->id();

// no legacy lock file? no lock.
if (file_exists($file) === false) {
return null;
}

$data = Data::read($file, 'yml', fail: false)[$id] ?? [];

// no valid lock entry? no lock.
if (isset($data['lock']) === false) {
return null;
}

// has the lock been unlocked? no lock.
if (isset($data['unlock']) === true) {
return null;
}

return new static(
user: $kirby->user($data['lock']['user']),
modified: $data['lock']['time'],
legacy: true
);
}

/**
* Returns the absolute path to a legacy lock file
*/
public static function legacyFile(ModelWithContent $model): string
{
$root = match ($model::CLASS_ALIAS) {
'file' => dirname($model->root()),
default => $model->root()
};
return $root . '/.lock';
}

/**
* Returns the timestamp when the locked content has
* been updated. You can pass a format to get a useful,
Expand All @@ -147,6 +209,7 @@ public function modified(
public function toArray(): array
{
return [
'isLegacy' => $this->isLegacy(),
'isLocked' => $this->isLocked(),
'modified' => $this->modified('c'),
'user' => [
Expand Down
132 changes: 132 additions & 0 deletions tests/Content/LockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Kirby\Cms\App;
use Kirby\Cms\Language;
use Kirby\Cms\User;
use Kirby\Data\Data;

/**
* @coversDefaultClass \Kirby\Content\Lock
Expand Down Expand Up @@ -155,6 +156,28 @@ public function testForWithLanguageWildcard()
$this->assertSame('admin', $lock->user()->id());
}

/**
* @covers ::for
*/
public function testForWithLegacyLock()
{
$page = $this->app->page('test');
$file = $page->root() . '/.lock';

Data::write($file, [
'/' . $page->id() => [
'lock' => [
'user' => 'editor',
'time' => $time = time()
]
]
], 'yml');

$lock = Lock::for($page->version('changes'));
$this->assertInstanceOf(Lock::class, $lock);
$this->assertTrue($lock->isLocked());
}

/**
* @covers ::isActive
*/
Expand Down Expand Up @@ -215,6 +238,18 @@ public function testIsEnabledWhenDisabled()
$this->assertFalse(Lock::isEnabled());
}

/**
* @covers ::isLegacy
*/
public function testIsLegacy()
{
$lock = new Lock();
$this->assertFalse($lock->isLegacy());

$lock = new Lock(legacy: true);
$this->assertTrue($lock->isLegacy());
}

/**
* @covers ::isLocked
*/
Expand Down Expand Up @@ -292,6 +327,102 @@ public function testIsLockedWithDifferentUserAndOldTimestamp()
$this->assertFalse($lock->isLocked());
}

/**
* @covers ::legacy
*/
public function testLegacy()
{
$page = $this->app->page('test');
$file = $page->root() . '/.lock';

Data::write($file, [
'/' . $page->id() => [
'lock' => [
'user' => 'editor',
'time' => $time = time()
]
]
], 'yml');

$lock = Lock::legacy($page);

$this->assertInstanceOf(Lock::class, $lock);
$this->assertTrue($lock->isLocked());
$this->assertTrue($lock->isLegacy());
$this->assertSame($this->app->user('editor'), $lock->user());
$this->assertSame($time, $lock->modified());
}

/**
* @covers ::legacy
*/
public function testLegacyWithoutLockInfo()
{
$page = $this->app->page('test');
$file = $page->root() . '/.lock';

Data::write($file, [], 'yml');

$lock = Lock::legacy($page);
$this->assertNull($lock);
}

/**
* @covers ::legacy
*/
public function testLegacyWithOutdatedFile()
{
$page = $this->app->page('test');
$file = $page->root() . '/.lock';

Data::write($file, [
'/' . $page->id() => [
'lock' => [
'user' => 'editor',
'time' => time() - 60 * 60 * 24
],
]
], 'yml');

$lock = Lock::legacy($page);

$this->assertInstanceOf(Lock::class, $lock);
$this->assertFalse($lock->isLocked());
}

/**
* @covers ::legacy
*/
public function testLegacyWithUnlockedFile()
{
$page = $this->app->page('test');
$file = $page->root() . '/.lock';

Data::write($file, [
'/' . $page->id() => [
'lock' => [
'user' => 'editor',
'time' => time()
],
'unlock' => ['admin']
]
], 'yml');

$lock = Lock::legacy($page);
$this->assertNull($lock);
}

/**
* @covers ::legacyFile
*/
public function testLegacyFile()
{
$page = $this->app->page('test');
$expected = $page->root() . '/.lock';

$this->assertSame($expected, Lock::legacyFile($page));
}

/**
* @covers ::modified
*/
Expand Down Expand Up @@ -319,6 +450,7 @@ public function testToArray()
);

$this->assertSame([
'isLegacy' => false,
'isLocked' => true,
'modified' => date('c', $modified),
'user' => [
Expand Down
3 changes: 3 additions & 0 deletions tests/Panel/Areas/SiteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public function testPage(): void

$this->assertSame('default', $props['blueprint']);
$this->assertSame([
'isLegacy' => false,
'isLocked' => false,
'modified' => null,
'user' => [
Expand Down Expand Up @@ -95,6 +96,7 @@ public function testPageFile(): void

$this->assertSame('image', $props['blueprint']);
$this->assertSame([
'isLegacy' => false,
'isLocked' => false,
'modified' => null,
'user' => [
Expand Down Expand Up @@ -190,6 +192,7 @@ public function testSiteFile(): void

$this->assertSame('image', $props['blueprint']);
$this->assertSame([
'isLegacy' => false,
'isLocked' => false,
'modified' => null,
'user' => [
Expand Down