Skip to content

FEATURE: Enable hierarchical asset collections #4160

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

Draft
wants to merge 7 commits into
base: 7.3
Choose a base branch
from
Draft
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
47 changes: 47 additions & 0 deletions Neos.Media/Classes/Domain/Model/AssetCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ class AssetCollection
*/
protected $tags;

/**
* @var AssetCollection
* @ORM\ManyToOne(inversedBy="children", cascade={"persist"})
* @ORM\JoinColumn(onDelete="SET NULL")
* @Flow\Lazy
*/
protected $parent;

/**
* @param string $title
*/
Expand Down Expand Up @@ -177,4 +185,43 @@ public function removeTag(Tag $tag)
}
return false;
}

public function getParent(): ?self
{
return $this->parent;
}

/**
* Throws an error if a circular dependency has been detected
* @throws \InvalidArgumentException
*/
private function validateCircularHierarchy(): void
{
$parent = $this->parent;
while ($parent !== null) {
$parent = $parent->getParent();
if ($parent && $parent === $this->parent) {
throw new \InvalidArgumentException(sprintf(
'Circular reference detected, parent AssetCollection "%s" appeared twice in the hierarchy',
$parent->getTitle()
), 1680328041);
}
}
}

public function setParent(self $parent): void
{
$this->parent = $parent;
$this->validateCircularHierarchy();
}

public function unsetParent(): void
{
$this->parent = null;
}

public function hasParent(): bool
{
return $this->parent !== null;
}
}
20 changes: 20 additions & 0 deletions Neos.Media/Classes/Domain/Repository/AssetCollectionRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@

use Neos\Flow\Annotations as Flow;
use Neos\Flow\Persistence\QueryInterface;
use Neos\Flow\Persistence\QueryResultInterface;
use Neos\Flow\Persistence\Repository;
use Neos\Media\Domain\Model\AssetCollection;

/**
* A repository for AssetCollections
*
* @method AssetCollection findOneByTitle(string $title)
* @method QueryResultInterface<AssetCollection> findByParent(AssetCollection $parent)
*
* @Flow\Scope("singleton")
*/
class AssetCollectionRepository extends Repository
Expand All @@ -26,4 +31,19 @@ class AssetCollectionRepository extends Repository
* @var array
*/
protected $defaultOrderings = ['title' => QueryInterface::ORDER_ASCENDING];

/**
* Remove all child collections recursively to prevent orphaned collections
*/
public function remove($object): void
{
/** @var AssetCollection $object */
$childCollections = $this->findByParent($object);
foreach ($childCollections as $childCollection) {
$this->remove($childCollection);
}
parent::remove($object);
}


}
35 changes: 35 additions & 0 deletions Neos.Media/Migrations/Mysql/Version20230401055019.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

declare(strict_types=1);

namespace Neos\Flow\Persistence\Doctrine\Migrations;

use Doctrine\DBAL\Platforms\MySqlPlatform;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

final class Version20230401055019 extends AbstractMigration
{
public function getDescription(): string
{
return 'Adds the parent property to the AssetCollection entity.';
}

public function up(Schema $schema): void
{
$this->abortIf(!$this->connection->getDatabasePlatform() instanceof MySqlPlatform, 'Migration can only be executed safely on "mysql".');

$this->addSql('ALTER TABLE neos_media_domain_model_assetcollection ADD parent VARCHAR(40) DEFAULT NULL');
$this->addSql('ALTER TABLE neos_media_domain_model_assetcollection ADD CONSTRAINT FK_74C770A3D8E604F FOREIGN KEY (parent) REFERENCES neos_media_domain_model_assetcollection (persistence_object_identifier) ON DELETE SET NULL');
$this->addSql('CREATE INDEX IDX_74C770A3D8E604F ON neos_media_domain_model_assetcollection (parent)');
}

public function down(Schema $schema): void
{
$this->abortIf(!$this->connection->getDatabasePlatform() instanceof MySqlPlatform, 'Migration can only be executed safely on "mysql".');

$this->addSql('ALTER TABLE neos_media_domain_model_assetcollection DROP FOREIGN KEY FK_74C770A3D8E604F');
$this->addSql('DROP INDEX IDX_74C770A3D8E604F ON neos_media_domain_model_assetcollection');
$this->addSql('ALTER TABLE neos_media_domain_model_assetcollection DROP parent');
}
}
35 changes: 35 additions & 0 deletions Neos.Media/Migrations/Postgresql/Version20230401171612.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

declare(strict_types=1);

namespace Neos\Flow\Persistence\Doctrine\Migrations;

use Doctrine\DBAL\Platforms\PostgreSQL100Platform;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

final class Version20230401171612 extends AbstractMigration
{
public function getDescription(): string
{
return 'Adds the parent property to AssetCollections';
}

public function up(Schema $schema): void
{
$this->abortIf(!$this->connection->getDatabasePlatform() instanceof PostgreSQL100Platform, 'Migration can only be executed safely on "postgresql".');

$this->addSql('ALTER TABLE neos_media_domain_model_assetcollection ADD parent VARCHAR(40) DEFAULT NULL');
$this->addSql('ALTER TABLE neos_media_domain_model_assetcollection ADD CONSTRAINT FK_74C770A3D8E604F FOREIGN KEY (parent) REFERENCES neos_media_domain_model_assetcollection (persistence_object_identifier) ON DELETE SET NULL NOT DEFERRABLE INITIALLY IMMEDIATE');
$this->addSql('CREATE INDEX IDX_74C770A3D8E604F ON neos_media_domain_model_assetcollection (parent)');
}

public function down(Schema $schema): void
{
$this->abortIf(!$this->connection->getDatabasePlatform()->getName() instanceof PostgreSQL100Platform, 'Migration can only be executed safely on "postgresql".');

$this->addSql('ALTER TABLE neos_media_domain_model_assetcollection DROP CONSTRAINT FK_74C770A3D8E604F');
$this->addSql('DROP INDEX IDX_74C770A3D8E604F');
$this->addSql('ALTER TABLE neos_media_domain_model_assetcollection DROP parent');
}
}
177 changes: 177 additions & 0 deletions Neos.Media/Tests/Functional/Domain/Model/AssetCollectionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
<?php
declare(strict_types=1);

