mark-gerarts / automapper-plus

An AutoMapper for PHP
MIT License
551 stars 30 forks source link

Operation::fromProperty with property path with dot notation #51

Closed ivoba closed 4 years ago

ivoba commented 4 years ago

This is probably a feature request: Is it possible to use a property path with dot notation with Operation::fromProperty?

Operation::fromProperty('children[0].firstName')

Its possible with the symfony PropertyAccessor: https://symfony.com/doc/current/components/property_access.html#accessing-public-properties

Of course this can be achieved by a callback, but its quite tedious to create the callback and also check for existance of the property path. It would be quite handy if this could be done automatically within fromProperty().

mark-gerarts commented 4 years ago

This would require some experimenting, since we can't use the Symfony property accessor as-is. The property accessor throws an error when it tries to read a private property, while this library has always supported this. It'll probably involve a decorated version of the accessor, as is done here for example (however, this still doesn't work on nested paths with private properties).

I'll see what I can do, but it'll be for the 2.0 release in any case. In the meantime this can be implemented the following way:

class DecoratedAccessor extends PropertyAccessor implements PropertyAccessorInterface {
  /**
   * @var SymfonyPropertyAccessorInterface
   */
  private $accessor;

  public function __construct(?SymfonyPropertyAccessorInterface $accessor = null) {
    $this->accessor = $accessor ?: PropertyAccess::createPropertyAccessor();
  }

  public function hasProperty($object, string $propertyName): bool {
    return $this->accessor->isReadable($object, $propertyName);
  }

  public function getProperty($object, string $propertyName) {
    return $this->accessor->getValue($object, $propertyName);
  }

}

class CustomFromProperty extends FromProperty {

  protected function getPropertyReader(): PropertyReaderInterface {
    return new DecoratedAccessor();
  }

  // The following methods have to be overridden because of a bug in the
  // parent class - I'll fix that in a next release :)
  protected function canMapProperty(string $propertyName, $source): bool {
    return $this->getPropertyReader()->hasProperty(
      $source,
      $this->getSourcePropertyName($propertyName)
    );
  }

  protected function getSourceValue($source, string $propertyName) {
    return $this->getPropertyReader()->getProperty(
      $source,
      $this->getSourcePropertyName($propertyName)
    );
  }

}

$config->registerMapping(Source::class, Destination::class)
  ->forMember('property', new CustomFromProperty('children[0].firstName'));

This is the least invasive, as the custom behaviour is restricted to the custom mapping operation. You could also try to register the decorated accessor globally. This way the default fromProperty would work as well. If you go all-out on the decorated accessor and provide a fallback to the parent, all existing mappings should still work as before (as in, I've run the test suite with the decorated accessor, and everything is green :)). The extended version:

class DecoratedAccessor extends PropertyAccessor implements PropertyAccessorInterface {
  /**
   * @var SymfonyPropertyAccessorInterface
   */
  private $accessor;

  public function __construct(?SymfonyPropertyAccessorInterface $accessor = null) {
    $this->accessor = $accessor ?: PropertyAccess::createPropertyAccessor();
  }

  public function hasProperty($object, string $propertyName): bool {
    if ($this->accessor->isReadable($object, $propertyName)) {
      return true;
    }

    return parent::hasProperty($object, $propertyName);
  }

  public function getProperty($object, string $propertyName) {
    try {
      return $this->accessor->getValue($object, $propertyName);
    }
    catch (NoSuchPropertyException $e) {
      return parent::getProperty($object);
    }
  }

}

$config->getOptions()->setPropertyAccessor(new DecoratedAccessor());
$config->registerMapping(Source::class, Destination::class)
  ->forMember('property', Operation::fromProperty('children[0].firstName'));
mark-gerarts commented 4 years ago

Release 1.3.8 now contains the mentioned fix, meaning the custom operation can be simplified:

class CustomFromProperty extends FromProperty {

  protected function getPropertyReader(): PropertyReaderInterface {
    return new DecoratedAccessor();
  }

}
ivoba commented 4 years ago

@mark-gerarts Great! I registered the custom accessor from above globaly and it works.

As i see there was a PR on the bundle that provides a symfony accessor: https://github.com/mark-gerarts/automapper-plus-bundle/pull/16/files

So basically using the symfony bridge accessor would also work, i guess? auto_mapper_plus.yaml:

services:
  AutoMapperPlus\AutoMapperPlusBundle\PropertyAccessor\SymfonyPropertyAccessorBridge:
    arguments:
      $propertyAccessor: '@property_accessor'
auto_mapper_plus:
  options:
    property_accessor: 'AutoMapperPlus\AutoMapperPlusBundle\PropertyAccessor\SymfonyPropertyAccessorBridge'

However not in my case as ->forMember isnt applied anymore on a registered mapping and i get an error with for not catched null values.

mark-gerarts commented 4 years ago

I've looked into this, and I'm honestly scratching my head why this is already sitting on the master branch of the bundle, because:

I apologize for this, and will dedicate some time in the weekend to fix the bugs and implement the proposed version of the Symfony-based property accessor instead of what is currently used in the bundle.

mark-gerarts commented 4 years ago

As an update: the changes are available on the bundle's master branch. I'm gonna tag a new release in the upcoming weeks, but I first want to include some more configuration options in the bundle while I'm at it.

ivoba commented 4 years ago

@mark-gerarts I tried the changes in the bundle. Work as expected :+1:

I guess we can close this issue.

ivoba commented 4 years ago

@mark-gerarts would you mind tagging a release of the bundle, even when not all configuration options are yet done?

mark-gerarts commented 4 years ago

Release 1.3.0 is now available!