Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unique_constraints, a more versatile replacement for django_get_or_create and sqlalchemy_get_or_create #794

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
34 changes: 32 additions & 2 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
@@ -1,10 +1,40 @@
ChangeLog
=========

3.1.1 (unreleased)
3.2.0 (unreleased)
------------------

- Nothing changed yet.
*New:*

- Add a generic "get-or-create" mechanism through :attr:`~FactoryOptions.unique_constraints`; this replaces
:attr:`~django.DjangoOptions.django_get_or_create` and
:attr:`~alchemy.SQLAlchemyOptions.sqlalchemy_get_or_create`.

Replace ``django_get_or_create = ['foo', 'bar']`` with
``unique_constraints = [ ['foo'], ['bar'] ]``.

- Only resolve non-constrained attributes once lookups have failed. This addresses
:issue:`69`.

- Add support for composite unique constraints; addressing :issue:`241`.

*Bugfix:*

- :issue:`781`: Don't leak memory by keeping references to call-time parameters
beyond the instance creation.

*Deprecation:*

- :attr:`~django.DjangoOptions.django_get_or_create` is deprecated;
:attr:`~FactoryOptions.unique_constraints` should be used instead.
- :attr:`~alchemy.SQLAlchemyOptions.sqlalchemy_get_or_create` is deprecated;
:attr:`~FactoryOptions.unique_constraints` should be used instead.

The changes brought to :attr:`~django.DjangoOptions.django_get_or_create`
and :attr:`~alchemy.SQLAlchemyOptions.sqlalchemy_get_or_create` might alter the
number and details of SQL queries performed when these attributes were defined with
more than one field: there is now one query per group until one matching instance is
found.


3.1.0 (2020-10-02)
Expand Down
22 changes: 21 additions & 1 deletion docs/internals.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,27 @@ Instantiating, Step 2: Preparing values
encountering a :class:`SubFactory`.


Instantiating, Step 3: Building the object
Instantiating, step 3: Lookup existing data (optional)
------------------------------------------------------

Once the ``StepBuilder`` and its ``Resolver`` are ready, the builder looks at the
:attr:`~Factory.Meta.unique_constraints`:

1. It provides the call-time parameters to the :meth:`FactoryOptions.get_lookup_groups`
method;
2. That method computes groups of lookups to try:

- Any call-time parameter that appear in any unique constraint will *always* be included;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Any call-time parameter that appear in any unique constraint will *always* be included;
- Any call-time parameter that appears in a unique constraint will *always* be included;

- The first lookups are performed on the unique constraintss sharing the most parameters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- The first lookups are performed on the unique constraintss sharing the most parameters
- The first lookups are performed on the unique constraints sharing the most parameters

with the call-time parameters

3. For each group, the ``StepBuilder`` computes the value of the required parameters,
and performs a database lookup;
4. If any lookup returns an instance, it is used; otherwise, the remaining parameters
will be resolved by the ``Resolver``.


Instantiating, Step 4: Building the object
------------------------------------------

1. The ``StepBuilder`` fetches the attributes computed by the ``Resolver``.
Expand Down
79 changes: 79 additions & 0 deletions docs/orms.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ All factories for a Django :class:`~django.db.models.Model` should use the
* The :attr:`~factory.FactoryOptions.model` attribute also supports the ``'app.Model'``
syntax
* :func:`~factory.Factory.create()` uses :meth:`Model.objects.create() <django.db.models.query.QuerySet.create>`
* :func:`~factory.FactoryOptions.unique_constraints` uses a provided :meth:`Factory._lookup`
implementation, performin a :meth:`Model.objects.get() <django.db.models.query.QuerySet.get>` call
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
implementation, performin a :meth:`Model.objects.get() <django.db.models.query.QuerySet.get>` call
implementation, performing a :meth:`Model.objects.get() <django.db.models.query.QuerySet.get>` call

* When using :class:`~factory.RelatedFactory` or :class:`~factory.PostGeneration`
attributes, the base object will be :meth:`saved <django.db.models.Model.save>`
once all post-generation hooks have run.
Expand All @@ -56,6 +58,43 @@ All factories for a Django :class:`~django.db.models.Model` should use the
.. attribute:: django_get_or_create

.. versionadded:: 2.4.0
.. deprecated:: 3.2.0

Use :attr:`FactoryOptions.unique_constraints` instead.

.. important::

Replace ``django_get_or_create = ['foo', 'bar']`` with ``unique_constraints = [ ['foo'], ['bar'] ]``.

Whereas :attr:`~django_get_or_create` could only provide a list
of independantly unique columns, :attr:`~FactoryOptions.unique_constraints`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
of independantly unique columns, :attr:`~FactoryOptions.unique_constraints`
of independently unique columns, :attr:`~FactoryOptions.unique_constraints`

