Merge pull request #2243 from pawelmhm/image-pipeline-2198

[MRG+1] [image & file pipeline] loading setting for user classes

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
         )

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
         )

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):

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
+        If file settings are defined but they are not defined for subclass
-        should be preserved.
+        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):

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
+        If image settings are defined but they are not defined for subclass default
-        should be preserved.
+        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()