namespace Neos\Media\Tests\Functional\Domain\Model;

/*
* This file is part of the Neos.Media package.
*
* (c) Contributors of the Neos Project - www.neos.io
*
* This package is Open Source Software. For the full copyright and license
* information, please view the LICENSE file which was distributed with this
* source code.
*/

use Doctrine\Common\Collections\ArrayCollection;
use Neos\Flow\Persistence\Doctrine\PersistenceManager;
use Neos\Media\Domain\Model\AssetCollection;
use Neos\Media\Domain\Repository\AssetCollectionRepository;
use Neos\Media\Tests\Functional\AbstractTest;

class AssetCollectionTest extends AbstractTest
{
/**
* @var boolean
*/
protected static $testablePersistenceEnabled = true;

/**
* @var AssetCollectionRepository
*/
protected $assetCollectionRepository;

public function setUp(): void
{
parent::setUp();
if (!$this->persistenceManager instanceof PersistenceManager) {
static::markTestSkipped('Doctrine persistence is not enabled');
}

$this->assetCollectionRepository = $this->objectManager->get(AssetCollectionRepository::class);
}

/**
* @test
*/
public function parentChildrenRelation(): void
{
$child1 = new AssetCollection('child1');
$child2 = new AssetCollection('child2');
$parent = new AssetCollection('parent');

$child1->setParent($parent);
$child2->setParent($parent);

$this->assetCollectionRepository->add($parent);
$this->assetCollectionRepository->add($child1);
$this->assetCollectionRepository->add($child2);

$this->persistenceManager->persistAll();
$this->persistenceManager->clearState();

$persistedChild = $this->assetCollectionRepository->findOneByTitle('child1');
$persistedParent = $persistedChild->getParent();

self::assertEquals('parent', $persistedParent->getTitle());
self::assertNull($persistedParent->getParent());

$children = $this->assetCollectionRepository->findByParent($parent);
self::assertEquals(2, $children->count());
self::assertEquals('child1', $children->offsetGet(0)->getTitle());
self::assertEquals('child2', $children->offsetGet(1)->getTitle());
}

/**
* Verifies the following hierarchie throws an error:
* first -> second -> third -> first
*
* @test
*/
public function circularParentChildrenRelationThrowsErrorWhenSettingParent(): void
{
$firstCollection = new AssetCollection('first');
$secondCollection = new AssetCollection('second');
$thirdCollection = new AssetCollection('third');

$secondCollection->setParent($firstCollection);
$thirdCollection->setParent($secondCollection);

$this->expectException(\InvalidArgumentException::class);
$firstCollection->setParent($thirdCollection);
}

/**
* @test
*/
public function unsettingTheParentRemovesChildFromParent(): void
{
$child = new AssetCollection('child');
$parent = new AssetCollection('parent');

$child->setParent($parent);

$this->assetCollectionRepository->add($parent);
$this->assetCollectionRepository->add($child);

$this->persistenceManager->persistAll();
$this->persistenceManager->clearState();

$persistedChild = $this->assetCollectionRepository->findOneByTitle('child');
$persistedChild->unsetParent(null);

$this->assetCollectionRepository->update($persistedChild);
$this->persistenceManager->persistAll();
$this->persistenceManager->clearState();

$persistedChild = $this->assetCollectionRepository->findOneByTitle('child');
$persistedParent = $this->assetCollectionRepository->findOneByTitle('parent');

$children = $this->assetCollectionRepository->findByParent($persistedParent);

self::assertNull($persistedChild->getParent());
self::assertCount(0, $children);
}

/**
* @test
*/
public function deletingTheParentDeletesTheChild(): void
{
$child = new AssetCollection('child');
$parent = new AssetCollection('parent');

$child->setParent($parent);

$this->assetCollectionRepository->add($parent);
$this->assetCollectionRepository->add($child);

$this->persistenceManager->persistAll();
$this->persistenceManager->clearState();

$persistedParent = $this->assetCollectionRepository->findOneByTitle('parent');
$this->assetCollectionRepository->remove($persistedParent);

$this->persistenceManager->persistAll();
$this->persistenceManager->clearState();

$persistedChild = $this->assetCollectionRepository->findOneByTitle('child');
$persistedParent = $this->assetCollectionRepository->findOneByTitle('parent');

self::assertNull($persistedChild);
self::assertNull($persistedParent);
}

/**
* @test
*/
public function hasParentReturnsTrueIfParentIsSet(): void
{
$child = new AssetCollection('child');
$parent = new AssetCollection('parent');

$child->setParent($parent);

$this->assetCollectionRepository->add($parent);
$this->assetCollectionRepository->add($child);

$this->persistenceManager->persistAll();
$this->persistenceManager->clearState();

$persistedChild = $this->assetCollectionRepository->findOneByTitle('child');
$persistedParent = $this->assetCollectionRepository->findOneByTitle('parent');

self::assertTrue($persistedChild->hasParent());
self::assertFalse($persistedParent->hasParent());
}
}
Loading