Improve non-root site handling of nikola auto. Fix #3715. (PR#3727)

AUTHORS.txt

@@ -3,6 +3,7 @@
 * `Alberto Berti <https://github.com/azazel75>`_
 * `Alex Popescu <https://github.com/al3xandru>`_
 * `Alex Walters <https://github.com/tritium21>`_
+* `Andreas Krüger <https://github.com/aknrdureegaesr>`_
 * `Andreas Linz <https://github.com/KLINGTdotNET>`_
 * `André Felipe Dias <https://github.com/andredias>`_
 * `Areski Belaid <https://github.com/areski>`_
@@ -13,14 +14,14 @@
 * `Boris Kaul <https://github.com/localvoid>`_
 * `Brad Miller <https://github.com/bnmnetp>`_
 * `Brandon W. Maister <https://github.com/quodlibetor>`_
-* `Bussonnier Matthias <https://gtihub.com/carreau>`_
+* `Bussonnier Matthias <https://github.com/carreau>`_
 * `Carlos Martín Sánchez <https://github.com/carlosvin>`_
 * `Carsten Grohmann <https://github.com/CarstenGrohmann>`_
 * `Casey M. Bessette <https://github.com/caseybessette>`_
 * `Chris Lee <https://github.com/clee>`_
 * `Chris Warrick <https://github.com/Kwpolska>`_
 * `Christian Lawson-Perfect <https://github.com/christianp>`_
-* `Christopher Arndt <https:/github.com/SpotlightKid>`_
+* `Christopher Arndt <https://github.com/SpotlightKid>`_
 * `Claudio Canepa <https://github.com/ccanepa>`_
 * `Clayton A. Davis <https://github.com/clayadavis>`_
 * `Damien Tournoud <https://github.com/damz>`_

CHANGES.txt

