Skip to content

Commit

Permalink
Short-circuit HTTP requests in tests (sphinx-doc#12772)
Browse files Browse the repository at this point in the history
  • Loading branch information
AA-Turner authored Aug 12, 2024
1 parent 070f2c1 commit 03b9134
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 51 deletions.
2 changes: 1 addition & 1 deletion sphinx/environment/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def __init__(self, app: Sphinx) -> None:
self.dlfiles: DownloadFiles = DownloadFiles()

# the original URI for images
self.original_image_uri: dict[str, str] = {}
self.original_image_uri: dict[_StrPath, str] = {}

# temporary data storage while reading a document
self.temp_data: dict[str, Any] = {}
Expand Down
5 changes: 3 additions & 2 deletions sphinx/environment/adapters/asset.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Assets adapter for sphinx.environment."""

from sphinx.environment import BuildEnvironment
from sphinx.util._pathlib import _StrPath


class ImageAdapter:
Expand All @@ -9,7 +10,7 @@ def __init__(self, env: BuildEnvironment) -> None:

def get_original_image_uri(self, name: str) -> str:
"""Get the original image URI."""
while name in self.env.original_image_uri:
name = self.env.original_image_uri[name]
while _StrPath(name) in self.env.original_image_uri:
name = self.env.original_image_uri[_StrPath(name)]

return name
107 changes: 59 additions & 48 deletions sphinx/transforms/post_transforms/images.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@
import re
from hashlib import sha1
from math import ceil
from pathlib import Path
from typing import TYPE_CHECKING, Any

from docutils import nodes

from sphinx.locale import __
from sphinx.transforms import SphinxTransform
from sphinx.util import logging, requests
from sphinx.util._pathlib import _StrPath
from sphinx.util.http_date import epoch_to_rfc1123, rfc1123_to_epoch
from sphinx.util.images import get_image_extension, guess_mimetype, parse_data_uri
from sphinx.util.osutil import ensuredir
Expand Down Expand Up @@ -65,50 +67,58 @@ def handle(self, node: nodes.image) -> None:
basename = CRITICAL_PATH_CHAR_RE.sub("_", basename)

uri_hash = sha1(node['uri'].encode(), usedforsecurity=False).hexdigest()
ensuredir(os.path.join(self.imagedir, uri_hash))
path = os.path.join(self.imagedir, uri_hash, basename)

headers = {}
if os.path.exists(path):
timestamp: float = ceil(os.stat(path).st_mtime)
headers['If-Modified-Since'] = epoch_to_rfc1123(timestamp)

config = self.app.config
r = requests.get(
node['uri'], headers=headers,
_user_agent=config.user_agent,
_tls_info=(config.tls_verify, config.tls_cacerts),
)
if r.status_code >= 400:
logger.warning(__('Could not fetch remote image: %s [%d]'),
node['uri'], r.status_code)
else:
self.app.env.original_image_uri[path] = node['uri']

if r.status_code == 200:
with open(path, 'wb') as f:
f.write(r.content)

last_modified = r.headers.get('last-modified')
if last_modified:
timestamp = rfc1123_to_epoch(last_modified)
os.utime(path, (timestamp, timestamp))

mimetype = guess_mimetype(path, default='*')
if mimetype != '*' and os.path.splitext(basename)[1] == '':
# append a suffix if URI does not contain suffix
ext = get_image_extension(mimetype)
newpath = os.path.join(self.imagedir, uri_hash, basename + ext)
os.replace(path, newpath)
self.app.env.original_image_uri.pop(path)
self.app.env.original_image_uri[newpath] = node['uri']
path = newpath
node['candidates'].pop('?')
node['candidates'][mimetype] = path
node['uri'] = path
self.app.env.images.add_file(self.env.docname, path)
path = Path(self.imagedir, uri_hash, basename)
path.parent.mkdir(parents=True, exist_ok=True)
self._download_image(node, path)

except Exception as exc:
logger.warning(__('Could not fetch remote image: %s [%s]'), node['uri'], exc)
msg = __('Could not fetch remote image: %s [%s]')
logger.warning(msg, node['uri'], exc)

def _download_image(self, node: nodes.image, path: Path) -> None:
headers = {}
if path.exists():
timestamp: float = ceil(path.stat().st_mtime)
headers['If-Modified-Since'] = epoch_to_rfc1123(timestamp)

config = self.app.config
r = requests.get(
node['uri'], headers=headers,
_user_agent=config.user_agent,
_tls_info=(config.tls_verify, config.tls_cacerts),
)
if r.status_code >= 400:
msg = __('Could not fetch remote image: %s [%d]')
logger.warning(msg, node['uri'], r.status_code)
else:
self.app.env.original_image_uri[_StrPath(path)] = node['uri']

if r.status_code == 200:
path.write_bytes(r.content)
if last_modified := r.headers.get('Last-Modified'):
timestamp = rfc1123_to_epoch(last_modified)
os.utime(path, (timestamp, timestamp))

self._process_image(node, path)

def _process_image(self, node: nodes.image, path: Path) -> None:
str_path = _StrPath(path)
self.app.env.original_image_uri[str_path] = node['uri']

mimetype = guess_mimetype(path, default='*')
if mimetype != '*' and path.suffix == '':
# append a suffix if URI does not contain suffix
ext = get_image_extension(mimetype) or ''
with_ext = path.with_name(path.name + ext)
os.replace(path, with_ext)
self.app.env.original_image_uri.pop(str_path)
self.app.env.original_image_uri[_StrPath(with_ext)] = node['uri']
path = with_ext
path_str = str(path)
node['candidates'].pop('?')
node['candidates'][mimetype] = path_str
node['uri'] = path_str
self.app.env.images.add_file(self.env.docname, path_str)


class DataURIExtractor(BaseImageConverter):
Expand All @@ -130,16 +140,17 @@ def handle(self, node: nodes.image) -> None:

ensuredir(os.path.join(self.imagedir, 'embeded'))
digest = sha1(image.data, usedforsecurity=False).hexdigest()
path = os.path.join(self.imagedir, 'embeded', digest + ext)
path = _StrPath(self.imagedir, 'embeded', digest + ext)
self.app.env.original_image_uri[path] = node['uri']

with open(path, 'wb') as f:
f.write(image.data)

path_str = str(path)
node['candidates'].pop('?')
node['candidates'][image.mimetype] = path
node['uri'] = path
self.app.env.images.add_file(self.env.docname, path)
node['candidates'][image.mimetype] = path_str
node['uri'] = path_str
self.app.env.images.add_file(self.env.docname, path_str)


def get_filename_for(filename: str, mimetype: str) -> str:
Expand Down Expand Up @@ -258,7 +269,7 @@ def handle(self, node: nodes.image) -> None:
node['candidates'][_to] = destpath
node['uri'] = destpath

self.env.original_image_uri[destpath] = srcpath
self.env.original_image_uri[_StrPath(destpath)] = srcpath
self.env.images.add_file(self.env.docname, destpath)

def convert(self, _from: str, _to: str) -> bool:
Expand Down
18 changes: 18 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import sys
from pathlib import Path
from types import SimpleNamespace
from typing import TYPE_CHECKING

import docutils
Expand Down Expand Up @@ -60,3 +61,20 @@ def _cleanup_docutils() -> Iterator[None]:
sys.path[:] = saved_path

_clean_up_global_state()


@pytest.fixture
def _http_teapot(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]:
"""Short-circuit HTTP requests.
Windows takes too long to fail on connections, hence this fixture.
"""
# https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/418
response = SimpleNamespace(status_code=418)

def _request(*args, **kwargs):
return response

with monkeypatch.context() as m:
m.setattr('sphinx.util.requests._Session.request', _request)
yield
1 change: 1 addition & 0 deletions tests/test_builders/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def test_numbered_circular_toctree(app):
) in warnings


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx('dummy', testroot='images')
def test_image_glob(app):
app.build(force_all=True)
Expand Down
1 change: 1 addition & 0 deletions tests/test_builders/test_build_epub.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ def test_xml_name_pattern_check():
assert not _XML_NAME_PATTERN.match('1bfda21')


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx('epub', testroot='images')
def test_copy_images(app):
app.build()
Expand Down
1 change: 1 addition & 0 deletions tests/test_builders/test_build_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ def test_html_inventory(app):
)


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx(
'html',
testroot='images',
Expand Down
2 changes: 2 additions & 0 deletions tests/test_builders/test_build_html_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import pytest


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx('html', testroot='images')
def test_html_remote_images(app):
app.build(force_all=True)
Expand Down Expand Up @@ -77,6 +78,7 @@ def test_html_scaled_image_link(app):
)


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx('html', testroot='images')
def test_copy_images(app):
app.build()
Expand Down
1 change: 1 addition & 0 deletions tests/test_builders/test_build_latex.py
Original file line number Diff line number Diff line change
Expand Up @@ -2157,6 +2157,7 @@ def test_latex_code_role(app):
) in content


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx('latex', testroot='images')
def test_copy_images(app):
app.build()
Expand Down
1 change: 1 addition & 0 deletions tests/test_builders/test_build_texinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ def test_texinfo_samp_with_variable(app):
assert '@code{Show @var{variable} in the middle}' in output


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx('texinfo', testroot='images')
def test_copy_images(app):
app.build()
Expand Down
1 change: 1 addition & 0 deletions tests/test_extensions/test_ext_math.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ def test_mathjax_numsep_html(app):
assert html in content


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx(
'html',
testroot='ext-math',
Expand Down
2 changes: 2 additions & 0 deletions tests/test_intl/test_intl.py
Original file line number Diff line number Diff line change
Expand Up @@ -1705,6 +1705,7 @@ def test_text_prolog_epilog_substitution(app):
)


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx(
'dummy',
testroot='images',
Expand Down Expand Up @@ -1782,6 +1783,7 @@ def test_image_glob_intl(app):
)


@pytest.mark.usefixtures('_http_teapot')
@pytest.mark.sphinx(
'dummy',
testroot='images',
Expand Down

0 comments on commit 03b9134

Please sign in to comment.