From 31c13a99bb9ebdaf12ccab4e880c5da930d86e79 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Tue, 16 Jul 2013 09:10:04 -0400 Subject: [PATCH] Fixed #14300 -- Fixed initial SQL location if models is a package. Thanks al_the_x for the report and fheinz for the draft patch. --- django/core/management/commands/loaddata.py | 2 +- django/core/management/sql.py | 20 ++++++++++++++++--- django/db/models/__init__.py | 2 +- django/db/models/loading.py | 16 +++++++++++---- docs/internals/deprecation.txt | 4 ++++ docs/releases/1.7.txt | 9 +++++++++ .../models/sql/book.sql | 2 ++ tests/fixtures_model_package/sql/book.sql | 1 + tests/fixtures_model_package/tests.py | 17 ++++++++++++++++ 9 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 tests/fixtures_model_package/models/sql/book.sql create mode 100644 tests/fixtures_model_package/sql/book.sql diff --git a/django/core/management/commands/loaddata.py b/django/core/management/commands/loaddata.py index 6856e85e45..aa879f6acc 100644 --- a/django/core/management/commands/loaddata.py +++ b/django/core/management/commands/loaddata.py @@ -233,7 +233,7 @@ class Command(BaseCommand): """ dirs = [] for path in get_app_paths(): - d = os.path.join(os.path.dirname(path), 'fixtures') + d = os.path.join(path, 'fixtures') if os.path.isdir(d): dirs.append(d) dirs.extend(list(settings.FIXTURE_DIRS)) diff --git a/django/core/management/sql.py b/django/core/management/sql.py index b58d89f60a..c5806086f9 100644 --- a/django/core/management/sql.py +++ b/django/core/management/sql.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import codecs import os import re +import warnings from django.conf import settings from django.core.management.base import CommandError @@ -168,7 +169,18 @@ def _split_statements(content): def custom_sql_for_model(model, style, connection): opts = model._meta - app_dir = os.path.normpath(os.path.join(os.path.dirname(upath(models.get_app(model._meta.app_label).__file__)), 'sql')) + app_dirs = [] + app_dir = models.get_app_path(model._meta.app_label) + app_dirs.append(os.path.normpath(os.path.join(app_dir, 'sql'))) + + # Deprecated location -- remove in Django 1.9 + old_app_dir = os.path.normpath(os.path.join(app_dir, 'models/sql')) + if os.path.exists(old_app_dir): + warnings.warn("Custom SQL location '/models/sql' is " + "deprecated, use '/sql' instead.", + PendingDeprecationWarning) + app_dirs.append(old_app_dir) + output = [] # Post-creation SQL should come before any initial SQL data is loaded. @@ -181,8 +193,10 @@ def custom_sql_for_model(model, style, connection): # Find custom SQL, if it's available. backend_name = connection.settings_dict['ENGINE'].split('.')[-1] - sql_files = [os.path.join(app_dir, "%s.%s.sql" % (opts.model_name, backend_name)), - os.path.join(app_dir, "%s.sql" % opts.model_name)] + sql_files = [] + for app_dir in app_dirs: + sql_files.append(os.path.join(app_dir, "%s.%s.sql" % (opts.model_name, backend_name))) + sql_files.append(os.path.join(app_dir, "%s.sql" % opts.model_name)) for sql_file in sql_files: if os.path.exists(sql_file): with codecs.open(sql_file, 'U', encoding=settings.FILE_CHARSET) as fp: diff --git a/django/db/models/__init__.py b/django/db/models/__init__.py index 4d310e480b..33151e068d 100644 --- a/django/db/models/__init__.py +++ b/django/db/models/__init__.py @@ -1,7 +1,7 @@ from functools import wraps from django.core.exceptions import ObjectDoesNotExist, ImproperlyConfigured -from django.db.models.loading import get_apps, get_app_paths, get_app, get_models, get_model, register_models, UnavailableApp +from django.db.models.loading import get_apps, get_app_path, get_app_paths, get_app, get_models, get_model, register_models, UnavailableApp from django.db.models.query import Q from django.db.models.expressions import F from django.db.models.manager import Manager diff --git a/django/db/models/loading.py b/django/db/models/loading.py index 7280051bd8..9a0cadaf37 100644 --- a/django/db/models/loading.py +++ b/django/db/models/loading.py @@ -154,6 +154,16 @@ class AppCache(object): return [elt[0] for elt in apps] + def _get_app_path(self, app): + if hasattr(app, '__path__'): # models/__init__.py package + app_path = app.__path__[0] + else: # models.py module + app_path = app.__file__ + return os.path.dirname(upath(app_path)) + + def get_app_path(self, app_label): + return self._get_app_path(self.get_app(app_label)) + def get_app_paths(self): """ Returns a list of paths to all installed apps. @@ -165,10 +175,7 @@ class AppCache(object): app_paths = [] for app in self.get_apps(): - if hasattr(app, '__path__'): # models/__init__.py package - app_paths.extend([upath(path) for path in app.__path__]) - else: # models.py module - app_paths.append(upath(app.__file__)) + app_paths.append(self._get_app_path(app)) return app_paths def get_app(self, app_label, emptyOK=False): @@ -321,6 +328,7 @@ cache = AppCache() # These methods were always module level, so are kept that way for backwards # compatibility. get_apps = cache.get_apps +get_app_path = cache.get_app_path get_app_paths = cache.get_app_paths get_app = cache.get_app get_app_errors = cache.get_app_errors diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index b0f5566cb3..7f93e1dc58 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -414,6 +414,10 @@ these changes. * ``django.utils.unittest`` will be removed. +* If models are organized in a package, Django will no longer look for + :ref:`initial SQL data` in ``myapp/models/sql/``. Move your + custom SQL files to ``myapp/sql/``. + 2.0 --- diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index bec24c94dc..bec5aaa12a 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -116,3 +116,12 @@ on all Python versions. Since ``unittest2`` became the standard library's :mod:`unittest` module in Python 2.7, and Django 1.7 drops support for older Python versions, this module isn't useful anymore. It has been deprecated. Use :mod:`unittest` instead. + +Custom SQL location for models package +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Previously, if models were organized in a package (``myapp/models/``) rather +than simply ``myapp/models.py``, Django would look for :ref:`initial SQL data +` in ``myapp/models/sql/``. This bug has been fixed so that Django +will search ``myapp/sql/`` as documented. The old location will continue to +work until Django 1.9. diff --git a/tests/fixtures_model_package/models/sql/book.sql b/tests/fixtures_model_package/models/sql/book.sql new file mode 100644 index 0000000000..9b3918f4d7 --- /dev/null +++ b/tests/fixtures_model_package/models/sql/book.sql @@ -0,0 +1,2 @@ +-- Deprecated search path for custom SQL -- remove in Django 1.9 +INSERT INTO fixtures_model_package_book (name) VALUES ('My Deprecated Book'); diff --git a/tests/fixtures_model_package/sql/book.sql b/tests/fixtures_model_package/sql/book.sql new file mode 100644 index 0000000000..21b1d9465b --- /dev/null +++ b/tests/fixtures_model_package/sql/book.sql @@ -0,0 +1 @@ +INSERT INTO fixtures_model_package_book (name) VALUES ('My Book'); diff --git a/tests/fixtures_model_package/tests.py b/tests/fixtures_model_package/tests.py index ad82267da3..1e22ac9833 100644 --- a/tests/fixtures_model_package/tests.py +++ b/tests/fixtures_model_package/tests.py @@ -5,6 +5,7 @@ import warnings from django.core import management from django.db import transaction from django.test import TestCase, TransactionTestCase +from django.utils.six import StringIO from .models import Article, Book @@ -110,3 +111,19 @@ class FixtureTestCase(TestCase): ], lambda a: a.headline, ) + + +class InitialSQLTests(TestCase): + + def test_custom_sql(self): + """ + #14300 -- Verify that custom_sql_for_model searches `app/sql` and not + `app/models/sql` (the old location will work until Django 1.9) + """ + out = StringIO() + management.call_command("sqlcustom", "fixtures_model_package", stdout=out) + output = out.getvalue() + self.assertTrue("INSERT INTO fixtures_model_package_book (name) " + "VALUES ('My Book')" in output) + # value from deprecated search path models/sql (remove in Django 1.9) + self.assertTrue("Deprecated Book" in output)