Values of integer-based enums are resolved as strings for GET requests parameters which leads to ConvertException

[teamush]

Bug description When the argument of GET route is annotated as integer-based enum (Enum with integer values or IntEnum), type conversion fails - received value is perceived as string, which leads to following errors:

1. convert_enum function fails with Value not in allowed values("1", "2"): "1"' if pure Enum with integers is used;
2. convert_int function fails with Cannot convert "1" to integer if IntEnum is used.

To Reproduce

1. Prepare a controller with some route_get;
2. Add a few parameters to route_get like: @route_get('/hello/{?param1,param2}');
3. Annotate underlying function parameters with integer-based enums:

from typing import Optional
from enum import Enum, IntEnum

import winter

class FooParam1(Enum)
    FOO = 1
    BAR = 2

class FooParam2(IntEnum)
    FOO = 1
    BAR = 2

@winter.controller
class HelloWorldController:
    @winter.route_get('/hello/{?param1,param2}')
    def hello(
            self,
            param1: Optional[FooParam1] = None,
            param2: Optional[FooParam2] = None):
        return f'Hello, world!'

4. Send a valid GET request with param1=1 or param2=1; 5.1 If param1 is given, endpoint returns HTTP 400 Bad Request with following error: Value not in allowed values("1", "2"): "1"' 5.2 If param2 is given, endpoint returns HTTP 400 Bad Request with following error: Cannot convert "1" to integer.

Expected behavior Parameter(s) are successfully resolved as enum values (or integers) and their belonging to the enum is validated.

Code snippets Minimal code snippet is given above.

Screenshots Screenshots are saying TemporaryHiringStatus and HiringStatus enums, sorry for that, but TemporaryHiringStatus stands for FooParam1 in the example above, and HiringStatus for FooParam2.

Environment(please complete the following information):

• OS: Ubuntu 18.04
• Python: CPython 3.7
• Winter version: 2.1.1

[andrey-berenda]

It seems you can not resolve it in winter but it can be resolved with changing enum like this Because converting from string to Enum is not trivial (my opinion)

class FooParam2(IntEnum):  # The same with FooParam1
    FOO = 1
    BAR = 2

    @classmethod
    def _missing_(cls, value):
        if isinstance(value, str) and value.isdigit():
            return cls(int(value))
        return super()._missing_(value)

[pristupa]

Trivial or not, I'd like we implement it! :-)

[andrey-berenda]

Because of it we will have bug(or not) with body: if we have enum with different types(int and string): '1' and 1 which one to use is not defined. Also with this implementation body {"param:": 1} and {"param": "1"} will valid for

@dataclasses.dataclass
class Data:
    param: FooParam2

But I think only {"param:": 1} valid and {"param": "1"} is not valid But I think the same with int(I haven't tried), so it is ok

[teamush]

Well, I can understand the concern about having heterogeneous data types inside Enum inheritors, but for IntEnum it shouldn't be a problem, as only integers could be used as it's values, but anyway, thanks for clarification and tips.

[pristupa]

@berenda-andrey we should probably distinguish between body and query string. In case it's a query string, we can understand that everything there is a string... And we could help with converting it to integer, no?

[mofr]

Both IntEnum and heterogeneous Enum are inspectable at the start time. So I think it can and should be implemented for both original use cases.

[andrey-berenda]

@pristupa Yes. I think distinguishing between body and query string can help

[mofr]

@teamush Please notice that the fix will go to 4.2.0. So probably it's a good time to update from 2.1.1 to 4.1.0, look at CHANGELOG.md for migration hints.