Fix __fields_set__ not using alias field names (#517) (#518)

* Fix __fields_set__ not using alias field names (#517)

fix #517

* Update HISTORY.rst

* Fix single quotes

* Don't need to check allow_population_by_alias

* Calculate fields_set in validate_model instead

* Fix formatting

* Add breaking change to history

* Minor fixed to validate_mode

* Minor fixes to tests

* Add comments for names_used and fields_set in validate_model

HISTORY.rst

@@ -8,6 +8,8 @@ v0.26 (unreleased)
 * fix to schema generation for ``IPvAnyAddress``, ``IPvAnyInterface``, ``IPvAnyNetwork`` #498 by @pilosus
 * fix variable length tuples support, #495 by @pilosus
 * fix return type hint for ``create_model``, #526 by @dmontagu
+* **Breaking Change:** fix ``.dict(skip_keys=True)`` skipping values set via alias (this involves changing
+  ``validate_model()`` to always returns ``Tuple[Dict[str, Any], Set[str], Optional[ValidationError]]``), #517 by @sommd
 
 v0.25 (2019-05-05)
 ..................

pydantic/dataclasses.py

@@ -25,7 +25,7 @@ if TYPE_CHECKING:  # pragma: no cover
 
 
 def _pydantic_post_init(self: 'DataclassType') -> None:
-    d = validate_model(self.__pydantic_model__, self.__dict__, cls=self.__class__)
+    d = validate_model(self.__pydantic_model__, self.__dict__, cls=self.__class__)[0]
     object.__setattr__(self, '__dict__', d)
     object.__setattr__(self, '__initialised__', True)
     if self.__post_init_original__:

pydantic/main.py

@@ -232,11 +232,8 @@ class BaseModel(metaclass=MetaModel):
         if TYPE_CHECKING:  # pragma: no cover
             self.__values__: Dict[str, Any] = {}
             self.__fields_set__: 'SetStr' = set()
-        object.__setattr__(self, '__values__', self._process_values(data))
-        if self.__config__.extra is Extra.allow:
-            fields_set = set(data.keys())
-        else:
-            fields_set = data.keys() & self.__values__.keys()  # type: ignore
+        values, fields_set, _ = validate_model(self, data)
+        object.__setattr__(self, '__values__', values)
         object.__setattr__(self, '__fields_set__', fields_set)
 
     @no_type_check
@@ -433,10 +430,6 @@ class BaseModel(metaclass=MetaModel):
             with change_exception(DictError, TypeError, ValueError):
                 return cls(**dict(value))  # type: ignore
 
-    def _process_values(self, input_data: Any) -> 'DictStrAny':
-        # (casting here is slow so use ignore)
-        return validate_model(self, input_data)  # type: ignore
-
     @classmethod
     def _get_value(cls, v: Any, by_alias: bool, skip_defaults: bool) -> Any:
         if isinstance(v, BaseModel):
@@ -576,13 +569,16 @@ def create_model(  # noqa: C901 (ignore complexity)
 
 def validate_model(  # noqa: C901 (ignore complexity)
     model: Union[BaseModel, Type[BaseModel]], input_data: 'DictStrAny', raise_exc: bool = True, cls: 'ModelOrDc' = None
-) -> Union['DictStrAny', Tuple['DictStrAny', Optional[ValidationError]]]:
+) -> Tuple['DictStrAny', 'SetStr', Optional[ValidationError]]:
     """
     validate data against a model.
     """
     values = {}
     errors = []
+    # input_data names, possibly alias
     names_used = set()
+    # field names, never aliases
+    fields_set = set()
     config = model.__config__
     check_extra = config.extra is not Extra.ignore
 
@@ -607,8 +603,10 @@ def validate_model(  # noqa: C901 (ignore complexity)
             if not model.__config__.validate_all and not field.validate_always:
                 values[name] = value
                 continue
-        elif check_extra:
-            names_used.add(field.name if using_name else field.alias)
+        else:
+            fields_set.add(name)
+            if check_extra:
+                names_used.add(field.name if using_name else field.alias)
 
         v_, errors_ = field.validate(value, values, loc=field.alias, cls=cls or model.__class__)  # type: ignore
         if isinstance(errors_, ErrorWrapper):
@@ -621,6 +619,7 @@ def validate_model(  # noqa: C901 (ignore complexity)
     if check_extra:
         extra = input_data.keys() - names_used
         if extra:
+            fields_set |= extra
             if config.extra is Extra.allow:
                 for f in extra:
                     values[f] = input_data[f]
@@ -629,8 +628,8 @@ def validate_model(  # noqa: C901 (ignore complexity)
                     errors.append(ErrorWrapper(ExtraError(), loc=f, config=config))
 
     if not raise_exc:
-        return values, ValidationError(errors) if errors else None
+        return values, fields_set, ValidationError(errors) if errors else None
 
     if errors:
         raise ValidationError(errors)
-    return values
+    return values, fields_set, None

tests/test_edge_cases.py

@@ -601,9 +601,14 @@ def test_return_errors_ok():
         foo: int
         bar: List[int]
 
-    assert validate_model(Model, {'foo': '123', 'bar': (1, 2, 3)}) == {'foo': 123, 'bar': [1, 2, 3]}
-    d, e = validate_model(Model, {'foo': '123', 'bar': (1, 2, 3)}, False)
+    assert validate_model(Model, {'foo': '123', 'bar': (1, 2, 3)}) == (
+        {'foo': 123, 'bar': [1, 2, 3]},
+        {'foo', 'bar'},
+        None,
+    )
+    d, f, e = validate_model(Model, {'foo': '123', 'bar': (1, 2, 3)}, False)
     assert d == {'foo': 123, 'bar': [1, 2, 3]}
+    assert f == {'foo', 'bar'}
     assert e is None
 
 
@@ -612,12 +617,14 @@ def test_return_errors_error():
         foo: int
         bar: List[int]
 
-    d, e = validate_model(Model, {'foo': '123', 'bar': (1, 2, 'x')}, False)
+    d, f, e = validate_model(Model, {'foo': '123', 'bar': (1, 2, 'x')}, False)
     assert d == {'foo': 123}
+    assert f == {'foo', 'bar'}
     assert e.errors() == [{'loc': ('bar', 2), 'msg': 'value is not a valid integer', 'type': 'type_error.integer'}]
 
-    d, e = validate_model(Model, {'bar': (1, 2, 3)}, False)
+    d, f, e = validate_model(Model, {'bar': (1, 2, 3)}, False)
     assert d == {'bar': [1, 2, 3]}
+    assert f == {'bar'}
     assert e.errors() == [{'loc': ('foo',), 'msg': 'field required', 'type': 'value_error.missing'}]
 
 

tests/test_main.py

@@ -598,6 +598,34 @@ def test_skip_defaults_recursive():
     assert dict(m) == {'c': 5, 'd': 2, 'e': {'a': 0, 'b': 1}}
 
 
+def test_dict_skip_defaults_populated_by_alias():
+    class MyModel(BaseModel):
+        a: str = Schema('default', alias='alias_a')
+        b: str = Schema('default', alias='alias_b')
+
+        class Config:
+            allow_population_by_alias = True
+
+    m = MyModel(alias_a='a')
+
+    assert m.dict(skip_defaults=True) == {'a': 'a'}
+    assert m.dict(skip_defaults=True, by_alias=True) == {'alias_a': 'a'}
+
+
+def test_dict_skip_defaults_populated_by_alias_with_extra():
+    class MyModel(BaseModel):
+        a: str = Schema('default', alias='alias_a')
+        b: str = Schema('default', alias='alias_b')
+
+        class Config:
+            extra = 'allow'
+
+    m = MyModel(alias_a='a', c='c')
+
+    assert m.dict(skip_defaults=True) == {'a': 'a', 'c': 'c'}
+    assert m.dict(skip_defaults=True, by_alias=True) == {'alias_a': 'a', 'c': 'c'}
+
+
 def test_dir_fields():
     class MyModel(BaseModel):
         attribute_a: int