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

[FEATURE] Add request_model_include/exclude #1357

Open
skorokithakis opened this issue Apr 30, 2020 · 15 comments
Open

[FEATURE] Add request_model_include/exclude #1357

skorokithakis opened this issue Apr 30, 2020 · 15 comments
Labels
enhancement

Comments

@skorokithakis
Copy link
Contributor

skorokithakis commented Apr 30, 2020

I have a model that include an ID field, which I return in responses. However, in requests (when POSTing new instances), I don't want the user to be able to specify the ID.

Similarly to how we can include/exclude fields in the response, we should be able to exclude fields in the request. Declaring more classes just to exclude specific fields is a bit cumbersome, and making the property optional is semantically wrong, since I don't want to let the user specify it at all.

@skorokithakis skorokithakis added the enhancement label Apr 30, 2020
@chris-allnutt
Copy link
Contributor

chris-allnutt commented May 6, 2020

You can create a special case CreateUser model, like what is used in the full stack example

https://github.com/tiangolo/full-stack-fastapi-postgresql/blob/master/%7B%7Bcookiecutter.project_slug%7D%7D/backend/app/app/schemas/user.py#L15

@skorokithakis
Copy link
Contributor Author

skorokithakis commented May 6, 2020

Sure, but it feels dirty and unsemantic to have to create different models for each field you want to include, and is asymmetric with response_model_exclude existing.

@chris-allnutt
Copy link
Contributor

chris-allnutt commented May 6, 2020

Is there any reason you can't default the ID to None in the pydantic model?

@skorokithakis
Copy link
Contributor Author

skorokithakis commented May 6, 2020

Yes, the ID is not optional, it must not be sent at all.

@Lunrtick
Copy link

Lunrtick commented Jun 23, 2020

As far as I can see, this would just require passing the exclude list straight to any calls to models.to_dict right? (as seen here). I should have a bit of time this weekend to give it a go.

@skorokithakis
Copy link
Contributor Author

skorokithakis commented Jun 23, 2020

I think it's a bit more complicated than that, because the request is also for docs/requests, not just responses. I might have misunderstood what you mean, though, @Lunrtick, please correct me if so.

@Lunrtick
Copy link

Lunrtick commented Jun 24, 2020

I was talking about excluding it in the request, but you're right, it's definitely more complicated than just skipping the property in serialisation (mostly because of needing it to be documented in openapi). I was hoping to see how the response_model is translated to the openapi schema. I'd imagine that it wouldn't necessarily make sense to have a different Schema entry simply to exclude an ID?

To be clear, I'd probably want the list of Schemas to contain a User, for example, like this

{
    "id": "integer",
    "email": "string",
    "password": "string",
    "created_at": "datetime",
    "updated_at": "datetime"
}

And then on a particular route (maybe the create route), I'd exclude the "id", "created_at", and "updated_at" keys. So the schema for this route would then just be

{
    "email": "string",
    "password": "string"
}

This wouldn't require a separate pydantic model, so the global list of schemas would only have a User, not a UserInDB + UserToCreate + UserOut etc.

@skorokithakis
Copy link
Contributor Author

skorokithakis commented Jun 24, 2020

Yes, exactly, this is the rationale for the PR, so we can avoid having three different models just to exclude three fields. That's a bit clunky, whereas this solution is much more elegant and is symmetrical with the response include/exclude.

@RohitJones
Copy link

RohitJones commented Nov 18, 2020

I think this also fits well with the OpenAPI 3.0 spec's Read-Only and Write-Only Properties. response_exclude maps well to writeOnly openapi property and a potential request_exclude would map nicely to the readOnly openapi property.
However, I think that feature implementation might suit Pydantic better and have fastapi read off it.

@databasedav
Copy link

databasedav commented Mar 11, 2021

This doesn't address all of the concerns above but suffices for what I was looking for so might be helpful.

from datetime import datetime
from typing import Iterable

from fastapi import FastAPI
from pydantic import BaseModel, create_model


app = FastAPI()

def require(model: BaseModel, *which: Iterable[str]):
    if not which:  # require all
        which = model.schema()['properties'].keys()
    return create_model(
        f"{model.__name__}__{'_'.join(which)}",
        __base__=model,
        **{w: (model.__fields__[w].outer_type_, ...) for w in which}
    )

