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

Automatic entity/object lookups #13

Open
iainmckay opened this issue Apr 28, 2016 · 12 comments
Open

Automatic entity/object lookups #13

iainmckay opened this issue Apr 28, 2016 · 12 comments

Comments

@iainmckay
Copy link
Collaborator

I'd like to open a discussion on a new piece of functionality we need in our app. I provide a rationale and some possible solutions.

Feedback would be appreciated before I start implementing this.

Intention

To provide an automatic system for resolving object references to instances.

Our service layer accepts a lot of commands/dto's which contain references to entities or value objects in our domain. We have a lot of repetitive lookup code that we would like to get rid of.

It is essentially the functionality provided by Symfony ParamConverter but without the requirement to pass a Request object.

This feels very much like reinventing the wheel when there is a battle tested implementation that already exists but we've discussed it internally and don't see a way around it.

Basic Use

This would work similarly to PR #11 in that you would annotate a property on a DTO. The binder would then create an instance of the registered converter to produce an object instance. If the conversion process fails a violation would be added to the violation list.

The converter would accept the object identifier and convert it by whatever means necessary to an instance of that object or fail trying.

Possible Solutions

I provide two possible solutions for making this feature work.

Example 1

In this example the naming and documentation of the properties are as if they are already instances. It is the most natural but may be a little confusing for the developer dispatching the DTO to the service layer as we're passing in object identifiers but the documentation (and auto-complete) says instances.

<?php
class MyDTO
{

    public $name;

    /**
     * @var ValueObject\Country
     * @Lookup(converter="country_converter")
    */
    public $country;

    /**
     * @var Entity\User
     * @Lookup(converter="doctrine_converter")
    */
    public $user;
}

$myDto = new MyDTO();

$result = $binder->bind([
    'name' => 'Some Person',
    'country' => 1,
    'user' => 1024
], $myDto);

if ($result->isValid()) {
    echo($myDto->user->getName());
}

Example 2

In this example the naming and documentation of the properties are more obvious for the consumer as to what the expectations on the service consuming the DTO are. Accessing the converted objects is more obtuse and lacks support for autocomplete.

<?php
class MyDTO
{

    public $name;

    /**
     * @var int
     * @Lookup(converter="country_converter")
    */
    public $countryId;

    /**
     * @var int
     * @Lookup(converter="doctrine_converter")
    */
    public $userId;
}

$myDto = new MyDTO();

$result = $binder->bind([
    'name' => 'Some Person',
    'countryId' => 1,
    'userId' => 1024
], $myDto);

if ($result->isValid()) {
    echo($myDto->get('user')->getName());
}

Converters

A converter would follow a very simple interface and would be registered with the Binder to be made available.

<?php
interface ConverterInterface
{

    /**
     * @param mixed $input
     * @param array options
     * @throws ConverterException
     * @returns mixed
    */
    public function convert($input, array $options);
}
@liuggio
Copy link
Member

liuggio commented May 2, 2016

Hi @iainmckay , sorry for the delay,
I do like this, how are the steps in order to create the bindto service?
My concern was not to push for this solution but suggest as great add-on (http://screencloud.net/v/rUPp)!

@iainmckay
Copy link
Collaborator Author

This is roughly what we're running to create the bindto service: https://gist.github.com/iainmckay/6ef89477aa1efe29bbefb582b1fc3df5

@iainmckay
Copy link
Collaborator Author

If the true on the ConvertingObjectMapper is set to false then exceptions are thrown when the conversion process fails. This option exists because we wanted a different flow but is much more opinionated than I would like to push on people.

@liuggio
Copy link
Member

liuggio commented May 2, 2016

@iainmckay I like it...
this is a big step forward...

@iainmckay
Copy link
Collaborator Author

Cool. We can either merge it in to Bindto or setup another project (e.g. Bindto-extensions) if you want to keep the main project slim.

Tests also need to be written.

@liuggio
Copy link
Member

liuggio commented May 2, 2016

I really like to see this finished...
About the external project as you wish but I think this project is really small and divide the features would not be the best for visibility...
I think also the next big step would be deal with collection...

@liuggio
Copy link
Member

liuggio commented May 2, 2016

👍 💯

@iainmckay
Copy link
Collaborator Author

What do you mean by deal with collection?

@liuggio
Copy link
Member

liuggio commented May 2, 2016

Maybe after this, the next biggest feature is when you have complex objects with arrays and staff like this...
Did ever had this problem?

@iainmckay
Copy link
Collaborator Author

Ah yes, we don't have this problem at the moment but it should be fairly straightforward to support lists. If you want to review the PR I added, I'll incorporate any changes you think should be made and write some tests.

@liuggio
Copy link
Member

liuggio commented May 2, 2016

Great!

@iainmckay
Copy link
Collaborator Author

I updated the PR with collections and some other usability improvements.

I also have an ObjectConverter which invokes the binder so you can have arrays of transfer objects converted waiting to be commited. We're just testing it some more internally.

Here's an example from our codebase:

$input = [
    'queue' => $uuid,
    'records' => [
        [
            'recordId' => 'D29D82ED332B4151809E7AC080DFC29D',
            'values' => [
                'classLevel' => 9,
            ],
        ],
    ],
];

$binder->bind($input, UpdateQueueRecordsCommand::class);

Transfer objects:

class UpdateQueueRecordsCommand extends AbstractCommand
{

    /**
     * @var Queue
     * @Convert(converter="doctrine", options={"entity": "ImportDomain:Entity\Queue"})
     * @Assert\NotNull()
     */
    public $queue;

    /**
     * List of records to update. These should be indexed by property name.
     *
     * @var UpdateQueueRecordTransferObject[]
     * @Convert(converter="object", options={"class": "TransferObject\UpdateQueueRecordTransferObject"})
     * @Assert\IsArray()
     * @Assert\AllIsInstanceOf(class="TransferObject\UpdateQueueRecordTransferObject")
     */
    public $records;
}

class UpdateQueueRecordTransferObject
{

    /**
     * @var UuidInterface
     * @Convert(converter="uuid")
     * @Assert\NotBlank()
     */
    public $recordId;

    /**
     * @var array
     * @Assert\IsArray()
     */
    public $values;
}

This recursively converts the input values of $records to instances of UpdateQueueRecordTransferObject and executes validators, converters or whatever configured mappers you're using.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants