piccolo-orm / piccolo_api

ASGI middleware for authentication, rate limiting, and building REST endpoints.
https://piccolo-api.readthedocs.io/en/latest/
MIT License
146 stars 26 forks source link

Feature to modify body and query_parameters in `PiccoloCRUD` #235

Open sarvesh4396 opened 1 year ago

sarvesh4396 commented 1 year ago

Currently, PiccoloCRUD exposes all endpoints and we can not change them internally. If we try to add middleware we can achieve that for query parameters but it can not be done for the body.

Proposed Solution

Add two callable parameters in PiccoloCRUD for manipulation of the body and parameters.

def __init__(
....
modify_body: t.Optional[t.Callable[[t.Dict[str, t.Any], t.Dict[str, t.Any]], t.Dict[str, t.Any]]] = None,
modify_query_params: t.Optional[t.Callable[[t.Dict[str, t.Any], t.Dict[str, t.Any]], t.Dict[str, t.Any]]] = None,
....
)->None

And we can pass

Then we can update the dict at desired places

data = await request.json()
if self.modify_body:
        data.update(self.modify_body(requests.headers,data)))

Use Cases

It will give the ability to manipulate the query operations in the desired way for example a notes app

If we want to add a note for that user without passing the user_id. If we are using jwt then we can get the user_id from the Authorization header and pass it into the body also, It will enable us to get all the notes from that user without passing that param explicitly.

@dantownsend I can do a PR, if the idea is helpful.

dantownsend commented 1 year ago

Hi @sarvesh4396

My initial thoughts was using middleware to modify the body. I looked around a bit and found this issue:

https://github.com/encode/starlette/issues/495#issuecomment-494008175

It's definitely not easy to do, but I think it's possible.

We have a couple of ways of modifying the behaviour of PiccoloCRUD. One is hooks, but probably not useful in this situation.

The other is validators. The intended purpose is to add extra validation to endpoints, but you may be able to use these to modify the request.

async def my_validator(piccolo_crud: PiccoloCRUD, request: Request):
    json = await request.json()
    request._json = {**json, 'custom_field': 'hello world'}

It might be worth giving that a go.

sarvesh4396 commented 1 year ago

hey @dantownsend , I tried the above solution but it doesn't work cause the data is extracted before the validators run.

dantownsend commented 1 year ago

@sarvesh4396 OK, interesting. When you do await request.json() does it just hang?

sarvesh4396 commented 1 year ago

@dantownsend Yes, It does, query params can be changed through middleware but not the body. For now, I have passed the modify_body arg for a quick patch it is working fine.

dantownsend commented 1 year ago

@sarvesh4396 OK, cool. Do you think this is something we could write a custom hook for?

sarvesh4396 commented 1 year ago

@dantownsend I didn't get it, Do you mean by Hooks which are there in piccolo-api? or something else. Though I believe we could write a function for it. and we can run it wherever we are doing await request.json() such that we do not have to await again and again which may hamper performance. accessing the request body in middleware has been an issue I couldn't find anything yet which can resolve it.

dantownsend commented 1 year ago

@sarvesh4396 Yeah, we have hooks for PiccoloAPI like pre_save. Could add a new one maybe? 🤔

In our CSRFMiddleware we access the request body, but then add it to the ASGI scope. This works OK, but the downstream app needs to know to look in the scope for the data.

https://github.com/piccolo-orm/piccolo_api/blob/16704dd5cfbe7c772efa94f7ca2cd6927c5e84c1/piccolo_api/csrf/middleware.py#L136-L139

sarvesh4396 commented 1 year ago

@dantownsend, It should work ideally, as we can receive the request Object in hooks.