Missing tests for #14731

[Jannis Leidel]

I tried fixing the tests that were added in r14891 later in r16400 but honestly can't figure out a sane way. The bug apparently only showed up in Postgres and MySQL.

I now create this ticket in the hope that the original author of #14731 is able to create a test that actual works.

[Jannis Leidel]

In the meantime I've reverted the previously committed tests in r16482.

[Christian Hammond]

I'd be happy to help, but need a bit more context into the problem. What tests specifically broke, and in what way? Is this a breakage limited to the test suite when used with MySQL/PostgreSQL? Any bug number I can reference to learn more?

[Jannis Leidel]

Well, the tests I committed in r14891 were broken in a way that they were simply ignored (no showing any sign of failure of the wrongly named fixture file).

I tried to fix those in r16400 (#16183).

But then again the tests failed with r16481 (before removing the broken tests) on Postgres 9.0.4:

[~/Code/git/django/tests] env:django-dev $ python runtests.py --settings=django_testing.postgres auth
Creating test database for alias 'default'...
......................................................................................................Problem installing fixture '/Users/jezdez/Code/git/django/django/contrib/auth/fixtures/permissionstestdata.json': Traceback (most recent call last):
  File "/Users/jezdez/Code/git/django/django/core/management/commands/loaddata.py", line 174, in handle
    obj.save(using=using)
  File "/Users/jezdez/Code/git/django/django/core/serializers/base.py", line 169, in save
    models.Model.save_base(self.object, using=using, raw=True)
  File "/Users/jezdez/Code/git/django/django/db/models/base.py", line 556, in save_base
    result = manager._insert(values, return_id=update_pk, using=using)
  File "/Users/jezdez/Code/git/django/django/db/models/manager.py", line 198, in _insert
    return insert_query(self.model, values, **kwargs)
  File "/Users/jezdez/Code/git/django/django/db/models/query.py", line 1456, in insert_query
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/Users/jezdez/Code/git/django/django/db/models/sql/compiler.py", line 810, in execute_sql
    cursor = super(SQLInsertCompiler, self).execute_sql(None)
  File "/Users/jezdez/Code/git/django/django/db/models/sql/compiler.py", line 754, in execute_sql
    cursor.execute(sql, params)
  File "/Users/jezdez/Code/git/django/django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
IntegrityError: duplicate key value violates unique constraint "django_content_type_app_label_model_key"
DETAIL:  Key (app_label, model)=(contenttypes, contenttype) already exists.

With MySQL:

......................................................................................................Problem installing fixture '/Users/jezdez/Code/git/django/django/contrib/auth/fixtures/permissionstestdata.json': Traceback (most recent call last):
  File "/Users/jezdez/Code/git/django/django/core/management/commands/loaddata.py", line 174, in handle
    obj.save(using=using)
  File "/Users/jezdez/Code/git/django/django/core/serializers/base.py", line 169, in save
    models.Model.save_base(self.object, using=using, raw=True)
  File "/Users/jezdez/Code/git/django/django/db/models/base.py", line 556, in save_base
    result = manager._insert(values, return_id=update_pk, using=using)
  File "/Users/jezdez/Code/git/django/django/db/models/manager.py", line 198, in _insert
    return insert_query(self.model, values, **kwargs)
  File "/Users/jezdez/Code/git/django/django/db/models/query.py", line 1456, in insert_query
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/Users/jezdez/Code/git/django/django/db/models/sql/compiler.py", line 810, in execute_sql
    cursor = super(SQLInsertCompiler, self).execute_sql(None)
  File "/Users/jezdez/Code/git/django/django/db/models/sql/compiler.py", line 754, in execute_sql
    cursor.execute(sql, params)
  File "/Users/jezdez/Code/git/django/django/db/backends/mysql/base.py", line 86, in execute
    return self.cursor.execute(query, args)
  File "/Library/Python/2.6/site-packages/MySQLdb/cursors.py", line 174, in execute
    self.errorhandler(self, exc, value)
  File "/Library/Python/2.6/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
IntegrityError: (1062, "Duplicate entry 'contenttypes-contenttype' for key 'app_label'")

[Jannis Leidel]

It works on SQLite, btw.

[Christian Hammond]

Thanks for the info. I'll try to play with it over the next couple of days. Heading out of town tonight but should have some free time. Obviously you'd want this fixed ASAP, but what are the release time constraints?

[Jannis Leidel]

The sooner the better :)

[Christian Hammond]

Alright, I have this figured out. Sorry for the delay, and thanks for the patience.

There were two problems. The first causing the error above, and the second that I uncovered after fixing the above.

1) On MySQL and PostgreSQL, the auto-increment field won't reset after clearing objects. This makes sense. I don't know why the same behavior wasn't showing for SQLite, but either way, this was causing problems when loading the fixtures.

