marshmallow-code / flask-smorest

DB agnostic framework to build auto-documented REST APIs with Flask and marshmallow
https://flask-smorest.readthedocs.io
MIT License
651 stars 72 forks source link

Passing a schema that has a field with a default load value to the arguments decorator does not work. #538

Closed AjeelAhmed1998 closed 1 year ago

AjeelAhmed1998 commented 1 year ago

How to reproduce:

import marshmallow as ma
from flask import Flask
from flask.views import MethodView
from flask_smorest import Api, Blueprint, abort

app = Flask(__name__)
app.config["API_TITLE"] = "My API"
app.config["API_VERSION"] = "v1"
app.config["OPENAPI_VERSION"] = "3.0.2"
api = Api(app)

class PetSchema(ma.Schema):
    name = ma.fields.String()
    age = ma.fields.Integer(required=False, load_default=24, dump_default=26)

class PetQueryArgsSchema(ma.Schema):
    name = ma.fields.String()

blp = Blueprint(
    "pets", "pets", url_prefix="/pets", description="Operations on pets"
)

@blp.route("/")
class Pets(MethodView):
    @blp.arguments(PetQueryArgsSchema(partial=True), location="query")
    @blp.response(200, PetSchema)
    def get(self, args):
        return "hello pets"

api.register_blueprint(blp)

To reiterate: the load_default parameter for the Marshmallow Schema does not work when it's passed to the arguments decorator. I replicated this in a regular Flask application and it works and from what I can tell this has something to do with webargs and it's FlaskParser.

What I expect to happen is that if the field does not exist in the request body, the app should create that field automatically with a default value that I defined but instead I don't get that field.

You will see that if you hit the endpoint defined above with no query parameters it will give you an empty dict but I am expecting {'age':24}.

Let me know if there are any other questions.

AjeelAhmed1998 commented 1 year ago

Hey @lafrech thanks for labeling but don't you think it's a bug? Can you please shed some light on this if possible? Thank you.

lafrech commented 1 year ago

I was replying then I realized I misunderstood the use case. It may be a bug indeed but I don't understand. Can you please clarify (trim down your sample code to a simpler case)?

In your example, the schema with load_default is not passed to arguments but to response. And it's not clear what you expect from the serialization of "hello pets".

AjeelAhmed1998 commented 1 year ago

@lafrech apologies for the confusion I have caused, I have simplified the code: let's start with the test:

@pytest.fixture
def client():
    with app.test_client() as client:
        yield client

def test_default_value_set(client):
    response = client.get(
        "/pets/?name=ajel",
        content_type="application/json",
    )
    print(response.data)      #{"age":26,"name":"ajel"}

    assert response.status_code == 200
class PetSchema(ma.Schema):
    name = ma.fields.String()
    age = ma.fields.Integer(required=False, load_default=24, dump_default=26)

blp = Blueprint(
    "pets", "pets", url_prefix="/pets", description="Operations on pets"
)

@blp.route("/")
class Pets(MethodView):
    @blp.arguments(PetSchema(partial=True), location="query")
    @blp.response(200, PetSchema)
    def get(self, args):
        print(args)      #{'name': 'ajel'}, I should be getting {"age":24, "name":"ajeel"}
        return args

api.register_blueprint(blp)

This endpoint should basically change the age of a person from 24(which it should by default, this part does not work) and return 26(which it should also set by default, this part works)

I hope this clarifies the code a little bit, I'm using the same schema now for the argument and the response. Please feel free to let me know if there is still any confusion.

lafrech commented 1 year ago

OK, this is what I was about to answer the first time. This is due to partial. When using partial, missing fields are ignored just like required fields, their default value is not used.

class MySchema(Schema):
    field_1=String(required=True)
    field_2=String(load_default="value")

MySchema(partial=True).load({})
# {}

The doc doesn't make that explicit. You may open an issue in marshmallow bugtracker about it, but since this has been working like this for a while, changing it would be a breaking change, so it will likely end-up filed as a doc issue.

However, it could be reconsidered for marshmallow 4.

AjeelAhmed1998 commented 1 year ago

Hey @lafrech thank you for the swift reply!