From 87b586bc2c20639a7789f5fd104323f919758047 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Tue, 6 Mar 2018 10:37:34 +0100 Subject: [PATCH] refactored version-subscriber to use uuid instead of paths --- CHANGELOG.md | 1 + lib/Subscriber/Behavior/VersionSubscriber.php | 50 +++++++++++-------- .../Behavior/VersionSubscriberTest.php | 46 ++++++++++++----- 3 files changed, 62 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae39420..8317a62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ CHANGELOG for Sulu Document Manager =================================== * dev-master + * HOTFIX #119 Refactored version-subscriber to use uuid instead of paths * HOTFIX #118 Set path and node-name after renaming node * HOTFIX #117 Execute rename in flush-event to avoid ItemNotFoundException diff --git a/lib/Subscriber/Behavior/VersionSubscriber.php b/lib/Subscriber/Behavior/VersionSubscriber.php index c1c06ad..32f258c 100644 --- a/lib/Subscriber/Behavior/VersionSubscriber.php +++ b/lib/Subscriber/Behavior/VersionSubscriber.php @@ -53,12 +53,12 @@ class VersionSubscriber implements EventSubscriberInterface /** * @var string[] */ - private $checkoutPaths = []; + private $checkoutUuids = []; /** * @var string[] */ - private $checkpointPaths = []; + private $checkpointUuids = []; public function __construct(SessionInterface $defaultSession, PropertyEncoder $propertyEncoder) { @@ -75,7 +75,7 @@ public static function getSubscribedEvents() return [ Events::PERSIST => [ ['setVersionMixin', 468], - ['rememberCheckoutPaths', -512], + ['rememberCheckoutUuids', -512], ], Events::PUBLISH => [ ['setVersionMixin', 468], @@ -132,21 +132,21 @@ public function setVersionsOnDocument(HydrateEvent $event) } /** - * Remember which paths need to be checked out after everything has been saved. + * Remember which uuids need to be checked out after everything has been saved. * * @param PersistEvent $event */ - public function rememberCheckoutPaths(PersistEvent $event) + public function rememberCheckoutUuids(PersistEvent $event) { if (!$this->support($event->getDocument())) { return; } - $this->checkoutPaths[] = $event->getNode()->getPath(); + $this->checkoutUuids[] = $event->getNode()->getIdentifier(); } /** - * Remember for which paths a new version has to be created. + * Remember for which uuids a new version has to be created. * * @param PublishEvent $event */ @@ -157,8 +157,8 @@ public function rememberCreateVersion(PublishEvent $event) return; } - $this->checkpointPaths[] = [ - 'path' => $event->getNode()->getPath(), + $this->checkpointUuids[] = [ + 'uuid' => $event->getNode()->getIdentifier(), 'locale' => $document->getLocale(), 'author' => $event->getOption('user'), ]; @@ -169,29 +169,35 @@ public function rememberCreateVersion(PublishEvent $event) */ public function applyVersionOperations() { - foreach ($this->checkoutPaths as $path) { + foreach ($this->checkoutUuids as $uuid) { + $node = $this->defaultSession->getNodeByIdentifier($uuid); + $path = $node->getPath(); + if (!$this->versionManager->isCheckedOut($path)) { $this->versionManager->checkout($path); } } - $this->checkoutPaths = []; + $this->checkoutUuids = []; /** @var NodeInterface[] $nodes */ $nodes = []; $nodeVersions = []; - foreach ($this->checkpointPaths as $versionInformation) { - $version = $this->versionManager->checkpoint($versionInformation['path']); + foreach ($this->checkpointUuids as $versionInformation) { + $node = $this->defaultSession->getNodeByIdentifier($versionInformation['uuid']); + $path = $node->getPath(); + + $version = $this->versionManager->checkpoint($path); - if (!array_key_exists($versionInformation['path'], $nodes)) { - $nodes[$versionInformation['path']] = $this->defaultSession->getNode($versionInformation['path']); + if (!array_key_exists($path, $nodes)) { + $nodes[$path] = $this->defaultSession->getNode($path); } - $versions = $nodes[$versionInformation['path']]->getPropertyValueWithDefault(static::VERSION_PROPERTY, []); + $versions = $nodes[$path]->getPropertyValueWithDefault(static::VERSION_PROPERTY, []); - if (!array_key_exists($versionInformation['path'], $nodeVersions)) { - $nodeVersions[$versionInformation['path']] = $versions; + if (!array_key_exists($path, $nodeVersions)) { + $nodeVersions[$path] = $versions; } - $nodeVersions[$versionInformation['path']][] = json_encode( + $nodeVersions[$path][] = json_encode( [ 'locale' => $versionInformation['locale'], 'version' => $version->getName(), @@ -201,12 +207,12 @@ public function applyVersionOperations() ); } - foreach ($nodes as $path => $node) { - $node->setProperty(static::VERSION_PROPERTY, $nodeVersions[$path]); + foreach ($nodes as $uuid => $node) { + $node->setProperty(static::VERSION_PROPERTY, $nodeVersions[$uuid]); } $this->defaultSession->save(); - $this->checkpointPaths = []; + $this->checkpointUuids = []; } /** diff --git a/tests/Unit/Subscriber/Behavior/VersionSubscriberTest.php b/tests/Unit/Subscriber/Behavior/VersionSubscriberTest.php index 0778403..eaba8f6 100644 --- a/tests/Unit/Subscriber/Behavior/VersionSubscriberTest.php +++ b/tests/Unit/Subscriber/Behavior/VersionSubscriberTest.php @@ -81,10 +81,10 @@ public function setUp() $this->versionSubscriber = new VersionSubscriber($this->session->reveal(), $this->propertyEncoder->reveal()); - $this->checkoutPathsReflection = new \ReflectionProperty(VersionSubscriber::class, 'checkoutPaths'); + $this->checkoutPathsReflection = new \ReflectionProperty(VersionSubscriber::class, 'checkoutUuids'); $this->checkoutPathsReflection->setAccessible(true); - $this->checkpointPathsReflection = new \ReflectionProperty(VersionSubscriber::class, 'checkpointPaths'); + $this->checkpointPathsReflection = new \ReflectionProperty(VersionSubscriber::class, 'checkpointUuids'); $this->checkpointPathsReflection->setAccessible(true); } @@ -185,11 +185,11 @@ public function testRememberCheckoutNodes() $event->getNode()->willReturn($node->reveal()); $event->getDocument()->willReturn($document->reveal()); - $node->getPath()->willReturn('/path/to/node'); + $node->getIdentifier()->willReturn('123-123-13'); - $this->versionSubscriber->rememberCheckoutPaths($event->reveal()); + $this->versionSubscriber->rememberCheckoutUuids($event->reveal()); - $this->assertEquals(['/path/to/node'], $this->checkoutPathsReflection->getValue($this->versionSubscriber)); + $this->assertEquals(['123-123-13'], $this->checkoutPathsReflection->getValue($this->versionSubscriber)); } public function testRememberCheckoutNodesWithoutVersionBehavior() @@ -199,7 +199,7 @@ public function testRememberCheckoutNodesWithoutVersionBehavior() $event->getDocument()->willReturn($document); - $this->versionSubscriber->rememberCheckoutPaths($event->reveal()); + $this->versionSubscriber->rememberCheckoutUuids($event->reveal()); $this->assertEmpty($this->checkoutPathsReflection->getValue($this->versionSubscriber)); } @@ -216,12 +216,12 @@ public function testRememberCreateVersionNodes() $event->getDocument()->willReturn($document->reveal()); $event->getOption('user')->willReturn(2); - $node->getPath()->willReturn('/path/to/node'); + $node->getIdentifier()->willReturn('123-123-123'); $this->versionSubscriber->rememberCreateVersion($event->reveal()); $this->assertEquals( - [['path' => '/path/to/node', 'locale' => 'de', 'author' => 2]], + [['uuid' => '123-123-123', 'locale' => 'de', 'author' => 2]], $this->checkpointPathsReflection->getValue($this->versionSubscriber) ); } @@ -241,11 +241,23 @@ public function testApplyVersionOperations() ClockMock::register(VersionSubscriber::class); ClockMock::withClockMock(true); - $this->checkoutPathsReflection->setValue($this->versionSubscriber, ['/node1', '/node2']); + $this->checkoutPathsReflection->setValue($this->versionSubscriber, ['1-1-1-1', '2-2-2-2']); $this->checkpointPathsReflection->setValue($this->versionSubscriber, [ - ['path' => '/node3', 'locale' => 'de', 'author' => 1], + ['uuid' => '3-3-3-3', 'locale' => 'de', 'author' => 1], ]); + $node = $this->prophesize(NodeInterface::class); + $node->getPath()->willReturn('/node1'); + $this->session->getNodeByIdentifier('1-1-1-1')->willReturn($node->reveal()); + + $node = $this->prophesize(NodeInterface::class); + $node->getPath()->willReturn('/node2'); + $this->session->getNodeByIdentifier('2-2-2-2')->willReturn($node->reveal()); + + $node = $this->prophesize(NodeInterface::class); + $node->getPath()->willReturn('/node3'); + $this->session->getNodeByIdentifier('3-3-3-3')->willReturn($node->reveal()); + $this->versionManager->isCheckedOut('/node1')->willReturn(false); $this->versionManager->isCheckedOut('/node2')->willReturn(true); @@ -283,12 +295,20 @@ public function testApplyVersionOperationsWithMultipleCheckpoints() $this->checkpointPathsReflection->setValue( $this->versionSubscriber, [ - ['path' => '/node1', 'locale' => 'de', 'author' => 2], - ['path' => '/node1', 'locale' => 'en', 'author' => 3], - ['path' => '/node2', 'locale' => 'en', 'author' => 1], + ['uuid' => '1-1-1-1', 'locale' => 'de', 'author' => 2], + ['uuid' => '1-1-1-1', 'locale' => 'en', 'author' => 3], + ['uuid' => '2-2-2-2', 'locale' => 'en', 'author' => 1], ] ); + $node = $this->prophesize(NodeInterface::class); + $node->getPath()->willReturn('/node1'); + $this->session->getNodeByIdentifier('1-1-1-1')->willReturn($node->reveal()); + + $node = $this->prophesize(NodeInterface::class); + $node->getPath()->willReturn('/node2'); + $this->session->getNodeByIdentifier('2-2-2-2')->willReturn($node->reveal()); + $node1 = $this->prophesize(NodeInterface::class); $node1->getPropertyValueWithDefault('sulu:versions', [])->willReturn(['{"locale":"fr","version":"0","author":1,"authored":"2016-12-05T19:47:22+01:00"}']); $this->session->getNode('/node1')->willReturn($node1->reveal());