Skip to content

Commit

Permalink
Completed testing framework - mypy, pylint compliance
Browse files Browse the repository at this point in the history
 - testing framework now (hopefully) up to full coverage
 - modifications made to tests to accomodate bandit concerns
 - typing complete - including hack
 - windows compatibility checked
 - tidied up rogue file from aiopath-typed accidental inclusion
  • Loading branch information
ajCameron committed Aug 8, 2023
1 parent f48f52e commit 86b3a24
Show file tree
Hide file tree
Showing 13 changed files with 1,116 additions and 806 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,17 @@

from __future__ import annotations

from typing import Any, Literal, Sequence, Union
from typing import Literal, Sequence, Union

import asyncio
import dataclasses
import fcntl
import functools
import logging
import os
import pathlib

import aiofiles
from aiofiles.threadpool.binary import AsyncBufferedIOBase as BytesIOBase
from aiofiles.threadpool.text import AsyncTextIOWrapper as TestIOBase

from mewbot.api.v1 import Input, IOConfig, Output, OutputEvent
from mewbot.io.file_storage.portable_locking import AsyncFileLock


class FileStorage(IOConfig):
Expand Down Expand Up @@ -173,12 +169,12 @@ async def output(self, event: OutputEvent) -> bool:
self._logger.warning("Cannot output - base path '%s' does not exist", self._path)
return False

path = self._path.joinpath(event.path).resolve()
path = self._path.joinpath(event.path).resolve().absolute()

if not path.is_relative_to(self._path):
if not path.is_relative_to(self._path.resolve().absolute()):
self._logger.error(
"Refusing to write to %s, as it would result in a file outside of %s",
event.path,
path,
self._path,
)
return False
Expand All @@ -203,7 +199,11 @@ async def _process_directory_create_event(path: pathlib.Path) -> bool:
Create the given directory (and any parent directories).
"""

path.mkdir(parents=True, exist_ok=True)
try:
path.mkdir(parents=True, exist_ok=True)
except PermissionError:
return False

return True

async def _process_file_write_event(
Expand Down Expand Up @@ -235,6 +235,8 @@ async def _process_file_write_event(
return True
except FileExistsError:
return False
except PermissionError:
return False

@staticmethod
async def _do_file_write_str(
Expand All @@ -256,69 +258,19 @@ async def _process_delete_file_event(self, path: pathlib.Path) -> bool:
try:
path.unlink()
except FileNotFoundError:
self._logger.warning("Unable to delete non-existent path %s", path)
self._logger.warning(
"Unable to delete non-existent path %s - FileNotFoundError", path
)
return False
except PermissionError:
self._logger.warning(
"Unable to delete non-existent path %s - PermissionError", path
)
return False

return True


class AsyncFileLock:
"""
Asynchronous context-manager for locking a file descriptor with fcntl.
This will acquire an exclusive lock using `fcntl.flock` in an executor
thread, and wait up to the timeout to return.
"""

_file: TestIOBase | BytesIOBase
_timeout: float

def __init__(self, file: TestIOBase | BytesIOBase, timeout: float = 5) -> None:
"""
Asynchronous context-manager for locking a file descriptor with fcntl.
This will acquire an exclusive lock using fcntl.flock in an executor
thread, and wait up to the timeout to return.
"""

self._file = file
self._timeout = timeout

async def __aenter__(self) -> AsyncFileLock:
"""Acquire the lock as part of an async context manager."""

await self.acquire()
return self

async def __aexit__(
self,
exc_type: type[BaseException] | None,
exc_val: BaseException | None,
exc_tb: Any | None,
) -> None:
"""Release the lock as part of an async context manager."""

await self.release()

async def acquire(self) -> None:
"""
Acquire an exclusive a lock on the current file descriptor.
"""

call = functools.partial(fcntl.flock, self._file.fileno(), fcntl.LOCK_EX)
loop = asyncio.get_running_loop()
future = loop.run_in_executor(None, call)

await asyncio.wait_for(future, self._timeout)

async def release(self) -> None:
"""
Release any held locks on the file descriptor.
"""

fcntl.flock(self._file.fileno(), fcntl.LOCK_UN)


@dataclasses.dataclass
class FSOutputEvent(OutputEvent):
"""
Expand Down Expand Up @@ -358,4 +310,5 @@ class DeleteFileOutputEvent(FSOutputEvent):
"CreateDirectoryOutputEvent",
"WriteToFileOutputEvent",
"DeleteFileOutputEvent",
"AsyncFileLock",
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
# SPDX-FileCopyrightText: 2023 Mewbot Developers <[email protected]>
#
# SPDX-License-Identifier: BSD-2-Clause

"""
The locking apis on Windows and linux are sufficiently different a compatibility layer is needed.
"""

from types import ModuleType
from typing import IO, TYPE_CHECKING, Any, Optional, Type

import asyncio
import functools

import portalocker
from aiofiles.threadpool.binary import AsyncBufferedIOBase as BytesIOBase
from aiofiles.threadpool.text import AsyncTextIOWrapper as TestIOBase

