Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Stopped calling apps.get_model with only_installed=False.

ContentTypes are only created for installed applications, and I could
make a case for not returning a model that isn't installed any more.
The check for stale ContentTypes in update_contenttypes doesn't use
model_class.

ModelSignal actually needs get_registered_model since the lookup happens
at import time. I took this opportunity to perform a small refactoring.
  • Loading branch information...
commit 81354b82bf647fd100e836dae2639e9c8d76c5eb 1 parent bbdf01e
@aaugustin aaugustin authored
View
2  django/contrib/contenttypes/models.py
@@ -157,7 +157,7 @@ def __str__(self):
def model_class(self):
"Returns the Python model class for this type of content."
- return apps.get_model(self.app_label, self.model, only_installed=False)
+ return apps.get_model(self.app_label, self.model)
def get_object_for_this_type(self, **kwargs):
"""
View
15 django/db/models/signals.py
@@ -1,5 +1,3 @@
-from collections import defaultdict
-
from django.apps import apps
from django.dispatch import Signal
from django.utils import six
@@ -16,7 +14,7 @@ class ModelSignal(Signal):
def __init__(self, *args, **kwargs):
super(ModelSignal, self).__init__(*args, **kwargs)
- self.unresolved_references = defaultdict(list)
+ self.unresolved_references = {}
class_prepared.connect(self._resolve_references)
def _resolve_references(self, sender, **kwargs):
@@ -35,18 +33,17 @@ def _resolve_references(self, sender, **kwargs):
def connect(self, receiver, sender=None, weak=True, dispatch_uid=None):
if isinstance(sender, six.string_types):
try:
- app_label, object_name = sender.split('.')
+ app_label, model_name = sender.split('.')
@charettes Collaborator

object_name was chosen instead of model_name here to be consistent with the model options naming. Maybe we should keep those distinctive concepts around? See ec469ad.

@aaugustin Owner

Oh, I see. Sorry.

To be honest, carrying around model_name and object_name seems useless to me. I don't see any value in denormalizing the lower-cased version of a string. Do we need it at all?

@charettes Collaborator

Using grep it looks like both have valid use cases.

object_name:
1. Class name generation;
2. Validation and runtime error messages.

Note that other instances of object_name.lower() have been introduced since ec469ad, mostly in django.db.migrations.

model_name (aka lower-cased object_name):
1. Template name and HTML id and class generation;
2. Permission and ContentType;
3. Related query name;
4. Model reference through django.apps.

@aaugustin Owner

I fixed object_name.lower => model_name in 20d487c.

Given that Django splits "app_label.ModelName" strings in ten different places, I should just move this logic into the app registry. Generally I'd like the case-insensitivity of model names to be entirely abstracted away by the app registry. Almost nothing in Django is case insensitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
except ValueError:
raise ValueError(
"Specified sender must either be a model or a "
"model name of the 'app_label.ModelName' form."
)
- sender = apps.get_model(app_label, object_name, only_installed=False)
+ sender = apps.get_registered_model(app_label, model_name)
if sender is None:
- reference = (app_label, object_name)
- self.unresolved_references[reference].append(
- (receiver, weak, dispatch_uid)
- )
+ ref = (app_label, model_name)
+ refs = self.unresolved_references.setdefault(ref, [])
+ refs.append((receiver, weak, dispatch_uid))
return
super(ModelSignal, self).connect(
receiver, sender=sender, weak=weak, dispatch_uid=dispatch_uid
View
6 tests/app_loading/tests.py
@@ -75,12 +75,6 @@ def test_get_model_only_returns_installed_models(self):
self.assertEqual(
apps.get_model("not_installed", "NotInstalledModel"), None)
- def test_get_model_with_not_installed(self):
- self.assertEqual(
- apps.get_model(
- "not_installed", "NotInstalledModel", only_installed=False),
- self.not_installed_module.NotInstalledModel)
-
def test_get_models_only_returns_installed_models(self):
self.assertNotIn(
"NotInstalledModel",
Please sign in to comment.
Something went wrong with that request. Please try again.