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

Example for admin metadata implementation, #19

Merged
merged 7 commits into from
Jun 5, 2016
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 0 additions & 1 deletion .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 Down
117 changes: 117 additions & 0 deletions Description/Description.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
<?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\Description;

use Puli\Repository\Api\Resource\PuliResource;

/**
* Descriptive metadata for resources.
*/
class Description
{
/**
* @var array
Copy link
Member

Choose a reason for hiding this comment

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

what are the values in this array? strings, or something more complex? a list of label => url?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can limit them to be scalar values. If there is a use case for array values we could allow that later.

Copy link
Member Author

Choose a reason for hiding this comment

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

We should allow arrays also, f.e. to indicate the valid child payload types.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

*/
private $descriptors = [];

/**
* @var PuliResource
*/
private $resource;

/**
* @param PuliResource $resource
*/
public function __construct(PuliResource $resource)
{
$this->resource = $resource;
}

/**
* Return the descriptors value for the given descriptor.
*
* @param string $descriptor
*
* @return mixed
Copy link
Member

Choose a reason for hiding this comment

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

can we be more specific what this is? how can the consumer work with the result?

Copy link
Member Author

Choose a reason for hiding this comment

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

well, the consumer can use the result in any way they please - for example the result could be the name of an .png file to use as an icon, or it could be the URL, or a vendor-assigned human title for the resource class (e.g. "Page"). Its pretty generic.

Copy link
Member

Choose a reason for hiding this comment

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

so you would split parts of the information into separate descriptions? like edit.html.icon and edit.html.url?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly, see the Descriptor class.

Then, when something needs a certain piece of information, it calls $description->has(Descriptor::LINK_HTML_EDIT).

*/
public function get($descriptor)
{
if (!isset($this->descriptors[$descriptor])) {
throw new \InvalidArgumentException(sprintf(
'Descriptor "%s" not supported for resource "%s" of class "%s". Supported descriptors: "%s"',
$descriptor,
$this->resource->getPath(),
get_class($this->resource),
implode('", "', array_keys($this->descriptors))
));
}

return $this->descriptors[$descriptor];
}

/**
* Return true if the given descriptor has been set.
*
* @param string $descriptor
*
* @return bool
*/
public function has($descriptor)
{
return isset($this->descriptors[$descriptor]);
}

/**
* Return all of the descriptors.
*
* @return array
*/
public function all()
{
return $this->descriptors;
}

/**
* Set value for descriptors descriptor.
*
* Note that:
*
* - It is possible to overwrite existing descriptors.
*
* - Where possible the descriptor should be the value of one of the constants
* defined in the Descriptor class.
*
* @param string $descriptor
* @param mixed $value
*/
public function set($descriptor, $value)
{
if (null !== $value && !is_scalar($value)) {
throw new \InvalidArgumentException(sprintf(
'Only scalar values are allowed as descriptor values, got "%s" when setting descriptor "%s"',
gettype($value), $descriptor
));
}

$this->descriptors[$descriptor] = $value;
}
Copy link
Member

Choose a reason for hiding this comment

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

What about also implementing the other methods mentioned in http://symfony.com/doc/current/contributing/code/conventions.html#method-names ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we are missing has() and possibly all().

Copy link
Member

Choose a reason for hiding this comment

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

something to set several descriptors in one call, by providing a hashmap that is merged with $this->descriptors?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess that would be merge() then.

Copy link
Member

Choose a reason for hiding this comment

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

or setDescriptor($key, $value) and setDescriptors($descriptors)

Copy link
Member Author

Choose a reason for hiding this comment

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

But would setDescriptors replace all the descriptors or merge them? I would assume without prior knowledge that it would replace them.

Copy link
Member

Choose a reason for hiding this comment

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

ah, true. maybe addDescriptors? mergeDescriptors feels strange. that adding something that already exists overwrites the existing value seems no big surprise to me, and we can state it explicitly in the phpdoc

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer merge, as that correlates with array_merge which is what you are actually doing. However, I think the ParameterBag uses add() with the same semantic.

I guess suffixing Descriptor makes sense.

Copy link
Member

@dbu dbu May 27, 2016 via email

Choose a reason for hiding this comment

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


/**
* Return the resource for which this is the description.
*
* @return PuliResource
*/
public function getResource()
{
return $this->resource;
}
}
35 changes: 35 additions & 0 deletions Description/DescriptionEnhancerInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?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\Description;

use Puli\Repository\Api\Resource\PuliResource;

interface DescriptionEnhancerInterface
{
/**
* Enrich the payload description.
*
* @param Description $description
*
* @return Description
*/
public function enhance(Description $description);

/**
* Return true if the provider supports the given type.
*
* @param CmfResource $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.

s/Cmf/Puli

*
* @return bool
*/
public function supports(PuliResource $resource);
}
52 changes: 52 additions & 0 deletions Description/DescriptionFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?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\Description;

use Puli\Repository\Api\Resource\PuliResource;

