Merge pull request #402 from goodtune/issue-302

Refactor ImproperlyConfigured to system check framework

.editorconfig

@@ -0,0 +1,20 @@
+# EditorConfig is awesome: http://EditorConfig.org
+
+root = true
+
+# Unix-style newlines with a newline ending every file
+[*]
+end_of_line = lf
+trim_trailing_whitespace = true
+insert_final_newline = true
+
+[*.{js,py}]
+charset = utf-8
+
+[*.py]
+indent_style = space
+indent_size = 4
+
+[*.{css,js,less}]
+indent_style = space
+indent_size = 2

docs/install.rst

@@ -103,7 +103,18 @@ To make use of shared and tenant-specific applications, there are two settings c
         'myapp.houses', 
     )
 
-    INSTALLED_APPS = list(SHARED_APPS) + [app for app in TENANT_APPS if app not in SHARED_APPS]
+    INSTALLED_APPS = (
+        'customers',
+        'django.contrib.contenttypes',
+        'django.contrib.auth',
+        'django.contrib.sessions',
+        'django.contrib.sites',
+        'django.contrib.messages',
+        'django.contrib.admin',
+        'myapp.hotels',
+        'myapp.houses',
+        'tenant_schemas',
+    )
 
 You also have to set where your tenant model is.
 

dts_test_project/.gitignore

@@ -0,0 +1 @@
+.coverage

dts_test_project/dts_test_project/settings.py

@@ -48,7 +48,16 @@
 
 TEST_RUNNER = 'django.test.runner.DiscoverRunner'
 
