Merge pull request scrapy#961 from ldmberman/task_977_request_dropped

An attempt to resolve scrapy#957, add signal to be sent when request is dropped by the scheduler

Author: pablohoffman

docs/topics/signals.rst

@@ -200,6 +200,23 @@ request_scheduled
     :param spider: the spider that yielded the request
     :type spider: :class:`~scrapy.spider.Spider` object
 
+request_dropped
+-----------------
+
+.. signal:: request_dropped
+.. function:: request_dropped(request, spider)
+
+    Sent when a :class:`~scrapy.http.Request`, scheduled by the engine to be
+    downloaded later, is rejected by the scheduler.
+
+    The signal does not support returning deferreds from their handlers.
+
+    :param request: the request that reached the scheduler
+    :type request: :class:`~scrapy.http.Request` object
+
+    :param spider: the spider that yielded the request
+    :type spider: :class:`~scrapy.spider.Spider` object
+
 response_received
 -----------------
 

scrapy/core/engine.py

@@ -173,7 +173,9 @@ def crawl(self, request, spider):
     def schedule(self, request, spider):
         self.signals.send_catch_log(signal=signals.request_scheduled,
                 request=request, spider=spider)
-        return self.slot.scheduler.enqueue_request(request)
+        if not self.slot.scheduler.enqueue_request(request):
+            self.signals.send_catch_log(signal=signals.request_dropped,
+                                        request=request, spider=spider)
 
     def download(self, request, spider):
         slot = self.slot

scrapy/core/scheduler.py

@@ -47,14 +47,15 @@ def close(self, reason):
     def enqueue_request(self, request):
         if not request.dont_filter and self.df.request_seen(request):
             self.df.log(request, self.spider)
-            return
+            return False
         dqok = self._dqpush(request)
         if dqok:
             self.stats.inc_value('scheduler/enqueued/disk', spider=self.spider)
         else:
             self._mqpush(request)
             self.stats.inc_value('scheduler/enqueued/memory', spider=self.spider)
         self.stats.inc_value('scheduler/enqueued', spider=self.spider)
+        return True
 
     def next_request(self):
         request = self.mqs.pop()

scrapy/signals.py

@@ -12,6 +12,7 @@
 spider_closed = object()
 spider_error = object()
 request_scheduled = object()
+request_dropped = object()
 response_received = object()
 response_downloaded = object()
 item_scraped = object()

tests/test_engine.py

@@ -59,6 +59,12 @@ def parse_item(self, response):
             item['price'] = m.group(1)
         return item
 
+
+class TestDupeFilterSpider(TestSpider):
+    def make_requests_from_url(self, url):
+        return Request(url)  # dont_filter=False
+
+
 def start_test_site(debug=False):
     root_dir = os.path.join(tests_datadir, "test_site")
     r = static.File(root_dir)
@@ -75,26 +81,31 @@ def start_test_site(debug=False):
 class CrawlerRun(object):
     """A class to run the crawler and keep track of events occurred"""
 
-    def __init__(self):
+    def __init__(self, with_dupefilter=False):
         self.spider = None
         self.respplug = []
         self.reqplug = []
+        self.reqdropped = []
         self.itemresp = []
         self.signals_catched = {}
+        self.spider_class = TestSpider if not with_dupefilter else \
+            TestDupeFilterSpider
 
     def run(self):
         self.port = start_test_site()
         self.portno = self.port.getHost().port
 
-        start_urls = [self.geturl("/"), self.geturl("/redirect")]
+        start_urls = [self.geturl("/"), self.geturl("/redirect"),
+                      self.geturl("/redirect")]  # a duplicate
 
         for name, signal in vars(signals).items():
             if not name.startswith('_'):
                 dispatcher.connect(self.record_signal, signal)
 
-        self.crawler = get_crawler(TestSpider)
+        self.crawler = get_crawler(self.spider_class)
         self.crawler.signals.connect(self.item_scraped, signals.item_scraped)
         self.crawler.signals.connect(self.request_scheduled, signals.request_scheduled)
+        self.crawler.signals.connect(self.request_dropped, signals.request_dropped)
         self.crawler.signals.connect(self.response_downloaded, signals.response_downloaded)
         self.crawler.crawl(start_urls=start_urls)
         self.spider = self.crawler.spider
@@ -123,6 +134,9 @@ def item_scraped(self, item, spider, response):
     def request_scheduled(self, request, spider):
         self.reqplug.append((request, spider))
 
+    def request_dropped(self, request, spider):
+        self.reqdropped.append((request, spider))
+
     def response_downloaded(self, response, spider):
         self.respplug.append((response, spider))
 
@@ -141,10 +155,14 @@ def test_crawler(self):
         self.run = CrawlerRun()
         yield self.run.run()
         self._assert_visited_urls()
-        self._assert_scheduled_requests()
+        self._assert_scheduled_requests(urls_to_visit=8)
         self._assert_downloaded_responses()
         self._assert_scraped_items()
         self._assert_signals_catched()
+        self.run = CrawlerRun(with_dupefilter=True)
+        yield self.run.run()
+        self._assert_scheduled_requests(urls_to_visit=7)
+        self._assert_dropped_requests()
 
     def _assert_visited_urls(self):
         must_be_visited = ["/", "/redirect", "/redirected",
@@ -153,18 +171,26 @@ def _assert_visited_urls(self):
         urls_expected = set([self.run.geturl(p) for p in must_be_visited])
         assert urls_expected <= urls_visited, "URLs not visited: %s" % list(urls_expected - urls_visited)
 
-    def _assert_scheduled_requests(self):
-        self.assertEqual(6, len(self.run.reqplug))
+    def _assert_scheduled_requests(self, urls_to_visit=None):
+        self.assertEqual(urls_to_visit, len(self.run.reqplug))
 
         paths_expected = ['/item999.html', '/item2.html', '/item1.html']
 
         urls_requested = set([rq[0].url for rq in self.run.reqplug])
         urls_expected = set([self.run.geturl(p) for p in paths_expected])
         assert urls_expected <= urls_requested
+        scheduled_requests_count = len(self.run.reqplug)
+        dropped_requests_count = len(self.run.reqdropped)
+        responses_count = len(self.run.respplug)
+        self.assertEqual(scheduled_requests_count,
+                         dropped_requests_count + responses_count)
+
+    def _assert_dropped_requests(self):
+        self.assertEqual(len(self.run.reqdropped), 1)
 
     def _assert_downloaded_responses(self):
         # response tests
-        self.assertEqual(6, len(self.run.respplug))
+        self.assertEqual(8, len(self.run.respplug))
 
         for response, _ in self.run.respplug:
             if self.run.getpath(response.url) == '/item999.html':