class OmniUser(BaseModel):
    id: int = None
    email: str = None
    password: str = None
    created_at: datetime = None
    updated_at: datetime = None

@app.post('/create_user', response_model=require(OmniUser))
async def create_user(payload: require(OmniUser, 'email', 'password')):
    ...

This also carries over any @validator's for the required fields.

edit: change model.__fields__[w].outer_type_ to typing.Optional[model.__fields__[w].outer_type_] to require the fields but allow None

@PrettyWood
Copy link
Contributor

PrettyWood commented Apr 20, 2021

pydantic/pydantic#2231 may be helpful for this.
The remaining thing to do would be to change the generated schemas (add readOnly: true?)

@sadakmed
Copy link

sadakmed commented Jul 25, 2021

        __base__=model,

create_model function inherits from __base__ so last argument in the function will filter the desired fields, but all the fields will be inherited from model, giving u the same inputted model,

__base__ should be BaseModel

Edit: however the downside of this is the validators are also lost,

what I ended up doing is

In [98]: def delete(model: BaseModel, *fields_to_delete):
    ...:     new_model = type('new_model', model.__bases__, dict(model.__dict__))
    ...:     for field in fields_to_delete:
    ...:         if field in new_model.__fields__:
    ...:             del new_model.__fields__[field]
    ...:     return new_model

@tdonovic
Copy link

tdonovic commented Oct 6, 2021

A thought when I was trying to implement this was to create a base, request class, that excludes the id, created_at, updated_at fields, and then create another class that uses the first as its __base__. Is this an approach that would work? Still have the duplicate classes, but at least there is inheritance?

@mseimys
Copy link

mseimys commented Oct 31, 2021

IMHO It's perfectly fine that we need to define two different types for this use case. It's also fine to get those types by combining classes.

My reasoning: these objects are created and used in different context, it's just a coincidence that they match most of the properties, but imagine we add one more property like client_id which should register a user to some client, but it shouldn't be part of the User class. It will get confusing pretty fast. Also if you tried converting such "type" to other typed programming languages like TypeScript - you couldn't. You can't have two objects named the same, but in some context one has more properties than the other - no way to specify that.

For example, GraphQL even has dedicated "Input" types which kinda try addressing the same issue you described:

Could you please explain why if input argument of mutation is object it should be input type? I think much simpler just reuse type without providing id.

The accepted answer describes some principles better than I did here :)

@InfinityMod
Copy link

InfinityMod commented Dec 23, 2021

        __base__=model,

create_model function inherits from __base__ so last argument in the function will filter the desired fields, but all the fields will be inherited from model, giving u the same inputted model,

__base__ should be BaseModel

Edit: however the downside of this is the validators are also lost,

what I ended up doing is

In [98]: def delete(model: BaseModel, *fields_to_delete):
    ...:     new_model = type('new_model', model.__bases__, dict(model.__dict__))
    ...:     for field in fields_to_delete:
    ...:         if field in new_model.__fields__:
    ...:             del new_model.__fields__[field]
    ...:     return new_model

I extended the function a little bit:

def filter(model: BaseModel, *which: Iterable[str], exclude=False):
    """Filters Pydantic Models and includes 
       (or excludes if set to True) the specified properties
       to abbreviate a model with a reduced field-set.
       All other properties are shared with the originated model."""
    if not which:  # require all
        which = model.schema()['properties'].keys()
    new_model = type(f"{model.__name__}__{'_'.join(which)}", model.__bases__,
                     dict(model.__dict__, __fields__=model.__fields__.copy()))
    if exclude:
        for field in which:
            if field in new_model.__fields__:
                del new_model.__fields__[field]
    else:
        for field in list(new_model.__fields__.keys()):
            if field not in which:
                del new_model.__fields__[field]
    return new_model

One small bug in the old one was, that fields property of the new model pointed to the same fields property of the old model, because dict(model.dict) only performed only a shallow copy. I solved it by adding an explicite copy of fields to the new properties dict.

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

No branches or pull requests

10 participants