Skip to content
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

Add tests to ModelManager and DateFilter #368

Merged
merged 2 commits into from
Jul 26, 2020
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
6 changes: 6 additions & 0 deletions UPGRADE-3.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ UPGRADE 3.x
UPGRADE FROM 3.x to 3.x
=======================

### Sonata\DoctrineMongoDBAdminBundle\Model\ModelManager

Deprecated `camelize()` method.

Deprecated not passing an instance of `PropertyAccessInterface` as second argument in the constructor.

### Sonata\DoctrineMongoDBAdminBundle\Admin\FieldDescription

Deprecated `getTargetEntity()`, use `getTargetModel()` instead.
Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"symfony/doctrine-bridge": "^4.4",
"symfony/form": "^4.4",
"symfony/http-kernel": "^4.4",
"symfony/property-access": "^4.4",
"twig/twig": "^2.6"
},
"provide": {
Expand Down
41 changes: 34 additions & 7 deletions src/Filter/AbstractDateFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,43 +50,47 @@ public function filter(ProxyQueryInterface $queryBuilder, $alias, $field, $data)
$data['type'] = !isset($data['type']) || !is_numeric($data['type']) ? DateOperatorType::TYPE_EQUAL : $data['type'];

// Some types do not require a value to be set (NULL, NOT NULL).
if (!$this->typeRequiresValue($data['type']) && !$data['value']) {
if (!isset($data['value']) && $this->typeDoesRequireValue($data['type'])) {
return;
}

switch ($data['type']) {
case DateOperatorType::TYPE_EQUAL:
$this->active = true;

$this->applyTypeIsEqual($queryBuilder, $field, $data);

return;

case DateOperatorType::TYPE_GREATER_THAN:
if (!\array_key_exists('value', $data) || !$data['value']) {
return;
}
$this->active = true;

$this->applyTypeIsGreaterThan($queryBuilder, $field, $data);

return;

case DateOperatorType::TYPE_LESS_EQUAL:
if (!\array_key_exists('value', $data) || !$data['value']) {
return;
}
$this->active = true;

$this->applyTypeIsLessEqual($queryBuilder, $field, $data);

return;

case DateOperatorType::TYPE_NULL:
case DateOperatorType::TYPE_NOT_NULL:
$this->active = true;

$this->applyType($queryBuilder, $this->getOperator($data['type']), $field, null);

return;

case DateOperatorType::TYPE_GREATER_EQUAL:
case DateOperatorType::TYPE_LESS_THAN:
$this->active = true;

$this->applyType($queryBuilder, $this->getOperator($data['type']), $field, $data['value']);

return;
}
}

Expand Down Expand Up @@ -120,6 +124,12 @@ public function getRenderSettings()
]];
}

abstract protected function applyTypeIsLessEqual(ProxyQueryInterface $queryBuilder, string $field, array $data);
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK this is a BC break, because you introduce new API methods without that MUST be implemented.

Copy link
Member

@VincentLanglet VincentLanglet Jun 21, 2020

Choose a reason for hiding this comment

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


abstract protected function applyTypeIsGreaterThan(ProxyQueryInterface $queryBuilder, string $field, array $data);

abstract protected function applyTypeIsEqual(ProxyQueryInterface $queryBuilder, string $field, array $data);
greg0ire marked this conversation as resolved.
Show resolved Hide resolved

