South migration ignoring on_delete on ForeignKey

[George Lund]

I have a problem where South is not persisting the on_delete option
passed to a ForeignKey, when it saves the ORM into a migration.

The default behaviour of ForeignKey is a Cascade Delete, whereas my
models are specifying on_delete=models.SET_NULL, asking Django to
avoid the default behaviour in this case, and keep the related models.

In my specific case, I've a data migration creating some rows and
adding references to them from an existing table, so when the reverse
migration deletes the rows it creates, it's deleting all the related
rows. A large chunk of my database disappears up in smoke, so this is
a pretty serious bug :-)

To illustrate this here are two simple models:
*** models.py
from django.db import models

class RelationA(models.Model):
pass

class RelationB(models.Model):
a = models.ForeignKey(RelationA, null=True, blank=True,
on_delete=models.SET_NULL)
***

And here are some tests (I created a migration to use in the test,
with --initial, but any migration created on those models would be the
same): the django_test passes, the south version fails.
*** test.py
from django.test import TestCase
import models as django_models
from south.migration.base import Migrations

class NoCascadeDeleteTest(TestCase):
def test_south(self):
mm = Migrations('experiment')
migration = mm[0]
self._test(migration.orm())

def test_django(self):
self._test(django_models)

def _test(self, orm):
a = orm.RelationA()
a.save()
b = orm.RelationB()
b.a = a
b.save()

self.assertEqual(orm.RelationA.objects.all().count(), 1, "Test
setup failed - expected 1 entry in each table")
self.assertEqual(orm.RelationB.objects.all().count(), 1, "Test
setup failed - expected 1 entry in each table")

orm.RelationA.objects.all().delete()

self.assertEqual(orm.RelationA.objects.all().count(), 0,
"RelationA not cleared for some reason.")
self.assertEqual(orm.RelationB.objects.all().count(), 1, "The
test failed: cascade delete happened.")
***

Having decided that this is a bug in South, I don't mind having a go
at finding a fix, but being quite new I'd appreciate some confirmation
before I get stuck in.

thanks
George

[Andrew Godwin]

On 02/02/12 17:15, George Lund wrote:
> I have a problem where South is not persisting the on_delete option
> passed to a ForeignKey, when it saves the ORM into a migration.

>

> Having decided that this is a bug in South, I don't mind having a go
> at finding a fix, but being quite new I'd appreciate some confirmation
> before I get stuck in.

Yep, it's probably because on_delete is relatively new (well, compared
to South's last release) and it's not in the models introspector.

Andrew

[George Lund]

On Thursday, 2 February 2012 18:43:43 UTC, Andrew Godwin wrote:

On 02/02/12 17:15, George Lund wrote:
> I have a problem where South is not persisting the on_delete option
> passed to a ForeignKey, when it saves the ORM into a migration.

... 

Yep, it's probably because on_delete is relatively new (well, compared 

to South's last release) and it's not in the models introspector.

Andrew

So I made a patch for this issue, which I think works. See http://south.aeracode.org/ticket/763

Unfortunately I've really struggled with making test cases.

I created a new class south.tests.ondelete.TestMigration, like the other tests.

This test is different to the others in that I want to first freeze the ORM, as in a migration, then generate the models, and make sure that they behave as expected. Just testing a previously-frozen set of models in a pre-created migration wouldn't really test that the freezing process still includes the correct field attributes.

To do this, I tried to create a dummy migration class, from which I could extract a working ORM. My function (to be used by my test class) looks like this:

_TEST_APP_NAME = "ondeleteapp"

def _get_orm():

    class _DummyMigration(object):

        models = None

    migration_class = _DummyMigration

    migration_class.models = freeze_apps(_TEST_APP_NAME)

    orm = FakeORM(migration_class, _TEST_APP_NAME)

    return orm

Unfortunately, freeze_apps doesn't work from here. I don't know why -- I'm just getting nothing back.

The problem actually happens in freezer.py:25, where this standard Django call

 models.get_models(models.get_app(app))

is returning an empty list.

I just don't know how to get my test case's models to appear. (Adding the test case into INSTALLED_APPS doesn't seem to actually help.)

It looked like the monkey-patching base test class might be trying to fix this kind of thing but what it was doing left me confused, and deriving from that didn't seem to help.

Hopefully someone else can pick up where I'm going here, unfortunately I don't have any more time to look at this right now (though let me know if I can clarify any of the above).

thanks

George