From c793e5c3949a382f4cf8d1e7b67362d6067d6f1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 10 Nov 2018 20:53:04 +0100 Subject: [PATCH 1/3] Add a test for false-positive warnings about intersite links. With the current implementation, links to articles/pages that have not been processed yet are reported with a warning, yet later replaced with a correct value. That's not wanted. --- .../cyclic_intersite_links/first-article.rst | 7 ++++++ .../cyclic_intersite_links/second-article.rst | 7 ++++++ .../cyclic_intersite_links/third-article.rst | 7 ++++++ pelican/tests/test_pelican.py | 23 +++++++++++++++++++ 4 files changed, 44 insertions(+) create mode 100644 pelican/tests/cyclic_intersite_links/first-article.rst create mode 100644 pelican/tests/cyclic_intersite_links/second-article.rst create mode 100644 pelican/tests/cyclic_intersite_links/third-article.rst diff --git a/pelican/tests/cyclic_intersite_links/first-article.rst b/pelican/tests/cyclic_intersite_links/first-article.rst new file mode 100644 index 00000000..b9eb3f93 --- /dev/null +++ b/pelican/tests/cyclic_intersite_links/first-article.rst @@ -0,0 +1,7 @@ +First article +############# + +:date: 2018-11-10 +:summary: Here's the `second <{filename}/second-article.rst>`_, + `third <{filename}/third-article.rst>`_ and a + `nonexistent article <{filename}/nonexistent.rst>`_. diff --git a/pelican/tests/cyclic_intersite_links/second-article.rst b/pelican/tests/cyclic_intersite_links/second-article.rst new file mode 100644 index 00000000..acb87d47 --- /dev/null +++ b/pelican/tests/cyclic_intersite_links/second-article.rst @@ -0,0 +1,7 @@ +Second article +############## + +:date: 2018-11-10 +:summary: Here's the `first <{filename}/first-article.rst>`_, + `third <{filename}/third-article.rst>`_ and a + `nonexistent article <{filename}/nonexistent.rst>`_. diff --git a/pelican/tests/cyclic_intersite_links/third-article.rst b/pelican/tests/cyclic_intersite_links/third-article.rst new file mode 100644 index 00000000..4c7c1b76 --- /dev/null +++ b/pelican/tests/cyclic_intersite_links/third-article.rst @@ -0,0 +1,7 @@ +Third article +############# + +:date: 2018-11-10 +:summary: Here's the `first <{filename}/first-article.rst>`_, + `second <{filename}/second-article.rst>`_ and a + `nonexistent article <{filename}/nonexistent.rst>`_. diff --git a/pelican/tests/test_pelican.py b/pelican/tests/test_pelican.py index 273fc430..e547a69b 100644 --- a/pelican/tests/test_pelican.py +++ b/pelican/tests/test_pelican.py @@ -211,6 +211,29 @@ class TestPelican(LoggedTestCase): msg="Writing .*", level=logging.INFO) + def test_cyclic_intersite_links_no_warnings(self): + settings = read_settings(path=None, override={ + 'PATH': os.path.join(CURRENT_DIR, 'cyclic_intersite_links'), + 'OUTPUT_PATH': self.temp_path, + 'CACHE_PATH': self.temp_cache, + }) + pelican = Pelican(settings=settings) + mute(True)(pelican.run)() + # There are four different intersite links: + # - one pointing to the second article from first and third + # - one pointing to the first article from second and third + # - one pointing to the third article from first and second + # - one pointing to a nonexistent from each + # If everything goes well, only the warning about the nonexistent + # article should be printed. Only two articles are not sufficient, + # since the first will always have _context['generated_content'] empty + # (thus skipping the link resolving) and the second will always have it + # non-empty, containing the first, thus always succeeding. + self.assertLogCountEqual( + count=1, + msg="Unable to find '.*\\.rst', skipping url replacement.", + level=logging.WARNING) + def test_md_extensions_deprecation(self): """Test that a warning is issued if MD_EXTENSIONS is used""" settings = read_settings(path=None, override={ From 673e33840d124bc274805fe8ca3acb199d5c9cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 12 Sep 2018 12:56:24 +0200 Subject: [PATCH 2/3] Don't process metadata links before all files are present. The refresh_metadata_intersite_links() is called again later, so this bit was only causing everything to be processed twice. Besides that, it makes the processing dependent on file order -- in particular, when metadata references a file that was not parsed yet, it reported an "Unable to find" warning. But everything is found in the second pass, so this only causes a superflous false warning and no change to the output. The related test now needs to call the refresh_metadata_intersite_links() explicitly. That function is called from Pelican.run() and all generators but the test processes just one page so it has no chance of being called implicitly. Related discussion: https://github.com/getpelican/pelican/pull/2288/files#r204337359 --- pelican/contents.py | 4 ---- pelican/tests/test_contents.py | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pelican/contents.py b/pelican/contents.py index 327adc72..dd2a74fa 100644 --- a/pelican/contents.py +++ b/pelican/contents.py @@ -142,10 +142,6 @@ class Content(object): if not hasattr(self, 'status'): self.status = getattr(self, 'default_status', None) - if (len(self._context.get('generated_content', [])) > 0 or - len(self._context.get('static_content', [])) > 0): - self.refresh_metadata_intersite_links() - signals.content_object_init.send(self) def __str__(self): diff --git a/pelican/tests/test_contents.py b/pelican/tests/test_contents.py index 21766359..da1a6ae2 100644 --- a/pelican/tests/test_contents.py +++ b/pelican/tests/test_contents.py @@ -333,6 +333,10 @@ class TestPage(LoggedTestCase): args['metadata']['custom'] = parsed args['context']['localsiteurl'] = 'http://notmyidea.org' p = Page(**args) + # This is called implicitly from all generators and Pelican.run() once + # all files are processed. Here we process just one page so it needs + # to be called explicitly. + p.refresh_metadata_intersite_links() self.assertEqual(p.summary, linked) self.assertEqual(p.custom, linked) From 4df3b5156fb4da751b4a31aa77a1c39faf4f891d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 9 Oct 2018 23:39:41 +0200 Subject: [PATCH 3/3] Bring back Content._summary. It's now just a (mutable) copy of metadata["summary"] and changes to it are integrated back to the original. --- pelican/contents.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/pelican/contents.py b/pelican/contents.py index dd2a74fa..8d534976 100644 --- a/pelican/contents.py +++ b/pelican/contents.py @@ -142,6 +142,10 @@ class Content(object): if not hasattr(self, 'status'): self.status = getattr(self, 'default_status', None) + # store the summary metadata if it is set + if 'summary' in metadata: + self._summary = metadata['summary'] + signals.content_object_init.send(self) def __str__(self): @@ -465,7 +469,7 @@ class Content(object): def refresh_metadata_intersite_links(self): for key in self.settings['FORMATTED_FIELDS']: - if key in self.metadata: + if key in self.metadata and key != 'summary': value = self._update_content( self.metadata[key], self.get_siteurl() @@ -473,6 +477,16 @@ class Content(object): self.metadata[key] = value setattr(self, key.lower(), value) + # _summary is an internal variable that some plugins may be writing to, + # so ensure changes to it are picked up + if ('summary' in self.settings['FORMATTED_FIELDS'] and + 'summary' in self.metadata): + self._summary = self._update_content( + self._summary, + self.get_siteurl() + ) + self.metadata['summary'] = self._summary + class Page(Content): mandatory_properties = ('title',)