Reduce the amount of test suite gymnastics with new WebishServer API

Instead of forcing the test suite to try to discover the location of an
unnamed temporary file, let it just assert that the file is created in the
directory specified in the temporary file factory.
This commit is contained in:
Jean-Paul Calderone 2023-07-07 16:00:00 -04:00
parent 1b99e23b9a
commit 4b23b779e4
4 changed files with 61 additions and 77 deletions

View File

@ -1,5 +1,5 @@
"""
Ported to Python 3.
Functionality related to operating a Tahoe-LAFS node (client _or_ server).
"""
from __future__ import annotations
@ -7,6 +7,7 @@ import os
import stat
import time
import weakref
import tempfile
from typing import Optional, Iterable
from base64 import urlsafe_b64encode
from functools import partial
@ -1029,14 +1030,14 @@ class _Client(node.Node, pollmixin.PollMixin):
def init_web(self, webport):
self.log("init_web(webport=%s)", args=(webport,))
from allmydata.webish import WebishServer
from allmydata.webish import WebishServer, anonymous_tempfile
nodeurl_path = self.config.get_config_path("node.url")
staticdir_config = self.config.get_config("node", "web.static", "public_html")
staticdir = self.config.get_config_path(staticdir_config)
ws = WebishServer(
self,
webport,
self._get_tempdir(),
anonymous_tempfile(self._get_tempdir()),
nodeurl_path,
staticdir,
)

View File

