Changed get_model to raise an exception on errors.
Returning None on errors required unpythonic error checking and was inconsistent with get_app_config. get_model was a private API until the previous commit, but given that it was certainly used in third party software, the change is explained in the release notes. Applied the same change to get_registered_model, which is a new private API introduced during the recent refactoring.
This commit is contained in:
parent
54790e669d
commit
ba7206cd81
|
@ -248,14 +248,11 @@ class Apps(object):
|
||||||
|
|
||||||
model_name is case-insensitive.
|
model_name is case-insensitive.
|
||||||
|
|
||||||
Returns None if no application exists with this label, or no model
|
Raises LookupError if no application exists with this label, or no
|
||||||
exists with this name in the application.
|
model exists with this name in the application.
|
||||||
"""
|
"""
|
||||||
self.populate_models()
|
self.populate_models()
|
||||||
try:
|
return self.get_app_config(app_label).get_model(model_name.lower())
|
||||||
return self.get_app_config(app_label).get_model(model_name.lower())
|
|
||||||
except LookupError:
|
|
||||||
return None
|
|
||||||
|
|
||||||
def register_model(self, app_label, model):
|
def register_model(self, app_label, model):
|
||||||
# Since this method is called when models are imported, it cannot
|
# Since this method is called when models are imported, it cannot
|
||||||
|
@ -289,9 +286,13 @@ class Apps(object):
|
||||||
the given app_label.
|
the given app_label.
|
||||||
|
|
||||||
It's safe to call this method at import time, even while the registry
|
It's safe to call this method at import time, even while the registry
|
||||||
is being populated. It returns None for models that aren't loaded yet.
|
is being populated.
|
||||||
"""
|
"""
|
||||||
return self.all_models[app_label].get(model_name.lower())
|
model = self.all_models[app_label].get(model_name.lower())
|
||||||
|
if model is None:
|
||||||
|
raise LookupError(
|
||||||
|
"Model '%s.%s' not registered." % (app_label, model_name))
|
||||||
|
return model
|
||||||
|
|
||||||
def set_available_apps(self, available):
|
def set_available_apps(self, available):
|
||||||
"""
|
"""
|
||||||
|
|
|
@ -188,8 +188,9 @@ class ModelDetailView(BaseAdminDocsView):
|
||||||
apps.get_app_config(self.kwargs['app_label'])
|
apps.get_app_config(self.kwargs['app_label'])
|
||||||
except LookupError:
|
except LookupError:
|
||||||
raise Http404(_("App %(app_label)r not found") % self.kwargs)
|
raise Http404(_("App %(app_label)r not found") % self.kwargs)
|
||||||
model = apps.get_model(self.kwargs['app_label'], self.kwargs['model_name'])
|
try:
|
||||||
if model is None:
|
model = apps.get_model(self.kwargs['app_label'], self.kwargs['model_name'])
|
||||||
|
except LookupError:
|
||||||
raise Http404(_("Model %(model_name)r not found in app %(app_label)r") % self.kwargs)
|
raise Http404(_("Model %(model_name)r not found in app %(app_label)r") % self.kwargs)
|
||||||
|
|
||||||
opts = model._meta
|
opts = model._meta
|
||||||
|
|
|
@ -129,8 +129,9 @@ def get_user_model():
|
||||||
app_label, model_name = settings.AUTH_USER_MODEL.split('.')
|
app_label, model_name = settings.AUTH_USER_MODEL.split('.')
|
||||||
except ValueError:
|
except ValueError:
|
||||||
raise ImproperlyConfigured("AUTH_USER_MODEL must be of the form 'app_label.model_name'")
|
raise ImproperlyConfigured("AUTH_USER_MODEL must be of the form 'app_label.model_name'")
|
||||||
user_model = apps.get_model(app_label, model_name)
|
try:
|
||||||
if user_model is None:
|
user_model = apps.get_model(app_label, model_name)
|
||||||
|
except LookupError:
|
||||||
raise ImproperlyConfigured("AUTH_USER_MODEL refers to model '%s' that has not been installed" % settings.AUTH_USER_MODEL)
|
raise ImproperlyConfigured("AUTH_USER_MODEL refers to model '%s' that has not been installed" % settings.AUTH_USER_MODEL)
|
||||||
return user_model
|
return user_model
|
||||||
|
|
||||||
|
|
|
@ -61,7 +61,9 @@ def _check_permission_clashing(custom, builtin, ctype):
|
||||||
|
|
||||||
|
|
||||||
def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kwargs):
|
def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kwargs):
|
||||||
if apps.get_model('auth', 'Permission') is None:
|
try:
|
||||||
|
apps.get_model('auth', 'Permission')
|
||||||
|
except LookupError:
|
||||||
return
|
return
|
||||||
|
|
||||||
if not router.allow_migrate(db, auth_app.Permission):
|
if not router.allow_migrate(db, auth_app.Permission):
|
||||||
|
@ -117,7 +119,9 @@ def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kw
|
||||||
|
|
||||||
|
|
||||||
def create_superuser(app, created_models, verbosity, db, **kwargs):
|
def create_superuser(app, created_models, verbosity, db, **kwargs):
|
||||||
if apps.get_model('auth', 'Permission') is None:
|
try:
|
||||||
|
apps.get_model('auth', 'Permission')
|
||||||
|
except LookupError:
|
||||||
return
|
return
|
||||||
|
|
||||||
UserModel = get_user_model()
|
UserModel = get_user_model()
|
||||||
|
|
|
@ -54,7 +54,7 @@ def post_comment(request, next=None, using=None):
|
||||||
except TypeError:
|
except TypeError:
|
||||||
return CommentPostBadRequest(
|
return CommentPostBadRequest(
|
||||||
"Invalid content_type value: %r" % escape(ctype))
|
"Invalid content_type value: %r" % escape(ctype))
|
||||||
except AttributeError:
|
except LookupError:
|
||||||
return CommentPostBadRequest(
|
return CommentPostBadRequest(
|
||||||
"The given content-type %r does not resolve to a valid model." % \
|
"The given content-type %r does not resolve to a valid model." % \
|
||||||
escape(ctype))
|
escape(ctype))
|
||||||
|
|
|
@ -12,7 +12,9 @@ def update_contenttypes(app, created_models, verbosity=2, db=DEFAULT_DB_ALIAS, *
|
||||||
Creates content types for models in the given app, removing any model
|
Creates content types for models in the given app, removing any model
|
||||||
entries that no longer have a matching model class.
|
entries that no longer have a matching model class.
|
||||||
"""
|
"""
|
||||||
if apps.get_model('contenttypes', 'ContentType') is None:
|
try:
|
||||||
|
apps.get_model('contenttypes', 'ContentType')
|
||||||
|
except LookupError:
|
||||||
return
|
return
|
||||||
|
|
||||||
if not router.allow_migrate(db, ContentType):
|
if not router.allow_migrate(db, ContentType):
|
||||||
|
|
|
@ -157,7 +157,10 @@ class ContentType(models.Model):
|
||||||
|
|
||||||
def model_class(self):
|
def model_class(self):
|
||||||
"Returns the Python model class for this type of content."
|
"Returns the Python model class for this type of content."
|
||||||
return apps.get_model(self.app_label, self.model)
|
try:
|
||||||
|
return apps.get_model(self.app_label, self.model)
|
||||||
|
except LookupError:
|
||||||
|
return None
|
||||||
|
|
||||||
def get_object_for_this_type(self, **kwargs):
|
def get_object_for_this_type(self, **kwargs):
|
||||||
"""
|
"""
|
||||||
|
|
|
@ -81,8 +81,9 @@ def kml(request, label, model, field_name=None, compress=False, using=DEFAULT_DB
|
||||||
must be that of a geographic field.
|
must be that of a geographic field.
|
||||||
"""
|
"""
|
||||||
placemarks = []
|
placemarks = []
|
||||||
klass = apps.get_model(label, model)
|
try:
|
||||||
if not klass:
|
klass = apps.get_model(label, model)
|
||||||
|
except LookupError:
|
||||||
raise Http404('You must supply a valid app label and module name. Got "%s.%s"' % (label, model))
|
raise Http404('You must supply a valid app label and module name. Got "%s.%s"' % (label, model))
|
||||||
|
|
||||||
if field_name:
|
if field_name:
|
||||||
|
|
|
@ -66,8 +66,9 @@ class Command(BaseCommand):
|
||||||
for exclude in excludes:
|
for exclude in excludes:
|
||||||
if '.' in exclude:
|
if '.' in exclude:
|
||||||
app_label, model_name = exclude.split('.', 1)
|
app_label, model_name = exclude.split('.', 1)
|
||||||
model = apps.get_model(app_label, model_name)
|
try:
|
||||||
if not model:
|
model = apps.get_model(app_label, model_name)
|
||||||
|
except LookupError:
|
||||||
raise CommandError('Unknown model in excludes: %s' % exclude)
|
raise CommandError('Unknown model in excludes: %s' % exclude)
|
||||||
excluded_models.add(model)
|
excluded_models.add(model)
|
||||||
else:
|
else:
|
||||||
|
@ -96,8 +97,9 @@ class Command(BaseCommand):
|
||||||
raise CommandError("Unknown application: %s" % app_label)
|
raise CommandError("Unknown application: %s" % app_label)
|
||||||
if app_config.models_module is None or app_config in excluded_apps:
|
if app_config.models_module is None or app_config in excluded_apps:
|
||||||
continue
|
continue
|
||||||
model = apps.get_model(app_label, model_label)
|
try:
|
||||||
if model is None:
|
model = apps.get_model(app_label, model_label)
|
||||||
|
except LookupError:
|
||||||
raise CommandError("Unknown model: %s.%s" % (app_label, model_label))
|
raise CommandError("Unknown model: %s.%s" % (app_label, model_label))
|
||||||
|
|
||||||
app_list_value = app_list.setdefault(app_config, [])
|
app_list_value = app_list.setdefault(app_config, [])
|
||||||
|
|
|
@ -42,7 +42,9 @@ def get_validation_errors(outfile, app=None):
|
||||||
except ValueError:
|
except ValueError:
|
||||||
e.add(opts, "%s is not of the form 'app_label.app_name'." % opts.swappable)
|
e.add(opts, "%s is not of the form 'app_label.app_name'." % opts.swappable)
|
||||||
continue
|
continue
|
||||||
if not apps.get_model(app_label, model_name):
|
try:
|
||||||
|
apps.get_model(app_label, model_name)
|
||||||
|
except LookupError:
|
||||||
e.add(opts, "Model has been swapped out for '%s' which has not been installed or is abstract." % opts.swapped)
|
e.add(opts, "Model has been swapped out for '%s' which has not been installed or is abstract." % opts.swapped)
|
||||||
# No need to perform any other validation checks on a swapped model.
|
# No need to perform any other validation checks on a swapped model.
|
||||||
continue
|
continue
|
||||||
|
|
|
@ -156,8 +156,6 @@ def _get_model(model_identifier):
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
Model = apps.get_model(*model_identifier.split("."))
|
Model = apps.get_model(*model_identifier.split("."))
|
||||||
except TypeError:
|
except (LookupError, TypeError):
|
||||||
Model = None
|
|
||||||
if Model is None:
|
|
||||||
raise base.DeserializationError("Invalid model identifier: '%s'" % model_identifier)
|
raise base.DeserializationError("Invalid model identifier: '%s'" % model_identifier)
|
||||||
return Model
|
return Model
|
||||||
|
|
|
@ -278,9 +278,7 @@ class Deserializer(base.Deserializer):
|
||||||
% (node.nodeName, attr))
|
% (node.nodeName, attr))
|
||||||
try:
|
try:
|
||||||
Model = apps.get_model(*model_identifier.split("."))
|
Model = apps.get_model(*model_identifier.split("."))
|
||||||
except TypeError:
|
except (LookupError, TypeError):
|
||||||
Model = None
|
|
||||||
if Model is None:
|
|
||||||
raise base.DeserializationError(
|
raise base.DeserializationError(
|
||||||
"<%s> node has invalid model identifier: '%s'"
|
"<%s> node has invalid model identifier: '%s'"
|
||||||
% (node.nodeName, model_identifier))
|
% (node.nodeName, model_identifier))
|
||||||
|
|
|
@ -192,11 +192,12 @@ class ModelState(object):
|
||||||
meta_contents["unique_together"] = list(meta_contents["unique_together"])
|
meta_contents["unique_together"] = list(meta_contents["unique_together"])
|
||||||
meta = type("Meta", tuple(), meta_contents)
|
meta = type("Meta", tuple(), meta_contents)
|
||||||
# Then, work out our bases
|
# Then, work out our bases
|
||||||
bases = tuple(
|
try:
|
||||||
(apps.get_model(*base.split(".", 1)) if isinstance(base, six.string_types) else base)
|
bases = tuple(
|
||||||
for base in self.bases
|
(apps.get_model(*base.split(".", 1)) if isinstance(base, six.string_types) else base)
|
||||||
)
|
for base in self.bases
|
||||||
if None in bases:
|
)
|
||||||
|
except LookupError:
|
||||||
raise InvalidBasesError("Cannot resolve one or more bases from %r" % (self.bases,))
|
raise InvalidBasesError("Cannot resolve one or more bases from %r" % (self.bases,))
|
||||||
# Turn fields into a dict for the body, add other bits
|
# Turn fields into a dict for the body, add other bits
|
||||||
body = dict(self.fields)
|
body = dict(self.fields)
|
||||||
|
|
|
@ -151,9 +151,10 @@ class ModelBase(type):
|
||||||
new_class._base_manager = new_class._base_manager._copy_to_model(new_class)
|
new_class._base_manager = new_class._base_manager._copy_to_model(new_class)
|
||||||
|
|
||||||
# Bail out early if we have already created this class.
|
# Bail out early if we have already created this class.
|
||||||
m = new_class._meta.apps.get_registered_model(new_class._meta.app_label, name)
|
try:
|
||||||
if m is not None:
|
return new_class._meta.apps.get_registered_model(new_class._meta.app_label, name)
|
||||||
return m
|
except LookupError:
|
||||||
|
pass
|
||||||
|
|
||||||
# Add all attributes to the class.
|
# Add all attributes to the class.
|
||||||
for obj_name, obj in attrs.items():
|
for obj_name, obj in attrs.items():
|
||||||
|
|
|
@ -68,13 +68,14 @@ def add_lazy_relation(cls, field, relation, operation):
|
||||||
# string right away. If get_model returns None, it means that the related
|
# string right away. If get_model returns None, it means that the related
|
||||||
# model isn't loaded yet, so we need to pend the relation until the class
|
# model isn't loaded yet, so we need to pend the relation until the class
|
||||||
# is prepared.
|
# is prepared.
|
||||||
model = cls._meta.apps.get_registered_model(app_label, model_name)
|
try:
|
||||||
if model:
|
model = cls._meta.apps.get_registered_model(app_label, model_name)
|
||||||
operation(field, model, cls)
|
except LookupError:
|
||||||
else:
|
|
||||||
key = (app_label, model_name)
|
key = (app_label, model_name)
|
||||||
value = (cls, field, operation)
|
value = (cls, field, operation)
|
||||||
cls._meta.apps._pending_lookups.setdefault(key, []).append(value)
|
cls._meta.apps._pending_lookups.setdefault(key, []).append(value)
|
||||||
|
else:
|
||||||
|
operation(field, model, cls)
|
||||||
|
|
||||||
|
|
||||||
def do_pending_lookups(sender, **kwargs):
|
def do_pending_lookups(sender, **kwargs):
|
||||||
|
|
|
@ -39,8 +39,9 @@ class ModelSignal(Signal):
|
||||||
"Specified sender must either be a model or a "
|
"Specified sender must either be a model or a "
|
||||||
"model name of the 'app_label.ModelName' form."
|
"model name of the 'app_label.ModelName' form."
|
||||||
)
|
)
|
||||||
sender = apps.get_registered_model(app_label, model_name)
|
try:
|
||||||
if sender is None:
|
sender = apps.get_registered_model(app_label, model_name)
|
||||||
|
except LookupError:
|
||||||
ref = (app_label, model_name)
|
ref = (app_label, model_name)
|
||||||
refs = self.unresolved_references.setdefault(ref, [])
|
refs = self.unresolved_references.setdefault(ref, [])
|
||||||
refs.append((receiver, weak, dispatch_uid))
|
refs.append((receiver, weak, dispatch_uid))
|
||||||
|
|
|
@ -202,5 +202,5 @@ Application registry
|
||||||
.. method:: apps.get_model(app_label, model_name)
|
.. method:: apps.get_model(app_label, model_name)
|
||||||
|
|
||||||
Returns the :class:`~django.db.models.Model` with the given ``app_label``
|
Returns the :class:`~django.db.models.Model` with the given ``app_label``
|
||||||
and ``model_name``. Returns ``None`` if no such application or model
|
and ``model_name``. Raises :exc:`~exceptions.LookupError` if no such
|
||||||
exists. ``model_name`` is case-insensitive.
|
application or model exists. ``model_name`` is case-insensitive.
|
||||||
|
|
|
@ -602,9 +602,15 @@ in addition to application modules, you should review code that accesses this
|
||||||
setting directly and use the app registry (:attr:`django.apps.apps`) instead.
|
setting directly and use the app registry (:attr:`django.apps.apps`) instead.
|
||||||
|
|
||||||
The "app registry" that manages the list of installed applications doesn't
|
The "app registry" that manages the list of installed applications doesn't
|
||||||
have the same features as the old "app cache". However, even though the "app
|
have the same features as the old "app cache". Even though the "app cache" was
|
||||||
cache" was a private API, most of its methods were temporarily preserved and
|
a private API, obsolete methods will be removed after a standard deprecation
|
||||||
will go through a deprecation path.
|
period. In addition, the following changes take effect immediately:
|
||||||
|
|
||||||
|
* ``get_model`` raises :exc:`~exceptions.LookupError` instead of returning
|
||||||
|
``None`` when no model is found.
|
||||||
|
|
||||||
|
* The ``only_installed`` and ``seed_cache`` arguments of ``get_model`` no
|
||||||
|
longer exist.
|
||||||
|
|
||||||
While the new implementation is believed to be more robust, regressions cannot
|
While the new implementation is believed to be more robust, regressions cannot
|
||||||
be ruled out, especially during the import sequence. Circular imports that
|
be ruled out, especially during the import sequence. Circular imports that
|
||||||
|
|
|
@ -72,8 +72,8 @@ class GetModelsTest(TestCase):
|
||||||
self.not_installed_module = models
|
self.not_installed_module = models
|
||||||
|
|
||||||
def test_get_model_only_returns_installed_models(self):
|
def test_get_model_only_returns_installed_models(self):
|
||||||
self.assertEqual(
|
with self.assertRaises(LookupError):
|
||||||
apps.get_model("not_installed", "NotInstalledModel"), None)
|
apps.get_model("not_installed", "NotInstalledModel")
|
||||||
|
|
||||||
def test_get_models_only_returns_installed_models(self):
|
def test_get_models_only_returns_installed_models(self):
|
||||||
self.assertNotIn(
|
self.assertNotIn(
|
||||||
|
|
|
@ -148,9 +148,11 @@ class AppsTests(TestCase):
|
||||||
Tests that the models in the models.py file were loaded correctly.
|
Tests that the models in the models.py file were loaded correctly.
|
||||||
"""
|
"""
|
||||||
self.assertEqual(apps.get_model("apps", "TotallyNormal"), TotallyNormal)
|
self.assertEqual(apps.get_model("apps", "TotallyNormal"), TotallyNormal)
|
||||||
self.assertEqual(apps.get_model("apps", "SoAlternative"), None)
|
with self.assertRaises(LookupError):
|
||||||
|
apps.get_model("apps", "SoAlternative")
|
||||||
|
|
||||||
self.assertEqual(new_apps.get_model("apps", "TotallyNormal"), None)
|
with self.assertRaises(LookupError):
|
||||||
|
new_apps.get_model("apps", "TotallyNormal")
|
||||||
self.assertEqual(new_apps.get_model("apps", "SoAlternative"), SoAlternative)
|
self.assertEqual(new_apps.get_model("apps", "SoAlternative"), SoAlternative)
|
||||||
|
|
||||||
def test_dynamic_load(self):
|
def test_dynamic_load(self):
|
||||||
|
@ -174,4 +176,6 @@ class AppsTests(TestCase):
|
||||||
old_models,
|
old_models,
|
||||||
apps.get_models(apps.get_app_config("apps").models_module),
|
apps.get_models(apps.get_app_config("apps").models_module),
|
||||||
)
|
)
|
||||||
|
with self.assertRaises(LookupError):
|
||||||
|
apps.get_model("apps", "SouthPonies")
|
||||||
self.assertEqual(new_apps.get_model("apps", "SouthPonies"), temp_model)
|
self.assertEqual(new_apps.get_model("apps", "SouthPonies"), temp_model)
|
||||||
|
|
Loading…
Reference in New Issue