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

@validates accepts multiple field names #1965

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

dharani7998
Copy link

Fix for #1960

@dharani7998 dharani7998 requested a review from deckar01 March 31, 2022 15:25
@sloria
Copy link
Member

sloria commented Jan 16, 2025

apologies for the long delay on this. i hesitated initially because i had the same concern as @deckar01 about this being a breaking change. now that we're closing to releasing the next major, i think this is one that we can get merged. i'll update this when i have some time

@sloria sloria added this to the 4.0 milestone Jan 16, 2025
@sloria
Copy link
Member

sloria commented Jan 16, 2025

should validator methods receive field_name?

from marshmallow import Schema, fields, validates, ValidationError

class UserSchema(Schema):
    name = fields.Str(required=True)
    nickname = fields.Str(required=True)

    @validates("name", "nickname")
    def validate_names(self, value: str, field_name: str) -> str:
        if len(value) < 3:
            raise ValidationError(f"{field_name} too short")
        return value

@sloria
Copy link
Member

sloria commented Jan 16, 2025

looks like SQLAlchemy does pass a key argument: https://docs.sqlalchemy.org/en/20/orm/mapped_attributes.html#simple-validators

from sqlalchemy.orm import validates


class EmailAddress(Base):
    __tablename__ = "address"

    id = mapped_column(Integer, primary_key=True)
    email = mapped_column(String)

    @validates("email")
    def validate_email(self, key, address):
        if "@" not in address:
            raise ValueError("failed simple email validation")
        return address

@sloria sloria changed the title multiple fields in validates decorator @validates accepts multiple field names Jan 18, 2025
@sloria
Copy link
Member

sloria commented Jan 18, 2025

Updated to pass data_key as a keyword argument to decorated methods. Would like one review on this since it's a breaking change

@sloria sloria requested a review from lafrech January 18, 2025 21:49
Copy link
Member

@lafrech lafrech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong opinion about this. It adds a bit of API surface for a case that may not be so frequent, but addressing it with validates_schema is less convenient so this makes sense.

I'm not a fan of the "string or list of strings" pattern. In fact I didn't see where the magic operates but I suppose it does, otherwise all tests would be broken. And expecting a list of strings for a single field (99% case) would be a pity.

@validates("name", "nickname")
def validate_names(self, value: str, data_key: str) -> None:
if len(value) < 3:
raise ValidationError("Too short")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be the right place to show the use of data_key like in the upgrading guide.

@sloria
Copy link
Member

sloria commented Jan 27, 2025

I'm not a fan of the "string or list of strings" pattern. In fact I didn't see where the magic operates but I suppose it does, otherwise all tests would be broken. And expecting a list of strings for a single field (99% case) would be a pity.

there's actually no special-casing here because

def validates(*field_names: str)

allows one or more field names to be passed positionally.

@lafrech
Copy link
Member

lafrech commented Jan 27, 2025

But then in validates, the code shall not iterate on the string if a single field name is passed. If we blindly iterate on field_names when a single string is passed, it won't work. I expected to see a type check such as iterable_not_string.

(I'm not talking about mypy / type checking but about runtime.)

@sloria
Copy link
Member

sloria commented Jan 27, 2025

field_names will always be a tuple, even when a single argument is passed.

>>> def validates(*field_names: str):
...     print(type(field_names))
...     
>>> validates("foo")
<class 'tuple'>

@lafrech
Copy link
Member

lafrech commented Jan 27, 2025

Oh sorry, I missed the obvious.

The user never passes a tuple or list, just N positional args.

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

Successfully merging this pull request may close these issues.

4 participants