@@ -12,6 +12,9 @@ Bugfixes
 * Fix margins of paragraphs at the end of sections (Issue #3704)
 * Ignore ``.DS_Store`` files in listing indexes (Issue #3698)
 * Fix baguetteBox.js invoking in the base theme (Issue #3687)
+* Fix development (preview) server `nikola auto`
+  for non-root SITE_URL, in particular when URL_TYPE is full_path.
+  (Issue #3715)
 
 New in v8.2.4
 =============

nikola/plugins/command/auto/__init__.py

@@ -35,6 +35,7 @@
 import subprocess
 import sys
 import typing
+import urllib.parse
 import webbrowser
 
 import blinker
@@ -67,6 +68,17 @@
     asyncio.set_event_loop(asyncio.ProactorEventLoop())
 
 
+def base_path_from_siteuri(siteuri: str) -> str:
+    """Extract the path part from a URI such as site['SITE_URL'].
+
+    The path never ends with a "/". (If only "/" is intended, it is empty.)
+    """
+    path = urllib.parse.urlsplit(siteuri).path
+    if path.endswith("/"):
+        path = path[:-1]
+    return path
+
+
 class CommandAuto(Command):
     """Automatic rebuilds for Nikola."""
 
@@ -239,8 +251,10 @@ def _execute(self, options, args):
         # Server can be disabled (Issue #1883)
         self.has_server = not options['no-server']
 
+        base_path = base_path_from_siteuri(self.site.config['SITE_URL'])
+
         if self.has_server:
-            loop.run_until_complete(self.set_up_server(host, port, out_folder))
+            loop.run_until_complete(self.set_up_server(host, port, base_path, out_folder))
 
         # Run an initial build so we are up-to-date. The server is running, but we are not watching yet.
         loop.run_until_complete(self.run_initial_rebuild())
@@ -293,9 +307,12 @@ def _execute(self, options, args):
         if browser:
             # Some browsers fail to load 0.0.0.0 (Issue #2755)
             if host == '0.0.0.0':
-                server_url = "http://127.0.0.1:{0}/".format(port)
-            self.logger.info("Opening {0} in the default web browser...".format(server_url))
-            webbrowser.open(server_url)
+                browser_url = "http://127.0.0.1:{0}/{1}".format(port, base_path.lstrip("/"))
+            else:
+                # server_url always ends with a "/":
+                browser_url = "{0}{1}".format(server_url, base_path.lstrip("/"))
+            self.logger.info("Opening {0} in the default web browser...".format(browser_url))
+            webbrowser.open(browser_url)
 
         # Run the event loop forever and handle shutdowns.
         try:
@@ -320,13 +337,13 @@ def _execute(self, options, args):
             self.wd_observer.join()
         loop.close()
 
-    async def set_up_server(self, host: str, port: int, out_folder: str) -> None:
+    async def set_up_server(self, host: str, port: int, base_path: str, out_folder: str) -> None:
         """Set up aiohttp server and start it."""
         webapp = web.Application()
         webapp.router.add_get('/livereload.js', self.serve_livereload_js)
         webapp.router.add_get('/robots.txt', self.serve_robots_txt)
         webapp.router.add_route('*', '/livereload', self.websocket_handler)
-        resource = IndexHtmlStaticResource(True, self.snippet, '', out_folder)
+        resource = IndexHtmlStaticResource(True, self.snippet, base_path, out_folder)
         webapp.router.register_resource(resource)
         webapp.on_shutdown.append(self.remove_websockets)
 

tests/integration/test_dev_server.py

@@ -3,9 +3,10 @@
 from nikola.utils import get_logger
 import pytest
 import pathlib
-import socket
-from typing import Optional
 import requests
+import socket
+import sys
+from typing import Optional, Tuple, Any, Dict
 
 from ..helper import FakeSite
 
@@ -35,7 +36,21 @@ def find_unused_port() -> int:
         s.close()
 
 
-def test_serves_root_dir(site, expected_text) -> None:
+class MyFakeSite(FakeSite):
+    def __init__(self, config: Dict[str, Any], configuration_filename="conf.py"):
+        self.configured = True
+        self.debug = True
+        self.THEMES = []
+        self._plugin_places = []
+        self.registered_auto_watched_folders = set()
+        self.config = config
+        self.configuration_filename = configuration_filename
+
+
+def test_serves_root_dir(
+    site_and_base_path: Tuple[MyFakeSite, str], expected_text: str
+) -> None:
+    site, base_path = site_and_base_path
     command_auto = auto.CommandAuto()
     command_auto.set_site(site)
     options = {
@@ -55,75 +70,108 @@ def test_serves_root_dir(site, expected_text) -> None:
     test_was_successful = False
     test_problem_description = "Async test setup apparently broken"
     test_inner_error: Optional[BaseException] = None
+    loop_for_this_test = None
 
     async def grab_loop_and_run_test() -> None:
-        loop = asyncio.get_running_loop()
-        watchdog_handle = loop.call_later(TEST_MAX_DURATION, lambda: loop.stop())
-        nonlocal test_problem_description
+        nonlocal test_problem_description, loop_for_this_test
+
+        loop_for_this_test = asyncio.get_running_loop()
+        watchdog_handle = loop_for_this_test.call_later(TEST_MAX_DURATION, lambda: loop_for_this_test.stop())
         test_problem_description = f"Test did not complete within {TEST_MAX_DURATION} seconds."
 
         def run_test() -> None:
             nonlocal test_was_successful, test_problem_description, test_inner_error
             try:
                 with requests.Session() as session:
+                    server_root_uri = f"http://{options['address']}:{options['port']}"
+
                     # First subtest: Grab the document root index.html file:
-                    server_base_uri = f"http://{options['address']}:{options['port']}/"
+                    server_base_uri = f"{server_root_uri}{base_path}"
+                    LOGGER.info("Attempting to fetch HTML from %s", server_base_uri)
                     res = session.get(server_base_uri)
                     res.raise_for_status()
                     assert "text/html; charset=utf-8" == res.headers['content-type']
                     assert expected_text in res.text
 
                     # Second subtest: Does the dev server serve something for the livereload JS?
-                    res_js = session.get(f"{server_base_uri}livereload.js?snipver=1")
+                    js_uri = f"{server_root_uri}/livereload.js?snipver=1"
+                    LOGGER.info("Attempting to fetch JS from %s", js_uri)
+                    res_js = session.get(js_uri)
                     res_js.raise_for_status()
                     content_type_js = res_js.headers['content-type']
                     assert "javascript" in content_type_js
 
                 test_was_successful = True
                 test_problem_description = "No problem. All is well."
             except BaseException as be:
+                LOGGER.error("Could not receive HTTP as expected.", exc_info=True)
                 test_inner_error = be
+                test_was_successful = False
+                test_problem_description = "(see exception)"
             finally:
-                LOGGER.info("Test completed, %s: %s",
-                            "successfully" if test_was_successful else "failing",
-                            test_problem_description)
-                loop.call_soon_threadsafe(lambda: watchdog_handle.cancel())
+                if test_was_successful:
+                    LOGGER.info("Test completed successfully.")
+                else:
+                    LOGGER.error("Test failed: %s", test_problem_description)
+                loop_for_this_test.call_soon_threadsafe(lambda: watchdog_handle.cancel())
 
                 # We give the outer grab_loop_and_run_test a chance to complete
                 # before burning the bridge:
-                loop.call_soon_threadsafe(lambda: loop.call_later(0.05, lambda: loop.stop()))
-        await loop.run_in_executor(None, run_test)
+                loop_for_this_test.call_soon_threadsafe(lambda: loop_for_this_test.call_later(0.05, lambda: loop_for_this_test.stop()))
+
+        await loop_for_this_test.run_in_executor(None, run_test)
 
     # We defeat the nikola site building functionality, so this does not actually get called.
-    # But the setup code sets this up and needs a command list for that.
+    # But the code setting up site building wants a command list:
     command_auto.nikola_cmd = ["echo"]
 
     # Defeat the site building functionality, and instead insert the test:
     command_auto.run_initial_rebuild = grab_loop_and_run_test
 
-    # Start the development server
-    # which under the hood runs our test when trying to build the site:
-    command_auto.execute(options=options)
-
-    # Verify the test succeeded:
-    if test_inner_error is not None:
-        raise test_inner_error
-    assert test_was_successful, test_problem_description
-
-
-class MyFakeSite(FakeSite):
-    def __init__(self, config, configuration_filename):
-        self.configured = True
-        self.debug = True
-        self.THEMES = []
-        self._plugin_places = []
-        self.registered_auto_watched_folders = set()
-        self.config = config
-        self.configuration_filename = configuration_filename
-
-
-@pytest.fixture(scope="module")
-def site() -> MyFakeSite:
+    try:
+        # Start the development server
+        # which under the hood runs our test when trying to build the site:
+        command_auto.execute(options=options)
+
+        # Verify the test succeeded:
+        if test_inner_error is not None:
+            raise test_inner_error
+        assert test_was_successful, test_problem_description
+    finally:
+        # Nikola is written with the assumption that it can
+        # create the event loop at will without ever cleaning it up.
+        # As this tests runs several times in succession,
+        # that assumption becomes a problem.
+        LOGGER.info("Cleaning up loop.")
+        # Loop cleanup:
+        assert loop_for_this_test is not None
+        assert not loop_for_this_test.is_running()
+        loop_for_this_test.close()
+        asyncio.set_event_loop(None)
+        # We would like to leave it at that,
+        # but doing so causes the next test to fail.
+        #
+        # We did not find asyncio - API to reset the loop
+        # to "back to square one, as if just freshly started".
+        #
+        # The following code does not feel right, it's a kludge,
+        # but it apparently works for now:
+        if sys.platform == 'win32':
+            # For this case, the auto module has special code
+            # (at module load time! 😟) which we reluctantly reproduce here:
+            asyncio.set_event_loop(asyncio.ProactorEventLoop())
+        else:
+            asyncio.set_event_loop(asyncio.new_event_loop())
+
+
+@pytest.fixture(scope="module",
+                params=["https://example.org",
+                        "https://example.org:1234/blog",
+                        "https://example.org:3456/blog/",
+                        "http://example.org/deep/down/a/rabbit/hole"
+                        ])
+def site_and_base_path(request) -> Tuple[MyFakeSite, str]:
+    """Return a fake site and the base_path (root) the dev server should be serving."""
     assert OUTPUT_FOLDER.is_dir(), \
         f"Could not find dev server test fixture {OUTPUT_FOLDER.as_posix()}"
 
@@ -133,9 +181,10 @@ def site() -> MyFakeSite:
         "GALLERY_FOLDERS": [],
         "LISTINGS_FOLDERS": [],
         "IMAGE_FOLDERS": [],
+        "SITE_URL": request.param,
         "OUTPUT_FOLDER": OUTPUT_FOLDER.as_posix(),
     }
-    return MyFakeSite(config, "conf.py")
+    return (MyFakeSite(config), auto.base_path_from_siteuri(request.param))
 
 
 @pytest.fixture(scope="module")
@@ -148,3 +197,27 @@ def expected_text():
     with open(OUTPUT_FOLDER / "index.html", encoding="utf-8") as html_file:
         all_html = html_file.read()
         return all_html[all_html.find("<body>"):]
+
+
+@pytest.mark.parametrize(("uri", "expected_basepath"), [
+    ("http://localhost", ""),
+    ("http://local.host", ""),
+    ("http://localhost/", ""),
+    ("http://local.host/", ""),
+    ("http://localhost:123/", ""),
+    ("http://local.host:456/", ""),
+    ("https://localhost", ""),
+    ("https://local.host", ""),
+    ("https://localhost/", ""),
+    ("https://local.host/", ""),
+    ("https://localhost:123/", ""),
+    ("https://local.host:456/", ""),
+    ("http://example.org/blog", "/blog"),
+    ("https://lorem.ipsum/dolet/", "/dolet"),
+    ("http://example.org:124/blog", "/blog"),
+    ("http://example.org:124/Deep/Rab_bit/hol.e/", "/Deep/Rab_bit/hol.e"),
+    # Would anybody in a sane mind actually do this?
+    ("http://example.org:124/blog?lorem=ipsum&dol=et", "/blog"),
+])
+def test_basepath(uri: str, expected_basepath: Optional[str]) -> None:
+    assert expected_basepath == auto.base_path_from_siteuri(uri)