Skip to content

Conversation

zairigimad
Copy link
Contributor

@zairigimad zairigimad commented Oct 24, 2020

New Command to generate API DataPersister

I Like Symfony and API Platform, I would like to leverage the power of Maker Bundle to generate code for API Platform.

Makers for API Platform:
In this PR:

  • DataPersister

Next:

  • DataProvider

  • DataTransformer

This PR is for DataPersister Maker.

Capture d’écran 2020-10-24 à 22 11 52

generated code :

ArticleDataPersister.php
<?php

namespace App\DataPersister;

use ApiPlatform\Core\DataPersister\ContextAwareDataPersisterInterface;

final class ArticleDataPersister implements ContextAwareDataPersisterInterface
{
    public function supports($data, array $context = []): bool
    {
        // implement your logic to check whether the given data is supported by this data persister
        return true;
    }

    public function persist($data, array $context = [])
    {
        // call your persistence layer to save $data
        return $data;
    }

    public function remove($data, array $context = [])
    {
        // call your persistence layer to delete $data
    }
}

Updating my fork repository
@zairigimad zairigimad force-pushed the feature/make-api-data-persister branch 2 times, most recently from 2c406ab to 70552e0 Compare October 24, 2020 20:39
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

I would absolutely LOVE to have a whole series of API Platform generates. So big 💯 from me.

I've added some notes to take the make:api:data-persister to the next level :). And let's make just 1 PR per command so that we can merge each as soon as its done and stay focused.

Thanks!

$input->getArgument('name'),
'DataPersister\\',
'DataPersister'
);
Copy link
Member

Choose a reason for hiding this comment

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

It might make sense to allow the user to interactively specify what class the data persister will persist for - e.g. App\Entity\CheeseListing. It would be even cooler if this read the API Platform metadata to list all the API Platform resource classes :). This would help us generate a better supports() method. Or, they could leave this blank and we would generate the supports() method like they have now.

And, to go further, if the resource class they choose is an entity, we could ask them something like this:

Would you like your persister to call the core Doctrine persister? This would allow you to
add custom logic without needing to worry about the save logic.

If they said yes, we would inject the core Doctrine ORM persister like done here - https://symfonycasts.com/screencast/api-platform-extending/decoration-deep-dive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, great ideas, I'm gonna Try to add this, and make Seperate PRs for the other Makers :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@weaverryan the Maker is now working with the update of services and with the different cases.
I would love to see your feedback if there are somethings to refactor in the code.

@zairigimad zairigimad force-pushed the feature/make-api-data-persister branch from aed83a5 to 4e8cf2a Compare October 29, 2020 19:32
@zairigimad zairigimad force-pushed the feature/make-api-data-persister branch 2 times, most recently from 198b2ff to 9b2f919 Compare October 29, 2020 23:06
@zairigimad zairigimad force-pushed the feature/make-api-data-persister branch 2 times, most recently from 09facc0 to b270583 Compare October 30, 2020 13:52
@zairigimad zairigimad force-pushed the feature/make-api-data-persister branch from b270583 to 424b749 Compare October 30, 2020 13:58
@zairigimad zairigimad requested a review from weaverryan October 30, 2020 20:21
@zairigimad zairigimad changed the title Feature/make api data persister new Maker for api-platform data persisters Oct 30, 2020
@weaverryan weaverryan changed the base branch from master to main November 16, 2020 20:00
@weaverryan
Copy link
Member

Hey @zairigimad!

I just talked with @dunglas and he was also thinking we should have these maker commands (and so I pointed him to your PR!). But we think it makes most sense for these to live inside of API Platform itself, so that once you install API Platform, you have the commands. Would you be willing to re-make this PR to API Platform? I'm not sure if it would just "drop in" or if some extra work would be needed - I'd be happy to help.

Thank you!

@weaverryan
Copy link
Member

Ah, @dunglas just told me that a Maker has been started already on API Platform! api-platform/core#3850

That is probably the PR we should use - it's clearly a good idea, as multiple people are proposing it (and I also think it's a good idea). So, unfortunately @zairigimad even though this is aa very good PR, I'm going to close it in favor of the other one on API Platform. I hope you could review that one over there and propose any improvements from your PR.

Thanks!

@weaverryan weaverryan closed this Nov 27, 2020
@zairigimad
Copy link
Contributor Author

Hey @zairigimad!

I just talked with @dunglas and he was also thinking we should have these maker commands (and so I pointed him to your PR!). But we think it makes most sense for these to live inside of API Platform itself, so that once you install API Platform, you have the commands. Would you be willing to re-make this PR to API Platform? I'm not sure if it would just "drop in" or if some extra work would be needed - I'd be happy to help.

Thank you!

Hey @weaverryan ,

Sure, no problem, I will check the PR in Api-Platform, and maybe will add other Makers :)

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

Successfully merging this pull request may close these issues.

2 participants