can support :attr:`~django.db.models.Options.unique_together`:
Comment on lines +69 to +71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth emphasizing that all unique_constraints are combined to generate the lookup, even though they are separate lists.


.. code-block:: python

class City(models.Model):
region = models.ForeignKey(Region)
name = models.TextField()
zipcode = models.TextField(unique=True)

class Meta:
unique_together = [
('region', 'name'),
]

class CityFactory(factory.django.DjangoModelFactory):
class Meta:
model = models.City
unique_constraints = [
['zipcode'],
['region', 'name'],
]

...

.. note:: The :meth:`django.DjangoModelFactory._lookup` method will only
perform database lookups when using the
:data:`~factory.CREATE_STRATEGY` strategy.

Fields whose name are passed in this list will be used to perform a
:meth:`Model.objects.get_or_create() <django.db.models.query.QuerySet.get_or_create>`
Expand Down Expand Up @@ -347,6 +386,46 @@ To work, this class needs an `SQLAlchemy`_ session object affected to the :attr:
.. attribute:: sqlalchemy_get_or_create

.. versionadded:: 3.0.0
.. deprecated:: 3.2.0

Use :attr:`FactoryOptions.unique_constraints` instead.

.. important::

Replace ``sqlalchemy_get_or_create = ['foo', 'bar']`` with
``unique_constraints = [ ['foo'], ['bar'] ]``.

Whereas :attr:`~sqlalchemy_get_or_create` could only provide a list
of independantly unique columns, :attr:`~FactoryOptions.unique_constraints`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
of independantly unique columns, :attr:`~FactoryOptions.unique_constraints`
of independently unique columns, :attr:`~FactoryOptions.unique_constraints`

can support :class:`sqlalchemy.schema.UniqueConstraint`:

.. code-block:: python

class City(Base):
__tablename__ = 'cities'

region_code = Column(Unicode(10))
name = Column(Unicode(200))
zipcode = Column(Unicode(10), unique=True)

__table__args = (
UniqueConstraint('region_code', 'name'),
)


class CityFactory(factory.django.DjangoModelFactory):
class Meta:
model = models.City
unique_constraints = [
['zipcode'],
['region_code', 'name'],
]

...

.. note:: The :meth:`alchemy.SQLAlchemyModelFactory._lookup` method will only
perform database lookups when using the
:data:`~factory.CREATE_STRATEGY` strategy.

Fields whose name are passed in this list will be used to perform a
:meth:`Model.query.one_or_none() <sqlalchemy.orm.query.Query.one_or_none>`
Expand Down
124 changes: 120 additions & 4 deletions docs/reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,111 @@ Meta options

.. versionadded: 2.6.0

.. attribute:: unique_constraints

Some models have unicity constraints on parts of their fields.
Instead of failing to save an instance, factories may define
these unicity constraints here:

.. code-block:: python

class EmployeeFactory(factory.Factory):
class Meta:
model = Employee
unique_constraints = [
['email'],
['access_card_id'],
['company', 'employee_id'],
]

Each group is a list of fields which should be *collectively unique*;
in this example, the ``employee_id`` is unique within each company.

When a new instance is required, the factory will start by resolving
the parameter used for each unique constraint, and attempt a lookup for this
combination of fields:
Comment on lines +159 to +160
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if an example should illustrate the combination of fields? When reading the unique constraints, it may not be obvious they will be combined with an AND.


.. code-block:: pycon

>>> e1 = EmployeeFactory()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying the access_card_id in the first factory explains why e1 is returned when e2 is built.

Suggested change
>>> e1 = EmployeeFactory()
>>> e1 = EmployeeFactory(access_card_id=42)

>>> e2 = EmployeeFactory(access_card_id=42)
lookup(email='[email protected]') -> None; continue
lookup(access_card_id=42) -> e1; return
>>> e1 == e2
True

If the lookup succeeds, the instance is used as is; post-generation hooks
will still be called.

.. note:: The :attr:`~FactoryOptions.unique_constraints` feature requires
a :meth:`Factory._lookup` definition on the factory class
or one of its parents.

A native implementation is provided for
:class:`django.DjangoModelFactory` and
:class:`alchemy.SQLAlchemyModelFactory`.

.. tip:: Group parameters are resolved lazily: in the above example,
the `company` declaration will only be evaluated if the `email`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EmployeeFactory does not have declarations. It would be useful to document a least a SubFactory to Company, to illustrate that the SubFactory, which usually creates a database row, will not be called.

and `access_card_id` lookup failed.

This avoids polluting the database with unneeded objects.

.. note::

**Lookup priority**