-INSTALLED_APPS = list(set(TENANT_APPS + SHARED_APPS))
+INSTALLED_APPS = (
+    'tenant_schemas',
+    'dts_test_app',
+    'customers',
+    'django.contrib.auth',
+    'django.contrib.contenttypes',
+    'django.contrib.sessions',
+    'django.contrib.messages',
+    'django.contrib.staticfiles',
+)
 
 MIDDLEWARE_CLASSES = (
     'django.contrib.sessions.middleware.SessionMiddleware',

dts_test_project/dts_test_project/urls.py

@@ -1,21 +1 @@
-"""wtf URL Configuration
-
-The `urlpatterns` list routes URLs to views. For more information please see:
-    https://docs.djangoproject.com/en/1.9/topics/http/urls/
-Examples:
-Function views
-    1. Add an import:  from my_app import views
-    2. Add a URL to urlpatterns:  url(r'^$', views.home, name='home')
-Class-based views
-    1. Add an import:  from other_app.views import Home
-    2. Add a URL to urlpatterns:  url(r'^$', Home.as_view(), name='home')
-Including another URLconf
-    1. Add an import:  from blog import urls as blog_urls
-    2. Import the include() function: from django.conf.urls import url, include
-    3. Add a URL to urlpatterns:  url(r'^blog/', include(blog_urls))
-"""
-from django.conf.urls import url
-from django.contrib import admin
-
-urlpatterns = [
-]
+urlpatterns = []

examples/tenant_tutorial/tenant_tutorial/settings.py

@@ -1,3 +1,5 @@
+import os
+
 # Django settings for tenant_tutorial project.
 
 DEBUG = True
@@ -121,7 +123,6 @@
 # Python dotted path to the WSGI application used by Django's runserver.
 WSGI_APPLICATION = 'tenant_tutorial.wsgi.application'
 
-import os
 TEMPLATE_DIRS = (os.path.join(os.path.dirname(__file__), '..', 'templates').replace('\\', '/'),)
 
 SHARED_APPS = (

examples/tenant_tutorial/tenant_tutorial/wsgi.py

@@ -14,6 +14,7 @@
 
 """
 import os
+from django.core.wsgi import get_wsgi_application
 
 # We defer to a DJANGO_SETTINGS_MODULE already in the environment. This breaks
 # if running multiple sites in the same mod_wsgi process. To fix this, use
@@ -24,7 +25,6 @@
 # This application object is used by any WSGI server configured to use this
 # file. This includes Django's development server, if the WSGI_APPLICATION
 # setting points here.
-from django.core.wsgi import get_wsgi_application
 application = get_wsgi_application()
 
 # Apply WSGI middleware here.

setup.cfg

@@ -0,0 +1,3 @@
+[flake8]
+exclude = .tox,docs,build,migrations,__init__.py
+ignore = C901,E501,E731

tenant_schemas/__init__.py

@@ -1,40 +1 @@
-from django.conf import settings
-from django.core.exceptions import ImproperlyConfigured
-
-from tenant_schemas.utils import get_public_schema_name, get_tenant_model
-
-recommended_config = """
-Warning: You should put 'tenant_schemas' at the end of INSTALLED_APPS:
-INSTALLED_APPS = TENANT_APPS + SHARED_APPS + ('tenant_schemas',)
-This is necessary to overwrite built-in django management commands with
-their schema-aware implementations.
-"""
-# Test for configuration recommendations. These are best practices,
-# they avoid hard to find bugs and unexpected behaviour.
-if not hasattr(settings, 'TENANT_APPS'):
-    raise ImproperlyConfigured('TENANT_APPS setting not set')
-
-if not settings.TENANT_APPS:
-    raise ImproperlyConfigured("TENANT_APPS is empty. "
-                               "Maybe you don't need this app?")
-
-if not hasattr(settings, 'TENANT_MODEL'):
-    raise ImproperlyConfigured('TENANT_MODEL setting not set')
-
-if 'tenant_schemas.routers.TenantSyncRouter' not in settings.DATABASE_ROUTERS:
-    raise ImproperlyConfigured("DATABASE_ROUTERS setting must contain "
-                               "'tenant_schemas.routers.TenantSyncRouter'.")
-
-if hasattr(settings, 'PG_EXTRA_SEARCH_PATHS'):
-    if get_public_schema_name() in settings.PG_EXTRA_SEARCH_PATHS:
-        raise ImproperlyConfigured(
-            "%s can not be included on PG_EXTRA_SEARCH_PATHS."
-            % get_public_schema_name())
-
-    # make sure no tenant schema is in settings.PG_EXTRA_SEARCH_PATHS
-    invalid_schemas = set(settings.PG_EXTRA_SEARCH_PATHS).intersection(
-        get_tenant_model().objects.all().values_list('schema_name', flat=True))
-    if invalid_schemas:
-        raise ImproperlyConfigured(
-            "Do not include tenant schemas (%s) on PG_EXTRA_SEARCH_PATHS."
-            % list(invalid_schemas))
+default_app_config = 'tenant_schemas.apps.TenantSchemaConfig'

tenant_schemas/apps.py

@@ -0,0 +1,85 @@
+from django.apps import AppConfig
+from django.conf import settings
+from django.core.checks import Critical, Error, Warning, register
+
+from tenant_schemas.utils import get_public_schema_name, get_tenant_model
+
+
+class TenantSchemaConfig(AppConfig):
+    name = 'tenant_schemas'
+
+
+@register('config')
+def best_practice(app_configs, **kwargs):
+    """
+    Test for configuration recommendations. These are best practices, they
+    avoid hard to find bugs and unexpected behaviour.
+    """
+    # Take the app_configs and turn them into *old style* application names.
+    # This is what we expect in the SHARED_APPS and TENANT_APPS settings.
+    INSTALLED_APPS = [
+        config.name
+        for config in app_configs
+    ]
+
+    if not hasattr(settings, 'TENANT_APPS'):
+        return [Critical('TENANT_APPS setting not set')]
+
+    if not hasattr(settings, 'TENANT_MODEL'):
+        return [Critical('TENANT_MODEL setting not set')]
+
+    if not hasattr(settings, 'SHARED_APPS'):
+        return [Critical('SHARED_APPS setting not set')]
+
+    if 'tenant_schemas.routers.TenantSyncRouter' not in settings.DATABASE_ROUTERS:
+        return [
+            Critical("DATABASE_ROUTERS setting must contain "
+                     "'tenant_schemas.routers.TenantSyncRouter'.")
+        ]
+
+    errors = []
+
+    if INSTALLED_APPS[0] != 'tenant_schemas':
+        errors.append(
+            Warning("You should put 'tenant_schemas' first in INSTALLED_APPS.",
+                    obj="django.conf.settings",
+                    hint="This is necessary to overwrite built-in django "
+                         "management commands with their schema-aware "
+                         "implementations."))
+
+    if not settings.TENANT_APPS:
+        errors.append(
+            Error("TENANT_APPS is empty.",
+                  hint="Maybe you don't need this app?"))
+
+    if hasattr(settings, 'PG_EXTRA_SEARCH_PATHS'):
+        if get_public_schema_name() in settings.PG_EXTRA_SEARCH_PATHS:
+            errors.append(Critical(
+                "%s can not be included on PG_EXTRA_SEARCH_PATHS."
+                % get_public_schema_name()))
+
+        # make sure no tenant schema is in settings.PG_EXTRA_SEARCH_PATHS
+        invalid_schemas = set(settings.PG_EXTRA_SEARCH_PATHS).intersection(
+            get_tenant_model().objects.all().values_list('schema_name', flat=True))
+        if invalid_schemas:
+            errors.append(Critical(
+                "Do not include tenant schemas (%s) on PG_EXTRA_SEARCH_PATHS."
+                % ", ".join(sorted(invalid_schemas))))
+
+    if not settings.SHARED_APPS:
+        errors.append(
+            Warning("SHARED_APPS is empty."))
+
+    if not set(settings.TENANT_APPS).issubset(INSTALLED_APPS):
+        delta = set(settings.TENANT_APPS).difference(INSTALLED_APPS)
+        errors.append(
+            Error("You have TENANT_APPS that are not in INSTALLED_APPS",
+                  hint=[a for a in settings.TENANT_APPS if a in delta]))
+
+    if not set(settings.SHARED_APPS).issubset(INSTALLED_APPS):
+        delta = set(settings.SHARED_APPS).difference(INSTALLED_APPS)
+        errors.append(
+            Error("You have SHARED_APPS that are not in INSTALLED_APPS",
+                  hint=[a for a in settings.SHARED_APPS if a in delta]))
+
+    return errors

tenant_schemas/management/commands/migrate_schemas.py

@@ -4,17 +4,16 @@
 from django.core.management.commands.migrate import Command as MigrateCommand
 from django.db import connection
 
+from tenant_schemas.management.commands import SyncCommon
+from tenant_schemas.utils import get_tenant_model, get_public_schema_name, schema_exists
+
 if django.VERSION >= (1, 9, 0):
     from django.db.migrations.exceptions import MigrationSchemaMissing
 else:
     class MigrationSchemaMissing(django.db.utils.DatabaseError):
         pass
 
 
-from tenant_schemas.management.commands import SyncCommon
-from tenant_schemas.utils import get_tenant_model, get_public_schema_name, schema_exists
-
-
 class Command(SyncCommon):
     help = "Updates database schema. Manages both apps with migrations and those without."
 

tenant_schemas/template_loaders.py

@@ -13,7 +13,7 @@
 from django.template.loaders.base import Loader as BaseLoader
 
 from tenant_schemas.postgresql_backend.base import FakeTenant
-import django
+
 
 class CachedLoader(BaseLoader):
     is_usable = True

tenant_schemas/tests/models.py

@@ -1,4 +1,3 @@
-from django.db import models
 from tenant_schemas.models import TenantMixin
 
 

tenant_schemas/tests/test_apps.py

@@ -0,0 +1,117 @@
+from django.apps import apps
+from django.core.checks import Critical, Error, Warning
+from django.test import TestCase
+from django.test.utils import override_settings
+
+from tenant_schemas.apps import best_practice
+from tenant_schemas.utils import get_tenant_model
+
+
+class AppConfigTests(TestCase):
+
+    maxDiff = None
+
+    def assertBestPractice(self, expected):
+        actual = best_practice(apps.get_app_configs())
+        self.assertEqual(expected, actual)
+
+    @override_settings()
+    def test_unset_tenant_apps(self):
+        from django.conf import settings
+        del settings.TENANT_APPS
+        self.assertBestPractice([
+            Critical('TENANT_APPS setting not set'),
+        ])
+
+    @override_settings()
+    def test_unset_tenant_model(self):
+        from django.conf import settings
+        del settings.TENANT_MODEL
+        self.assertBestPractice([
+            Critical('TENANT_MODEL setting not set'),
+        ])
+
+    @override_settings()
+    def test_unset_shared_apps(self):
+        from django.conf import settings
+        del settings.SHARED_APPS
+        self.assertBestPractice([
+            Critical('SHARED_APPS setting not set'),
+        ])
+
+    @override_settings(DATABASE_ROUTERS=())
+    def test_database_routers(self):
+        self.assertBestPractice([
+            Critical("DATABASE_ROUTERS setting must contain "
+                     "'tenant_schemas.routers.TenantSyncRouter'."),
+        ])
+
+    @override_settings(INSTALLED_APPS=[
+        'dts_test_app',
+        'customers',
+        'django.contrib.auth',
+        'django.contrib.contenttypes',
+        'tenant_schemas',
+        'django.contrib.sessions',
+        'django.contrib.messages',
+        'django.contrib.staticfiles',
+    ])
+    def test_tenant_schemas_last_installed_apps(self):
+        self.assertBestPractice([
+            Warning("You should put 'tenant_schemas' first in INSTALLED_APPS.",
+                    obj="django.conf.settings",
+                    hint="This is necessary to overwrite built-in django "
+                         "management commands with their schema-aware "
+                         "implementations."),
+        ])
+
+    @override_settings(TENANT_APPS=())
+    def test_tenant_apps_empty(self):
+        self.assertBestPractice([
+            Error("TENANT_APPS is empty.",
+                  hint="Maybe you don't need this app?"),
+        ])
+
+    @override_settings(PG_EXTRA_SEARCH_PATHS=['public', 'demo1', 'demo2'])
+    def test_public_schema_on_extra_search_paths(self):
+        TenantModel = get_tenant_model()
+        TenantModel.objects.create(
+            schema_name='demo1', domain_url='demo1.example.com')
+        TenantModel.objects.create(
+            schema_name='demo2', domain_url='demo2.example.com')
+        self.assertBestPractice([
+            Critical("public can not be included on PG_EXTRA_SEARCH_PATHS."),
+            Critical("Do not include tenant schemas (demo1, demo2) on PG_EXTRA_SEARCH_PATHS."),
+        ])
+
+    @override_settings(SHARED_APPS=())
+    def test_shared_apps_empty(self):
+        self.assertBestPractice([
+            Warning("SHARED_APPS is empty."),
+        ])
+
+    @override_settings(TENANT_APPS=(
+        'dts_test_app',
+        'django.contrib.flatpages',
+    ))
+    def test_tenant_app_missing_from_install_apps(self):
+        self.assertBestPractice([
+            Error("You have TENANT_APPS that are not in INSTALLED_APPS",
+                  hint=['django.contrib.flatpages']),
+        ])
+
+    @override_settings(SHARED_APPS=(
+        'tenant_schemas',
+        'customers',
+        'django.contrib.auth',
+        'django.contrib.contenttypes',
+        'django.contrib.flatpages',
+        'django.contrib.messages',
+        'django.contrib.sessions',
+        'django.contrib.staticfiles',
+    ))
+    def test_shared_app_missing_from_install_apps(self):
+        self.assertBestPractice([
+            Error("You have SHARED_APPS that are not in INSTALLED_APPS",
+                  hint=['django.contrib.flatpages']),
+        ])