Merge pull request #5847 from namelessGonbai/feat/FixFeedExport

Fix FeedExporter not to export empty file

docs/topics/feed-exports.rst

@@ -572,9 +572,12 @@ to ``.json`` or ``.xml``.
 FEED_STORE_EMPTY
 ----------------
 
-Default: ``False``
+Default: ``True``
 
 Whether to export empty feeds (i.e. feeds with no items).
+If ``False``, and there are no items to export, no new files are created and 
+existing files are not modified, even if the :ref:`overwrite feed option 
+<feed-options>` is enabled.
 
 .. setting:: FEED_STORAGES
 

scrapy/extensions/feedexport.py

@@ -312,8 +312,6 @@ class FTPFeedStorage(BlockingFeedStorage):
 class FeedSlot:
     def __init__(
         self,
-        file,
-        exporter,
         storage,
         uri,
         format,
@@ -321,9 +319,14 @@ class FeedSlot:
         batch_id,
         uri_template,
         filter,
+        feed_options,
+        spider,
+        exporters,
+        settings,
+        crawler,
     ):
-        self.file = file
-        self.exporter = exporter
+        self.file = None
+        self.exporter = None
         self.storage = storage
         # feed params
         self.batch_id = batch_id
@@ -332,15 +335,44 @@ class FeedSlot:
         self.uri_template = uri_template
         self.uri = uri
         self.filter = filter
+        # exporter params
+        self.feed_options = feed_options
+        self.spider = spider
+        self.exporters = exporters
+        self.settings = settings
+        self.crawler = crawler
         # flags
         self.itemcount = 0
         self._exporting = False
+        self._fileloaded = False
 
     def start_exporting(self):
+        if not self._fileloaded:
+            self.file = self.storage.open(self.spider)
+            if "postprocessing" in self.feed_options:
+                self.file = PostProcessingManager(
+                    self.feed_options["postprocessing"], self.file, self.feed_options
+                )
+            self.exporter = self._get_exporter(
+                file=self.file,
+                format=self.feed_options["format"],
+                fields_to_export=self.feed_options["fields"],
+                encoding=self.feed_options["encoding"],
+                indent=self.feed_options["indent"],
+                **self.feed_options["item_export_kwargs"],
+            )
+            self._fileloaded = True
+
         if not self._exporting:
             self.exporter.start_exporting()
             self._exporting = True
 
+    def _get_instance(self, objcls, *args, **kwargs):
+        return create_instance(objcls, self.settings, self.crawler, *args, **kwargs)
+
+    def _get_exporter(self, file, format, *args, **kwargs):
+        return self._get_instance(self.exporters[format], file, *args, **kwargs)
+
     def finish_exporting(self):
         if self._exporting:
             self.exporter.finish_exporting()
@@ -444,11 +476,16 @@ class FeedExporter:
                 return slot_.file.file
             return slot_.file
 
-        slot.finish_exporting()
-        if not slot.itemcount and not slot.store_empty:
-            # We need to call slot.storage.store nonetheless to get the file
-            # properly closed.
-            return defer.maybeDeferred(slot.storage.store, get_file(slot))
+        if slot.itemcount:
+            # Normal case
+            slot.finish_exporting()
+        elif slot.store_empty and slot.batch_id == 1:
+            # Need to store the empty file
+            slot.start_exporting()
+            slot.finish_exporting()
+        else:
+            # In this case, the file is not stored, so no processing is required.
+            return None
 
         logmsg = f"{slot.format} feed ({slot.itemcount} items) in: {slot.uri}"
         d = defer.maybeDeferred(slot.storage.store, get_file(slot))
@@ -493,23 +530,7 @@ class FeedExporter:
         :param uri_template: template of uri which contains %(batch_time)s or %(batch_id)d to create new uri
         """
         storage = self._get_storage(uri, feed_options)
-        file = storage.open(spider)
-        if "postprocessing" in feed_options:
-            file = PostProcessingManager(
-                feed_options["postprocessing"], file, feed_options
-            )
-
-        exporter = self._get_exporter(
-            file=file,
-            format=feed_options["format"],
-            fields_to_export=feed_options["fields"],
-            encoding=feed_options["encoding"],
-            indent=feed_options["indent"],
-            **feed_options["item_export_kwargs"],
-        )
         slot = FeedSlot(
-            file=file,
-            exporter=exporter,
             storage=storage,
             uri=uri,
             format=feed_options["format"],
@@ -517,9 +538,12 @@ class FeedExporter:
             batch_id=batch_id,
             uri_template=uri_template,
             filter=self.filters[uri_template],
+            feed_options=feed_options,
+            spider=spider,
+            exporters=self.exporters,
+            settings=self.settings,
+            crawler=getattr(self, "crawler", None),
         )
-        if slot.store_empty:
-            slot.start_exporting()
         return slot
 
     def item_scraped(self, item, spider):
@@ -603,14 +627,6 @@ class FeedExporter:
         else:
             logger.error("Unknown feed storage scheme: %(scheme)s", {"scheme": scheme})
 
-    def _get_instance(self, objcls, *args, **kwargs):
-        return create_instance(
-            objcls, self.settings, getattr(self, "crawler", None), *args, **kwargs
-        )
-
-    def _get_exporter(self, file, format, *args, **kwargs):
-        return self._get_instance(self.exporters[format], file, *args, **kwargs)
-
     def _get_storage(self, uri, feed_options):
         """Fork of create_instance specific to feed storage classes
 