Unique constraints have a specific interaction with call-time parameters:
if a parameter is explicitly set when calling the factory, and appears
in any unique constraint, it will be included in each lookup: it is likely
a unique identifier of some sort.

Moreover, unique constraints containing more call-time parameters will be
tried first: they are more likely to succeed.

With the above definition, the following will happen:

.. code-block:: pycon

>>> EmployeeFactory()
# 1. lookup(email=...)
# 2. lookup(access_card_id=...)
# 3. lookup(company=..., employee_id=...)

>>> EmployeeFactory(access_card_id='42')
# 1. lookup(access_card_id=42)
# 2. lookup(access_card_id=42, email=...)
# 3. lookup(access_card_id=42, company=..., employee_id=...)

>>> EmployeeFactory(access_card_id='42', name="John Doe")
# The `name=` is not included in lookups, since it doesn't appear
# in unique constraints.
# 1. lookup(access_card_id=42)
# 2. lookup(access_card_id=42, email=...)
# 3. lookup(access_card_id=42, company=..., employee_id=...)

This feature *replaces* :attr:`django.DjangoOptions.django_get_or_create`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This feature *replaces* :attr:`django.DjangoOptions.django_get_or_create`
This feature *replaces* the :attr:`django.DjangoOptions.django_get_or_create`

and :attr:`alchemy.SQLAlchemyOptions.sqlalchemy_get_or_create` options.

.. warning:: :attr:`~django.DjangoOptions.django_get_or_create` and
:attr:`~alchemy.SQLAlchemyOptions.sqlalchemy_get_or_create`
where list of fields, understood as "a list of separately
unique columns"; whereas :attr:`~FactoryOptions.unique_constraints`
is a list of *lists of collectively unique columns*.
Comment on lines +224 to +228
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. warning:: :attr:`~django.DjangoOptions.django_get_or_create` and
:attr:`~alchemy.SQLAlchemyOptions.sqlalchemy_get_or_create`
where list of fields, understood as "a list of separately
unique columns"; whereas :attr:`~FactoryOptions.unique_constraints`
is a list of *lists of collectively unique columns*.
.. warning:: :attr:`~django.DjangoOptions.django_get_or_create` and
:attr:`~alchemy.SQLAlchemyOptions.sqlalchemy_get_or_create`
were the list of fields, understood as "a list of separately
unique columns"; whereas :attr:`~FactoryOptions.unique_constraints`
is a list of *lists of collectively unique columns*.


Migrate as follow:

.. code-block:: python

class EmployeeFactory(factory.django.DjangoModelFactory):
class Meta:
django_get_or_create = ['email', 'employee_id']
unique_constraints = [
['email'],
['employee_id'],
]
Comment on lines +236 to +240
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is that some users would otherwise keep both.

Suggested change
django_get_or_create = ['email', 'employee_id']
unique_constraints = [
['email'],
['employee_id'],
]
# Replace this django_get_or_create declaration
django_get_or_create = ['email', 'employee_id']
# With this unique_constraints declaration
unique_constraints = [
['email'],
['employee_id'],
]


.. versionadded:: 3.2.0

.. attribute:: strategy

Expand All @@ -150,7 +255,7 @@ Attributes and methods
.. class:: Factory


**Class-level attributes:**
.. rubric:: Class-level attributes:

.. attribute:: Meta
.. attribute:: _meta
Expand Down Expand Up @@ -179,7 +284,7 @@ Attributes and methods
the :class:`Factory`-building metaclass can use it instead.


**Base functions:**
.. rubric:: Base functions

The :class:`Factory` class provides a few methods for getting objects;
the usual way being to simply call the class:
Expand Down Expand Up @@ -245,7 +350,7 @@ Attributes and methods
according to :obj:`create`.


**Extension points:**
.. rubric:: Extension points

A :class:`Factory` subclass may override a couple of class methods to adapt
its behaviour:
Expand Down Expand Up @@ -319,6 +424,17 @@ Attributes and methods

.. OHAI_VIM*

.. classmethod:: _lookup(cls, model_class, strategy, fields)

Lookup an instance from the database, using the passed-in fields.

This is required for the :attr:`~FactoryOptions.unique_constraints` feature.

The ``strategy`` field can be used to disable lookups with specific
strategies - typically for :data:`BUILD_STRATEGY` and :data:`STUB_STRATEGY`.

.. versionadded:: 3.2.0

.. classmethod:: _after_postgeneration(cls, obj, create, results=None)

:arg object obj: The object just generated
Expand All @@ -333,7 +449,7 @@ Attributes and methods
values, for instance.


**Advanced functions:**
.. rubric:: Advanced functions:


.. classmethod:: reset_sequence(cls, value=None, force=False)
Expand Down
Loading