Repository: FactoryBoy/factory_boy (python)

Issue:
The unreleased `Transformer` has a conflicting API with the overall design
#### Description
For most declarations, I can override the default mechanism by passing an explicit value:

```python
>>> UserFactory().email
"john.doe@example.org"
>>> UserFactory(email="jane.doe@example.org").email
"jane.doe@example.org"
```

If the factory uses a `factory.Transformer` field, I can't force a value:

```python
>>> UserFactory().password
"pbkdf2_sha256$216000$Gj2b9f0Ln1ms$1dBlLEYrtGiwEA219JU831VAdscD2f9nFY37PrNfCDU="
>>> UserFactory(
...     password="pbkdf2_sha256$216000$Gj2b9f0Ln1ms$1dBlLEYrtGiwEA219JU831VAdscD2f9nFY37PrNfCDU="
... ).password
"pbkdf2_sha256$216000$EUKlFdNViyB5$vcfPaK6H7pDiQAa9ZIbFoj5oj55tUyHKHDUfxI4GIeY="
```

As a user, it is quite confusing to have very different behaviours for fields; if I have a specific password hash (in this example), I need to have a way to set it.

#### Next steps

I'm not sure we can officially release the `factory.Transformer` (and related code) without addressing this topic first; how can a user bypass the transform?

Pull request:
Handle declarations in Transformers
Transformers init changed from

```python
def __init__(self, transform, declaration):
```
to
```python
def __init__(self, declaration, *, transform):
```

That allows Password to specify `make_transform` as the default
transform, leaving an option to override the transform and facilitates
subclassing Transformer.
Having a transform keyword argument also clarifies the instantiation of
transformers.

Thanks to @rbarrois for providing additional test cases!

Fixes #886

