Repository: samuelcolvin/pydantic
Pull request #3413 (2021-11-14 03:55:28)

bug: Add annotation to ModelField for use in BaseModel.__signature__
Currently, `ModelField.outer_type_` is used for `Model.__signature__`.

This leads to signatures that differ from dataclasses or classes with a manual `__init__` of the same types.

```python
from dataclasses import dataclass
from inspect import signature
from typing import Optional

from pydantic import BaseModel

@dataclass
class Dataclass:
    foo: Optional[int] = None

class CustomClass:
    def __init__(self, foo: Optional[int] = None) -> None:
        ...

class PydanticModel(BaseModel):
    foo: Optional[int] = None


print(signature(Dataclass).parameters["foo"].annotation)  # Optional[int]
print(signature(CustomClass).parameters["foo"].annotation)  # Optional[int]
print(signature(PydanticModel).parameters["foo"].annotation)  # <class 'int'>
```

This PR fixes this by storing the annotation in `ModelField.annotation` and then using that for the signature instead of `outer_type_`.
I realize this is adding a public attribute to `ModelField`, and that it is not as simple as storing the `type_` passed into the constructor (I tried to handle the case where the type is inferred from the default value, I think successfully), so this may not be the best solution, but it is the best I could come up with today.

samuelcolvin: please update.
adriangb: Hmm. While I am adding a public attribute, and that could be considered a feature, I am only doing it to fix what I would consider a bug (incorrect results from `inspect.signsture`). One of my main questions here was: can you think of a way of doing this that does not add a public attribute?
samuelcolvin: I don't mind the public attribute, I don't see another (simply) way.

Which of my points were you questioning? The current description, out which release this goes in?
adriangb: I was just questioning if this is a bug fix or a feature. I think the underlying problem is a bug, but if we're adding a public attribute it's more like a feature.

I'll add the change description.
adriangb: @samuelcolvin I guess we didn't merge this 😅
samuelcolvin: > @samuelcolvin I guess we didn't merge this sweat_smile

Got there in the end :sweat:.

Thank so much, as ever, _sorry for the delay_.

Patch:
diff --git a/changes/3337-rglsk.md b/changes/3337-rglsk.md
new file mode 100644
index 000000000..c0f2da372
--- /dev/null
+++ b/changes/3337-rglsk.md
@@ -0,0 +1,1 @@
+Support multi hosts validation in `PostgresDsn`.
\ No newline at end of file
diff --git a/changes/3413-fix-inspect-signature.md b/changes/3413-fix-inspect-signature.md
new file mode 100644
index 000000000..157650845
--- /dev/null
+++ b/changes/3413-fix-inspect-signature.md
@@ -0,0 +1,1 @@
+fix mangling of `inspect.signature` for `BaseModel`
diff --git a/docs/usage/types.md b/docs/usage/types.md
index 15bb9f9cb..4e6ad2068 100644
--- a/docs/usage/types.md
+++ b/docs/usage/types.md
@@ -641,7 +641,10 @@ For URI/URL validation the following types are available:
 - `AnyHttpUrl`: scheme `http` or `https`, TLD not required, host required
 - `HttpUrl`: scheme `http` or `https`, TLD required, host required, max length 2083
 - `FileUrl`: scheme `file`, host not required
-- `PostgresDsn`: scheme `postgres`, `postgresql`, user info required, TLD not required, host required. Also, its supported DBAPI dialects:
+- `PostgresDsn`: user info required, TLD not required, host required,
+  as of V.10 `PostgresDsn` supports multiple hosts. The following schemes are supported:
+  - `postgres`
+  - `postgresql`
   - `postgresql+asyncpg`
   - `postgresql+pg8000`
   - `postgresql+psycopg2`
