Streamlined REST API Methods then using the default endpoint generator

[waza-ari]

Is your feature request related to a problem? Please describe. First of all, thanks for your work and sorry for opening a million issues. This one is more of a debate I assume, as there is no "right" way of doing it. I do have the feeling that the current standard set of endpoints I suboptimal. Basically using the example:

router = crud_router(
    session=get_db,
    model=Ability,
    create_schema=AbilityCreateSchema,
    update_schema=AbilityUpdateSchema,
    path="/abilities",
    tags=["abilities"],
)

Yields these endpoints:

One example is: DELETE /api/v1/abilities/delete/{id} - we already know its deleting because of the HTTP verb - why does it need to be in the path again? The same applies to create (POST), update (PATCH) and get (GET) as well.

Describe the solution you'd like I'd love to be able to configure something "cleaner" (although I'm fully aware that's very opinionated) like this:

The HTTP Verb here together with the endpoint is quite descriptive in itself (and obviously I should change the default endpoint description, but its just for the demo).

Its not possible to achieve this with customised endpoint names either, as it would yield a double slash even if setting the endpoint name for delete to an empty string.

[igorbenav]

To be honest, I thought passing an empty string was working. Thanks for all the issues, by the way! Really great that you caught all of this stuff.

About the standard one, I don't have a formed opinion. I think if more people would comment here in favor, maybe we could change it to the cleaner option.

[JakNowy]

I had similar thoughts to @waza-ari, I think such simplification would be nice. Maybe we could even unify paginated and not paginated endpoints by introducing some optional pagination param, which makes it return all records if None or something like that.

[igorbenav]

This pagination part has been dealt with in #62, probably not the best option to make unpaginated a default endpoint (people might just add it blindly and then all of a sudden you allow users to get all your data at once). I think we could simplify the standard one though, go for the cleaner version.

Any of you guys want to do it?

[waza-ari]

Before saying yes, my question would be how to handle this change. If we change the default endpoint names and go for the "cleaner" version, that has the potential of breaking the code for other users relying on the current default behaviour. What would be your idea around that?

[igorbenav]

We add a warning in the release and in the docs and change version from 0.11.x to 0.12.0 to indicate breaking changes. Since we're at 0.y.z, breaking changes are acceptable (we're still figuring things out). We could also add a warning message when using EndpointCreator for the next couple of releases to indicate that things changed.

[JakNowy]

@waza-ari would be great if you could help with this!

I would go for GET PATCH and DELETE /hero/{id} and POST /hero/. We may leave GET hero/get_multi/ and GET /hero/get_paginated/ as it is, while personally, I would love the get_paginated to become an optional feature of /get_multi/ controlled by some query parameters. Like optional page and page_size defaulting to None or whatever you find appropriate.

We can think about the warnings together once you issue a PR.

[igorbenav]

@waza-ari do you still plan to do this?

[JakNowy]

@igorbenav I did some investigation on this.

Simplest approach is to pass custom endpoint_names to fastcrud.crud_router():

endpoint_names = {
    "create": "",
    "read": "",
    "update": "",
    "delete": "",
    "db_delete": "",
    "read_multi": "get_multi",
    "read_paginated": "get_paginated",
}

Names for read_multi and read_paginated must be left inplace or they will be registerred twice as the same users/ route (probably read_paginated taking the place as it's the last one registerred). The downside of this is that the route paths are incorrectly containing reduntant slashes:

This can be further adjusted in EndpointCreator.add_routes_to_router by overriding paths for all views

        path = f"{self.path}/{endpoint_name}" if endpoint_name else self.path

        self.router.add_api_route(
            path,
            ...

I consider this the simplest approach, however personally I would opt for making the streamlined version the default one (and bumping version to 2 when ready?). I volounteer to work on this if I get your approval.

As I also mentioned, I would like to see EndpointCreator._read_items and EndpointCreator._read_paginated unified to single read_items where pagination is optionally supported. That will be easy as they all use just read_multi under the hood. This way we could also simplify get_multi and get_paginated endpoints names to just /items/.

Lastly, I would love to see some easy way to override generic router endpoints.

[JakNowy]

cc @waza-ari