scrapy/settings/default_settings.py

@@ -141,7 +141,7 @@ EXTENSIONS_BASE = {
 FEED_TEMPDIR = None
 FEEDS = {}
 FEED_URI_PARAMS = None  # a function to extend uri arguments
-FEED_STORE_EMPTY = False
+FEED_STORE_EMPTY = True
 FEED_EXPORT_ENCODING = None
 FEED_EXPORT_FIELDS = None
 FEED_STORAGES = {}

tests/test_feedexport.py

@@ -747,10 +747,9 @@ class FeedExportTest(FeedExportTestBase):
                 yield crawler.crawl()
 
             for file_path, feed_options in FEEDS.items():
-                if not Path(file_path).exists():
-                    continue
-
-                content[feed_options["format"]] = Path(file_path).read_bytes()
+                content[feed_options["format"]] = (
+                    Path(file_path).read_bytes() if Path(file_path).exists() else None
+                )
 
         finally:
             for file_path in FEEDS.keys():
@@ -956,9 +955,10 @@ class FeedExportTest(FeedExportTestBase):
                 "FEEDS": {
                     self._random_temp_filename(): {"format": fmt},
                 },
+                "FEED_STORE_EMPTY": False,
             }
             data = yield self.exported_no_data(settings)
-            self.assertEqual(b"", data[fmt])
+            self.assertEqual(None, data[fmt])
 
     @defer.inlineCallbacks
     def test_start_finish_exporting_items(self):
@@ -1074,8 +1074,7 @@ class FeedExportTest(FeedExportTestBase):
         with LogCapture() as log:
             yield self.exported_no_data(settings)
 
-        print(log)
-        self.assertEqual(str(log).count("Storage.store is called"), 3)
+        self.assertEqual(str(log).count("Storage.store is called"), 0)
 
     @defer.inlineCallbacks
     def test_export_multiple_item_classes(self):
@@ -1742,10 +1741,9 @@ class FeedPostProcessedExportsTest(FeedExportTestBase):
                 yield crawler.crawl()
 
             for file_path, feed_options in FEEDS.items():
-                if not Path(file_path).exists():
-                    continue
-
-                content[str(file_path)] = Path(file_path).read_bytes()
+                content[str(file_path)] = (
+                    Path(file_path).read_bytes() if Path(file_path).exists() else None
+                )
 
         finally:
             for file_path in FEEDS.keys():
@@ -2246,6 +2244,9 @@ class BatchDeliveriesTest(FeedExportTestBase):
 
             for path, feed in FEEDS.items():
                 dir_name = Path(path).parent
+                if not dir_name.exists():
+                    content[feed["format"]] = []
+                    continue
                 for file in sorted(dir_name.iterdir()):
                     content[feed["format"]].append(file.read_bytes())
         finally:
@@ -2429,10 +2430,11 @@ class BatchDeliveriesTest(FeedExportTestBase):
                     / self._file_mark: {"format": fmt},
                 },
                 "FEED_EXPORT_BATCH_ITEM_COUNT": 1,
+                "FEED_STORE_EMPTY": False,
             }
             data = yield self.exported_no_data(settings)
             data = dict(data)
-            self.assertEqual(b"", data[fmt][0])
+            self.assertEqual(0, len(data[fmt]))
 
     @defer.inlineCallbacks
     def test_export_no_items_store_empty(self):
@@ -2546,9 +2548,6 @@ class BatchDeliveriesTest(FeedExportTestBase):
             for expected_batch, got_batch in zip(expected, data[fmt]):
                 self.assertEqual(expected_batch, got_batch)
 
-    @pytest.mark.skipif(
-        sys.platform == "win32", reason="Odd behaviour on file creation/output"
-    )
     @defer.inlineCallbacks
     def test_batch_path_differ(self):
         """
@@ -2570,7 +2569,7 @@ class BatchDeliveriesTest(FeedExportTestBase):
             "FEED_EXPORT_BATCH_ITEM_COUNT": 1,
         }
         data = yield self.exported_data(items, settings)
-        self.assertEqual(len(items), len([_ for _ in data["json"] if _]))
+        self.assertEqual(len(items), len(data["json"]))
 
     @defer.inlineCallbacks
     def test_stats_batch_file_success(self):
@@ -2656,7 +2655,7 @@ class BatchDeliveriesTest(FeedExportTestBase):
             crawler = get_crawler(TestSpider, settings)
             yield crawler.crawl()
 
-        self.assertEqual(len(CustomS3FeedStorage.stubs), len(items) + 1)
+        self.assertEqual(len(CustomS3FeedStorage.stubs), len(items))
         for stub in CustomS3FeedStorage.stubs[:-1]:
             stub.assert_no_pending_responses()