# Hideous hack to make mypy "happy"
if not TYPE_CHECKING:
FCNTL_INSTALLED = None
try:
import fcntl as FCNTL_INSTALLED
except ModuleNotFoundError:
fcntl: Optional[ModuleType] = None
fcntl = FCNTL_INSTALLED
else:
fcntl: Optional[ModuleType] = None

FCNTL = globals()["fcntl"]


class FileHandlePortalocker(portalocker.Lock):
"""
By default, portalocker opens a new file handle for use with a lock.
We want to reuse an existing file handle - the one aiofiles is using.
"""

_internal_fh: IO[bytes] | IO[str]

def __init__(self, *args: Any, **kwargs: Any) -> None:
"""
Stores the file handle to return inplace of the new one portalocker usually creates.
:param args:
:param kwargs:
"""
super().__init__(*args, **kwargs)

def set_internal_fh(self, file_handle: IO[bytes] | IO[str]) -> None:
"""
Set the internal file handle.
:param file_handle:
:return:
"""
self._internal_fh = file_handle

def _get_fh(self) -> IO[bytes] | IO[str]:
"""
Return the internal file handle.
:return:
"""
return self._internal_fh

def release(self) -> None:
"""
Needed for mypy.
:return:
"""
super_status: Any = super().release() # type: ignore[no-untyped-call]
assert super_status is None, "Unexpected return from super().release()"


class FcntlAsyncFileLock:
"""
Asynchronous context-manager for locking a file descriptor with fcntl.
This will acquire an exclusive lock using `fcntl.flock` in an executor
thread, and wait up to the timeout to return.
"""

_file: TestIOBase | BytesIOBase
_timeout: float

fcntl: Optional[ModuleType]

def __init__(self, file: TestIOBase | BytesIOBase, timeout: float = 5) -> None:
"""
Asynchronous context-manager for locking a file descriptor with fcntl.
This will acquire an exclusive lock using fcntl.flock in an executor
thread, and wait up to the timeout to return.
"""

self._file = file
self._timeout = timeout

self.fcntl = FCNTL

async def __aenter__(self) -> "FcntlAsyncFileLock":
"""Acquire the lock as part of an async context manager."""

await self.acquire()
return self

async def __aexit__(
self,
exc_type: type[BaseException] | None,
exc_val: BaseException | None,
exc_tb: Any | None,
) -> None:
"""Release the lock as part of an async context manager."""

await self.release()

async def acquire(self) -> None:
"""
Acquire an exclusive a lock on the current file descriptor.
"""

assert self.fcntl is not None, "This position should never be reached."

call = functools.partial(self.fcntl.flock, self._file.fileno(), self.fcntl.LOCK_EX)
loop = asyncio.get_running_loop()
future = loop.run_in_executor(None, call)

await asyncio.wait_for(future, self._timeout)

async def release(self) -> None:
"""
Release any held locks on the file descriptor.
"""

assert self.fcntl is not None, "This position should never be reached."

self.fcntl.flock(self._file.fileno(), self.fcntl.LOCK_UN)


class PortaLockerAsyncFileLock:
"""
Asynchronous context-manager for locking a file descriptor with portalocker.
This will acquire an exclusive lock using `fcntl.flock` in an executor
thread, and wait up to the timeout to return.
"""

_file: TestIOBase | BytesIOBase
_timeout: float
_lock: FileHandlePortalocker

def __init__(self, file: TestIOBase | BytesIOBase, timeout: float = 5) -> None:
"""
Asynchronous context-manager for locking a file descriptor with fcntl.
This will acquire an exclusive lock using fcntl.flock in an executor
thread, and wait up to the timeout to return.
"""

self._file = file
self._timeout = timeout

self._lock = FileHandlePortalocker(filename="bypassed")
self._lock.set_internal_fh(self._file._file) # type: ignore[union-attr]

async def __aenter__(self) -> "PortaLockerAsyncFileLock":
"""Acquire the lock as part of an async context manager."""

await self.acquire()
return self

async def __aexit__(
self,
exc_type: type[BaseException] | None,
exc_val: BaseException | None,
exc_tb: Any | None,
) -> None:
"""Release the lock as part of an async context manager."""

await self.release()

async def acquire(self) -> None:
"""
Acquire an exclusive a lock on the current file descriptor.
"""
# From the examples - portalocker.lock(file, portalocker.LockFlags.EXCLUSIVE)
call = functools.partial(
self._lock.acquire,
)
loop = asyncio.get_running_loop()
future = loop.run_in_executor(None, call)

# No clue why this is different between Windows and Linux
try:
await asyncio.wait_for(future, self._timeout)
except asyncio.exceptions.TimeoutError as exp:
raise TimeoutError from exp

async def release(self) -> None:
"""
Release any held locks on the file descriptor.
"""

self._lock.release()


AsyncFileLock: Type[PortaLockerAsyncFileLock] | Type[FcntlAsyncFileLock]
if FCNTL is None:
AsyncFileLock = PortaLockerAsyncFileLock
else:
AsyncFileLock = FcntlAsyncFileLock
Loading

0 comments on commit 86b3a24

Please sign in to comment.