From 02f26f55ce54e99e5fc67fe8a36e73c18985743f Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 19 Sep 2017 17:55:14 -0400 Subject: [PATCH] Remove unused imports. Push model imports down into relevant methods. Mock out the static_replace modules in the proper location. --- common/djangoapps/static_replace/__init__.py | 5 ++--- .../static_replace/test/test_static_replace.py | 14 +++++++------- common/lib/xmodule/xmodule/modulestore/django.py | 4 +++- lms/djangoapps/branding/__init__.py | 4 +++- lms/djangoapps/commerce/signals.py | 11 ++++++++--- openedx/core/djangoapps/coursegraph/tasks.py | 9 +++++++-- openedx/core/djangoapps/credit/signals.py | 1 - .../core/djangoapps/site_configuration/helpers.py | 7 ++++++- openedx/core/djangoapps/waffle_utils/__init__.py | 11 +++++++---- openedx/core/djangolib/testing/utils.py | 3 ++- 10 files changed, 45 insertions(+), 24 deletions(-) diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index a4a63430f5..3fb2b0c9a0 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -5,9 +5,6 @@ from django.contrib.staticfiles.storage import staticfiles_storage from django.contrib.staticfiles import finders from django.conf import settings -from static_replace.models import AssetBaseUrlConfig, AssetExcludedExtensionsConfig -from xmodule.modulestore.django import modulestore -from xmodule.modulestore import ModuleStoreEnum from xmodule.contentstore.content import StaticContent from opaque_keys.edx.locator import AssetLocator @@ -191,6 +188,8 @@ def replace_static_urls(text, data_directory=None, course_id=None, static_asset_ else: # if not, then assume it's courseware specific content and then look in the # Mongo-backed database + # Import is placed here to avoid model import at project startup. + from static_replace.models import AssetBaseUrlConfig, AssetExcludedExtensionsConfig base_url = AssetBaseUrlConfig.get_base_url() excluded_exts = AssetExcludedExtensionsConfig.get_excluded_extensions() url = StaticContent.get_canonicalized_asset_path(course_id, rest, base_url, excluded_exts) diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index 802571a777..759e50abad 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -116,9 +116,9 @@ def test_storage_url_not_exists(mock_storage): @patch('static_replace.StaticContent', autospec=True) -@patch('static_replace.modulestore', autospec=True) -@patch('static_replace.AssetBaseUrlConfig.get_base_url') -@patch('static_replace.AssetExcludedExtensionsConfig.get_excluded_extensions') +@patch('xmodule.modulestore.django.modulestore', autospec=True) +@patch('static_replace.models.AssetBaseUrlConfig.get_base_url') +@patch('static_replace.models.AssetExcludedExtensionsConfig.get_excluded_extensions') def test_mongo_filestore(mock_get_excluded_extensions, mock_get_base_url, mock_modulestore, mock_static_content): mock_modulestore.return_value = Mock(MongoModuleStore) @@ -139,7 +139,7 @@ def test_mongo_filestore(mock_get_excluded_extensions, mock_get_base_url, mock_m @patch('static_replace.settings', autospec=True) -@patch('static_replace.modulestore', autospec=True) +@patch('xmodule.modulestore.django.modulestore', autospec=True) @patch('static_replace.staticfiles_storage', autospec=True) def test_data_dir_fallback(mock_storage, mock_modulestore, mock_settings): mock_modulestore.return_value = Mock(XMLModuleStore) @@ -165,7 +165,7 @@ def test_raw_static_check(): @pytest.mark.django_db @patch('static_replace.staticfiles_storage', autospec=True) -@patch('static_replace.modulestore', autospec=True) +@patch('xmodule.modulestore.django.modulestore', autospec=True) def test_static_url_with_query(mock_modulestore, mock_storage): """ Make sure that for urls with query params: @@ -201,7 +201,7 @@ def test_regex(): @patch('static_replace.staticfiles_storage', autospec=True) -@patch('static_replace.modulestore', autospec=True) +@patch('xmodule.modulestore.django.modulestore', autospec=True) def test_static_url_with_xblock_resource(mock_modulestore, mock_storage): """ Make sure that for URLs with XBlock resource URL, which start with /static/, @@ -216,7 +216,7 @@ def test_static_url_with_xblock_resource(mock_modulestore, mock_storage): @patch('static_replace.staticfiles_storage', autospec=True) -@patch('static_replace.modulestore', autospec=True) +@patch('xmodule.modulestore.django.modulestore', autospec=True) @override_settings(STATIC_URL='https://example.com/static/') def test_static_url_with_xblock_resource_on_cdn(mock_modulestore, mock_storage): """ diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 094890c194..45a8b8b987 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -29,7 +29,6 @@ from xmodule.contentstore.django import contentstore from xmodule.modulestore.draft_and_published import BranchSettingMixin from xmodule.modulestore.mixed import MixedModuleStore from xmodule.util.django import get_current_request_hostname -import xblock.reference.plugins try: # We may not always have the request_cache module available @@ -245,6 +244,9 @@ def create_modulestore_instance( """ This will return a new instance of a modulestore given an engine and options """ + # Import is placed here to avoid model import at project startup. + import xblock.reference.plugins + class_ = load_function(engine) _options = {} diff --git a/lms/djangoapps/branding/__init__.py b/lms/djangoapps/branding/__init__.py index b8d909af53..534c0c4faf 100644 --- a/lms/djangoapps/branding/__init__.py +++ b/lms/djangoapps/branding/__init__.py @@ -10,7 +10,6 @@ such as the site visible courses, university name and logo. from django.conf import settings from opaque_keys.edx.keys import CourseKey -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers @@ -25,6 +24,9 @@ def get_visible_courses(org=None, filter_=None): filter_ (dict): Optional parameter that allows custom filtering by fields on the course. """ + # Import is placed here to avoid model import at project startup. + from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + courses = [] current_site_orgs = configuration_helpers.get_current_site_orgs() diff --git a/lms/djangoapps/commerce/signals.py b/lms/djangoapps/commerce/signals.py index 58e7ffe712..25cb10ebc1 100644 --- a/lms/djangoapps/commerce/signals.py +++ b/lms/djangoapps/commerce/signals.py @@ -10,12 +10,9 @@ from urlparse import urljoin import requests from django.conf import settings from django.contrib.auth import get_user_model -from django.contrib.auth.models import AnonymousUser from django.dispatch import receiver from django.utils.translation import ugettext as _ -from commerce.models import CommerceConfiguration -from openedx.core.djangoapps.commerce.utils import ecommerce_api_client, is_commerce_service_configured from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming import helpers as theming_helpers from request_cache.middleware import RequestCache @@ -31,6 +28,10 @@ def handle_refund_order(sender, course_enrollment=None, **kwargs): Signal receiver for unenrollments, used to automatically initiate refunds when applicable. """ + # Import is placed here to avoid model import at project startup. + from django.contrib.auth.models import AnonymousUser + from openedx.core.djangoapps.commerce.utils import is_commerce_service_configured + if not is_commerce_service_configured(): return @@ -83,6 +84,10 @@ def refund_seat(course_enrollment): exceptions.SlumberBaseException: for any unhandled HTTP error during communication with the E-Commerce Service. exceptions.Timeout: if the attempt to reach the commerce service timed out. """ + # Import is placed here to avoid model import at project startup. + from commerce.models import CommerceConfiguration + from openedx.core.djangoapps.commerce.utils import ecommerce_api_client + User = get_user_model() # pylint:disable=invalid-name course_key_str = unicode(course_enrollment.course_id) enrollee = course_enrollment.user diff --git a/openedx/core/djangoapps/coursegraph/tasks.py b/openedx/core/djangoapps/coursegraph/tasks.py index 9138612ae7..ee73e25c35 100644 --- a/openedx/core/djangoapps/coursegraph/tasks.py +++ b/openedx/core/djangoapps/coursegraph/tasks.py @@ -13,10 +13,8 @@ from opaque_keys.edx.keys import CourseKey from py2neo import Graph, Node, Relationship, authenticate, NodeSelector from py2neo.compat import integer, string, unicode as neo4j_unicode from request_cache.middleware import RequestCache -from xmodule.modulestore.django import modulestore from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES -from openedx.core.djangoapps.content.course_structures.models import CourseStructure log = logging.getLogger(__name__) celery_log = logging.getLogger('edx.celery.task') @@ -135,6 +133,8 @@ def get_course_last_published(course_key): text, or None, if there's no record of the last time this course was published. """ + # Import is placed here to avoid model import at project startup. + from openedx.core.djangoapps.content.course_structures.models import CourseStructure try: structure = CourseStructure.objects.get(course_id=course_key) course_last_published_date = six.text_type(structure.modified) @@ -154,6 +154,9 @@ def serialize_course(course_id): nodes: a list of py2neo Node objects relationships: a list of py2neo Relationships objects """ + # Import is placed here to avoid model import at project startup. + from xmodule.modulestore.django import modulestore + # create a location to node mapping we'll need later for # writing relationships location_to_node = {} @@ -292,6 +295,8 @@ class ModuleStoreSerializer(object): For example, ["course-v1:org+course+run"]. skip: Also a list of string serializations of course keys. """ + # Import is placed here to avoid model import at project startup. + from xmodule.modulestore.django import modulestore if courses: course_keys = [CourseKey.from_string(course.strip()) for course in courses] else: diff --git a/openedx/core/djangoapps/credit/signals.py b/openedx/core/djangoapps/credit/signals.py index 0a3e5e2af8..20872871e6 100644 --- a/openedx/core/djangoapps/credit/signals.py +++ b/openedx/core/djangoapps/credit/signals.py @@ -9,7 +9,6 @@ from django.utils import timezone from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED -from xmodule.modulestore.django import SignalHandler log = logging.getLogger(__name__) diff --git a/openedx/core/djangoapps/site_configuration/helpers.py b/openedx/core/djangoapps/site_configuration/helpers.py index 559cc36f3d..fc6e0084be 100644 --- a/openedx/core/djangoapps/site_configuration/helpers.py +++ b/openedx/core/djangoapps/site_configuration/helpers.py @@ -4,7 +4,6 @@ Helpers methods for site configuration. from django.conf import settings from microsite_configuration import microsite -from openedx.core.djangoapps.site_configuration.models import SiteConfiguration def get_current_site_configuration(): @@ -20,6 +19,8 @@ def get_current_site_configuration(): from openedx.core.djangoapps.theming.helpers import get_current_site site = get_current_site() + # Import is placed here to avoid model import at project startup. + from openedx.core.djangoapps.site_configuration.models import SiteConfiguration try: return getattr(site, "configuration", None) except SiteConfiguration.DoesNotExist: @@ -183,6 +184,8 @@ def get_value_for_org(org, val_name, default=None): """ # Here we first look for the asked org inside site configuration, and if org is not present in site configuration # then we go ahead and look it inside microsite configuration. + # Import is placed here to avoid model import at project startup. + from openedx.core.djangoapps.site_configuration.models import SiteConfiguration if SiteConfiguration.has_org(org): return SiteConfiguration.get_value_for_org(org, val_name, default) else: @@ -212,6 +215,8 @@ def get_all_orgs(): Returns: A list of all organizations present in either microsite configuration or site configuration. """ + # Import is placed here to avoid model import at project startup. + from openedx.core.djangoapps.site_configuration.models import SiteConfiguration site_configuration_orgs = SiteConfiguration.get_all_orgs() microsite_orgs = microsite.get_all_orgs() diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index 30a8460e8b..c0a8b2da64 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -51,10 +51,6 @@ from contextlib import contextmanager from opaque_keys.edx.keys import CourseKey from request_cache import get_cache as get_request_cache, get_request from waffle import flag_is_active, switch_is_active -from waffle.models import Flag -from waffle.testutils import override_switch as waffle_override_switch - -from .models import WaffleFlagCourseOverrideModel log = logging.getLogger(__name__) @@ -154,6 +150,8 @@ class WaffleSwitchNamespace(WaffleNamespace): contextmanager. Note: The value is overridden in the model, not the request cache. """ + # Import is placed here to avoid model import at project startup. + from waffle.testutils import override_switch as waffle_override_switch namespaced_switch_name = self._namespaced_name(switch_name) with waffle_override_switch(namespaced_switch_name, active): log.info(u"%sSwitch '%s' set to %s in model.", self.log_prefix, namespaced_switch_name, active) @@ -206,6 +204,9 @@ class WaffleFlagNamespace(WaffleNamespace): flag_undefined_default (Boolean): A default value to be returned if the waffle flag is to be checked, but doesn't exist. """ + # Import is placed here to avoid model import at project startup. + from waffle.models import Flag + # validate arguments namespaced_flag_name = self._namespaced_name(flag_name) value = None @@ -295,6 +296,8 @@ class CourseWaffleFlag(WaffleFlag): namespaced_flag_name (String): A namespaced version of the flag to check. """ + # Import is placed here to avoid model import at project startup. + from .models import WaffleFlagCourseOverrideModel cache_key = u'{}.{}'.format(namespaced_flag_name, unicode(course_key)) force_override = self.waffle_namespace._cached_flags.get(cache_key) diff --git a/openedx/core/djangolib/testing/utils.py b/openedx/core/djangolib/testing/utils.py index dc734c033a..29278d6d02 100644 --- a/openedx/core/djangolib/testing/utils.py +++ b/openedx/core/djangolib/testing/utils.py @@ -16,7 +16,6 @@ import crum from django import db from django.conf import settings from django.contrib import sites -from django.contrib.auth.models import AnonymousUser from django.core.cache import caches from django.db import DEFAULT_DB_ALIAS, connections from django.test import RequestFactory, TestCase, override_settings @@ -244,6 +243,8 @@ def get_mock_request(user=None): """ Create a request object for the user, if specified. """ + # Import is placed here to avoid model import at project startup. + from django.contrib.auth.models import AnonymousUser request = RequestFactory().get('/') if user is not None: request.user = user