From e979787128e3c959eb3246594853236517753905 Mon Sep 17 00:00:00 2001 From: Anton Agestam Date: Sun, 27 Oct 2019 20:17:23 +0100 Subject: [PATCH] fix #159: don't load strategy when disabled --- collectfast/__init__.py | 2 +- .../management/commands/collectstatic.py | 25 ++++++---- collectfast/strategies/__init__.py | 3 +- collectfast/strategies/base.py | 11 +++++ collectfast/tests/command/__init__.py | 0 .../tests/{ => command}/test_command.py | 45 ++++-------------- collectfast/tests/command/test_disable.py | 46 +++++++++++++++++++ collectfast/tests/command/utils.py | 13 ++++++ 8 files changed, 98 insertions(+), 47 deletions(-) create mode 100644 collectfast/tests/command/__init__.py rename collectfast/tests/{ => command}/test_command.py (76%) create mode 100644 collectfast/tests/command/test_disable.py create mode 100644 collectfast/tests/command/utils.py diff --git a/collectfast/__init__.py b/collectfast/__init__.py index 67bc602..9c73af2 100644 --- a/collectfast/__init__.py +++ b/collectfast/__init__.py @@ -1 +1 @@ -__version__ = "1.3.0" +__version__ = "1.3.1" diff --git a/collectfast/management/commands/collectstatic.py b/collectfast/management/commands/collectstatic.py index 040b4bd..d7a4433 100644 --- a/collectfast/management/commands/collectstatic.py +++ b/collectfast/management/commands/collectstatic.py @@ -2,6 +2,7 @@ from typing import Any from typing import Dict from typing import List +from typing import Optional from typing import Tuple from typing import Type @@ -13,6 +14,7 @@ from collectfast import __version__ from collectfast import settings +from collectfast.strategies import DisabledStrategy from collectfast.strategies import guess_strategy from collectfast.strategies import load_strategy from collectfast.strategies import Strategy @@ -27,7 +29,7 @@ def __init__(self, *args, **kwargs): self.num_copied_files = 0 self.tasks = [] # type: List[Task] self.collectfast_enabled = settings.enabled - self.strategy = self._load_strategy()(self.storage) + self.strategy = DisabledStrategy(Storage()) # type: Strategy @staticmethod def _load_strategy(): @@ -62,8 +64,11 @@ def add_arguments(self, parser): def set_options(self, **options): # type: (Any) -> None """Set options and handle deprecation.""" - disable = options.pop("disable_collectfast", False) - self.collectfast_enabled = not disable + self.collectfast_enabled = self.collectfast_enabled and not options.pop( + "disable_collectfast" + ) + if self.collectfast_enabled: + self.strategy = self._load_strategy()(self.storage) super().set_options(**options) def collect(self): @@ -73,14 +78,18 @@ def collect(self): Command.copy_file() which is called by super().collect(). """ ret = super().collect() + if not self.collectfast_enabled: + return ret if settings.threads: Pool(settings.threads).map(self.maybe_copy_file, self.tasks) return ret def handle(self, *args, **options): - # type: (Any, Any) -> str + # type: (Any, Any) -> Optional[str] """Override handle to suppress summary output.""" - super().handle(**options) + ret = super().handle(**options) + if not self.collectfast_enabled: + return ret return "{} static file{} copied.".format( self.num_copied_files, "" if self.num_copied_files == 1 else "s" ) @@ -90,9 +99,9 @@ def maybe_copy_file(self, args): """Determine if file should be copied or not and handle exceptions.""" path, prefixed_path, source_storage = args - self.strategy.pre_should_copy_hook() - if self.collectfast_enabled and not self.dry_run: + self.strategy.pre_should_copy_hook() + if not self.strategy.should_copy_file(path, prefixed_path, source_storage): self.log("Skipping '%s'" % path) return @@ -107,7 +116,7 @@ def copy_file(self, path, prefixed_path, source_storage): file with a blocking call. """ args = (path, prefixed_path, source_storage) - if settings.threads: + if settings.threads and self.collectfast_enabled: self.tasks.append(args) else: self.maybe_copy_file(args) diff --git a/collectfast/strategies/__init__.py b/collectfast/strategies/__init__.py index 9d52a9d..4d93b5e 100644 --- a/collectfast/strategies/__init__.py +++ b/collectfast/strategies/__init__.py @@ -1,5 +1,6 @@ +from .base import DisabledStrategy from .base import guess_strategy from .base import load_strategy from .base import Strategy -__all__ = ("load_strategy", "guess_strategy", "Strategy") +__all__ = ("load_strategy", "guess_strategy", "Strategy", "DisabledStrategy") diff --git a/collectfast/strategies/base.py b/collectfast/strategies/base.py index 1705b3f..198e2d4 100644 --- a/collectfast/strategies/base.py +++ b/collectfast/strategies/base.py @@ -9,6 +9,7 @@ from io import BytesIO from typing import ClassVar from typing import Generic +from typing import NoReturn from typing import Optional from typing import Tuple from typing import Type @@ -138,6 +139,16 @@ def get_gzipped_local_file_hash(self, uncompressed_file_hash, path, contents): return str(file_hash) +class DisabledStrategy(Strategy): + def should_copy_file(self, path, prefixed_path, local_storage): + # type: (str, str, Storage) -> NoReturn + raise NotImplementedError + + def pre_should_copy_hook(self): + # type: () -> NoReturn + raise NotImplementedError + + def load_strategy(klass: Union[str, type, object]) -> Type[Strategy[Storage]]: if isinstance(klass, str): klass = pydoc.locate(klass) diff --git a/collectfast/tests/command/__init__.py b/collectfast/tests/command/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/collectfast/tests/test_command.py b/collectfast/tests/command/test_command.py similarity index 76% rename from collectfast/tests/test_command.py rename to collectfast/tests/command/test_command.py index c63908d..002f1a9 100644 --- a/collectfast/tests/test_command.py +++ b/collectfast/tests/command/test_command.py @@ -1,18 +1,17 @@ -from io import StringIO -from typing import Any from unittest import TestCase from django.core.exceptions import ImproperlyConfigured -from django.core.management import call_command from django.test import override_settings as override_django_settings -from .utils import clean_static_dir -from .utils import create_static_file -from .utils import make_test -from .utils import override_setting -from .utils import override_storage_attr -from .utils import test_many +from .utils import call_collectstatic from collectfast.management.commands.collectstatic import Command +from collectfast.tests.utils import clean_static_dir +from collectfast.tests.utils import create_static_file +from collectfast.tests.utils import make_test +from collectfast.tests.utils import override_setting +from collectfast.tests.utils import override_storage_attr +from collectfast.tests.utils import test_many + aws_backend_confs = { "boto3": override_django_settings( @@ -43,15 +42,6 @@ make_test_all_backends = test_many(**all_backend_confs) -def call_collectstatic(*args, **kwargs): - # type: (Any, Any) -> str - out = StringIO() - call_command( - "collectstatic", *args, verbosity=3, interactive=False, stdout=out, **kwargs - ) - return out.getvalue() - - @make_test_all_backends def test_basics(case): # type: (TestCase) -> None @@ -73,25 +63,6 @@ def test_threads(case): case.assertIn("0 static files copied.", call_collectstatic()) -@make_test -@override_django_settings( - STATICFILES_STORAGE="django.contrib.staticfiles.storage.StaticFilesStorage" -) -def test_disable_collectfast_with_default_storage(case): - # type: (TestCase) -> None - clean_static_dir() - create_static_file() - case.assertIn("1 static file copied.", call_collectstatic(disable_collectfast=True)) - - -@make_test -def test_disable_collectfast(case): - # type: (TestCase) -> None - clean_static_dir() - create_static_file() - case.assertIn("1 static file copied.", call_collectstatic(disable_collectfast=True)) - - @make_test def test_dry_run(case): # type: (TestCase) -> None diff --git a/collectfast/tests/command/test_disable.py b/collectfast/tests/command/test_disable.py new file mode 100644 index 0000000..6a04820 --- /dev/null +++ b/collectfast/tests/command/test_disable.py @@ -0,0 +1,46 @@ +from unittest import mock +from unittest import TestCase + +from django.test import override_settings as override_django_settings + +from .utils import call_collectstatic +from collectfast.tests.utils import clean_static_dir +from collectfast.tests.utils import create_static_file +from collectfast.tests.utils import make_test +from collectfast.tests.utils import override_setting + + +@make_test +@override_django_settings( + STATICFILES_STORAGE="django.contrib.staticfiles.storage.StaticFilesStorage" +) +def test_disable_collectfast_with_default_storage(case): + # type: (TestCase) -> None + clean_static_dir() + create_static_file() + case.assertIn("1 static file copied", call_collectstatic(disable_collectfast=True)) + + +@make_test +def test_disable_collectfast(case): + # type: (TestCase) -> None + clean_static_dir() + create_static_file() + case.assertIn("1 static file copied.", call_collectstatic(disable_collectfast=True)) + + +@override_setting("enabled", False) +@mock.patch("collectfast.management.commands.collectstatic.Command._load_strategy") +def test_no_load_with_disable_setting(mocked_load_strategy): + # type: (mock.MagicMock) -> None + clean_static_dir() + call_collectstatic() + mocked_load_strategy.assert_not_called() + + +@mock.patch("collectfast.management.commands.collectstatic.Command._load_strategy") +def test_no_load_with_disable_flag(mocked_load_strategy): + # type: (mock.MagicMock) -> None + clean_static_dir() + call_collectstatic(disable_collectfast=True) + mocked_load_strategy.assert_not_called() diff --git a/collectfast/tests/command/utils.py b/collectfast/tests/command/utils.py new file mode 100644 index 0000000..64061e7 --- /dev/null +++ b/collectfast/tests/command/utils.py @@ -0,0 +1,13 @@ +from io import StringIO +from typing import Any + +from django.core.management import call_command + + +def call_collectstatic(*args, **kwargs): + # type: (Any, Any) -> str + out = StringIO() + call_command( + "collectstatic", *args, verbosity=3, interactive=False, stdout=out, **kwargs + ) + return out.getvalue()