/**
* @param string $operation
* @param string $field
Expand All @@ -132,14 +142,23 @@ protected function applyType(ProxyQueryInterface $queryBuilder, $operation, $fie
}

/**
* NEXT_MAJOR: Remove this method.
*
* Returns if the filter type requires a value to be set.
*
* @param int $type
*
* @deprecated since sonata-project/doctrine-mongodb-admin-bundle 3.x, to be removed in 4.0.'.
*
* @return bool
*/
protected function typeRequiresValue($type)
{
@trigger_error(sprintf(
'"%s()" is deprecated since sonata-project/doctrine-mongodb-admin-bundle 3.x and will be removed in version 4.0.',
__METHOD__
), E_USER_DEPRECATED);

return \in_array($type, [
DateOperatorType::TYPE_NULL,
DateOperatorType::TYPE_NOT_NULL,
Expand Down Expand Up @@ -167,4 +186,12 @@ protected function getOperator($type)

return $choices[(int) $type];
}

private function typeDoesRequireValue(int $type): bool
{
return !\in_array($type, [
DateOperatorType::TYPE_NULL,
DateOperatorType::TYPE_NOT_NULL,
], true);
}
}
6 changes: 6 additions & 0 deletions src/Filter/DateFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,15 @@
namespace Sonata\DoctrineMongoDBAdminBundle\Filter;

use Sonata\AdminBundle\Datagrid\ProxyQueryInterface;
use Symfony\Component\Form\Extension\Core\Type\DateType;

class DateFilter extends AbstractDateFilter
{
public function getFieldType(): string
{
return $this->getOption('field_type', DateType::class);
}

/**
* @param string $field
* @param array $data
Expand Down
6 changes: 6 additions & 0 deletions src/Filter/DateTimeFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace Sonata\DoctrineMongoDBAdminBundle\Filter;

use Sonata\AdminBundle\Datagrid\ProxyQueryInterface;
use Symfony\Component\Form\Extension\Core\Type\DateTimeType;

class DateTimeFilter extends AbstractDateFilter
{
Expand All @@ -24,6 +25,11 @@ class DateTimeFilter extends AbstractDateFilter
*/
protected $time = true;

public function getFieldType(): string
{
return $this->getOption('field_type', DateTimeType::class);
}

/**
* @param string $field
* @param array $data
Expand Down
111 changes: 76 additions & 35 deletions src/Model/ModelManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
namespace Sonata\DoctrineMongoDBAdminBundle\Model;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Persistence\Mapping\ClassMetadata as CommonClassMetadata;
use Doctrine\ODM\MongoDB\Query\Builder;
use Doctrine\Persistence\Mapping\ClassMetadata;
use Sonata\AdminBundle\Admin\FieldDescriptionInterface;
use Sonata\AdminBundle\Datagrid\DatagridInterface;
use Sonata\AdminBundle\Datagrid\ProxyQueryInterface;
Expand All @@ -23,16 +25,44 @@
use Sonata\DoctrineMongoDBAdminBundle\Datagrid\ProxyQuery;
use Sonata\Exporter\Source\DoctrineODMQuerySourceIterator;
use Symfony\Bridge\Doctrine\ManagerRegistry;
use Symfony\Component\Form\Exception\PropertyAccessDeniedException;
use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;

class ModelManager implements ModelManagerInterface
{
public const ID_SEPARATOR = '-';

/**
* @var ManagerRegistry
*/
protected $registry;

public function __construct(ManagerRegistry $registry)
/**
* @var PropertyAccessorInterface
*/
private $propertyAccessor;

/**
* NEXT_MAJOR: Make $propertyAccessor mandatory.
*/
public function __construct(ManagerRegistry $registry, ?PropertyAccessorInterface $propertyAccessor = null)
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

And what about requiring the param in the next major ? (And add a deprecation in 3.x if it's not provided)

{
$this->registry = $registry;

// NEXT_MAJOR: Remove this block.
if (!$propertyAccessor instanceof PropertyAccessorInterface) {
@trigger_error(sprintf(
'Not passing an object implementing "%s" as argument 2 for "%s()" is deprecated since'
.' sonata-project/doctrine-mongodb-admin-bundle 3.x and will throw a %s error in 4.0.',
PropertyAccessorInterface::class,
__METHOD__,
\TypeError::class
), E_USER_DEPRECATED);

$propertyAccessor = PropertyAccess::createPropertyAccessor();
}

$this->propertyAccessor = $propertyAccessor;
}

/**
Expand Down Expand Up @@ -364,6 +394,21 @@ public function getExportFields($class)
*/
public function getModelInstance($class)
{
if (!class_exists($class)) {
throw new \InvalidArgumentException(sprintf('Class "%s" not found', $class));
}

$r = new \ReflectionClass($class);
franmomu marked this conversation as resolved.
Show resolved Hide resolved
if ($r->isAbstract()) {
throw new \InvalidArgumentException(sprintf('Cannot initialize abstract class: %s', $class));
}

$constructor = $r->getConstructor();

if (null !== $constructor && (!$constructor->isPublic() || $constructor->getNumberOfRequiredParameters() > 0)) {
return $r->newInstanceWithoutConstructor();
}

return new $class();
}

Expand Down Expand Up @@ -438,39 +483,10 @@ public function modelReverseTransform($class, array $array = [])
$instance = $this->getModelInstance($class);
$metadata = $this->getMetadata($class);

$reflClass = $metadata->reflClass;
foreach ($array as $name => $value) {
$reflection_property = false;
// property or association ?
if (\array_key_exists($name, $metadata->fieldMappings)) {
$property = $metadata->fieldMappings[$name]['fieldName'];
$reflection_property = $metadata->reflFields[$name];
} elseif (\array_key_exists($name, $metadata->associationMappings)) {
$property = $metadata->associationMappings[$name]['fieldName'];
} else {
$property = $name;
}
$property = $this->getFieldName($metadata, $name);

$setter = 'set'.$this->camelize($name);

if ($reflClass->hasMethod($setter)) {
if (!$reflClass->getMethod($setter)->isPublic()) {
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $setter, $reflClass->getName()));
}

$instance->$setter($value);
} elseif ($reflClass->hasMethod('__set')) {
// needed to support magic method __set
$instance->$property = $value;
} elseif ($reflClass->hasProperty($property)) {
if (!$reflClass->getProperty($property)->isPublic()) {
throw new PropertyAccessDeniedException(sprintf('Property "%s" is not public in class "%s". Maybe you should create the method "set%s()"?', $property, $reflClass->getName(), ucfirst($property)));
}

$instance->$property = $value;
} elseif ($reflection_property) {
$reflection_property->setValue($instance, $value);
}
$this->propertyAccessor->setValue($instance, $property, $value);
franmomu marked this conversation as resolved.
Show resolved Hide resolved
franmomu marked this conversation as resolved.
Show resolved Hide resolved
}

return $instance;
Expand Down Expand Up @@ -517,15 +533,40 @@ public function collectionRemoveElement(&$collection, &$element)
}