@ -341,7 +341,7 @@ class WebMixin(TimezoneMixin):
self.ws = webish.WebishServer(
self.s,
"0",
tempdir=tempdir.path,
webish.anonymous_tempfile(tempdir.path),
staticdir=self.staticdir,
clock=self.clock,
now_fn=lambda:self.fakeTime,

View File

@ -1,17 +1,8 @@
"""
Tests for ``allmydata.webish``.
Ported to Python 3.
"""
from __future__ import absolute_import
from __future__ import division
from __future__ import print_function
from __future__ import unicode_literals
from future.utils import PY2
if PY2:
from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401
import tempfile
from uuid import (
uuid4,
)
@ -59,6 +50,7 @@ from ..common import (
from ...webish import (
TahoeLAFSRequest,
TahoeLAFSSite,
anonymous_tempfile,
)
@ -183,8 +175,14 @@ class TahoeLAFSSiteTests(SyncTestCase):
:return: ``None`` if the logging looks good.
"""
logPath = self.mktemp()
tempdir = self.mktemp()
FilePath(tempdir).makedirs()
site = TahoeLAFSSite(self.mktemp(), Resource(), logPath=logPath)
site = TahoeLAFSSite(
anonymous_tempfile(tempdir),
Resource(),
logPath=logPath,
)
site.startFactory()
channel = DummyChannel()
@ -257,11 +255,17 @@ class TahoeLAFSSiteTests(SyncTestCase):
Create and return a new ``TahoeLAFSRequest`` hooked up to a
``TahoeLAFSSite``.
:param bytes tempdir: The temporary directory to give to the site.
:param FilePath tempdir: The temporary directory to configure the site
to write large temporary request bodies to. The temporary files
will be named for ease of testing.
:return TahoeLAFSRequest: The new request instance.
"""
site = TahoeLAFSSite(tempdir.path, Resource(), logPath=self.mktemp())
site = TahoeLAFSSite(
lambda: tempfile.NamedTemporaryFile(dir=tempdir.path),
Resource(),
logPath=self.mktemp(),
)
site.startFactory()
channel = DummyChannel()
@ -275,6 +279,7 @@ class TahoeLAFSSiteTests(SyncTestCase):
A request body smaller than 1 MiB is kept in memory.
"""
tempdir = FilePath(self.mktemp())
tempdir.makedirs()
request = self._create_request(tempdir)
request.gotLength(request_body_size)
self.assertThat(
@ -284,57 +289,21 @@ class TahoeLAFSSiteTests(SyncTestCase):
def _large_request_test(self, request_body_size):
"""
Assert that when a request with a body of of the given size is received
its content is written to the directory the ``TahoeLAFSSite`` is
configured with.
Assert that when a request with a body of the given size is
received its content is written a temporary file created by the given
tempfile factory.
"""
tempdir = FilePath(self.mktemp())
tempdir.makedirs()
request = self._create_request(tempdir)
# So. Bad news. The temporary file for the uploaded content is
# unnamed (and this isn't even necessarily a bad thing since it is how
# you get automatic on-process-exit cleanup behavior on POSIX). It's
# not visible by inspecting the filesystem. It has no name we can
# discover. Then how do we verify it is written to the right place?
# The question itself is meaningless if we try to be too precise. It
# *has* no filesystem location. However, it is still stored *on* some
# filesystem. We still want to make sure it is on the filesystem we
# specified because otherwise it might be on a filesystem that's too
# small or undesirable in some other way.
#
# I don't know of any way to ask a file descriptor which filesystem
# it's on, either, though. It might be the case that the [f]statvfs()
# result could be compared somehow to infer the filesystem but
# ... it's not clear what the failure modes might be there, across
# different filesystems and runtime environments.
#
# Another approach is to make the temp directory unwriteable and
# observe the failure when an attempt is made to create a file there.
# This is hardly a lovely solution but at least it's kind of simple.
#
# It would be nice if it worked consistently cross-platform but on
# Windows os.chmod is more or less broken.
if platform.isWindows():
request.gotLength(request_body_size)
self.assertThat(
tempdir.children(),
HasLength(1),
)
else:
tempdir.chmod(0o550)
with self.assertRaises(OSError) as ctx:
request.gotLength(request_body_size)
raise Exception(
"OSError not raised, instead tempdir.children() = {}".format(
tempdir.children(),
),
)
self.assertThat(
ctx.exception.errno,
Equals(EACCES),
)
request.gotLength(request_body_size)
# We can see the temporary file in the temporary directory we
# specified because _create_request makes a request that uses named
# temporary files instead of the usual anonymous temporary files.
self.assertThat(
tempdir.children(),
HasLength(1),
)
def test_unknown_request_size(self):
"""

View File

@ -4,7 +4,7 @@ General web server-related utilities.
from __future__ import annotations
from six import ensure_str
from typing import BinaryIO, Callable, Optional
import re, time, tempfile
from urllib.parse import parse_qsl, urlencode
@ -217,36 +217,50 @@ def censor(queryargs: bytes) -> bytes:
return urlencode(result, safe="[]").encode("ascii")
def anonymous_tempfile(tempdir: bytes) -> BinaryIO:
"""
Create a no-argument callable for creating a new temporary file in the
given directory.
:param tempdir: The directory in which temporary files with be created.
:return: The callable.
"""
return lambda: tempfile.TemporaryFile(dir=tempdir)
class TahoeLAFSSite(Site, object):
"""
The HTTP protocol factory used by Tahoe-LAFS.
Among the behaviors provided:
* A configurable temporary directory where large request bodies can be
written so they don't stay in memory.
* A configurable temporary file factory for large request bodies to avoid
keeping them in memory.
* A log formatter that writes some access logs but omits capability
strings to help keep them secret.
"""
requestFactory = TahoeLAFSRequest
def __init__(self, tempdir, *args, **kwargs):
def __init__(self, make_tempfile: Callable[[], BinaryIO], *args, **kwargs):
Site.__init__(self, *args, logFormatter=_logFormatter, **kwargs)
self._tempdir = tempdir
assert callable(make_tempfile)
with make_tempfile():
pass
self._make_tempfile = make_tempfile
def getContentFile(self, length):
def getContentFile(self, length: Optional[int]) -> BinaryIO:
if length is None or length >= 1024 * 1024:
return tempfile.TemporaryFile(dir=self._tempdir)
return self._make_tempfile()
return BytesIO()
class WebishServer(service.MultiService):
# The type in Twisted for services is wrong in 22.10...
# https://github.com/twisted/twisted/issues/10135
name = "webish" # type: ignore[assignment]
def __init__(self, client, webport, tempdir, nodeurl_path=None, staticdir=None,
def __init__(self, client, webport, make_tempfile, nodeurl_path=None, staticdir=None,
clock=None, now_fn=time.time):
service.MultiService.__init__(self)
# the 'data' argument to all render() methods default to the Client
@ -256,7 +270,7 @@ class WebishServer(service.MultiService):
# time in a deterministic manner.
self.root = root.Root(client, clock, now_fn)
self.buildServer(webport, tempdir, nodeurl_path, staticdir)
self.buildServer(webport, make_tempfile, nodeurl_path, staticdir)
# If set, clock is a twisted.internet.task.Clock that the tests
# use to test ophandle expiration.
@ -266,9 +280,9 @@ class WebishServer(service.MultiService):
self.root.putChild(b"storage-plugins", StoragePlugins(client))
def buildServer(self, webport, tempdir, nodeurl_path, staticdir):
def buildServer(self, webport, make_tempfile, nodeurl_path, staticdir):
self.webport = webport
self.site = TahoeLAFSSite(tempdir, self.root)
self.site = TahoeLAFSSite(make_tempfile, self.root)
self.staticdir = staticdir # so tests can check
if staticdir:
self.root.putChild(b"static", static.File(staticdir))
@ -346,4 +360,4 @@ class IntroducerWebishServer(WebishServer):
def __init__(self, introducer, webport, nodeurl_path=None, staticdir=None):
service.MultiService.__init__(self)
self.root = introweb.IntroducerRoot(introducer)
self.buildServer(webport, tempfile.tempdir, nodeurl_path, staticdir)
self.buildServer(webport, tempfile.TemporaryFile, nodeurl_path, staticdir)