diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 8cdc548f6..843b4d3ec 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -233,7 +233,8 @@ class FilesPipeline(MediaPipeline): cls_name = "FilesPipeline" self.store = self._get_store(store_uri) resolve = functools.partial(self._key_for_pipe, - base_class_name=cls_name) + base_class_name=cls_name, + settings=settings) self.expires = settings.getint( resolve('FILES_EXPIRES'), self.EXPIRES ) diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index af5825c0b..5796bfb80 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -55,7 +55,8 @@ class ImagesPipeline(FilesPipeline): settings = Settings(settings) resolve = functools.partial(self._key_for_pipe, - base_class_name="ImagesPipeline") + base_class_name="ImagesPipeline", + settings=settings) self.expires = settings.getint( resolve("IMAGES_EXPIRES"), self.EXPIRES ) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 82b4b462e..57f70499e 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -28,7 +28,8 @@ class MediaPipeline(object): self.download_func = download_func - def _key_for_pipe(self, key, base_class_name=None): + def _key_for_pipe(self, key, base_class_name=None, + settings=None): """ >>> MediaPipeline()._key_for_pipe("IMAGES") 'IMAGES' @@ -38,9 +39,11 @@ class MediaPipeline(object): 'MYPIPE_IMAGES' """ class_name = self.__class__.__name__ - if class_name == base_class_name or not base_class_name: + formatted_key = "{}_{}".format(class_name.upper(), key) + if class_name == base_class_name or not base_class_name \ + or (settings and not settings.get(formatted_key)): return key - return "{}_{}".format(class_name.upper(), key) + return formatted_key @classmethod def from_crawler(cls, crawler): diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index bda2a2199..157c21a89 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -255,16 +255,17 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): def test_subclass_attrs_preserved_custom_settings(self): """ - If file settings are defined but they are not defined for subclass class attributes - should be preserved. + If file settings are defined but they are not defined for subclass + settings should be preserved. """ pipeline_cls = self._generate_fake_pipeline() settings = self._generate_fake_settings() pipeline = pipeline_cls.from_settings(Settings(settings)) for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map: value = getattr(pipeline, pipe_ins_attr) + setting_value = settings.get(settings_attr) self.assertNotEqual(value, self.default_cls_settings[pipe_attr]) - self.assertEqual(value, getattr(pipeline, pipe_attr)) + self.assertEqual(value, setting_value) def test_no_custom_settings_for_subclasses(self): """ @@ -321,6 +322,24 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase): self.assertEqual(pipeline.files_urls_field, "that") + def test_user_defined_subclass_default_key_names(self): + """Test situation when user defines subclass of FilesPipeline, + but uses attribute names for default pipeline (without prefixing + them with pipeline class name). + """ + settings = self._generate_fake_settings() + + class UserPipe(FilesPipeline): + pass + + pipeline_cls = UserPipe.from_settings(Settings(settings)) + + for pipe_attr, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map: + expected_value = settings.get(settings_attr) + self.assertEqual(getattr(pipeline_cls, pipe_inst_attr), + expected_value) + + class TestS3FilesStore(unittest.TestCase): @defer.inlineCallbacks def test_persist(self): diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 8286582de..6c1976b63 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -309,17 +309,19 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): def test_subclass_attrs_preserved_custom_settings(self): """ - If image settings are defined but they are not defined for subclass class attributes - should be preserved. + If image settings are defined but they are not defined for subclass default + values taken from settings should be preserved. """ pipeline_cls = self._generate_fake_pipeline_subclass() settings = self._generate_fake_settings() pipeline = pipeline_cls.from_settings(Settings(settings)) for pipe_attr, settings_attr in self.img_cls_attribute_names: - # Instance attribute (lowercase) must be equal to class attribute (uppercase). + # Instance attribute (lowercase) must be equal to + # value defined in settings. value = getattr(pipeline, pipe_attr.lower()) self.assertNotEqual(value, self.default_pipeline_settings[pipe_attr]) - self.assertEqual(value, getattr(pipeline, pipe_attr)) + setings_value = settings.get(settings_attr) + self.assertEqual(value, setings_value) def test_no_custom_settings_for_subclasses(self): """ @@ -370,11 +372,26 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase): class UserDefinedImagePipeline(ImagesPipeline): DEFAULT_IMAGES_URLS_FIELD = "something" DEFAULT_IMAGES_RESULT_FIELD = "something_else" - pipeline = UserDefinedImagePipeline.from_settings(Settings({"IMAGES_STORE": self.tempdir})) self.assertEqual(pipeline.images_result_field, "something_else") self.assertEqual(pipeline.images_urls_field, "something") + def test_user_defined_subclass_default_key_names(self): + """Test situation when user defines subclass of ImagePipeline, + but uses attribute names for default pipeline (without prefixing + them with pipeline class name). + """ + settings = self._generate_fake_settings() + + class UserPipe(ImagesPipeline): + pass + + pipeline_cls = UserPipe.from_settings(Settings(settings)) + + for pipe_attr, settings_attr in self.img_cls_attribute_names: + expected_value = settings.get(settings_attr) + self.assertEqual(getattr(pipeline_cls, pipe_attr.lower()), + expected_value) def _create_image(format, *a, **kw): buf = TemporaryFile()