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

[WIP] move, rename delete resource of editable repository #31

Closed
Closed
Show file tree
Hide file tree
Changes from 5 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
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ php:
- 5.6
- 7.0
- hhvm

sudo: false

cache:
Expand Down
105 changes: 102 additions & 3 deletions Controller/ResourceController.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@

namespace Symfony\Cmf\Bundle\ResourceRestBundle\Controller;

use PHPCR\Util\PathHelper;
use Puli\Repository\Api\EditableRepository;
use Puli\Repository\Api\ResourceRepository;
use Symfony\Cmf\Component\Resource\RepositoryRegistryInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use JMS\Serializer\SerializerInterface;
use JMS\Serializer\SerializationContext;
use Symfony\Component\Routing\Exception\RouteNotFoundException;

class ResourceController
{
Expand All @@ -29,7 +34,8 @@ class ResourceController
private $serializer;

/**
* @param RepositoryInterface
* @param SerializerInterface $serializer
* @param RepositoryRegistryInterface $registry
*/
public function __construct(
SerializerInterface $serializer,
Expand All @@ -39,11 +45,104 @@ public function __construct(
$this->registry = $registry;
}

public function resourceAction($repositoryName, $path)
public function getResourceAction($repositoryName, $path)
{
$repository = $this->registry->get($repositoryName);
$resource = $repository->get('/'.$path);

return $this->createResponse($resource);
}

/**
* There are two different changes that can currently be done on a cmf resource:
Copy link
Member

Choose a reason for hiding this comment

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

the styleci complains because the first line of a comment is the short description.

we should say something like "Change an existing resource." and then add a new line.

*
* - move
* - rename
Copy link
Member

Choose a reason for hiding this comment

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

can move only change the parents but not the name itself? does rename allow to only specify the new name but not the path? not sure if that would make worth it to have two different operations. you still need to know the full identifier of the resource.

or is this a puli thing or otherwise required to be two different operations?

Copy link
Member

Choose a reason for hiding this comment

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

the way I understand it, you move the resources to a new parent path and you cannot rename them with move.

Copy link
Member

@dbu dbu Jun 2, 2016 via email

Choose a reason for hiding this comment

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

Copy link
Member

@wouterj wouterj Jun 2, 2016

Choose a reason for hiding this comment

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

At least my move PR on Puli does support renaming in the move action (as @dbu said, both actions are just about changing paths): https://github.com/puli/repository/pull/26/files#diff-14ae094154456c4c7c9fa3ddb909c559R388

Copy link
Member

Choose a reason for hiding this comment

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

that makes sense. it works the same as the mv command in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dantleech that would be awesome if it fieels equal.

The Comment maybe can be seen as a hint, what to document afterwards, cause we need to explain it.

*
* changing payload properties isn't supported yet.
*
* @param string $repositoryName
* @param string $path
* @param Request $request
*
* @return Response
*/
public function patchResourceAction($repositoryName, $path, Request $request)
{
$repository = $this->registry->get($repositoryName);
$this->failOnNotEditable($repository, $repositoryName);

$resourcePath = $request->get('path');
$resourceName = $request->get('name');

$targetPath = null;
if ($path !== $resourcePath) {
$targetPath = $resourcePath;
} elseif ($resourceName !== PathHelper::getNodeName($path)) {
$targetPath = $targetPath = PathHelper::absolutizePath($repositoryName, PathHelper::getParentPath($path));
}

if (null == $targetPath) {
return $this->badRequestRespons('Move and rename is supported only.');
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.

typo: badRequestRespons

}

try {
$repository->move($path, $targetPath);
} catch (\InvalidArgumentException $e) {
return $this->badRequestResponse($e->getMessage());
}

$resource = $repository->get($targetPath);

return $this->createResponse($resource);
}

/**
* Delete a resource of a repository.
*
* @param string $repositoryName
* @param string $path
*
* @return Response
*/
public function deleteResourceAction($repositoryName, $path)
{
$repository = $this->registry->get($repositoryName);
$this->failOnNotEditable($repository, $repositoryName);

try {
$repository->remove($path);
} catch (\InvalidArgumentException $e) {
return $this->badRequestResponse($e->getMessage());
}

return $this->createResponse('', Response::HTTP_NO_CONTENT);
}

private function failOnNotEditable(ResourceRepository $repository, $repositoryName)
{
if (!$repository instanceof EditableRepository) {
throw new RouteNotFoundException(sprintf('Repository %s is not editable.', $repositoryName));
}
}

/**
* @param string $message
*
* @return Response
*/
private function badRequestResponse($message)
{
return $this->createResponse(['message' => $message], Response::HTTP_BAD_REQUEST);
}

/**
* @param object|array $resource
* @param int $httpStatusCode
* @return Response
*/
private function createResponse($resource, $httpStatusCode = Response::HTTP_OK)
{
$context = SerializationContext::create();
$context->enableMaxDepthChecks();
$context->setSerializeNull(true);
Expand All @@ -53,7 +152,7 @@ public function resourceAction($repositoryName, $path)
$context
);

$response = new Response($json);
$response = new Response($json, $httpStatusCode);
$response->headers->set('Content-Type', 'application/json');

return $response;
Expand Down
9 changes: 4 additions & 5 deletions Enhancer/EnhancerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@

namespace Symfony\Cmf\Bundle\ResourceRestBundle\Enhancer;

use JMS\Serializer\Context;
use Puli\Repository\Api\Resource\PuliResource;
use Puli\Repository\Api\Resource\Resource;

/**
* Enhancer classes enhance the REST response for resources.
Expand All @@ -28,8 +27,8 @@ interface EnhancerInterface
*
* $context->addData('foobar', 'Some value');
*
* @param Context Serialization context
* @param resource The resource being serialized
* @param [] $data Context Serialization context
* @param Resource $resource The resource being serialized
*/
public function enhance(array $data, PuliResource $resource);
public function enhance(array $data, Resource $resource);
Copy link
Member Author

Choose a reason for hiding this comment

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

@wouterj i did the change here, but i think it is the other way around as you said it. Or do we we have some version proplems in our chain from resource -> resource-bundle -> resource-restbundle ->sandbox?
The major problem is that master on Puli is behind several beta-* branches, otherwise we should use dev-master at all instances to got the same behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now I am completely confused. Localy i still got Resource. Your Link sais the change was the other way around, and i trust you, so i have the wrong version and i think i did the same wrong change on resource-bundle too. No clue if @dantleech took that error too. But at least we should try to force to get the latest version of that bundle.

Copy link
Member

Choose a reason for hiding this comment

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

I had to explicitly include Puli @beta in ResourceBundle:

https://github.com/symfony-cmf/resource-bundle/pull/31/files#diff-b5d0ee8c97c7abd7e3fa29b9a27d1780R17

On Fri, Jun 03, 2016 at 06:38:48AM -0700, Maximilian Berghoff wrote:

In [1]Enhancer/EnhancerInterface.php:

  */
  • public function enhance(array $data, PuliResource $resource);
  • public function enhance(array $data, Resource $resource);

Now I am completely confused. Localy i still got Resource. Your Link sais
the change was the other way around, and i trust you, so i have the wrong
version and i think i did the same wrong change on resource-bundle too. No
clue if [2]@dantleech took that error too. But at least we should try to
force to get the latest version of that bundle.


You are receiving this because you were mentioned.
Reply to this email directly, [3]view it on GitHub, or [4]mute the thread.

Reverse link: [5]unknown

References

Visible links

  1. [WIP] move, rename delete resource of editable repository #31 (comment)
  2. https://github.com/dantleech
  3. https://github.com/symfony-cmf/resource-rest-bundle/pull/31/files/fcd3f0aae763a6b30211a5e3c1e51fc71508a7c9#r65707989
  4. https://github.com/notifications/unsubscribe/AAgZcXom6yfXsS9L4BesP9QqZ8tUMXAcks5qIC5ogaJpZM4Ik8Qk
  5. https://github.com/symfony-cmf/resource-rest-bundle/pull/31/files/fcd3f0aae763a6b30211a5e3c1e51fc71508a7c9#r65707989

}
4 changes: 2 additions & 2 deletions Enhancer/PayloadEnhancer.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

namespace Symfony\Cmf\Bundle\ResourceRestBundle\Enhancer;

use Puli\Repository\Api\Resource\PuliResource;
use Puli\Repository\Api\Resource\Resource;

/**
* Serialize the payload.
Expand All @@ -23,7 +23,7 @@ class PayloadEnhancer implements EnhancerInterface
/**
* {@inheritdoc}
*/
public function enhance(array $data, PuliResource $resource)
public function enhance(array $data, Resource $resource)
{
$payload = $resource->getPayload();
$data['payload'] = $payload;
Expand Down
4 changes: 2 additions & 2 deletions Enhancer/SonataAdminEnhancer.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use Sonata\AdminBundle\Admin\Pool;
use Doctrine\Common\Util\ClassUtils;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Puli\Repository\Api\Resource\PuliResource;
use Puli\Repository\Api\Resource\Resource;

/**
* Add links and meta-info from Sonata Admin.
Expand Down Expand Up @@ -48,7 +48,7 @@ public function __construct(Pool $pool, UrlGeneratorInterface $urlGenerator)
/**
* {@inheritdoc}
*/
public function enhance(array $data, PuliResource $resource)
public function enhance(array $data, Resource $resource)
{
$object = $resource->getPayload();

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Symfony CMF Resource REST API Bundle

[![Build Status](https://secure.travis-ci.org/symfony-cmf/ResourceRestBundle.png?branch=master)](http://travis-ci.org/symfony-cmf/ResourceRestBundle)
[![Build Status](https://travis-ci.org/symfony-cmf/resource-rest-bundle.svg?branch=master)](https://travis-ci.org/symfony-cmf/resource-rest-bundle)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the build icon change all other bundles still have.

[![StyleCI](https://styleci.io/repos/29090266/shield)](https://styleci.io/repos/29090266)
[![Latest Stable Version](https://poser.pugx.org/symfony-cmf/resource-rest-bundle/version.png)](https://packagist.org/packages/symfony-cmf/resource-rest-bundle)
[![Total Downloads](https://poser.pugx.org/symfony-cmf/resource-rest-bundle/d/total.png)](https://packagist.org/packages/symfony-cmf/resource-rest-bundle)
Expand Down
4 changes: 3 additions & 1 deletion Registry/EnhancerRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Cmf\Bundle\ResourceRestBundle\Registry;

use Symfony\Cmf\Bundle\ResourceRestBundle\Enhancer\EnhancerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
Expand Down Expand Up @@ -65,7 +66,8 @@ public function getEnhancers($repositoryAlias)
}

$aliases = $this->enhancerMap[$repositoryAlias];

$enhancers = [];

foreach ($aliases as $alias) {
if (!isset($this->aliasMap[$alias])) {
throw new \InvalidArgumentException(sprintf(
Expand Down
6 changes: 3 additions & 3 deletions Registry/PayloadAliasRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@

namespace Symfony\Cmf\Bundle\ResourceRestBundle\Registry;

use Puli\Repository\Api\Resource\Resource;
use Symfony\Cmf\Component\Resource\RepositoryRegistryInterface;
use Symfony\Cmf\Component\Resource\Repository\Resource\CmfResource;
use Puli\Repository\Api\Resource\PuliResource;

/**
* Registry for resource payload aliases.
Expand Down Expand Up @@ -54,11 +54,11 @@ public function __construct(
/**
* Return the alias for the given PHPCR resource.
*
* @param resource $resource
* @param Resource $resource
*
* @return string
*/
public function getPayloadAlias(PuliResource $resource)
public function getPayloadAlias(Resource $resource)
{
$repositoryType = $this->repositoryRegistry->getRepositoryType(
$resource->getRepository()
Expand Down
23 changes: 21 additions & 2 deletions Resources/config/routing.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,26 @@
_cmf_resource:
_delete_cmf_resource:
path: /api/{repositoryName}/{path}
methods: ['delete']
requirements:
path: .*
defaults:
_controller: cmf_resource_rest.controller.resource:resourceAction
_controller: cmf_resource_rest.controller.resource:deleteResourceAction
_format: json

_patch_cmf_resource:
path: /api/{repositoryName}/{path}
methods: ['patch']
requirements:
path: .*
defaults:
_controller: cmf_resource_rest.controller.resource:patchResourceAction
_format: json
Copy link
Member

Choose a reason for hiding this comment

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

this wouldn't change anything (unless I'm wrong)


_get_cmf_resource:
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to put the vendor in front: _cmf_get_resource

path: /api/{repositoryName}/{path}
methods: ['get']
requirements:
path: .*
defaults:
_controller: cmf_resource_rest.controller.resource:getResourceAction
_format: json
7 changes: 3 additions & 4 deletions Serializer/Jms/EventSubscriber/ResourceSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
use JMS\Serializer\EventDispatcher\EventSubscriberInterface;
use JMS\Serializer\EventDispatcher\Events;
use JMS\Serializer\EventDispatcher\PreSerializeEvent;
use Puli\Repository\Api\ResourceCollection;
use Puli\Repository\Api\Resource\PuliResource;
use Puli\Repository\Api\Resource\Resource;

/**
* Force instaces of ResourceCollection to type "ResourceCollection".
Expand All @@ -41,8 +40,8 @@ public function onPreSerialize(PreSerializeEvent $event)
{
$object = $event->getObject();

if ($object instanceof PuliResource) {
$event->setType('Puli\Repository\Api\Resource\PuliResource');
if ($object instanceof Resource) {
$event->setType('Puli\Repository\Api\Resource\Resource');
}
}
}
8 changes: 4 additions & 4 deletions Serializer/Jms/Handler/ResourceHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use Symfony\Cmf\Bundle\ResourceRestBundle\Registry\PayloadAliasRegistry;
use Symfony\Cmf\Bundle\ResourceRestBundle\Registry\EnhancerRegistry;
use Symfony\Cmf\Component\Resource\Repository\Resource\CmfResource;
use Puli\Repository\Api\Resource\PuliResource;
use Puli\Repository\Api\Resource\Resource;

/**
* Handle PHPCR resource serialization.
Expand Down Expand Up @@ -51,7 +51,7 @@ public static function getSubscribingMethods()
array(
'event' => GraphNavigator::DIRECTION_SERIALIZATION,
'format' => 'json',
'type' => 'Puli\Repository\Api\Resource\PuliResource',
'type' => 'Puli\Repository\Api\Resource\Resource',
'method' => 'serializeResource',
),
);
Expand All @@ -65,15 +65,15 @@ public static function getSubscribingMethods()
*/
public function serializeResource(
JsonSerializationVisitor $visitor,
PuliResource $resource,
Resource $resource,
array $type,
Context $context
) {
$data = $this->doSerializeResource($resource);
$context->accept($data);
}

private function doSerializeResource(PuliResource $resource, $depth = 0)
private function doSerializeResource(Resource $resource, $depth = 0)
{
$data = array();
$repositoryAlias = $this->registry->getRepositoryAlias($resource->getRepository());
Expand Down
1 change: 1 addition & 0 deletions Tests/Resources/app/config/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/

$container->setParameter('cmf_testing.bundle_fqn', 'Symfony\Cmf\Bundle\ResourceRestBundle');
$container->setParameter('kernel.environment', 'test');
Copy link
Member

Choose a reason for hiding this comment

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

should not be needed if the Kernel is instantiated correctly (with the test env).

$loader->import(CMF_TEST_CONFIG_DIR.'/dist/parameters.yml');
$loader->import(CMF_TEST_CONFIG_DIR.'/dist/framework.php');
$loader->import(CMF_TEST_CONFIG_DIR.'/dist/monolog.yml');
Expand Down
4 changes: 3 additions & 1 deletion Tests/Unit/Registry/PayloadAliasRegistryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@

class PayloadAliasRegistryTest extends \PHPUnit_Framework_TestCase
{
private $registry;
private $repositoryRegistry;
private $resource;
private $repository;

public function setUp()
{
Expand Down
2 changes: 1 addition & 1 deletion Tests/Unit/Serializer/Jms/Handler/ResourceHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public function testHandler()
$this->enhancerRegistry->getEnhancers('repo')->willReturn(array(
$this->enhancer,
));
$this->enhancer->enhance(Argument::type('array'), Argument::type('Puli\Repository\Api\Resource\PuliResource'))
$this->enhancer->enhance(Argument::type('array'), Argument::type('Puli\Repository\Api\Resource\Resource'))
->will(function ($data, $resource) {
return $data[0];
});
Expand Down
13 changes: 11 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,20 @@
"homepage": "https://github.com/symfony-cmf/symfony-cmf/contributors"
}
],
"repositories": [
{
"type": "vcs",
"url": "https://github.com/ElectricMaxxx/resource-bundle.git"
},
{
"type": "vcs",
"url": "https://github.com/ElectricMaxxx/resource.git"
}
],
"minimum-stability": "dev",
"prefer-stable": true,
"require": {
"php": "^5.5.6|^7.0",
"symfony-cmf/resource-bundle": "1.*",
"symfony-cmf/resource-bundle": "dev-test_move_add_remove",
Copy link
Member Author

Choose a reason for hiding this comment

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

will remove that when both PR are merged.

"jms/serializer-bundle": "1.*"
},
"require-dev": {
Expand Down