Ge0rg3 / flask-parameter-validation

Get and validate all Flask input parameters with ease.
https://pypi.org/project/Flask-Parameter-Validation/
28 stars 12 forks source link

Conversion of empty string to None #35

Open summersz opened 9 months ago

summersz commented 9 months ago

For query and form data, null values are transmitted as empty strings.

I would like to suggest a method to convert these to None during the convert function of the Query class.

I think the user would need to specify that this is the functionality they want. I am thinking either checking for Union[str, None] as the type or adding an extra attribute like allow_none=True.

Ge0rg3 commented 9 months ago

Got it... I guess checking for Union[str,None] has the same effect as just setting as Optional[str], and then either returning None or a str?

summersz commented 9 months ago

yes, that's a nicer way to declare it.

summersz commented 9 months ago

Having thought about this a little more I think passing an option would be a better solution, rather than relying on Optional[str], as this could lead to unexpected behaviour for users.

I was thinking blank_none=True and this would set an empty user_value to None in the validate function.

# If blank_none is true and the input is "", set input to None
if expected_delivery_type.blank_none and user_input == "":
    user_input = None

This would precede where the default values are set, so would also work in this instance. i.e. if an empty string is passed with the blank_none setting and a default value it would be set to None and then the default logic would pick it up and set it to the default value.

Let me know what you think, I can raise a pull request.

Ge0rg3 commented 8 months ago

For sure blank_none could be a good option.. What unexpected behaviour do you reckon using Optional[str] may cause?

summersz commented 7 months ago

One example I have is using a search string query parameter. I am using the same endpoint to retrieve html on the first load as well as getting specific data based on the search query.

I first check if the search query is None which indicates this is the first page loads so retrieve all the html

If I get a search query (which can be an empty string) I then retrieve an html fragment, so in this instance I wouldn't want the empty string to be converted to None.

@endpoint.get("")
@ValidateParameters()
def index(search: Optional[str] = Query(default=None)):
    if search is None: # first load
        return full_html
    else:
        return partial_filtered_html

Although there are workarounds for this I don't think it would be the expected default behaviour of Optional[] to return None, so could trip people up trying to do something like this.

smt5541 commented 2 months ago

What are your thoughts on blank_none defaulting to False to keep things backwards compatible?

Additionally, a variable in current_app.config could be set by end-users to override this default globally in their app, while still allowing the use of blank_none to change the behavior for individual parameters.

smt5541 commented 1 month ago

Wanted to follow up on the above - if there aren't any further comments, I'll go ahead and implement a blank_none option that defaults to False with a variable in current_app.config for overriding this globally for an app.