Merge pull request #5758 from MattyMay/finish_exporting_for_each_start_exporting

Finish exporting for each start exporting

scrapy/extensions/feedexport.py

@@ -350,11 +350,11 @@ class FeedExporter:
         return defer.DeferredList(deferred_list) if deferred_list else None
 
     def _close_slot(self, slot, spider):
+        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, slot.file)
-        slot.finish_exporting()
         logmsg = f"{slot.format} feed ({slot.itemcount} items) in: {slot.uri}"
         d = defer.maybeDeferred(slot.storage.store, slot.file)
 

tests/test_feedexport.py

@@ -33,8 +33,9 @@ from zope.interface.verify import verifyObject
 
 import scrapy
 from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning
-from scrapy.exporters import CsvItemExporter
+from scrapy.exporters import CsvItemExporter, JsonItemExporter
 from scrapy.extensions.feedexport import (
+    _FeedSlot,
     BlockingFeedStorage,
     FeedExporter,
     FileFeedStorage,
@@ -680,6 +681,47 @@ class FeedExportTestBase(ABC, unittest.TestCase):
         return result
 
 
+class InstrumentedFeedSlot(_FeedSlot):
+    """Instrumented _FeedSlot subclass for keeping track of calls to
+    start_exporting and finish_exporting."""
+    def start_exporting(self):
+        self.update_listener('start')
+        super().start_exporting()
+
+    def finish_exporting(self):
+        self.update_listener('finish')
+        super().finish_exporting()
+
+    @classmethod
+    def subscribe__listener(cls, listener):
+        cls.update_listener = listener.update
+
+
+class IsExportingListener:
+    """When subscribed to InstrumentedFeedSlot, keeps track of when
+    a call to start_exporting has been made without a closing call to
+    finish_exporting and when a call to finish_exporting has been made
+    before a call to start_exporting."""
+    def __init__(self):
+        self.start_without_finish = False
+        self.finish_without_start = False
+
+    def update(self, method):
+        if method == 'start':
+            self.start_without_finish = True
+        elif method == 'finish':
+            if self.start_without_finish:
+                self.start_without_finish = False
+            else:
+                self.finish_before_start = True
+
+
+class ExceptionJsonItemExporter(JsonItemExporter):
+    """JsonItemExporter that throws an exception every time export_item is called."""
+    def export_item(self, _):
+        raise Exception('foo')
+
+
 class FeedExportTest(FeedExportTestBase):
     __test__ = True
 
@@ -890,6 +932,88 @@ class FeedExportTest(FeedExportTestBase):
             data = yield self.exported_no_data(settings)
             self.assertEqual(b'', data[fmt])
 
+    @defer.inlineCallbacks
+    def test_start_finish_exporting_items(self):
+        items = [
+            self.MyItem({'foo': 'bar1', 'egg': 'spam1'}),
+        ]
+        settings = {
+            'FEEDS': {
+                self._random_temp_filename(): {'format': 'json'},
+            },
+            'FEED_EXPORT_INDENT': None,
+        }
+
+        listener = IsExportingListener()
+        InstrumentedFeedSlot.subscribe__listener(listener)
+
+        with mock.patch('scrapy.extensions.feedexport._FeedSlot',
+                        InstrumentedFeedSlot):
+            _ = yield self.exported_data(items, settings)
+            self.assertFalse(listener.start_without_finish)
+            self.assertFalse(listener.finish_without_start)
+
+    @defer.inlineCallbacks
+    def test_start_finish_exporting_no_items(self):
+        items = []
+        settings = {
+            'FEEDS': {
+                self._random_temp_filename(): {'format': 'json'},
+            },
+            'FEED_EXPORT_INDENT': None,
+        }
+
+        listener = IsExportingListener()
+        InstrumentedFeedSlot.subscribe__listener(listener)
+
+        with mock.patch('scrapy.extensions.feedexport._FeedSlot',
+                        InstrumentedFeedSlot):
+            _ = yield self.exported_data(items, settings)
+            self.assertFalse(listener.start_without_finish)
+            self.assertFalse(listener.finish_without_start)
+
+    @defer.inlineCallbacks
+    def test_start_finish_exporting_items_exception(self):
+        items = [
+            self.MyItem({'foo': 'bar1', 'egg': 'spam1'}),
+        ]
+        settings = {
+            'FEEDS': {
+                self._random_temp_filename(): {'format': 'json'},
+            },
+            'FEED_EXPORTERS': {'json': ExceptionJsonItemExporter},
+            'FEED_EXPORT_INDENT': None,
+        }
+
+        listener = IsExportingListener()
+        InstrumentedFeedSlot.subscribe__listener(listener)
+
+        with mock.patch('scrapy.extensions.feedexport._FeedSlot',
+                        InstrumentedFeedSlot):
+            _ = yield self.exported_data(items, settings)
+            self.assertFalse(listener.start_without_finish)
+            self.assertFalse(listener.finish_without_start)
+
+    @defer.inlineCallbacks
+    def test_start_finish_exporting_no_items_exception(self):
+        items = []
+        settings = {
+            'FEEDS': {
+                self._random_temp_filename(): {'format': 'json'},
+            },
+            'FEED_EXPORTERS': {'json': ExceptionJsonItemExporter},
+            'FEED_EXPORT_INDENT': None,
+        }
+
+        listener = IsExportingListener()
+        InstrumentedFeedSlot.subscribe__listener(listener)
+
+        with mock.patch('scrapy.extensions.feedexport._FeedSlot',
+                        InstrumentedFeedSlot):
+            _ = yield self.exported_data(items, settings)
+            self.assertFalse(listener.start_without_finish)
+            self.assertFalse(listener.finish_without_start)
+
     @defer.inlineCallbacks
     def test_export_no_items_store_empty(self):
         formats = (