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

Make sure correct routes are generated from the config in DI Extension #94

Closed
saimaz opened this issue Dec 15, 2015 · 4 comments
Closed
Milestone

Comments

@saimaz
Copy link
Member

saimaz commented Dec 15, 2015

Look at the configuration docs: https://github.com/ongr-io/ApiBundle/blob/9610b53ea5bc7dd3390b4be27e7a94452a3e4cb8/Resources/doc/configuration.md

Say we have:

ongr_api:
   output_format: json
   version_in_url: true
   versions:
       v3:
           endpoints:
               person:
                   repository: es.manager.default.person
                   allow_extra_fields: false
                   allow_get_all: true

Routes we expect to have generated dynamicaly:
/api/v1/person POST
/api/v1/person/{id} PUT
/api/v1/person/{id} GET
/api/v1/person/_all GET
/api/v1/person/{id} DELETE
/api/v1/person/_batch POST

Routes must be pointed to specific RestController action.

@trandangtri
Copy link
Contributor

IMO, I think we should separate 2 cleaned parts:

  • GET, POST, PUT, DELETE will be handled by RestController
  • All the others (with '_' prefix, _all, _batch for instance) which I called commands, will be handled by CommandController. It's very easy to implement new commands

You could take a look on my work (still be in progress)
trandangtri@a9e214b#diff-47c6f7be84a1fcdde5ffc18ec6958135R20

What do you think about this, @saimaz?

@saimaz
Copy link
Member Author

saimaz commented Dec 23, 2015

I'll take a look tomorrow.

As of two parts. CommandController is not very accurate name for _batch and _all. I know that it might be more some same routes, but it should be named something like ColelctionController or the other way is create separate controller for batch and for all, then it will make more sense. Even if we will add some other not traditional route, it will be clean way to do it with separate controller. Also imagine if there will be needed to add some additional parameters to both controllers, then it will become more complex. So I vote for separate controller for each action _batch and _all.

Maybe @mvar has some thoughts and advises?

@mvar
Copy link
Member

mvar commented Dec 28, 2015

Sorry, no suggestions. I just don't like the name command. Besides that everything sounds good. I like the idea to separate REST from non-REST.

@trandangtri
Copy link
Contributor

Thanks @mvar. I changed Command to Collection.

And I added a mapping configuration into the Abstract Collection class, so you could define yours commands in an individual class or separated classed.

/**
     * Mapping of the commands
     *
     * @var array
     */
    private $mapping = [
        '_all' => [
            '_controller' => 'ONGRApiBundle:Collection:all',
            'methods' => ['GET'],
            'validator' => 'allow_get_all'
        ],
        '_batch' => [
            '_controller' => 'ONGRApiBundle:Collection:batch',
            'methods' => ['POST'],
            'validator' => 'allow_batch'
        ]
    ];

@trandangtri trandangtri mentioned this issue Dec 31, 2015
Merged
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

3 participants