class DescriptionFactory
{
/**
* @var DescriptionEnhancerInterface[]
*/
private $enhancers = [];

/**
* @param array $enhancers
*/
public function __construct(array $enhancers)
{
$this->enhancers = $enhancers;
}

/**
* Return a description of the given (CMF) Resource.
*
* @param PuliResource $resource
*
* @return Description
*/
Copy link
Member

Choose a reason for hiding this comment

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

missing @return

public function getPayloadDescriptionFor(PuliResource $resource)
{
$description = new Description($resource);

foreach ($this->enhancers as $enhancer) {
if (false === $enhancer->supports($resource)) {
continue;
}

$enhancer->enhance($description);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of these by reference edits. What about doing $description = $enhancer->enhance($description);? That would be more flexible.

Copy link
Member Author

Choose a reason for hiding this comment

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

$description is an object, even if you did return it it would still be modified by reference - unless we made it immutable. But I don't really see the benefit.

Copy link
Member

Choose a reason for hiding this comment

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

We had the same thing in RoutingAuto, which we later deprecated because it turned out that some people needed to not modify by reference: symfony-cmf/routing-auto#53

Copy link
Member Author

@dantleech dantleech May 25, 2016

Choose a reason for hiding this comment

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

Yes, but in that case it was the Entity which came from "outside". In this case, we create the Description, nobody is relying on its state before this factory produces it.

Copy link
Member Author

Choose a reason for hiding this comment

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

And returning or not returning, the result will always be the same and the Description will always be the same instance.

}

return $description;
}
}
54 changes: 54 additions & 0 deletions Description/Descriptor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?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\Description;

/**
* Class containing recommended descriptors for use by enhancers in order to
* provide a level of interoperability.
*/
final class Descriptor
{
/**
* Alias for the resource type for example `app.page`.
*/
const TYPE_ALIAS = 'type.alias';

/**
* Humanized representation of the resource type.
*/
const TYPE_TITLE = 'type.title';

/**
* Title of the actual payload, e.g. "My Blog Post".
*/
const PAYLOAD_TITLE = 'title';

/**
* Descriptors for HTML links.
*/
const LINK_EDIT_HTML = 'link.edit.html';
const LINK_CREATE_HTML = 'link.create.html';
const LINK_UPDATE_HTML = 'link.update.html';
const LINK_REMOVE_HTML = 'link.remove.html';
const LINK_SHOW_HTML = 'link.show.html';
const LINK_LIST_HTML = 'link.list.html';

/**
* Descriptors for REST links.
*/
const LINK_EDIT_REST = 'link.edit.rest';
const LINK_CREATE_REST = 'link.create.rest';
const LINK_UPDATE_REST = 'link.update.rest';
const LINK_REMOVE_REST = 'link.remove.rest';
const LINK_SHOW_REST = 'link.show.rest';
const LINK_LIST_REST = 'link.show.rest';
}
120 changes: 120 additions & 0 deletions Description/Enhancer/Sonata/AdminEnhancer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
<?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\Description\Enhancer\Sonata;

use Sonata\AdminBundle\Admin\Pool;
use Doctrine\Common\Util\ClassUtils;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Cmf\Component\Resource\Description\DescriptionEnhancerInterface;
use Symfony\Cmf\Component\Resource\Description\Description;
use Puli\Repository\Api\Resource\PuliResource;
use Symfony\Cmf\Component\Resource\Description\Descriptor;

/**
* Add links and meta-info from Sonata Admin.
*
* @author Daniel Leech <[email protected]>
*/
class AdminEnhancer implements DescriptionEnhancerInterface
Copy link
Member Author

Choose a reason for hiding this comment

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

Still need to "manually" test this or setup a full Sonata environment here for integration testing.

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 may as well wait until this filters up to the resource-rest-bundle.

{
/**
* @var Pool
*/
private $pool;

/**
* @var UrlGeneratorInterface
*/
private $urlGenerator;

/**
* @param Pool $pool
* @param UrlGeneratorInterface $urlGenerator
*/
public function __construct(Pool $pool, UrlGeneratorInterface $urlGenerator)
{
$this->pool = $pool;
$this->urlGenerator = $urlGenerator;
}

/**
* {@inheritdoc}
*/
public function enhance(Description $description)
{
$object = $description->getResource()->getPayload();

// sonata has dependency on ClassUtils so this is fine.
$class = ClassUtils::getClass($object);
$admin = $this->pool->getAdminByClass($class);

$links = array();

$routeCollection = $admin->getRoutes();

foreach ($routeCollection->getElements() as $code => $route) {
$routeName = $route->getDefault('_sonata_name');
$url = $this->urlGenerator->generate($routeName, array(
$admin->getIdParameter() => $admin->getUrlsafeIdentifier($object),
), true);

$routeRole = substr($code, strlen($admin->getCode()) + 1);

$links[$routeRole] = $url;
}

if (isset($links['list'])) {
$description->set('list', $links['list']);
unset($links['list']);
}

if (isset($links['create'])) {
$description->set(Descriptor::LINK_CREATE_HTML, $links['create']);
unset($links['create']);
}

if (isset($links['edit'])) {
$description->set(Descriptor::LINK_EDIT_HTML, $links['edit']);
unset($links['edit']);
}

if (isset($links['delete'])) {
$description->set(Descriptor::LINK_REMOVE_HTML, $links['delete']);
unset($links['delete']);
}

if (isset($links['show'])) {
$description->set(Descriptor::LINK_SHOW_HTML, $links['show']);
unset($links['show']);
}

$description->set(Descriptor::PAYLOAD_TITLE, $admin->toString($object));
$description->set(Descriptor::TYPE_ALIAS, $admin->getLabel());
}

/**
* {@inheritdoc}
*/
public function supports(PuliResource $resource)
{
if (false === $resource instanceof CmfResource) {
return false;
}

$payload = $resource->getPayload();

// sonata has dependency on ClassUtils so this is fine.
$class = ClassUtils::getClass($payload);

return $this->pool->hasAdminByClass($class);
}
}
Loading