diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 0aa27e417..c3382d9d8 100644 --- a/scrapy/extensions/feedexport.py +++ b/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) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 97c3a74b3..feaba5dab 100644 --- a/tests/test_feedexport.py +++ b/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 = (