Patch:
diff --git a/docs/reference.rst b/docs/reference.rst
index a7e9be0..9efd1a2 100644
--- a/docs/reference.rst
+++ b/docs/reference.rst
@@ -907,21 +907,22 @@ return value of the method:
 Transformer
 """""""""""
 
-.. class:: Transformer(transform, value)
+.. class:: Transformer(default_value, *, transform)
 
 A :class:`Transformer` applies a ``transform`` function to the provided value
 before to set the transformed value on the generated object.
 
-It expects two arguments:
+It expects one positional argument and one keyword argument:
 
-- ``transform``: function taking the value as parameter and returning the
+- ``default_value``: the default value, which passes through the ``transform``
+  function.
+- ``transform``: a function taking the value as parameter and returning the
   transformed value,
-- ``value``: the default value.
 
 .. code-block:: python
 
-   class UpperFactory(Factory):
-       name = Transformer(lambda x: x.upper(), "Joe")
+   class UpperFactory(factory.Factory):
+       name = factory.Transformer("Joe", transform=str.upper)
 
        class Meta:
            model = Upper
@@ -933,6 +934,14 @@ It expects two arguments:
    >>> UpperFactory(name="John").name
    'JOHN'
 
+Disabling
+~~~~~~~~~
+To disable a :class:`Transformer`, wrap the value in ``Transformer.Force``:
+
+.. code-block:: pycon
+
+   >>> UpperFactory(name=factory.Transformer.Force("John")).name
+   'John'
 
 Sequence
 """"""""
diff --git a/factory/builder.py b/factory/builder.py
index 9d810fb..e76e755 100644
--- a/factory/builder.py
+++ b/factory/builder.py
@@ -2,7 +2,7 @@
 
 import collections
 
-from . import declarations, enums, errors, utils
+from . import enums, errors, utils
 
 DeclarationWithContext = collections.namedtuple(
     'DeclarationWithContext',
@@ -134,6 +134,14 @@ class DeclarationSet:
         return '<DeclarationSet: %r>' % self.as_dict()
 
 
+def _captures_overrides(declaration_with_context):
+    declaration = declaration_with_context.declaration
+    if enums.get_builder_phase(declaration) == enums.BuilderPhase.ATTRIBUTE_RESOLUTION:
+        return declaration.CAPTURE_OVERRIDES
+    else:
+        return False
+
+
 def parse_declarations(decls, base_pre=None, base_post=None):
     pre_declarations = base_pre.copy() if base_pre else DeclarationSet()
     post_declarations = base_post.copy() if base_post else DeclarationSet()
@@ -156,10 +164,6 @@ def parse_declarations(decls, base_pre=None, base_post=None):
             # Set it as `key__`
             magic_key = post_declarations.join(k, '')
             extra_post[magic_key] = v
-        elif k in pre_declarations and isinstance(
-            pre_declarations[k].declaration, declarations.Transformer
-        ):
-            extra_maybenonpost[k] = pre_declarations[k].declaration.function(v)
         else:
             extra_maybenonpost[k] = v
 
@@ -173,6 +177,12 @@ def parse_declarations(decls, base_pre=None, base_post=None):
     for k, v in extra_maybenonpost.items():
         if k in post_overrides:
             extra_post_declarations[k] = v
+        elif k in pre_declarations and _captures_overrides(pre_declarations[k]):
+            # Send the overriding value to the existing declaration.
+            # By symmetry with the behaviour of PostGenerationDeclaration,
+            # we send it as `key__` -- i.e under the '' key.
+            magic_key = pre_declarations.join(k, '')
+            extra_pre_declarations[magic_key] = v
         else:
             # Anything else is pre_declarations
             extra_pre_declarations[k] = v
diff --git a/factory/declarations.py b/factory/declarations.py
index 40ae99c..70abe35 100644
--- a/factory/declarations.py
+++ b/factory/declarations.py
@@ -20,6 +20,11 @@ class BaseDeclaration(utils.OrderedBase):
 
     FACTORY_BUILDER_PHASE = enums.BuilderPhase.ATTRIBUTE_RESOLUTION
 
+    #: Whether this declaration has a special handling for call-time overrides
+    #: (e.g. Tranformer).
+    #: Overridden values will be passed in the `extra` args.
+    CAPTURE_OVERRIDES = False
+
     #: Whether to unroll the context before evaluating the declaration.
     #: Set to False on declarations that perform their own unrolling.
     UNROLL_CONTEXT_BEFORE_EVALUATION = True
@@ -43,6 +48,20 @@ class BaseDeclaration(utils.OrderedBase):
         subfactory = factory.base.DictFactory
         return step.recurse(subfactory, full_context, force_sequence=step.sequence)
 
+    def _unwrap_evaluate_pre(self, wrapped, *, instance, step, overrides):
+        """Evaluate a wrapped pre-declaration.
+
+        This is especially useful for declarations wrapping another one,
+        e.g. Maybe or Transformer.
+        """
+        if isinstance(wrapped, BaseDeclaration):
+            return wrapped.evaluate_pre(
+                instance=instance,
+                step=step,
+                overrides=overrides,
+            )
+        return wrapped
+
     def evaluate_pre(self, instance, step, overrides):
         context = self.unroll_context(instance, step, overrides)
         return self.evaluate(instance, step, context)
@@ -100,20 +119,47 @@ class LazyAttribute(BaseDeclaration):
         return self.function(instance)
 
 
-class Transformer(LazyFunction):
-    """Transform value using given function.
+class Transformer(BaseDeclaration):
+    CAPTURE_OVERRIDES = True
+    UNROLL_CONTEXT_BEFORE_EVALUATION = False
 
-    Attributes:
-        transform (function): returns the transformed value.
-        value: passed as the first argument to the transform function.
-    """
+    class Force:
+        """
+        Bypass a transformer's transformation.
 
