Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
-+-
 Reporter:  aneil|Owner:  koirikivi
 Type:  Bug  |   Status:  closed
Component:  Core |  Version:  1.2
  (Serialization)|   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  easy-pickings|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by Ramiro Morales ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"a75324c6544d728d3bd8f678b1b8021fdff18332"]:
 {{{
 #!CommitTicketReference repository=""
 revision="a75324c6544d728d3bd8f678b1b8021fdff18332"
 Fixed #14226 -- Dependency calculation for complex M2M relations.

 `sort_dependencies` incorrectly interpreted 'complex' M2M relations
 (with explicit through models) as dependencies for a model. This caused
 circular complex M2M relations to be unserializable by dumpdata.

 Thanks to aneil for the report and outofculture for initial tests.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.cb8d099dfc365f53f2c7002ad4bbae81%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
-+-
 Reporter:  aneil|Owner:  koirikivi
 Type:  Bug  |   Status:  assigned
Component:  Core |  Version:  1.2
  (Serialization)|   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  easy-pickings|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by timo):

 * easy:  1 => 0


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.efba098398267964f8e727e27ce97d09%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
-+-
 Reporter:  aneil|Owner:  koirikivi
 Type:  Bug  |   Status:  assigned
Component:  Core |  Version:  1.2
  (Serialization)|   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  easy-pickings|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  1|
-+-

Comment (by koirikivi):

 I think the correct way to fix this is to remove models referenced by
 complex (with explicit intermediate models i.e. through=...) M2M relations
 from the dependency chain, but keep simple (with automatic intermediate
 models) in the dependency chain.

 This is also the check done by
 django.core.serializers.python.Serializer.handle_m2m_field. In serialized
 data, simple M2M relations are shown inline with the model that defines
 them, which makes dependencies of the referenced models. For complex M2M
 relations, however, the intermediate models should be serialized along
 with the other models, and should be included as models in the serialized
 data. The model defining the M2M relation thus has no dependency to the
 other model, but the intermediate model will have a dependency to both of
 the M2M models.

 Development branch at
 https://github.com/koirikivi/django/tree/ticket_14226
 Pull request at https://github.com/django/django/pull/1495

 All tests pass under sqlite and postgres.

 I created quite a few tests to first learn about the issue and then to be
 sure that everything works. The ''test_dump_and_load_m2m_complex_*'' tests
 are most likely redundant with the other tests in the PR, and can be
 removed as seemed fit.

 FWIW I also tried the patch posted by aneil, which broke a couple of
 tests, and removing the M2M checks altogether, which didn't break any
 tests but resulted in a regression that's caught in the tests in the PR
 (namely ''test_dependency_sorting_m2m_simple'')

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.3446b87c7de987194fdeb4ed8e50b63c%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
-+-
 Reporter:  aneil|Owner:  koirikivi
 Type:  Bug  |   Status:  assigned
Component:  Core |  Version:  1.2
  (Serialization)|   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  easy-pickings|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  1|
-+-
Changes (by koirikivi):

 * needs_better_patch:  1 => 0


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.4a35b8684090d3b9522193735abe71d1%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
-+-
 Reporter:  aneil|Owner:  koirikivi
 Type:  Bug  |   Status:  assigned
Component:  Core |  Version:  1.2
  (Serialization)|   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  easy-pickings|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  1|
-+-
Changes (by koirikivi):

 * owner:  outofculture => koirikivi


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.a0f80cb1bcbdfa9a741c923c3a69bd2f%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
+
   Reporter:  aneil |Owner:  outofculture
   Type:  Bug   |   Status:  assigned
  Milestone:|Component:  Core (Serialization)
Version:  1.2   | Severity:  Normal
 Resolution:| Keywords:  easy-pickings
   Triage Stage:  Accepted  |Has patch:  1
Needs documentation:  0 |  Needs tests:  0
Patch needs improvement:  1 |
+
Changes (by graham_king):

 * type:   => Bug
 * severity:   => Normal


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
+---
  Reporter:  aneil  | Owner:  outofculture 
Status:  assigned   | Milestone:   
 Component:  Serialization  |   Version:  1.2  
Resolution: |  Keywords:  easy-pickings
 Stage:  Accepted   | Has_patch:  1
Needs_docs:  0  |   Needs_tests:  0
Needs_better_patch:  1  |  
+---
Comment (by russellm):

 The test cases look good, but running the full test suite reveals some
 additional breakages -- most notably in the fixtures tests. Some of these
 breakages are output ordering problems, but some appear to be more than
 that.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
+---
  Reporter:  aneil  | Owner:  outofculture 
Status:  assigned   | Milestone:   
 Component:  Serialization  |   Version:  1.2  
Resolution: |  Keywords:  easy-pickings
 Stage:  Accepted   | Has_patch:  1
Needs_docs:  0  |   Needs_tests:  0
Needs_better_patch:  1  |  
+---
Changes (by outofculture):

  * needs_tests:  1 => 0

Comment:

 active development of this at http://github.com/outofculture/django

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
+---
  Reporter:  aneil  | Owner:  outofculture 
Status:  assigned   | Milestone:   
 Component:  Serialization  |   Version:  1.2  
Resolution: |  Keywords:  easy-pickings
 Stage:  Accepted   | Has_patch:  1
Needs_docs:  0  |   Needs_tests:  1
Needs_better_patch:  1  |  
+---
Comment (by outofculture):

 The proposed code patch appeared to break
 fixtures_regress:test_dependency_sorting_dangling, but on closer
 inspection, should not be considered to be the case.  That test explicitly
 expected the dangling, un-related model to have a position in the
 resultant dependencies, but such an expectation is invalid.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
+---
  Reporter:  aneil  | Owner:  outofculture 
Status:  assigned   | Milestone:   
 Component:  Serialization  |   Version:  1.2  
Resolution: |  Keywords:  easy-pickings
 Stage:  Accepted   | Has_patch:  1
Needs_docs:  0  |   Needs_tests:  1
Needs_better_patch:  1  |  
+---
Comment (by outofculture):

 also, this test does not account for russelm's concerns.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
+---
  Reporter:  aneil  | Owner:  outofculture 
Status:  assigned   | Milestone:   
 Component:  Serialization  |   Version:  1.2  
Resolution: |  Keywords:  easy-pickings
 Stage:  Accepted   | Has_patch:  1
Needs_docs:  0  |   Needs_tests:  1
Needs_better_patch:  1  |  
+---
Comment (by outofculture):

 the patch to add tests will only be usefully applied after the genocide of
 doc tests has been merged into master.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
+---
  Reporter:  aneil  | Owner:  outofculture 
Status:  assigned   | Milestone:   
 Component:  Serialization  |   Version:  1.2  
Resolution: |  Keywords:  easy-pickings
 Stage:  Accepted   | Has_patch:  1
Needs_docs:  0  |   Needs_tests:  1
Needs_better_patch:  1  |  
+---
Changes (by outofculture):

  * owner:  nobody => outofculture
  * status:  new => assigned

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
+---
  Reporter:  aneil  | Owner:  nobody   
Status:  new| Milestone:   
 Component:  Serialization  |   Version:  1.2  
Resolution: |  Keywords:  easy-pickings
 Stage:  Accepted   | Has_patch:  1
Needs_docs:  0  |   Needs_tests:  1
Needs_better_patch:  1  |  
+---
Changes (by russellm):

 * cc: ja...@b-list.org, and...@eternicode.com (removed)
  * keywords:  database postgres schema => easy-pickings

Comment:

 Not sure how the CC and keywords got modified. Sorry James and Andrew.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
+---
  Reporter:  aneil  | Owner:  nobody  
Status:  new| Milestone:  
 Component:  Serialization  |   Version:  1.2 
Resolution: |  Keywords:  database postgres schema
 Stage:  Accepted   | Has_patch:  1   
Needs_docs:  0  |   Needs_tests:  1   
Needs_better_patch:  1  |  
+---
Changes (by russellm):

 * cc: ja...@b-list.org, and...@eternicode.com (added)
  * needs_better_patch:  => 1
  * needs_tests:  => 1
  * keywords:  => database postgres schema
  * needs_docs:  => 0
  * has_patch:  0 => 1
  * stage:  Unreviewed => Accepted

Comment:

 This needs tests; fixtures_regress already has tests for the
 sort_dependencies utility method, which is what is being modified here.

 Also - I'm not completely convinced the patch is correct. On first
 inspection, I'm fairly certain the "add the through model to the
 dependency chain" will result in objects being added to the fixture that
 aren't required for the simple case. The simple case (normal m2m) can be
 satisfied by simply removing m2m checks from the dependency chain. In the
 complex case (manually specified m2m model), checks aren't required
 either, because the manually specified m2m model will be processed as a
 standalone model.

 Tests would help to validate this :-)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

[Django] #14226: Bug in dumpdata dependency calculation involving ManyToManyFields

[Django]

#14226: Bug in dumpdata dependency calculation involving ManyToManyFields
---+
 Reporter:  aneil  |   Owner:  nobody
   Status:  new|   Milestone:
Component:  Serialization  | Version:  1.2   
 Keywords: |   Stage:  Unreviewed
Has_patch:  0  |  
---+
 The manage.py dumpdata command incorrectly interprets !ManyToMany
 relationships as dependencies of the model that declares them (rather than
 the other way around).

 In the example below are 5 models - User, Tag and Posting, where both User
 and Posting have M2M relationships to Tag via !UserTag and !PostingTag,
 respectively.  This should be serializable.

 Here are the actual dependencies:
 {{{
 User:  None
 Tag: None
 Posting:  User
 PostingTag: Posting, Tag
 UserTag:   User, Tag
 }}}
 However, dumpdata fails with this error:
 Error: Can't resolve dependencies for main.Posting, main.!PostingTag,
 main.Tag, main.User, main.!UserTag in serialized app list.

 {{{
 from django.db.models import
 Model,CharField,ForeignKey,ManyToManyField,TextField,DateTimeField
 class User(Model):
 username  = CharField(max_length=20)
 password = CharField(max_length=20)
 topics = ManyToManyField("Tag",through="UserTag")

 def natural_key(self):
 return (self.username,)

 class Posting(Model):
 user = ForeignKey(User)
 text = TextField()
 time = DateTimeField()

 def natural_key(self):
 return (self.user.username,self.time)

 natural_key.dependencies=['main.User']

 class Tag(Model):
 name  = CharField(max_length=20)
 postings = ManyToManyField(Posting,through="PostingTag")

 def natural_key(self):
 return (self.name,)

 class PostingTag(Model):
 tag = ForeignKey(Tag)
 posting = ForeignKey(Posting)

 def natural_key(self):
 return (self.tag.natural_key(),self.posting.natural_key())

 class UserTag(Model):
 user = ForeignKey(User)
 tag = ForeignKey(Tag)

 def natural_key(self):
 return (self.tag.natural_key(),self.user.natural_key())
 }}}

 The reason this occurs is invalid logic in
 django/core/management/commands/dumpdata.py in lines 152-155.  Here is the
 relevant code & context:

 {{{
 145# Now add a dependency for any FK or M2M relation with
 146   # a model that defines a natural key
 147for field in model._meta.fields:
 148if hasattr(field.rel, 'to'):
 149rel_model = field.rel.to
 150if hasattr(rel_model, 'natural_key'):
 151deps.append(rel_model)
 152for field in model._meta.many_to_many:
 153rel_model = field.rel.to
 154if hasattr(rel_model, 'natural_key'):
 155deps.append(rel_model)
 156model_dependencies.append((model, deps))
 }}}

 Lines 152-155 treat M2M relations like FK relations.  This is incorrect.
 A Model named by an FK is a dependency, however, the model named by an M2M
 is not.

 The fix requires adding the M2M *table* to the model_list, and processing
 its dependencies accordingly.

 I've attached a simple test project that demonstrates the problem.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.