@@ -650,7 +653,7 @@ For URI/URL validation the following types are available:
   - `postgresql+pygresql`
 - `AmqpDsn`: schema `amqp` or `amqps`, user info not required, TLD not required, host not required
 - `RedisDsn`: scheme `redis` or `rediss`, user info not required, tld not required, host not required (CHANGED: user info
-- `MongoDsn` : scheme `mongodb`, user info not required, database name not required, port not required  
+- `MongoDsn` : scheme `mongodb`, user info not required, database name not required, port
   not required from **v1.6** onwards), user info may be passed without user part (e.g., `rediss://:pass@localhost`)
 - `stricturl`: method with the following keyword arguments:
     - `strip_whitespace: bool = True`
diff --git a/pydantic/fields.py b/pydantic/fields.py
index ab32be9f2..cb763959d 100644
--- a/pydantic/fields.py
+++ b/pydantic/fields.py
@@ -356,6 +356,7 @@ class ModelField(Representation):
     __slots__ = (
         'type_',
         'outer_type_',
+        'annotation',
         'sub_fields',
         'sub_fields_mapping',
         'key_field',
@@ -396,6 +397,7 @@ class ModelField(Representation):
         self.name: str = name
         self.has_alias: bool = alias is not None
         self.alias: str = alias if alias is not None else name
+        self.annotation = type_
         self.type_: Any = convert_generics(type_)
         self.outer_type_: Any = type_
         self.class_validators = class_validators or {}
@@ -557,6 +559,7 @@ class ModelField(Representation):
         if default_value is not None and self.type_ is Undefined:
             self.type_ = default_value.__class__
             self.outer_type_ = self.type_
+            self.annotation = self.type_
 
         if self.type_ is Undefined:
             raise errors_.ConfigError(f'unable to infer type for attribute "{self.name}"')
diff --git a/pydantic/networks.py b/pydantic/networks.py
index 71396f852..4bab18856 100644
--- a/pydantic/networks.py
+++ b/pydantic/networks.py
@@ -15,6 +15,8 @@ from typing import (
     Collection,
     Dict,
     Generator,
+    List,
+    Match,
     Optional,
     Pattern,
     Set,
@@ -51,6 +53,13 @@ if TYPE_CHECKING:
         query: Optional[str]
         fragment: Optional[str]
 
+    class HostParts(TypedDict, total=False):
+        host: str
+        tld: Optional[str]
+        host_type: Optional[str]
+        port: Optional[str]
+        rebuild: bool
+
 else:
     email_validator = None
 
@@ -80,30 +89,54 @@ __all__ = [
 ]
 
 _url_regex_cache = None
+_multi_host_url_regex_cache = None
 _ascii_domain_regex_cache = None
 _int_domain_regex_cache = None
+_host_regex_cache = None
+
+_host_regex = (
+    r'(?:'
+    r'(?P<ipv4>(?:\d{1,3}\.){3}\d{1,3})(?=$|[/:#?])|'  # ipv4
+    r'(?P<ipv6>\[[A-F0-9]*:[A-F0-9:]+\])(?=$|[/:#?])|'  # ipv6
+    r'(?P<domain>[^\s/:?#]+)'  # domain, validation occurs later
+    r')?'
+    r'(?::(?P<port>\d+))?'  # port
+)
+_scheme_regex = r'(?:(?P<scheme>[a-z][a-z0-9+\-.]+)://)?'  # scheme https://tools.ietf.org/html/rfc3986#appendix-A
+_user_info_regex = r'(?:(?P<user>[^\s:/]*)(?::(?P<password>[^\s/]*))?@)?'
+_path_regex = r'(?P<path>/[^\s?#]*)?'
+_query_regex = r'(?:\?(?P<query>[^\s#]*))?'
+_fragment_regex = r'(?:#(?P<fragment>[^\s#]*))?'
 
 
 def url_regex() -> Pattern[str]:
     global _url_regex_cache
     if _url_regex_cache is None:
         _url_regex_cache = re.compile(
-            r'(?:(?P<scheme>[a-z][a-z0-9+\-.]+)://)?'  # scheme https://tools.ietf.org/html/rfc3986#appendix-A
-            r'(?:(?P<user>[^\s:/]*)(?::(?P<password>[^\s/]*))?@)?'  # user info
-            r'(?:'
-            r'(?P<ipv4>(?:\d{1,3}\.){3}\d{1,3})(?=$|[/:#?])|'  # ipv4
-            r'(?P<ipv6>\[[A-F0-9]*:[A-F0-9:]+\])(?=$|[/:#?])|'  # ipv6
-            r'(?P<domain>[^\s/:?#]+)'  # domain, validation occurs later
-            r')?'
-            r'(?::(?P<port>\d+))?'  # port
-            r'(?P<path>/[^\s?#]*)?'  # path
-            r'(?:\?(?P<query>[^\s#]*))?'  # query
-            r'(?:#(?P<fragment>[^\s#]*))?',  # fragment
+            rf'{_scheme_regex}{_user_info_regex}{_host_regex}{_path_regex}{_query_regex}{_fragment_regex}',
             re.IGNORECASE,
         )
     return _url_regex_cache
 
 
+def multi_host_url_regex() -> Pattern[str]:
+    """
+    Compiled multi host url regex.
+
+    Additionally to `url_regex` it allows to match multiple hosts.
+    E.g. host1.db.net,host2.db.net
+    """
+    global _multi_host_url_regex_cache
+    if _multi_host_url_regex_cache is None:
+        _multi_host_url_regex_cache = re.compile(
+            rf'{_scheme_regex}{_user_info_regex}'
+            r'(?P<hosts>([^/]*))'  # validation occurs later
+            rf'{_path_regex}{_query_regex}{_fragment_regex}',
+            re.IGNORECASE,
+        )
+    return _multi_host_url_regex_cache
+
+
 def ascii_domain_regex() -> Pattern[str]:
     global _ascii_domain_regex_cache
     if _ascii_domain_regex_cache is None:
@@ -124,6 +157,16 @@ def int_domain_regex() -> Pattern[str]:
     return _int_domain_regex_cache
 
 
+def host_regex() -> Pattern[str]:
+    global _host_regex_cache
+    if _host_regex_cache is None:
+        _host_regex_cache = re.compile(
+            _host_regex,
+            re.IGNORECASE,
+        )
+    return _host_regex_cache
+
+
 class AnyUrl(str):
     strip_whitespace = True
     min_length = 1
@@ -228,7 +271,7 @@ class AnyUrl(str):
             value = value.strip()
         url: str = cast(str, constr_length_validator(value, field, config))
 
-        m = url_regex().match(url)
+        m = cls._match_url(url)
         # the regex should always match, if it doesn't please report with details of the URL tried
         assert m, 'URL regex failed unexpectedly'
 
@@ -236,11 +279,19 @@ class AnyUrl(str):
         parts = cls.apply_default_parts(original_parts)
         parts = cls.validate_parts(parts)
 
-        host, tld, host_type, rebuild = cls.validate_host(parts)
-
         if m.end() != len(url):
             raise errors.UrlExtraError(extra=url[m.end() :])
 
+        return cls._build_url(m, url, parts)
+
+    @classmethod
+    def _build_url(cls, m: Match[str], url: str, parts: 'Parts') -> 'AnyUrl':
+        """
+        Validate hosts and build the AnyUrl object. Split from `validate` so this method
+        can be altered in `MultiHostDsn`.
+        """
+        host, tld, host_type, rebuild = cls.validate_host(parts)
+
         return cls(
             None if rebuild else url,
             scheme=parts['scheme'],
@@ -255,10 +306,19 @@ class AnyUrl(str):
             fragment=parts['fragment'],
         )
 
+    @staticmethod
+    def _match_url(url: str) -> Optional[Match[str]]:
+        return url_regex().match(url)
+
+    @staticmethod
+    def _validate_port(port: Optional[str]) -> None:
+        if port is not None and int(port) > 65_535:
+            raise errors.UrlPortError()
+
     @classmethod
-    def validate_parts(cls, parts: 'Parts') -> 'Parts':
+    def validate_parts(cls, parts: 'Parts', validate_port: bool = True) -> 'Parts':
         """
-        A method used to validate parts of an URL.
+        A method used to validate parts of a URL.
         Could be overridden to set default values for parts if missing
         """
         scheme = parts['scheme']
@@ -268,9 +328,8 @@ class AnyUrl(str):
         if cls.allowed_schemes and scheme.lower() not in cls.allowed_schemes:
             raise errors.UrlSchemePermittedError(set(cls.allowed_schemes))
 
-        port = parts['port']
-        if port is not None and int(port) > 65_535:
-            raise errors.UrlPortError()
+        if validate_port:
+            cls._validate_port(parts['port'])
 
         user = parts['user']
         if cls.user_required and user is None:
@@ -280,7 +339,7 @@ class AnyUrl(str):
 
     @classmethod
     def validate_host(cls, parts: 'Parts') -> Tuple[str, Optional[str], str, bool]:
-        host, tld, host_type, rebuild = None, None, None, False
+        tld, host_type, rebuild = None, None, False
         for f in ('domain', 'ipv4', 'ipv6'):
             host = parts[f]  # type: ignore[literal-required]
             if host:
@@ -360,7 +419,71 @@ class FileUrl(AnyUrl):
     __slots__ = ()
 
 
-class PostgresDsn(AnyUrl):
+class MultiHostDsn(AnyUrl):
+    __slots__ = AnyUrl.__slots__ + ('hosts',)
+
+    def __init__(self, *args: Any, hosts: Optional[List['HostParts']] = None, **kwargs: Any):
+        super().__init__(*args, **kwargs)
+        self.hosts = hosts
+
+    @staticmethod
+    def _match_url(url: str) -> Optional[Match[str]]:
+        return multi_host_url_regex().match(url)
+
+    @classmethod
+    def validate_parts(cls, parts: 'Parts', validate_port: bool = True) -> 'Parts':
+        return super().validate_parts(parts, validate_port=False)
+
+    @classmethod
+    def _build_url(cls, m: Match[str], url: str, parts: 'Parts') -> 'MultiHostDsn':
+        hosts_parts: List['HostParts'] = []
+        host_re = host_regex()
+        for host in m.groupdict()['hosts'].split(','):
+            d: Parts = host_re.match(host).groupdict()  # type: ignore
+            host, tld, host_type, rebuild = cls.validate_host(d)
+            port = d.get('port')
+            cls._validate_port(port)
+            hosts_parts.append(
+                {
+                    'host': host,
+                    'host_type': host_type,
+                    'tld': tld,
+                    'rebuild': rebuild,
+                    'port': port,
+                }
+            )
+
+        if len(hosts_parts) > 1:
+            return cls(
+                None if any([hp['rebuild'] for hp in hosts_parts]) else url,
+                scheme=parts['scheme'],
+                user=parts['user'],
+                password=parts['password'],
+                path=parts['path'],
+                query=parts['query'],
+                fragment=parts['fragment'],
+                host_type=None,
+                hosts=hosts_parts,
+            )
+        else:
+            # backwards compatibility with single host
+            host_part = hosts_parts[0]
+            return cls(
+                None if host_part['rebuild'] else url,
+                scheme=parts['scheme'],
+                user=parts['user'],
+                password=parts['password'],
+                host=host_part['host'],
+                tld=host_part['tld'],
+                host_type=host_part['host_type'],
+                port=host_part.get('port'),
+                path=parts['path'],
+                query=parts['query'],
+                fragment=parts['fragment'],
+            )
+
+
+class PostgresDsn(MultiHostDsn):
     allowed_schemes = {
         'postgres',
         'postgresql',
diff --git a/pydantic/utils.py b/pydantic/utils.py
index 31e747713..858862593 100644
--- a/pydantic/utils.py
+++ b/pydantic/utils.py
@@ -258,7 +258,7 @@ def generate_model_signature(
             # TODO: replace annotation with actual expected types once #1055 solved
             kwargs = {'default': field.default} if not field.required else {}
             merged_params[param_name] = Parameter(
-                param_name, Parameter.KEYWORD_ONLY, annotation=field.outer_type_, **kwargs
+                param_name, Parameter.KEYWORD_ONLY, annotation=field.annotation, **kwargs
             )
 
     if config.extra is Extra.allow:


Test patch:
diff --git a/tests/test_model_signature.py b/tests/test_model_signature.py
index 789372b9b..e74ebc60c 100644
--- a/tests/test_model_signature.py
+++ b/tests/test_model_signature.py
@@ -1,5 +1,7 @@
-from inspect import signature
-from typing import Any, Iterable, Union
+from inspect import Parameter, Signature, signature
+from typing import Any, Iterable, Optional, Union
+
+from typing_extensions import Annotated
 
 from pydantic import BaseModel, Extra, Field, create_model
 
@@ -138,3 +140,31 @@ def test_signature_is_class_only():
     assert _equals(str(signature(Model)), '(*, foo: int = 123) -> None')
     assert _equals(str(signature(Model())), '(a: int) -> bool')
     assert not hasattr(Model(), '__signature__')
+
+
+def test_optional_field():
+    class Model(BaseModel):
+        foo: Optional[int] = None
+
+    assert signature(Model) == Signature(
+        [Parameter('foo', Parameter.KEYWORD_ONLY, default=None, annotation=Optional[int])], return_annotation=None
+    )
+
+
+def test_annotated_field():
+    class Model(BaseModel):
+        foo: Annotated[int, 'foo'] = 1
+
+    assert signature(Model) == Signature(
+        [Parameter('foo', Parameter.KEYWORD_ONLY, default=1, annotation=Annotated[int, 'foo'])], return_annotation=None
+    )
+
+
+def test_annotated_optional_field():
+    class Model(BaseModel):
+        foo: Annotated[Optional[int], 'foo'] = None
+
+    assert signature(Model) == Signature(
+        [Parameter('foo', Parameter.KEYWORD_ONLY, default=None, annotation=Annotated[Optional[int], 'foo'])],
+        return_annotation=None,
+    )
diff --git a/tests/test_networks.py b/tests/test_networks.py
index a1b1702e9..851d39b45 100644
--- a/tests/test_networks.py
+++ b/tests/test_networks.py
@@ -419,31 +419,92 @@ def test_http_urls_default_port(url, port):
     assert m.v == url
 
 
-def test_postgres_dsns():
+@pytest.mark.parametrize(
+    'dsn',
+    [
+        'postgres://user:pass@localhost:5432/app',
+        'postgresql://user:pass@localhost:5432/app',
+        'postgresql+asyncpg://user:pass@localhost:5432/app',
+        'postgres://user:pass@host1.db.net,host2.db.net:6432/app',
+    ],
+)
+def test_postgres_dsns(dsn):
     class Model(BaseModel):
         a: PostgresDsn
 
-    assert Model(a='postgres://user:pass@localhost:5432/app').a == 'postgres://user:pass@localhost:5432/app'
-    assert Model(a='postgresql://user:pass@localhost:5432/app').a == 'postgresql://user:pass@localhost:5432/app'
-    assert (
-        Model(a='postgresql+asyncpg://user:pass@localhost:5432/app').a
-        == 'postgresql+asyncpg://user:pass@localhost:5432/app'
-    )
+    assert Model(a=dsn).a == dsn
 
-    with pytest.raises(ValidationError) as exc_info:
-        Model(a='http://example.org')
-    assert exc_info.value.errors()[0]['type'] == 'value_error.url.scheme'
-    assert exc_info.value.json().startswith('[')
 
-    with pytest.raises(ValidationError) as exc_info:
-        Model(a='postgres://localhost:5432/app')
-    error = exc_info.value.errors()[0]
-    assert error == {'loc': ('a',), 'msg': 'userinfo required in URL but missing', 'type': 'value_error.url.userinfo'}
+@pytest.mark.parametrize(
+    'dsn,error_message',
+    (
+        (
+            'postgres://user:pass@host1.db.net:4321,/foo/bar:5432/app',
+            {'loc': ('a',), 'msg': 'URL host invalid', 'type': 'value_error.url.host'},
+        ),
+        (
+            'postgres://user:pass@host1.db.net,/app',
+            {'loc': ('a',), 'msg': 'URL host invalid', 'type': 'value_error.url.host'},
+        ),
+        (
+            'postgres://user:pass@/foo/bar:5432,host1.db.net:4321/app',
+            {'loc': ('a',), 'msg': 'URL host invalid', 'type': 'value_error.url.host'},
+        ),
+        (
+            'postgres://localhost:5432/app',
+            {'loc': ('a',), 'msg': 'userinfo required in URL but missing', 'type': 'value_error.url.userinfo'},
+        ),
+        (
+            'postgres://user@/foo/bar:5432/app',
+            {'loc': ('a',), 'msg': 'URL host invalid', 'type': 'value_error.url.host'},
+        ),
+        (
+            'http://example.org',
+            {
+                'loc': ('a',),
+                'msg': 'URL scheme not permitted',
+                'type': 'value_error.url.scheme',
+                'ctx': {'allowed_schemes': PostgresDsn.allowed_schemes},
+            },
+        ),
+    ),
+)
+def test_postgres_dsns_validation_error(dsn, error_message):
+    class Model(BaseModel):
+        a: PostgresDsn
 
     with pytest.raises(ValidationError) as exc_info:
-        Model(a='postgres://user@/foo/bar:5432/app')
+        Model(a=dsn)
     error = exc_info.value.errors()[0]
-    assert error == {'loc': ('a',), 'msg': 'URL host invalid', 'type': 'value_error.url.host'}
+    assert error == error_message
+
+
+def test_multihost_postgres_dsns():
+    class Model(BaseModel):
+        a: PostgresDsn
+
+    any_multihost_url = Model(a='postgres://user:pass@host1.db.net:4321,host2.db.net:6432/app').a
+    assert any_multihost_url == 'postgres://user:pass@host1.db.net:4321,host2.db.net:6432/app'
+    assert any_multihost_url.scheme == 'postgres'
+    assert any_multihost_url.host is None
+    assert any_multihost_url.host_type is None
+    assert any_multihost_url.tld is None
+    assert any_multihost_url.port is None
+    assert any_multihost_url.path == '/app'
+    assert any_multihost_url.hosts == [
+        {'host': 'host1.db.net', 'port': '4321', 'tld': 'net', 'host_type': 'domain', 'rebuild': False},
+        {'host': 'host2.db.net', 'port': '6432', 'tld': 'net', 'host_type': 'domain', 'rebuild': False},
+    ]
+
+    any_multihost_url = Model(a='postgres://user:pass@host.db.net:4321/app').a
+    assert any_multihost_url.scheme == 'postgres'
+    assert any_multihost_url == 'postgres://user:pass@host.db.net:4321/app'
+    assert any_multihost_url.host == 'host.db.net'
+    assert any_multihost_url.host_type == 'domain'
+    assert any_multihost_url.tld == 'net'
+    assert any_multihost_url.port == '4321'
+    assert any_multihost_url.path == '/app'
+    assert any_multihost_url.hosts is None
 
 
 def test_amqp_dsns():