Repository: scrapy/scrapy
Pull request #3512 (2018-11-29 21:19:42)

[MRG+1] Add sitemap_filter function to SitemapSpider class
it makes it possible to filter sitemap urls by any available attribute

for example, you can filter urls with lastmod greater than a given datetime

it can be helpful when the url loc itself does not aggregate that information

codecov[bot]: # [Codecov](https://codecov.io/gh/scrapy/scrapy/pull/3512?src=pr&el=h1) Report
> Merging [#3512](https://codecov.io/gh/scrapy/scrapy/pull/3512?src=pr&el=desc) into [master](https://codecov.io/gh/scrapy/scrapy/commit/886513c3751b92e42dcc8cb180d4c15a5a11ccaf?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `50%`.


```diff
@@            Coverage Diff             @@
##           master    #3512      +/-   ##
==========================================
- Coverage   84.38%   84.37%   -0.02%     
==========================================
  Files         167      167              
  Lines        9376     9380       +4     
  Branches     1392     1393       +1     
==========================================
+ Hits         7912     7914       +2     
- Misses       1206     1207       +1     
- Partials      258      259       +1
```

| [Impacted Files](https://codecov.io/gh/scrapy/scrapy/pull/3512?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [scrapy/spiders/sitemap.py](https://codecov.io/gh/scrapy/scrapy/pull/3512/diff?src=pr&el=tree#diff-c2NyYXB5L3NwaWRlcnMvc2l0ZW1hcC5weQ==) | `73.43% <50%> (-1.57%)` | :arrow_down: |

raphapassini: @victor-torres seems like a nice feature. But we'll need some tests
victor-torres: Done @raphapassini 
kmike: Hey @victor-torres! I think this feature makes sense, and implementation is fine, but the current API won't be clear for users.
victor-torres: Should we discuss it with someone else, @kmike?
victor-torres: I'll rename the `url` param to `entries` as suggested by @dangra. 

Still need to fix the coverage problem stated by dodecov exten. 
dangra: please rebase to get travis-ci fix  https://github.com/scrapy/scrapy/pull/3538 
dangra: @victor-torres do you plan to add a test case to cover the lines that are missing coverage?
victor-torres: > @victor-torres do you plan to add a test case to cover the lines that are missing coverage?

I'm not sure about what additional tests I should write to increase the coverage on this case. 
victor-torres: All checks have passed :)
kmike: Hey @victor-torres! The implementation and argument name looks fine, but I still think we should document what's inside a single `entry` - what keys/values are available, what they mean.
victor-torres: @kmike, thanks for your contribution here.

I think this is already documented in the `docs/topics/spiders.rst` file when we show this example:

```

.. method:: sitemap_filter(entries)

    Specifies a function to filter sitemap entries and their attributes.

    For example::

        <url>
            <loc>http://example.com/</loc>
            <lastmod>2005-01-01</lastmod>
        </url>

    We can define a ``sitemap_filter`` function to filter ``entries`` by date::

        def sitemap_filter(entries):
            from datetime import datetime
            for entry in entries:
                date_time = datetime.strptime(entry['lastmod'], '%Y-%m-%d')
                if date_time.year >= 2005:
                    yield entry

    This would retrieve only ``entries`` modified on 2005 and the following
    years.

    If you omit this method, all entries found in sitemaps will be
    processed, observing other attributes and their settings.
```

Given the context, it's clear to me that entries come from the sitemap document and the keys/values will depend upon the document structure itself. Explaining the sitemap XML format here doesn't seem to be in the scope. 

Of course my opinion may be biased and I'd love to hear some suggestions from you guys. 
kmike: @victor-torres I agree that documenting sitemap XML doesn't make sense, though entries we return don't match attributes exactly - see https://github.com/scrapy/scrapy/blob/4306886ac6030c9312547bdbcff3d88eb89c11d0/scrapy/utils/sitemap.py#L22

For example, namespaces seem to be removed, there is an extra "alternate" key, and rows without "loc" are dropped - this is what I think is nice to have documented.

Patch:
diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst
index a08dc30f2..742a88659 100644
--- a/docs/topics/spiders.rst
+++ b/docs/topics/spiders.rst
@@ -680,6 +680,50 @@ SitemapSpider
 
         Default is ``sitemap_alternate_links`` disabled.
 
+    .. method:: sitemap_filter(entries)
+
+        This is a filter funtion that could be overridden to select sitemap entries
+        based on their attributes.
+
+        For example::
+
+            <url>
+                <loc>http://example.com/</loc>
+                <lastmod>2005-01-01</lastmod>
+            </url>
+
+        We can define a ``sitemap_filter`` function to filter ``entries`` by date::
+
+            from datetime import datetime
+            from scrapy.spiders import SitemapSpider
+
+            class FilteredSitemapSpider(SitemapSpider):
+                name = 'filtered_sitemap_spider'
+                allowed_domains = ['example.com']
+                sitemap_urls = ['http://example.com/sitemap.xml']
+
+                def sitemap_filter(self, entries):
+                    for entry in entries:
+                        date_time = datetime.strptime(entry['lastmod'], '%Y-%m-%d')
+                        if date_time.year >= 2005:
+                            yield entry
+
+        This would retrieve only ``entries`` modified on 2005 and the following
+        years.
+
+        Entries are dict objects extracted from the sitemap document.
+        Usually, the key is the tag name and the value is the text inside it.
+
+        It's important to notice that:
+
+        - as the loc attribute is required, entries without this tag are discarded
+        - alternate links are stored in a list with the key ``alternate``
+          (see ``sitemap_alternate_links``)
+        - namespaces are removed, so lxml tags named as ``{namespace}tagname`` become only ``tagname``
+
+        If you omit this method, all entries found in sitemaps will be
+        processed, observing other attributes and their settings.
+
 
 SitemapSpider examples
 ~~~~~~~~~~~~~~~~~~~~~~
diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py
index 0ee8ba5e7..534c45c70 100644
--- a/scrapy/spiders/sitemap.py
+++ b/scrapy/spiders/sitemap.py
@@ -31,6 +31,14 @@ class SitemapSpider(Spider):
         for url in self.sitemap_urls:
             yield Request(url, self._parse_sitemap)
 
+    def sitemap_filter(self, entries):
+        """This method can be used to filter sitemap entries by their
+        attributes, for example, you can filter locs with lastmod greater
+        than a given date (see docs).
+        """
+        for entry in entries:
+            yield entry
+
     def _parse_sitemap(self, response):
         if response.url.endswith('/robots.txt'):
             for url in sitemap_urls_from_robots(response.text, base_url=response.url):
@@ -43,12 +51,14 @@ class SitemapSpider(Spider):
                 return
 
             s = Sitemap(body)
+            it = self.sitemap_filter(s)
+
             if s.type == 'sitemapindex':
-                for loc in iterloc(s, self.sitemap_alternate_links):
+                for loc in iterloc(it, 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':
-                for loc in iterloc(s, self.sitemap_alternate_links):
+                for loc in iterloc(it, self.sitemap_alternate_links):
                     for r, c in self._cbs:
                         if r.search(loc):
                             yield Request(loc, callback=c)


Test patch:
diff --git a/tests/test_spider.py b/tests/test_spider.py
index f26da2334..fefdaa403 100644
--- a/tests/test_spider.py
+++ b/tests/test_spider.py
@@ -375,6 +375,104 @@ Sitemap: /sitemap-relative-url.xml
                           'http://www.example.com/schweiz-deutsch/',
                           'http://www.example.com/italiano/'])
 