-    def __init__(self, transform, value, *args, **kwargs):
-        super().__init__(transform, *args, **kwargs)
-        self.value = value
+        The forced value can be any declaration, and will be evaluated as if it
+        had been passed instead of the Transformer declaration.
+        """
+        def __init__(self, forced_value):
+            self.forced_value = forced_value
 
-    def evaluate(self, instance, step, extra):
-        return self.function(self.value)
+        def __repr__(self):
+            return f'Transformer.Force({repr(self.forced_value)})'
+
+    def __init__(self, default, *, transform):
+        super().__init__()
+        self.default = default
+        self.transform = transform
+
+    def evaluate_pre(self, instance, step, overrides):
+        # The call-time value, if present, is set under the "" key.
+        value_or_declaration = overrides.pop("", self.default)
+
+        if isinstance(value_or_declaration, self.Force):
+            bypass_transform = True
+            value_or_declaration = value_or_declaration.forced_value
+        else:
+            bypass_transform = False
+
+        value = self._unwrap_evaluate_pre(
+            value_or_declaration,
+            instance=instance,
+            step=step,
+            overrides=overrides,
+        )
+        if bypass_transform:
+            return value
+        return self.transform(value)
 
 
 class _UNSPECIFIED:
@@ -492,16 +538,14 @@ class Maybe(BaseDeclaration):
     def evaluate_pre(self, instance, step, overrides):
         choice = self.decider.evaluate(instance=instance, step=step, extra={})
         target = self.yes if choice else self.no
-
-        if isinstance(target, BaseDeclaration):
-            return target.evaluate_pre(
-                instance=instance,
-                step=step,
-                overrides=overrides,
-            )
-        else:
-            # Flat value (can't be POST_INSTANTIATION, checked in __init__)
-            return target
+        # The value can't be POST_INSTANTIATION, checked in __init__;
+        # evaluate it as `evaluate_pre`
+        return self._unwrap_evaluate_pre(
+            target,
+            instance=instance,
+            step=step,
+            overrides=overrides,
+        )
 
     def __repr__(self):
         return f'Maybe({self.decider!r}, yes={self.yes!r}, no={self.no!r})'
diff --git a/factory/django.py b/factory/django.py
index dee35d6..87b6fd5 100644
--- a/factory/django.py
+++ b/factory/django.py
@@ -192,8 +192,8 @@ class DjangoModelFactory(base.Factory):
 
 
 class Password(declarations.Transformer):
-    def __init__(self, password, *args, **kwargs):
-        super().__init__(make_password, password, *args, **kwargs)
+    def __init__(self, password, transform=make_password, **kwargs):
+        super().__init__(password, transform=transform, **kwargs)
 
 
 class FileField(declarations.BaseDeclaration):


Test patch:
diff --git a/tests/test_declarations.py b/tests/test_declarations.py
index c9458ff..c49bbba 100644
--- a/tests/test_declarations.py
+++ b/tests/test_declarations.py
@@ -134,7 +134,7 @@ class IteratorTestCase(unittest.TestCase):
 
 class TransformerTestCase(unittest.TestCase):
     def test_transform(self):
-        t = declarations.Transformer(lambda x: x.upper(), 'foo')
+        t = declarations.Transformer('foo', transform=str.upper)
         self.assertEqual("FOO", utils.evaluate_declaration(t))
 
 
diff --git a/tests/test_transformer.py b/tests/test_transformer.py
index 0065845..b21a8da 100644
--- a/tests/test_transformer.py
+++ b/tests/test_transformer.py
@@ -2,7 +2,7 @@
 
 from unittest import TestCase
 
-from factory import Factory, Transformer
+import factory
 
 
 class TransformCounter:
@@ -22,12 +22,13 @@ transform = TransformCounter()
 
 
 class Upper:
-    def __init__(self, name):
+    def __init__(self, name, **extra):
         self.name = name
+        self.extra = extra
 
 
-class UpperFactory(Factory):
-    name = Transformer(transform, "value")
+class UpperFactory(factory.Factory):
+    name = factory.Transformer("value", transform=transform)
 
     class Meta:
         model = Upper
@@ -44,3 +45,186 @@ class TransformerTest(TestCase):
     def test_transform_kwarg(self):
         self.assertEqual("TEST", UpperFactory(name="test").name)
         self.assertEqual(transform.calls_count, 1)
+        self.assertEqual("VALUE", UpperFactory().name)
+        self.assertEqual(transform.calls_count, 2)
+
+    def test_transform_faker(self):
+        value = UpperFactory(name=factory.Faker("first_name_female", locale="fr")).name
+        self.assertIs(value.isupper(), True)
+
+    def test_transform_linked(self):
+        value = UpperFactory(
+            name=factory.LazyAttribute(lambda o: o.username.replace(".", " ")),
+            username="john.doe",
+        ).name
+        self.assertEqual(value, "JOHN DOE")
+
+    def test_force_value(self):
+        value = UpperFactory(name=factory.Transformer.Force("Mia")).name
+        self.assertEqual(value, "Mia")
+
+    def test_force_value_declaration(self):
+        """Pretty unlikely use case, but easy enough to cover."""
+        value = UpperFactory(
+            name=factory.Transformer.Force(
+                factory.LazyFunction(lambda: "infinity")
+            )
+        ).name
+        self.assertEqual(value, "infinity")
+
+    def test_force_value_declaration_context(self):
+        """Ensure "forced" values run at the right level."""
+        value = UpperFactory(
+            name=factory.Transformer.Force(
+                factory.LazyAttribute(lambda o: o.username.replace(".", " ")),
+            ),
+            username="john.doe",
+        ).name
+        self.assertEqual(value, "john doe")
+
+
+class TestObject:
+    def __init__(self, one=None, two=None, three=None):
+        self.one = one
+        self.two = two
+        self.three = three
+
+
+class TransformDeclarationFactory(factory.Factory):
+    class Meta:
+        model = TestObject
+    one = factory.Transformer("", transform=str.upper)
+    two = factory.Transformer(factory.Sequence(int), transform=lambda n: n ** 2)
+
+
+class TransformerSequenceTest(TestCase):
+    def test_on_sequence(self):
+        instance = TransformDeclarationFactory(__sequence=2)
+        self.assertEqual(instance.one, "")
+        self.assertEqual(instance.two, 4)
+        self.assertIsNone(instance.three)
+
+    def test_on_user_supplied(self):
+        """A transformer can wrap a call-time declaration"""
+        instance = TransformDeclarationFactory(
+            one=factory.Sequence(str),
+            two=2,
+            __sequence=2,
+        )
+        self.assertEqual(instance.one, "2")
+        self.assertEqual(instance.two, 4)
+        self.assertIsNone(instance.three)
+
+
+class WithMaybeFactory(factory.Factory):
+    class Meta:
+        model = TestObject
+
+    one = True
+    two = factory.Maybe(
+        'one',
+        yes_declaration=factory.Transformer("yes", transform=str.upper),
+        no_declaration=factory.Transformer("no", transform=str.upper),
+    )
+    three = factory.Maybe('one', no_declaration=factory.Transformer("three", transform=str.upper))
+
+
+class TransformerMaybeTest(TestCase):
+    def test_default_transform(self):
+        instance = WithMaybeFactory()
+        self.assertIs(instance.one, True)
+        self.assertEqual(instance.two, "YES")
+        self.assertIsNone(instance.three)
+
+    def test_yes_transform(self):
+        instance = WithMaybeFactory(one=True)
+        self.assertIs(instance.one, True)
+        self.assertEqual(instance.two, "YES")
+        self.assertIsNone(instance.three)
+
+    def test_no_transform(self):
+        instance = WithMaybeFactory(one=False)
+        self.assertIs(instance.one, False)
+        self.assertEqual(instance.two, "NO")
+        self.assertEqual(instance.three, "THREE")
+
+    def test_override(self):
+        instance = WithMaybeFactory(one=True, two="NI")
+        self.assertIs(instance.one, True)
+        self.assertEqual(instance.two, "NI")
+        self.assertIsNone(instance.three)
+
+
+class RelatedTest(TestCase):
+    def test_default_transform(self):
+        cities = []
+
+        class City:
+            def __init__(self, capital_of, name):
+                self.capital_of = capital_of
+                self.name = name
+                cities.append(self)
+
+        class Country:
+            def __init__(self, name):
+                self.name = name
+
+        class CityFactory(factory.Factory):
+            class Meta:
+                model = City
+
+            name = "Rennes"
+
+        class CountryFactory(factory.Factory):
+            class Meta:
+                model = Country
+
+            name = "France"
+            capital_city = factory.RelatedFactory(
+                CityFactory,
+                factory_related_name="capital_of",
+                name=factory.Transformer("Paris", transform=str.upper),
+            )
+
+        instance = CountryFactory()
+        self.assertEqual(instance.name, "France")
+        [city] = cities
+        self.assertEqual(city.capital_of, instance)
+        self.assertEqual(city.name, "PARIS")
+
+
+class WithTraitFactory(factory.Factory):
+    class Meta:
+        model = TestObject
+
+    class Params:
+        upper_two = factory.Trait(
+            two=factory.Transformer("two", transform=str.upper)
+        )
+        odds = factory.Trait(
+            one="one",
+            three="three",
+        )
+    one = factory.Transformer("one", transform=str.upper)
+
+
+class TransformerTraitTest(TestCase):
+    def test_traits_off(self):
+        instance = WithTraitFactory()
+        self.assertEqual(instance.one, "ONE")
+        self.assertIsNone(instance.two)
+        self.assertIsNone(instance.three)
+
+    def test_trait_transform_applies(self):
+        """A trait-provided transformer should apply to existing values"""
+        instance = WithTraitFactory(upper_two=True)
+        self.assertEqual(instance.one, "ONE")
+        self.assertEqual(instance.two, "TWO")
+        self.assertIsNone(instance.three)
+
+    def test_trait_transform_applies_supplied(self):
+        """A trait-provided transformer should be overridden by caller-provided values"""
+        instance = WithTraitFactory(upper_two=True, two="two")
+        self.assertEqual(instance.one, "ONE")
+        self.assertEqual(instance.two, "two")
+        self.assertIsNone(instance.three)