Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

[WIP] Support for move and remove #23

Merged
merged 5 commits into from
Jun 18, 2016
Merged
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
3 changes: 1 addition & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
language: php

php:
- 5.4
- 5.5
- 5.6
- 7.0
Expand All @@ -12,7 +11,7 @@ sudo: false
cache:
directories:
- $HOME/.composer/cache/files

matrix:
fast_finish: true

Expand Down
115 changes: 98 additions & 17 deletions Repository/AbstractPhpcrRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,21 @@

namespace Symfony\Cmf\Component\Resource\Repository;

use Puli\Repository\Api\ChangeStream\VersionList;
use Puli\Repository\Api\NoVersionFoundException;
use Puli\Repository\Api\ResourceNotFoundException;
use DTL\Glob\FinderInterface;
use Puli\Repository\Api\ResourceRepository;
use Puli\Repository\Api\UnsupportedLanguageException;
use Puli\Repository\Resource\Collection\ArrayResourceCollection;
use Webmozart\PathUtil\Path;
use Webmozart\Assert\Assert;
use DTL\Glob\FinderInterface;
use Puli\Repository\AbstractRepository;
use Symfony\Cmf\Component\Resource\Repository\Api\EditableRepository;
use DTL\Glob\GlobHelper;

/**
* Abstract repository for both PHPCR and PHPCR-ODM repositories.
*
* @author Daniel Leech <[email protected]>
*/
abstract class AbstractPhpcrRepository implements ResourceRepository
abstract class AbstractPhpcrRepository extends AbstractRepository implements ResourceRepository, EditableRepository
{
/**
* Base path from which to serve nodes / nodes.
Expand All @@ -38,13 +39,19 @@ abstract class AbstractPhpcrRepository implements ResourceRepository
*/
private $finder;

/**
* @var GlobHelper
*/
private $globHelper;

/**
* @param string $basePath
*/
public function __construct(FinderInterface $finder, $basePath = null)
{
$this->finder = $finder;
$this->basePath = $basePath;
$this->globHelper = new GlobHelper();
}

/**
Expand All @@ -71,6 +78,74 @@ public function find($query, $language = 'glob')
return $this->buildCollection($nodes);
}

/**
* {@inheritdoc}
*/
public function remove($query, $language = 'glob')
{
$this->failUnlessGlob($language);
$nodes = $this->finder->find($this->resolvePath($query));

if (0 === count($nodes)) {
return 0;
}

try {
// delegate remove nodes to the implementation
$this->removeNodes($nodes);
} catch (\Exception $e) {
throw new \RuntimeException(sprintf(
'Error encountered when removing resource(s) using query "%s"',
$query
), null, $e);
}

return count($nodes);
}

/**
* {@inheritdoc}
*/
public function move($query, $targetPath, $language = 'glob')
{
$this->failUnlessGlob($language);
$nodes = $this->finder->find($this->resolvePath($query));

if (0 === count($nodes)) {
return 0;
}

$targetPath = $this->resolvePath($targetPath);

try {
// delegate moving to the implementation
$this->moveNodes($nodes, $query, $targetPath);
} catch (\Exception $e) {
throw new \RuntimeException(sprintf(
'Error encountered when moving resource(s) using query "%s"',
$query
), null, $e);
}

return count($nodes);
}

/**
* {@inheritdoc}
*/
public function clear()
{
throw new \BadMethodCallException('Clear not supported');
}

/**
* {@inheritdoc}
*/
public function add($path, $resource)
{
throw new \BadMethodCallException('Add not supported');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing add support for now. It doesn't make much sense to me as we would either:

a. create the payload first via. the implementation, then there is no point in an abstraction.
b. create an empty payload - but that will fail if there are required fields / properties.

Also we do not need this for any of our current use cases, so I think we can safely leave it out.

}

/**
* Return the path with the basePath prefix
* if it has been set.
Expand All @@ -81,8 +156,7 @@ public function find($query, $language = 'glob')
*/
protected function resolvePath($path)
{
Assert::stringNotEmpty($path, 'The path must be a non-empty string. Got: %s');
Assert::startsWith($path, '/', 'The path %s is not absolute.');
$path = $this->sanitizePath($path);

if ($this->basePath) {
$path = $this->basePath.$path;
Expand All @@ -107,6 +181,11 @@ protected function unresolvePath($path)
return $path;
}

protected function isGlobbed($string)
{
return $this->globHelper->isGlobbed($string);
}

/**
* Build a collection of PHPCR resources.
*
Expand All @@ -115,14 +194,16 @@ protected function unresolvePath($path)
abstract protected function buildCollection(array $nodes);

/**
* {@inheritdoc}
* Rmeove the given nodes.
*
* @param NodeInterface[]
*/
public function getVersions($path)
{
try {
return new VersionList($path, [$this->get($path)]);
} catch (ResourceNotFoundException $e) {
throw NoVersionFoundException::forPath($path, $e);
}
}
abstract protected function removeNodes(array $nodes);

/**
* Move the given nodes.
*
* @param NodeInterface[]
*/
abstract protected function moveNodes(array $nodes, $query, $targetPath);
}
42 changes: 42 additions & 0 deletions Repository/Api/EditableRepository.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

/*
* This file is part of the Symfony CMF package.
*
* (c) 2011-2015 Symfony CMF
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Cmf\Component\Resource\Repository\Api;

use InvalidArgumentException;
use Puli\Repository\Api\EditableRepository as PuliEditableRepository;
use Puli\Repository\Api\UnsupportedLanguageException;

/**
* Extends the Puli editable repository to implement the as-of-yet not
* implemented features.
*
* @author Maximilian Berghoff <[email protected]>
*/
interface EditableRepository extends PuliEditableRepository
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this need to have the remove() method as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
/**
* Move all resources and their subgraphs found by $sourceQuery to the
* target (parent) path and returns the number of nodes that have been
* *explicitly* moved (i.e. the number of resources found by the query, NOT
* the total number of nodes affected).
*
* @param string $sourceQuery
* @param string $targetPath
* @param string $language
*
* @return int
*
* @throws InvalidArgumentException If the sourceQuery is invalid.
* @throws UnsupportedLanguageException If the language is not supported.
*/
public function move($sourceQuery, $targetPath, $language = 'glob');
}
43 changes: 40 additions & 3 deletions Repository/PhpcrOdmRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@
namespace Symfony\Cmf\Component\Resource\Repository;

