removed unnecesary response ResponseSoup extension, and replaced by a utils function with cache support

docs/ref/extensions.rst

@@ -172,32 +172,6 @@ This extension only works on POSIX-compliant platforms (ie. not Windows).
 
 .. _SIGUSR1: http://en.wikipedia.org/wiki/SIGUSR1_and_SIGUSR2
 
-Response soup extension
------------------------
-
-.. module:: scrapy.contrib.response.soup
-   :synopsis: Response soup extension
-
-.. class:: scrapy.contrib.response.soup.ResponseSoup
-
-The ResponseSoup extension causes the :class:`~scrapy.http.Response` objects to
-grow a new method (``getsoup()``) which returns a cached `BeautifulSoup`_
-object of their body, and a ``soup`` attribute with the same effect. The
-``soup`` attribute is provided only for convenience, as you cannot pass pass
-any BeautifulSoup constructor arguments (use the ``getsoup()`` method for those
-cases). 
-
-The advantage of using the Response soup extension over instantiating a
-BeautifulSoup object directly is performance, as BeautifulSoup is known to be
-very slow.
-
-For example, if you have a downloader middleware and a spider that both need to
-construct a BeautifulSoup object of the responses, you would be constructing
-two BeautifulSoup objects unless you use this extension which caches the first
-one.
-
-.. _BeautifulSoup: http://www.crummy.com/software/BeautifulSoup/documentation.html
-
 StatsMailer extension
 ---------------------
 

docs/ref/settings.rst

@@ -402,7 +402,6 @@ Default::
         'scrapy.contrib.memdebug.MemoryDebugger',
         'scrapy.contrib.closedomain.CloseDomain',
         'scrapy.contrib.debug.StackTraceDump',
-        'scrapy.contrib.response.soup.ResponseSoup',
     ]
 
 The list of available extensions. Keep in mind that some of them need need to

scrapy/conf/default_settings.py

@@ -98,7 +98,6 @@ EXTENSIONS = [
     'scrapy.contrib.memdebug.MemoryDebugger',
     'scrapy.contrib.closedomain.CloseDomain',
     'scrapy.contrib.debug.StackTraceDump',
-    'scrapy.contrib.response.soup.ResponseSoup',
 ]
 
 GROUPSETTINGS_ENABLED = False

scrapy/contrib/response/soup.py

@@ -1,22 +0,0 @@
-"""
-ResponseSoup extension 
-
-See documentation in docs/ref/extensions.rst
-""" 
-
-from scrapy.xlib.BeautifulSoup import BeautifulSoup
-
-from scrapy.http import Response
-
-
-class ResponseSoup(object):
-    def __init__(self):
-        setattr(Response, 'getsoup', getsoup)
-        setattr(Response, 'soup', property(getsoup))
-
-def getsoup(response, **kwargs):
-    # TODO: use different cache buckets depending on constructor parameters
-    if 'soup' not in response.cache:
-        body = response.body if response.body is not None else ""
-        response.cache['soup'] = BeautifulSoup(body, **kwargs)
-    return response.cache['soup']

scrapy/tests/test_contrib_response_soup.py

@@ -1,37 +0,0 @@
-import unittest
-
-from scrapy.xlib.BeautifulSoup import BeautifulSoup
-
-from scrapy.http import Response
-from scrapy.contrib.response.soup import ResponseSoup
-
-class ResponseSoupTest(unittest.TestCase):
-
-    def setUp(self):
-        ResponseSoup()
-
-    def test_response_soup(self):
-        r1 = Response('http://www.example.com', body='')
-
-        soup1 = r1.getsoup()
-        soup2 = r1.getsoup()
-
-        assert isinstance(r1.soup, BeautifulSoup)
-        assert isinstance(soup2, BeautifulSoup)
-        # make sure it's cached
-        assert soup1 is soup2
-
-        # when body is None, an empty soup should be returned
-        r1 = Response('http://www.example.com')
-        assert r1.body == ""
-        assert isinstance(r1.getsoup(), BeautifulSoup)
-
-    def test_response_soup_caching(self):
-        r1 = Response('http://www.example.com', body='')
-        soup1 = r1.getsoup()
-        r2 = r1.copy()
-        soup2 = r1.getsoup()
-        soup3 = r2.getsoup()
-
-        assert soup1 is soup2
-        assert soup1 is not soup3

scrapy/tests/test_utils_response.py

@@ -1,6 +1,9 @@
 import unittest
+
+from scrapy.xlib.BeautifulSoup import BeautifulSoup
 from scrapy.http import Response, TextResponse
-from scrapy.utils.response import body_or_str, get_base_url, get_meta_refresh, response_httprepr
+from scrapy.utils.response import body_or_str, get_base_url, get_meta_refresh, \
+    response_httprepr, get_cached_beautifulsoup
 
 class ResponseUtilsTest(unittest.TestCase):
     dummy_response = TextResponse(url='http://example.org/', body='dummy_response')
@@ -58,6 +61,30 @@ class ResponseUtilsTest(unittest.TestCase):
         r1 = Response("http://www.example.com", status=404, headers={"Content-type": "text/html"}, body="Some body")
         self.assertEqual(response_httprepr(r1), 'HTTP/1.1 404 Not Found\r\nContent-Type: text/html\r\n\r\nSome body')
 
+    def test_get_cached_beautifulsoup(self):
+        r1 = Response('http://www.example.com', body='')
+
+        soup1 = get_cached_beautifulsoup(r1)
+        soup2 = get_cached_beautifulsoup(r1)
+
+        assert isinstance(soup1, BeautifulSoup)
+        assert isinstance(soup2, BeautifulSoup)
+        # make sure it's cached
+        assert soup1 is soup2
+
+        # when body is None, an empty soup should be returned
+        r1 = Response('http://www.example.com')
+        assert r1.body == ""
+        assert isinstance(get_cached_beautifulsoup(r1), BeautifulSoup)
+
+        r1 = Response('http://www.example.com', body='')
+        soup1 = get_cached_beautifulsoup(r1)
+        r2 = r1.copy()
+        soup2 = get_cached_beautifulsoup(r1)
+        soup3 = get_cached_beautifulsoup(r2)
+
+        assert soup1 is soup2
+        assert soup1 is not soup3
 
 if __name__ == "__main__":
     unittest.main()

scrapy/utils/response.py

@@ -3,11 +3,12 @@ This module provides some useful functions for working with
 scrapy.http.Response objects
 """
 
-import re
+import re, weakref
 
 from twisted.web import http
 from twisted.web.http import RESPONSES
 
+from scrapy.xlib.BeautifulSoup import BeautifulSoup
 from scrapy.http.response import Response
 
 def body_or_str(obj, unicode=True):
@@ -41,6 +42,14 @@ def get_meta_refresh(response):
         response.cache['meta_refresh_url'] = match.groups() if match else (None, None)
     return response.cache['meta_refresh_url']
 
+_beautifulsoup_cache = weakref.WeakKeyDictionary()
+def get_cached_beautifulsoup(response):
+    """Return BeautifulSoup object of the given response, with caching
+    support"""
+    if response not in _beautifulsoup_cache:
+        _beautifulsoup_cache[response] = BeautifulSoup(response.body)
+    return _beautifulsoup_cache[response]
+
 def response_status_message(status):
     """Return status code plus status text descriptive message