The fixture was assuming something would be at ID 2, let's say, but since we had previously created a bunch of entries and then deleted them, the next ID wouldn't be 2 but LAST_ID + 2.

This is easily solvable by doing a call_command('flush'). The IDs will then reset, and we can match properly.

2) The IDs in the fixture weren't matching the IDs from the order we created things, so there were mismatches. I've updated the IDs to match.

Sorry my original patch for the tests were so broken. I saw the update that fixed it to properly report the errors. Hopefully this latest patch won't cause as many problems :)

I'll have two patches up shortly, once I finish a full test suite run with each database.

One patch is against the most recent version prior to the backout. The other is a patch that applies to HEAD.

[Christian Hammond]

Hit some issues with making this work across databases.

In order to properly test, we have to clear out the existing auth/contenttypes tables in the database by dropping and bringing the tables back. There are a couple problems here. The first being that some test apps (the contenttypes.tests being the main one) register new models and never clean them up, causing us to have unexpected ContentTypes registered, which will break these tests.

I have a fix for this that does some AppCache manipulation. Sorta ugly, but there's not exactly an API for forgetting models.

Second part, which is where I'm currently stuck, is how best to handle resetting the auth and contenttypes tables without causing new problems. I have tried a "flush" command, which PostgreSQL disallows at this point in the test suite. I have also tried the equivalent of the old "reset" command, which does work for this test but breaks future tests due to entries that they expect to have already been created in the ContentTypes tables.

At the moment, I'm stuck. Too much of the test suite litters the database and caches, and it's very tricky to have this run in seclusion. Any advice would be very much appreciated.

[Aymeric Augustin]

Here's the history of the problem:

• r14413 optimized the creation of permissions, which is triggered by the post_syncdb signal. This had the side effect of changing the order in which permissions are created.
• #14731 reported this as a regression, because it made it impossible to load some fixtures containing permissions. Permissions are always created in post_syncdb, which means they always exist when one attempts to load a fixture. Then, when a fixture contains permissions, and by chance, it matches exactly the contents of the database, loading it is a no-op; if it doesn't match the contents of the database, a unique constraint is broken — the same ('content_type', 'codename') is loaded with a different pk — and an exception is raised.
• r14891 restored the previous order in which permissions are created, and introduced a test that didn't work.
• r16400 attempted to fix that test.
• r16482 and r16483 removed that test.

---

Upon further study, the primary keys of permissions aren't deterministic overall; they might appear to be because:

• manage.py syncdb emits the post_syncdb signal for each application in AppCache.get_apps(), and app_store is a SortedDict populated in the order of settings.INSTALLED_APPS;
• create_permissions doesn't rely on its created_models argument, which is a set; rather, the order in which it creates permissions is defined by:
  • get_models(app), which is deterministic because values in AppCache.app_models are instances of SortedDict — see django.db.models.loading.AppCache.register_models() — and models are registered by the django.db.models.base.ModelBase metaclass in the order in which they are defined;
  • ContentType.objects.get_for_models: this returns a dict, which isn't deterministic theoretically, but might be on a given version of CPython, when the same data is stored in the same order;
  • _get_all_permissions: this is deterministic.

So, code inspection indicates that the primary keys of permissions depend on the order in which a dictionary is iterated, and Python makes no guarantees there.

---

To sum up, putting permissions in a fixture is:

• extremely fragile: adding a model, changing the order of INSTALLED_APPS, or just using a different version of Python may break the fixture;
• at best, it works, and it is useless (loading them is a no-op);
• at worst, it crashes.

---

My conclusion is that permissions, like all auto-generated data, shouldn't be saved in fixtures, and that we shouldn't attempt to make their primary keys deterministic. However, we should mention in the documentation that it's a bad idea to save permissions in fixtures.

I believe that the most common use case for such fixtures is to save a bunch of groups and their permissions. Just dump the groups with natural keys and everything will work as expected! That should be mentioned in the documentation too.

[Aymeric Augustin]

By the way, the docs use the permissions and content types as an example for serialization: ​https://docs.djangoproject.com/en/dev/topics/serialization/#dependencies-during-serialization

For consistency, we should change that example or add a warning, if we decide that it's a bad idea to serialize permissions or content-types.

[Luke Plant]

I agree with your analysis Aymeric.

Also, I believe that tests any involving explicit auto-IDs will break with Oracle, which will probably thwart any attempt to add tests for this.

[Aymeric Augustin]

I'm requalifying this as a documentation issue, since the conclusion is that people should never serialize automatically created data.

[Gabriel Hurley]

Second pass at docs

[Gabriel Hurley]

Added a second patch with a couple additional corrections for typos/grammar in the affected docs. Should be ready to go. Marking RFC.

[Aymeric Augustin]

In [17355]:

Fixed #16364 -- Clarified why automatically created data shouldn't be saved in fixtures. Thanks Gabriel for the review.