Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions AUTHORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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>`_
Expand All @@ -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>`_
Expand Down
3 changes: 3 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
=============
Expand Down
29 changes: 23 additions & 6 deletions nikola/plugins/command/auto/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import subprocess
import sys
import typing
import urllib.parse
import webbrowser

import blinker
Expand Down Expand Up @@ -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."""

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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:
Expand All @@ -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)

Expand Down
151 changes: 112 additions & 39 deletions tests/integration/test_dev_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 = {
Expand All @@ -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:
Comment on lines +160 to +161
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was required to work around a bug in asyncio, IIRC it prevented shutting down the server with ^C. You may move it to some later point than loading the module.

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()}"

Expand All @@ -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")
Expand All @@ -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?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I doubt it would work in any reasonable way.

("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)