From 915d7cf247a87054a3026be7c12eee6f90edade9 Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Sun, 4 Aug 2013 16:08:06 +0200 Subject: [PATCH 1/4] also fetch alternate URLs from sitemaps, see #360 --- scrapy/contrib/spiders/sitemap.py | 13 +++++++++++-- scrapy/tests/test_utils_sitemap.py | 21 +++++++++++++++++++++ scrapy/utils/sitemap.py | 7 ++++++- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/scrapy/contrib/spiders/sitemap.py b/scrapy/contrib/spiders/sitemap.py index 4fc19e108..e4092c5d6 100644 --- a/scrapy/contrib/spiders/sitemap.py +++ b/scrapy/contrib/spiders/sitemap.py @@ -13,6 +13,10 @@ class SitemapSpider(BaseSpider): sitemap_follow = [''] def __init__(self, *a, **kw): + self._alternate = False + if 'alternate' in kw and kw.pop('alternate') == True: + self._alternate = True + super(SitemapSpider, self).__init__(*a, **kw) self._cbs = [] for r, c in self.sitemap_rules: @@ -37,7 +41,7 @@ class SitemapSpider(BaseSpider): s = Sitemap(body) if s.type == 'sitemapindex': - for loc in iterloc(s): + for loc in iterloc(s, self._alternate): if any(x.search(loc) for x in self._follow): yield Request(loc, callback=self._parse_sitemap) elif s.type == 'urlset': @@ -65,6 +69,11 @@ def regex(x): return re.compile(x) return x -def iterloc(it): +def iterloc(it, alt=False): for d in it: yield d['loc'] + + # Also consider alternate URLs (xhtml:link rel="alternate") + if alt == True and 'alternate' in d: + for l in d['alternate']: + yield l diff --git a/scrapy/tests/test_utils_sitemap.py b/scrapy/tests/test_utils_sitemap.py index 8be94a309..a338adfe2 100644 --- a/scrapy/tests/test_utils_sitemap.py +++ b/scrapy/tests/test_utils_sitemap.py @@ -159,6 +159,27 @@ Disallow: /forum/active/ {'lastmod': '2013-07-15', 'loc': 'http://www.example.com/sitemap3.xml'}, ]) + def test_alternate(self): + s = Sitemap(""" + + + http://www.example.com/english/ + + + + + """) + + self.assertEqual(list(s), [ + {'loc': 'http://www.example.com/english/', + 'alternate': ['http://www.example.com/deutsch/', 'http://www.example.com/schweiz-deutsch/', 'http://www.example.com/english/'] + } + ]) + if __name__ == '__main__': unittest.main() diff --git a/scrapy/utils/sitemap.py b/scrapy/utils/sitemap.py index 38e38d6a9..b69b4226f 100644 --- a/scrapy/utils/sitemap.py +++ b/scrapy/utils/sitemap.py @@ -23,7 +23,12 @@ class Sitemap(object): for el in elem.getchildren(): tag = el.tag name = tag.split('}', 1)[1] if '}' in tag else tag - d[name] = el.text.strip() if el.text else '' + + if name == 'link': + d.setdefault('alternate', []).append(el.get('href')) + else: + d[name] = el.text.strip() if el.text else '' + if 'loc' in d: yield d From 8ed2d0cda16cbe1c94373dacf85a632a25df221a Mon Sep 17 00:00:00 2001 From: Stefan Date: Sat, 7 Sep 2013 12:56:30 +0200 Subject: [PATCH 2/4] improved changes to allow retrieval of alternate links in sitemaps, see #360 --- docs/topics/spiders.rst | 19 +++++++++++++++++++ scrapy/contrib/spiders/sitemap.py | 8 ++------ scrapy/tests/test_utils_sitemap.py | 1 + scrapy/utils/sitemap.py | 7 ++++--- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 6586db668..e9afe1368 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -560,6 +560,25 @@ SitemapSpider By default, all sitemaps are followed. + .. attribute:: use_alternate_links + + Specifies if alternate links for one ``url`` should be followed. These + are links for the same website in another language passed within + the same ``url`` block. + + For example:: + + + http://example.com/ + + + + With ``use_alternate_links`` set, this would retrieve both URLs. With + ``use_alternate_links`` disabled, only ``http://example.com/`` would be + retrieved. + + Default is ``use_alternate_links`` disabled. + SitemapSpider examples ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/scrapy/contrib/spiders/sitemap.py b/scrapy/contrib/spiders/sitemap.py index e4092c5d6..eb14b614d 100644 --- a/scrapy/contrib/spiders/sitemap.py +++ b/scrapy/contrib/spiders/sitemap.py @@ -13,10 +13,6 @@ class SitemapSpider(BaseSpider): sitemap_follow = [''] def __init__(self, *a, **kw): - self._alternate = False - if 'alternate' in kw and kw.pop('alternate') == True: - self._alternate = True - super(SitemapSpider, self).__init__(*a, **kw) self._cbs = [] for r, c in self.sitemap_rules: @@ -41,7 +37,7 @@ class SitemapSpider(BaseSpider): s = Sitemap(body) if s.type == 'sitemapindex': - for loc in iterloc(s, self._alternate): + for loc in iterloc(s, self.use_alternate_links): if any(x.search(loc) for x in self._follow): yield Request(loc, callback=self._parse_sitemap) elif s.type == 'urlset': @@ -74,6 +70,6 @@ def iterloc(it, alt=False): yield d['loc'] # Also consider alternate URLs (xhtml:link rel="alternate") - if alt == True and 'alternate' in d: + if alt and 'alternate' in d: for l in d['alternate']: yield l diff --git a/scrapy/tests/test_utils_sitemap.py b/scrapy/tests/test_utils_sitemap.py index a338adfe2..7423d1782 100644 --- a/scrapy/tests/test_utils_sitemap.py +++ b/scrapy/tests/test_utils_sitemap.py @@ -171,6 +171,7 @@ Disallow: /forum/active/ href="http://www.example.com/schweiz-deutsch/"/> + """) diff --git a/scrapy/utils/sitemap.py b/scrapy/utils/sitemap.py index b69b4226f..24a540531 100644 --- a/scrapy/utils/sitemap.py +++ b/scrapy/utils/sitemap.py @@ -12,7 +12,7 @@ class Sitemap(object): (type=sitemapindex) files""" def __init__(self, xmltext): - xmlp = lxml.etree.XMLParser(recover=True) + xmlp = lxml.etree.XMLParser(recover=True, remove_comments=True) self._root = lxml.etree.fromstring(xmltext, parser=xmlp) rt = self._root.tag self.type = self._root.tag.split('}', 1)[1] if '}' in rt else rt @@ -25,10 +25,11 @@ class Sitemap(object): name = tag.split('}', 1)[1] if '}' in tag else tag if name == 'link': - d.setdefault('alternate', []).append(el.get('href')) + if 'href' in el.attrib: + d.setdefault('alternate', []).append(el.get('href')) else: d[name] = el.text.strip() if el.text else '' - + if 'loc' in d: yield d From 6994959181941529aecd8dd57369b99212555f30 Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 8 Sep 2013 10:38:28 +0200 Subject: [PATCH 3/4] renamed to sitemap_alternate_links and added default value, see #360 --- docs/topics/spiders.rst | 8 ++++---- scrapy/contrib/spiders/sitemap.py | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index e9afe1368..82e96df1e 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -560,7 +560,7 @@ SitemapSpider By default, all sitemaps are followed. - .. attribute:: use_alternate_links + .. attribute:: sitemap_alternate_links Specifies if alternate links for one ``url`` should be followed. These are links for the same website in another language passed within @@ -573,11 +573,11 @@ SitemapSpider - With ``use_alternate_links`` set, this would retrieve both URLs. With - ``use_alternate_links`` disabled, only ``http://example.com/`` would be + With ``sitemap_alternate_links`` set, this would retrieve both URLs. With + ``sitemap_alternate_links`` disabled, only ``http://example.com/`` would be retrieved. - Default is ``use_alternate_links`` disabled. + Default is ``sitemap_alternate_links`` disabled. SitemapSpider examples diff --git a/scrapy/contrib/spiders/sitemap.py b/scrapy/contrib/spiders/sitemap.py index eb14b614d..d8567f7f0 100644 --- a/scrapy/contrib/spiders/sitemap.py +++ b/scrapy/contrib/spiders/sitemap.py @@ -11,6 +11,7 @@ class SitemapSpider(BaseSpider): sitemap_urls = () sitemap_rules = [('', 'parse')] sitemap_follow = [''] + sitemap_alternate_links = False def __init__(self, *a, **kw): super(SitemapSpider, self).__init__(*a, **kw) @@ -37,7 +38,7 @@ class SitemapSpider(BaseSpider): s = Sitemap(body) if s.type == 'sitemapindex': - for loc in iterloc(s, self.use_alternate_links): + for loc in iterloc(s, self.sitemap_alternate_links): if any(x.search(loc) for x in self._follow): yield Request(loc, callback=self._parse_sitemap) elif s.type == 'urlset': From 855041ebfd58a3e0e4190455de305d60bfa104aa Mon Sep 17 00:00:00 2001 From: Stefan Date: Sat, 14 Sep 2013 12:17:34 +0200 Subject: [PATCH 4/4] added test case for comments, see #363 --- scrapy/tests/test_utils_sitemap.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/scrapy/tests/test_utils_sitemap.py b/scrapy/tests/test_utils_sitemap.py index 7423d1782..0049e5c40 100644 --- a/scrapy/tests/test_utils_sitemap.py +++ b/scrapy/tests/test_utils_sitemap.py @@ -159,6 +159,20 @@ Disallow: /forum/active/ {'lastmod': '2013-07-15', 'loc': 'http://www.example.com/sitemap3.xml'}, ]) + def test_comment(self): + s = Sitemap(""" + + + http://www.example.com/ + + + """) + + self.assertEqual(list(s), [ + {'loc': 'http://www.example.com/'} + ]) + def test_alternate(self): s = Sitemap("""