use Doctrine\Common\Persistence\ManagerRegistry;
use Doctrine\ODM\PHPCR\DocumentManagerInterface;
use DTL\Glob\Finder\PhpcrOdmTraversalFinder;
use DTL\Glob\FinderInterface;
use Symfony\Cmf\Component\Resource\Repository\Resource\PhpcrOdmResource;
use Puli\Repository\Resource\Collection\ArrayResourceCollection;
use Puli\Repository\Api\ResourceNotFoundException;
use Puli\Repository\Resource\Collection\ArrayResourceCollection;
use Symfony\Cmf\Component\Resource\Repository\Resource\PhpcrOdmResource;

class PhpcrOdmRepository extends AbstractPhpcrRepository
{
Expand All @@ -32,6 +33,9 @@ public function __construct(ManagerRegistry $managerRegistry, $basePath = null,
$this->managerRegistry = $managerRegistry;
}

/**
* @return DocumentManagerInterface
*/
protected function getManager()
{
return $this->managerRegistry->getManager();
Expand Down Expand Up @@ -82,7 +86,7 @@ public function contains($selector, $language = 'glob')
*/
public function findByTag($tag)
{
throw new \Exception('Get by tag not currently supported');
throw new \Exception('Find by tag not supported');
}

/**
Expand Down Expand Up @@ -116,4 +120,37 @@ protected function buildCollection(array $documents)

return $collection;
}

/**
* {@inheritdoc}
*/
protected function removeNodes(array $documents)
{
foreach ($documents as $document) {
$this->getManager()->remove($document);
}

$this->getManager()->flush();
}

/**
* {@inheritdoc}
*/
protected function moveNodes(array $documents, $sourceQuery, $targetPath)
{
$this->doMoveNodes($documents, $sourceQuery, $targetPath);
$this->getManager()->flush();
}

private function doMoveNodes(array $documents, $sourceQuery, $targetPath)
{
if (false === $this->isGlobbed($sourceQuery)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Symfony CS, this would be written as just if (!$this->isGlobbed($sourceQuery)) { I find that more readable, but I've no hard preference.

return $this->getManager()->move(current($documents), $targetPath);
}

foreach ($documents as $document) {
$node = $this->getManager()->getNodeForDocument($document);
$this->getManager()->move($document, $targetPath.'/'.$node->getName());
}
}
}
45 changes: 40 additions & 5 deletions Repository/PhpcrRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@

namespace Symfony\Cmf\Component\Resource\Repository;

use PHPCR\SessionInterface;
use DTL\Glob\Finder\PhpcrTraversalFinder;
use DTL\Glob\FinderInterface;
use Symfony\Cmf\Component\Resource\Repository\Resource\PhpcrResource;
use Puli\Repository\Resource\Collection\ArrayResourceCollection;
use PHPCR\SessionInterface;
use Puli\Repository\Api\ResourceNotFoundException;
use Puli\Repository\Resource\Collection\ArrayResourceCollection;
use Symfony\Cmf\Component\Resource\Repository\Resource\PhpcrResource;

/**
* Resource repository for PHPCR.
Expand All @@ -26,14 +26,14 @@
class PhpcrRepository extends AbstractPhpcrRepository
{
/**
* @var ManagerRegistry
* @var SessionInterface
*/
private $session;

/**
* @param SessionInterface $session
* @param FinderInterface $finder
* @param string $basePath
* @param FinderInterface $finder
*/
public function __construct(SessionInterface $session, $basePath = null, FinderInterface $finder = null)
{
Expand Down Expand Up @@ -68,6 +68,9 @@ public function get($path)
return $resource;
}

/**
* {@inheritdoc}
*/
public function listChildren($path)
{
$resource = $this->get($path);
Expand Down Expand Up @@ -119,4 +122,36 @@ protected function buildCollection(array $nodes)

return $collection;
}

/**
* {@inheritdoc}
*/
protected function removeNodes(array $nodes)
{
foreach ($nodes as $node) {
$node->remove();
}

$this->session->save();
}

/**
* {@inheritdoc}
*/
protected function moveNodes(array $nodes, $sourceQuery, $targetPath)
{
$this->doMoveNodes($nodes, $sourceQuery, $targetPath);
$this->session->save();
}

private function doMoveNodes(array $nodes, $sourceQuery, $targetPath)
{
if (false === $this->isGlobbed($sourceQuery)) {
Copy link
Member

@wouterj wouterj Jun 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's not globbed, wouldn't it not always result in $nodes containing just one element? In other words, why can't we simply loop through $nodes like done in removeNodes()?

Nvm, I spotted the difference now.

return $this->session->move(current($nodes)->getPath(), $targetPath);
}

foreach ($nodes as $node) {
$this->session->move($node->getPath(), $targetPath.'/'.$node->getName());
}
}
}
Loading