/**
* method taken from PropertyPath.
* NEXT_MAJOR: Remove this method.
*
* @deprecated since sonata-project/doctrine-mongodb-admin-bundle 3.x, to be removed in 4.0.'.
*
* @param string $property
*
* @return mixed
*/
protected function camelize($property)
{
return preg_replace(['/(^|_)+(.)/e', '/\.(.)/e'], ["strtoupper('\\2')", "'_'.strtoupper('\\1')"], $property);
@trigger_error(sprintf(
'Method "%s()" is deprecated since sonata-project/doctrine-mongodb-admin-bundle 3.x and will be removed in version 4.0.',
__METHOD__
), E_USER_DEPRECATED);

return str_replace(' ', '', ucwords(str_replace('_', ' ', $property)));
}

/**
* NEXT_MAJOR: Remove CommonClassMetadata and add ClassMetadata as type hint when dropping doctrine/mongodb-odm 1.3.x.
*
* @param ClassMetadata|CommonClassMetadata $metadata
*/
private function getFieldName($metadata, string $name): string
{
if (\array_key_exists($name, $metadata->fieldMappings)) {
return $metadata->fieldMappings[$name]['fieldName'];
}

if (\array_key_exists($name, $metadata->associationMappings)) {
return $metadata->associationMappings[$name]['fieldName'];
}

return $name;
}

private function isFieldAlreadySorted(FieldDescriptionInterface $fieldDescription, DatagridInterface $datagrid): bool
Expand Down
1 change: 1 addition & 0 deletions src/Resources/config/doctrine_mongodb.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<services>
<service id="sonata.admin.manager.doctrine_mongodb" class="Sonata\DoctrineMongoDBAdminBundle\Model\ModelManager">
<argument type="service" id="doctrine_mongodb" on-invalid="ignore"/>
<argument type="service" id="property_accessor"/>
<tag name="sonata.admin.manager"/>
</service>
<!-- FormBuilder -->
Expand Down
Loading