diff --git a/bdmodels/fields.py b/bdmodels/fields.py index 0ab0038..42029ed 100644 --- a/bdmodels/fields.py +++ b/bdmodels/fields.py @@ -150,7 +150,7 @@ def _check_from_field(self): else: return [] - def _check_on_delete(self): + def _check_on_delete(self, databases=None): on_delete = getattr(self.remote_field, 'on_delete', None) if on_delete in (SET_NULL, SET_DEFAULT): return [ diff --git a/bdmodels/models.py b/bdmodels/models.py index fc91973..76969e6 100644 --- a/bdmodels/models.py +++ b/bdmodels/models.py @@ -239,10 +239,14 @@ class BrokenDownManager(models.Manager.from_queryset(BrokenDownQuerySet)): Connects the model to a :py:class:`BrokenDownQuerySet` (and inherits its methods, as it is built from it). """ def get_queryset(self): - return super().get_queryset().update_fetched_parents({}, force_update_deferrals=True) + fetched_parents = self.model._meta.fetched_parents + return super().get_queryset().update_fetched_parents(fetched_parents, force_update_deferrals=True) class BrokenDownOptions(Options): + # Initialize _fetched_parents_raw to None by default + _fetched_parents_raw = None + @cached_property def _forward_fields_map(self): res = {} @@ -262,6 +266,59 @@ def _forward_fields_map(self): pass return res + @cached_property + def fetched_parents(self): + """ + Return the set of parent models that should be fetched by default. + + This can be configured via the model's ``Meta.fetched_parents`` attribute, + which should be a list or tuple of parent model classes that will be + automatically fetched (using joins) whenever the model is queried. + + By default, returns an empty frozenset, meaning no parents are fetched + automatically and all parent fields are deferred. + + **Example**:: + + class MyModel(BrokenDownModel, ParentA, ParentB): + # ... field definitions ... + + class Meta: + fetched_parents = [ParentA] # ParentA will be fetched by default + + **Validation:** + - All items in ``Meta.fetched_parents`` must be Django model classes + - All specified models must be actual parents of the model + - Invalid configurations will raise ``TypeError`` or ``ValueError`` + + **Returns:** + frozenset: A frozenset of parent model classes to fetch by default + """ + if self._fetched_parents_raw is None: + return frozenset() + + # Validate that all items are model classes and are actual parents + parent_models = set() + for parent_spec in self._fetched_parents_raw: + # Check if it's a model class + if not isinstance(parent_spec, type) or not issubclass(parent_spec, models.Model): + raise TypeError( + f"Meta.fetched_parents must contain only Django model classes, " + f"got {parent_spec!r}" + ) + + # Check if it's an actual parent of this model + if parent_spec not in self.parents: + raise ValueError( + f"Meta.fetched_parents contains {parent_spec._meta.label} which is not " + f"a parent of {self.label}. Valid parents are: " + f"{', '.join(p._meta.label for p in self.parents.keys())}" + ) + + parent_models.add(parent_spec) + + return frozenset(parent_models) + class BrokenDownModelBase(models.base.ModelBase): """A hack for using our own options class""" @@ -269,7 +326,16 @@ def add_to_class(cls, name, value): if name == '_meta': # We only mess with 'vanilla' Options if type(value) is Options: + # Extract fetched_parents from meta before it's processed + meta = value.meta if hasattr(value, 'meta') else None + fetched_parents_raw = getattr(meta, 'fetched_parents', None) if meta else None + # Remove it from meta so Django doesn't complain + if meta and hasattr(meta, 'fetched_parents'): + delattr(meta, 'fetched_parents') + value.__class__ = BrokenDownOptions + # Store the extracted fetched_parents + value._fetched_parents_raw = fetched_parents_raw else: # If anybody else already messed with it, we bail out raise TypeError(f"BrokenDownModel needs to mess with the Options, but we got {type(value)}") diff --git a/docs/optimizations.rst b/docs/optimizations.rst index 5b13dbb..507182b 100644 --- a/docs/optimizations.rst +++ b/docs/optimizations.rst @@ -78,6 +78,54 @@ To do this, modify your ``manage.py`` as follows: With this, every potential case of 1+N queries will be logged with a full stack-trace, so you can find exactly where it comes from. +Configuring Default Fetched Parents +------------------------------------ + +By default, when you query a broken-down model, only the model's own fields +are fetched, and all parent fields are deferred. However, if certain parent +models are accessed frequently together with the main model, you can configure +them to be fetched by default using the ``fetched_parents`` Meta option. + +This can be particularly useful when: + +* A parent contains fields that are accessed in most views or operations +* You want to avoid 1+N queries in common code paths without manually adding + ``select_related()`` everywhere +* Certain parents logically belong to the "core" of your model's functionality + +Example:: + + from bdmodels.models import BrokenDownModel + from bdmodels.fields import VirtualParentLink + + class Central(BrokenDownModel, CoreFields, FrequentFields, RareFields): + id = models.AutoField(primary_key=True) + corefields_ptr = VirtualParentLink(CoreFields, on_delete=models.DO_NOTHING) + frequentfields_ptr = VirtualParentLink(FrequentFields, on_delete=models.DO_NOTHING) + rarefields_ptr = VirtualParentLink(RareFields, on_delete=models.DO_NOTHING) + + class Meta: + # FrequentFields will be fetched by default along with Central's own fields + fetched_parents = [FrequentFields] + +With this configuration: + +* Queries like ``Central.objects.get(id=1)`` will automatically fetch both + ``Central`` and ``FrequentFields`` in a single query +* ``RareFields`` will still be deferred and only fetched when accessed +* You can still override this behavior: + + - Use ``select_related('rarefields_ptr')`` to fetch specific parents + - Use ``fetch_all_parents()`` to fetch all parents + - The ``fetched_parents`` setting only affects the default behavior + +Important notes: + +* ``fetched_parents`` must be a list or tuple of model classes (not strings) +* All specified models must be actual parents of the model +* This is a performance optimization - use it judiciously based on your + access patterns + If it's the User model ---------------------- diff --git a/test_bdmodels/testapp/migrations/0006_childwithfetchedparents.py b/test_bdmodels/testapp/migrations/0006_childwithfetchedparents.py new file mode 100644 index 0000000..7acffb4 --- /dev/null +++ b/test_bdmodels/testapp/migrations/0006_childwithfetchedparents.py @@ -0,0 +1,42 @@ +# Generated manually to demonstrate fetched_parents feature + +from django.db import migrations, models +import django.db.models.deletion + +import bdmodels.fields +import bdmodels.migration_ops as bdmigrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('testapp', '0005_childwithvirtualnonparent'), + ] + + operations = [ + migrations.CreateModel( + name='ChildWithFetchedParents', + fields=[ + ('id', models.AutoField(primary_key=True, serialize=False)), + ('child_name', models.CharField(max_length=10)), + ], + options={ + 'abstract': False, + }, + ), + bdmigrations.AddVirtualField( + model_name='childwithfetchedparents', + name='parenta_ptr', + field=bdmodels.fields.VirtualOneToOneField(from_field='id', on_delete=django.db.models.deletion.DO_NOTHING, parent_link=True, to='testapp.ParentA'), + ), + bdmigrations.AddVirtualField( + model_name='childwithfetchedparents', + name='parentb_ptr', + field=bdmodels.fields.VirtualOneToOneField(from_field='id', on_delete=django.db.models.deletion.DO_NOTHING, parent_link=True, to='testapp.ParentB'), + ), + bdmigrations.AddVirtualField( + model_name='childwithfetchedparents', + name='parentc_ptr', + field=bdmodels.fields.VirtualOneToOneField(from_field='id', on_delete=django.db.models.deletion.DO_NOTHING, parent_link=True, to='testapp.ParentC'), + ), + ] \ No newline at end of file diff --git a/test_bdmodels/testapp/models.py b/test_bdmodels/testapp/models.py index d7676c4..9a5dc50 100644 --- a/test_bdmodels/testapp/models.py +++ b/test_bdmodels/testapp/models.py @@ -92,3 +92,14 @@ class ChildWithVirtualNonParent(BrokenDownModel, ParentA): parenta_ptr = VirtualParentLink(ParentA, on_delete=models.DO_NOTHING) b = VirtualOneToOneField(ParentB, 'id', on_delete=models.DO_NOTHING) child_name = models.CharField(max_length=10) + + +class ChildWithFetchedParents(BrokenDownModel, ParentA, ParentB, ParentC): + id = models.AutoField(primary_key=True) + parenta_ptr = VirtualOneToOneField(ParentA, 'id', parent_link=True, on_delete=models.DO_NOTHING) + parentb_ptr = VirtualOneToOneField(ParentB, 'id', parent_link=True, on_delete=models.DO_NOTHING) + parentc_ptr = VirtualOneToOneField(ParentC, 'id', parent_link=True, on_delete=models.DO_NOTHING) + child_name = models.CharField(max_length=10) + + class Meta: + fetched_parents = [ParentA, ParentB] diff --git a/test_bdmodels/testapp/tests.py b/test_bdmodels/testapp/tests.py index 2a75e11..cf948e8 100644 --- a/test_bdmodels/testapp/tests.py +++ b/test_bdmodels/testapp/tests.py @@ -2,12 +2,12 @@ from django.db import DatabaseError, transaction from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature -from .models import Child, UserChild, Nephew, TimeStampedChild, ChildProxy, ChildWithVirtualNonParent, ParentB +from .models import Child, UserChild, Nephew, TimeStampedChild, ChildProxy, ChildWithVirtualNonParent, ParentB, \ + ChildWithFetchedParents # TODO: Rename test classes class SelectRelatedTestCase(TestCase): - ChildClass = Child def setUp(self): @@ -127,7 +127,6 @@ def test_refresh_from_db_all_parents(self): class AbstractBaseClassTestCase(SelectRelatedTestCase): - ChildClass = TimeStampedChild def test_the_abstract_base_works(self): @@ -142,7 +141,6 @@ def test_the_abstract_base_works(self): class ProxyChildClassTestCase(SelectRelatedTestCase): - ChildClass = ChildProxy @@ -212,7 +210,6 @@ def test_select_related_through_parent_with_id(self): class VirtualNonParentTestCase(TestCase): - ChildClass = ChildWithVirtualNonParent def setUp(self): @@ -258,7 +255,8 @@ def test_select_related_all(self): class ObjectUpdateTestCase(TestCase): def setUp(self) -> None: - self.child = Child.objects.create(id=12, para_name='A', parb_name='B', parc_name='C', parc_zit=True, child_name='Xerxes') + self.child = Child.objects.create(id=12, para_name='A', parb_name='B', parc_name='C', parc_zit=True, + child_name='Xerxes') def test_update_parent_field(self): c = Child.objects.get(id=12) @@ -281,6 +279,46 @@ def test_update_parent_field_with_other_parent_field_access(self): self.assertFalse(c.parc_zit) +class FetchedParentsTestCase(TestCase): + + def setUp(self): + super().setUp() + ChildWithFetchedParents.objects.create(para_name='A', parb_name='B', parc_name='C', child_name='Xerxes') + + def test_configured_parents_fetched_by_default(self): + """ + When fetched_parents is configured in Meta, those parents are fetched + by default without requiring select_related(). + """ + with self.assertNumQueries(1): + c = ChildWithFetchedParents.objects.get(child_name='Xerxes') + # ParentA and ParentB should be fetched + self.assertEqual((c.para_zit, c.para_name), (True, 'A')) + self.assertEqual((c.parb_zit, c.parb_name), (True, 'B')) + # ParentC should still require a query + with self.assertNumQueries(1): + self.assertEqual(c.parc_name, 'C') + + def test_select_related_overrides_fetched_parents(self): + """ + select_related() should still work and override the default fetched parents. + """ + with self.assertNumQueries(1): + c = ChildWithFetchedParents.objects.select_related('parentc_ptr').get(child_name='Xerxes') + # Only ParentC should be fetched via select_related + self.assertEqual(c.parc_name, 'C') + + def test_fetch_all_parents_still_works(self): + """ + fetch_all_parents() should still fetch all parents regardless of the fetched_parents config. + """ + with self.assertNumQueries(1): + c = ChildWithFetchedParents.objects.fetch_all_parents().get(child_name='Xerxes') + self.assertEqual((c.para_zit, c.para_name), (True, 'A')) + self.assertEqual((c.parb_zit, c.parb_name), (True, 'B')) + self.assertEqual(c.parc_name, 'C') + + class BulkCreateTestCase(TestCase): @skipIfDBFeature('can_return_rows_from_bulk_insert')