Issue 4867 - Add global get_pages_metadata to template converters

cms/converters.py

Index: cms/converters.py
===================================================================
--- a/cms/converters.py
+++ b/cms/converters.py
@@ -378,16 +378,17 @@
             'linkify': self.linkify,
             'toclist': self.toclist,
         }
 
         globals = {
             'get_string': self.get_string,
             'has_string': self.has_string,
             'get_page_content': self.get_page_content,
+            'get_pages_metadata': self.get_pages_metadata,
         }
 
         for dirname, dictionary in [('filters', filters),
                                     ('globals', globals)]:
             for filename in self._params['source'].list_files(dirname):
                 root, ext = os.path.splitext(filename)
                 if ext.lower() != '.py':
                     continue
@@ -466,16 +467,60 @@
         locale, url = self._params['source'].resolve_link(page, locale)
         return jinja2.Markup('<a{}>'.format(''.join(
             ' {}="{}"'.format(name, jinja2.escape(value)) for name, value in [
                 ('href', url),
                 ('hreflang', locale)
             ] + attrs.items()
         )))
 
+    def get_pages_metadata(self, filters=None):
+        if not isinstance(filters, dict) and filters:
+            raise TypeError('Filters are not a dictionary')
+
+        return_data = []
+        for page_name, _format in self._params['source'].list_pages():
+            data, filename = self._params['source'].read_page(page_name,
+                                                              _format)
+            page_data = self.parse_page_metadata(data, page_name)
+
+            if self.filter_metadata(filters, page_data) is True:
+                return_data.append(page_data)
+
+        return return_data
+
+    def parse_page_metadata(self, data, page):

[Jon Sonesen, 2017/06/23 10:09:26]

This code is essentially duplicating the logic in the init function of the
Converter class, Vasily and I discussed this and the options were to break the
logic out into a class function of Converters, make it an utils.py function or
use it as a function in the converters namespace. 

We chose to put it in the converters.py namespace as a function because it makes
no sense in utils since it is page specific logic, but it is not specific enough
to a given page's instance of its own converter class to be a class function.

I will break this out into its own function in the next patch set if everyone
agrees this makes sense

[Vasily Kuznetsov, 2017/06/23 14:15:24]

As discussed, this approach sounds good.

Now looking at these 3 functions that we're adding to `TemplateConverter` it
starts looking like we should separate all the default globals out into their
own file(s). They are not really part of the converter logic but are more like a
set of services that we provide to the template -- it doesn't seem right to
pollute the converter class with this stuff. The globals often access
`self._params`, which technically is a private attribute of the converter, but
logically that thing is a rendering context and it actually becomes the context
(in jinja sense) of the templates so we will be able to get it using
`contextfunction` decorator. There's also `self._get_locale_data()` that is used
by the globals, but I'm actually wondering if `self._params['localedata']`
should be used instead (it wouldn't load file from the disk the locale every
time and it also supports locale overrides...). I guess we should ask Wladimir
why it's done this way (it's from this change:
https://hg.adblockplus.org/cms/rev/b022896ef69a).

Anyway, you can do the metadata loading refactoring already and perhaps the
separation of the globals will land as a separate change.

[Jon Sonesen, 2017/06/26 07:22:43]

Yeah I totally agree here, and actually we talked about this in the past (not to
this extent detail wise) the fact that we could break out globals and/or filters
out of the converters file tp make it cleaner to extend in the future.

Regarding the locale_data changes I agree here, since instantiating any
converter will override the locale data with user specified parameters. But
maybe there is a side effect we are not considering, or are unaware of.

+        page_metadata = {'page': page}
+        lines = data.splitlines(True)
+        for i, line in enumerate(lines):
+            if not re.search(r'^\s*[\w\-]+\s*=', line):
+                break
+            name, value = line.split('=', 1)
+            value = value.strip()
+            if value.startswith('[') and value.endswith(']'):
+                value = [element.strip() for element in value[1:-1].split(',')]
+            page_metadata[name.strip()] = value
+        return page_metadata
+
+    def filter_metadata(self, filters, metadata):
+        if filters is None:
+            return True
+        for filter_name, filter_value in filters.items():
+            if filter_name not in metadata:
+                return False
+            if isinstance(metadata[filter_name], list):
+                if isinstance(filter_value, basestring):
+                    filter_value = [filter_value]
+                for option in filter_value:
+                    if str(option) not in metadata[filter_name]:
+                        return False
+            elif filter_value != metadata[filter_name]:
+                    return False
+        return True
+
     def toclist(self, content):
         toc_re = r'<h(\d)\s[^<>]*\bid="([^<>"]+)"[^<>]*>(.*?)</h\1>'
         flat = []
         for match in re.finditer(toc_re, content, re.S):
             flat.append({
                 'level': int(match.group(1)),
                 'anchor': jinja2.Markup(match.group(2)).unescape(),
                 'title': jinja2.Markup(match.group(3)).unescape(),