+    def test_sitemap_filter(self):
+        sitemap = b"""<?xml version="1.0" encoding="UTF-8"?>
+    <urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"
+        xmlns:xhtml="http://www.w3.org/1999/xhtml">
+        <url>
+            <loc>http://www.example.com/english/</loc>
+            <lastmod>2010-01-01</lastmod>
+        </url>
+        <url>
+            <loc>http://www.example.com/portuguese/</loc>
+            <lastmod>2005-01-01</lastmod>
+        </url>
+    </urlset>"""
+
+        class FilteredSitemapSpider(self.spider_class):
+            def sitemap_filter(self, entries):
+                from datetime import datetime
+                for entry in entries:
+                    date_time = datetime.strptime(entry['lastmod'], '%Y-%m-%d')
+                    if date_time.year > 2008:
+                        yield entry
+
+        r = TextResponse(url="http://www.example.com/sitemap.xml", body=sitemap)
+        spider = self.spider_class("example.com")
+        self.assertEqual([req.url for req in spider._parse_sitemap(r)],
+                         ['http://www.example.com/english/',
+                          'http://www.example.com/portuguese/'])
+
+        spider = FilteredSitemapSpider("example.com")
+        self.assertEqual([req.url for req in spider._parse_sitemap(r)],
+                         ['http://www.example.com/english/'])
+
+    def test_sitemap_filter_with_alternate_links(self):
+        sitemap = b"""<?xml version="1.0" encoding="UTF-8"?>
+    <urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"
+        xmlns:xhtml="http://www.w3.org/1999/xhtml">
+        <url>
+            <loc>http://www.example.com/english/article_1/</loc>
+            <lastmod>2010-01-01</lastmod>
+            <xhtml:link rel="alternate" hreflang="de"
+                href="http://www.example.com/deutsch/article_1/"/>
+        </url>
+        <url>
+            <loc>http://www.example.com/english/article_2/</loc>
+            <lastmod>2015-01-01</lastmod>
+        </url>
+    </urlset>"""
+
+        class FilteredSitemapSpider(self.spider_class):
+            def sitemap_filter(self, entries):
+                for entry in entries:
+                    alternate_links = entry.get('alternate', tuple())
+                    for link in alternate_links:
+                        if '/deutsch/' in link:
+                            entry['loc'] = link
+                            yield entry
+
+        r = TextResponse(url="http://www.example.com/sitemap.xml", body=sitemap)
+        spider = self.spider_class("example.com")
+        self.assertEqual([req.url for req in spider._parse_sitemap(r)],
+                         ['http://www.example.com/english/article_1/',
+                          'http://www.example.com/english/article_2/'])
+
+        spider = FilteredSitemapSpider("example.com")
+        self.assertEqual([req.url for req in spider._parse_sitemap(r)],
+                         ['http://www.example.com/deutsch/article_1/'])
+
+    def test_sitemapindex_filter(self):
+        sitemap = b"""<?xml version="1.0" encoding="UTF-8"?>
+    <sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
+        <sitemap>
+            <loc>http://www.example.com/sitemap1.xml</loc>
+            <lastmod>2004-01-01T20:00:00+00:00</lastmod>
+        </sitemap>
+        <sitemap>
+            <loc>http://www.example.com/sitemap2.xml</loc>
+            <lastmod>2005-01-01</lastmod>
+        </sitemap>
+    </sitemapindex>"""
+
+        class FilteredSitemapSpider(self.spider_class):
+            def sitemap_filter(self, entries):
+                from datetime import datetime
+                for entry in entries:
+                    date_time = datetime.strptime(entry['lastmod'].split('T')[0], '%Y-%m-%d')
+                    if date_time.year > 2004:
+                        yield entry
+
+        r = TextResponse(url="http://www.example.com/sitemap.xml", body=sitemap)
+        spider = self.spider_class("example.com")
+        self.assertEqual([req.url for req in spider._parse_sitemap(r)],
+                         ['http://www.example.com/sitemap1.xml',
+                          'http://www.example.com/sitemap2.xml'])
+
+        spider = FilteredSitemapSpider("example.com")
+        self.assertEqual([req.url for req in spider._parse_sitemap(r)],
+                         ['http://www.example.com/sitemap2.xml'])
+
 
 class DeprecationTest(unittest.TestCase):