From 24d87f90afedb0ec5863c5131808b005d47e4af3 Mon Sep 17 00:00:00 2001 From: Kwist Date: Sun, 16 Jun 2024 20:52:15 +0300 Subject: [PATCH 1/7] add test to check the number of `__init__` calls --- tests/test_filelock.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_filelock.py b/tests/test_filelock.py index 8ecd743b..ecf423ae 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -785,3 +785,20 @@ class Lock2(lock_type): # type: ignore[valid-type, misc] assert isinstance(Lock1._instances, WeakValueDictionary) # noqa: SLF001 assert isinstance(Lock2._instances, WeakValueDictionary) # noqa: SLF001 assert Lock1._instances is not Lock2._instances # noqa: SLF001 + + +def test_singleton_locks_when_inheriting_init_is_called_once(tmp_path: Path) -> None: + + init_calls = 0 + + class MyFileLock(FileLock): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + nonlocal init_calls + init_calls += 1 + + lock_path = tmp_path / "a" + MyFileLock(str(lock_path), is_singleton=True) + MyFileLock(str(lock_path), is_singleton=True) + + assert init_calls == 1 From c857e44b2a184f8d5cb905f0e5a97193442b0eaf Mon Sep 17 00:00:00 2001 From: Kwist Date: Sun, 16 Jun 2024 21:31:41 +0300 Subject: [PATCH 2/7] FileLockMeta --- src/filelock/_api.py | 95 +++++++++++++++++++++-------------------- src/filelock/asyncio.py | 31 +++++++++++++- tests/test_filelock.py | 7 ++- 3 files changed, 83 insertions(+), 50 deletions(-) diff --git a/src/filelock/_api.py b/src/filelock/_api.py index 2894e61b..1c5995a1 100644 --- a/src/filelock/_api.py +++ b/src/filelock/_api.py @@ -5,7 +5,7 @@ import os import time import warnings -from abc import ABC, abstractmethod +from abc import ABCMeta, abstractmethod from dataclasses import dataclass from threading import local from typing import TYPE_CHECKING, Any @@ -77,12 +77,8 @@ class ThreadLocalFileContext(FileLockContext, local): """A thread local version of the ``FileLockContext`` class.""" -class BaseFileLock(ABC, contextlib.ContextDecorator): - """Abstract base class for a file lock object.""" - - _instances: WeakValueDictionary[str, Self] - - def __new__( # noqa: PLR0913 +class FileLockMeta(ABCMeta): + def __call__( cls, lock_file: str | os.PathLike[str], timeout: float = -1, # noqa: ARG003 @@ -92,19 +88,53 @@ def __new__( # noqa: PLR0913 blocking: bool = True, # noqa: ARG003 is_singleton: bool = False, **kwargs: Any, # capture remaining kwargs for subclasses # noqa: ARG003, ANN401 - ) -> Self: - """Create a new lock object or if specified return the singleton instance for the lock file.""" - if not is_singleton: - return super().__new__(cls) - - instance = cls._instances.get(str(lock_file)) - if not instance: - self = super().__new__(cls) - cls._instances[str(lock_file)] = self - return self + ) -> BaseFileLock: + if is_singleton: + instance = cls._instances.get(str(lock_file)) + if instance: + params_to_check = { + "thread_local": (thread_local, instance.is_thread_local()), + "timeout": (timeout, instance.timeout), + "mode": (mode, instance.mode), + "blocking": (blocking, instance.blocking), + } + + non_matching_params = { + name: (passed_param, set_param) + for name, (passed_param, set_param) in params_to_check.items() + if passed_param != set_param + } + if not non_matching_params: + return instance + + # parameters do not match; raise error + msg = "Singleton lock instances cannot be initialized with differing arguments" + msg += "\nNon-matching arguments: " + for param_name, (passed_param, set_param) in non_matching_params.items(): + msg += f"\n\t{param_name} (existing lock has {set_param} but {passed_param} was passed)" + raise ValueError(msg) + + instance = super().__call__( + lock_file=lock_file, + timeout=timeout, + mode=mode, + thread_local=thread_local, + blocking=blocking, + is_singleton=is_singleton, + **kwargs, + ) + + if is_singleton: + cls._instances[str(lock_file)] = instance return instance # type: ignore[return-value] # https://github.com/python/mypy/issues/15322 + +class BaseFileLock(contextlib.ContextDecorator, metaclass=FileLockMeta): + """Abstract base class for a file lock object.""" + + _instances: WeakValueDictionary[str, Self] + def __init_subclass__(cls, **kwargs: dict[str, Any]) -> None: """Setup unique state for lock subclasses.""" super().__init_subclass__(**kwargs) @@ -136,34 +166,6 @@ def __init__( # noqa: PLR0913 to pass the same object around. """ - if is_singleton and hasattr(self, "_context"): - # test whether other parameters match existing instance. - if not self.is_singleton: - msg = "__init__ should only be called on initialized object if it is a singleton" - raise RuntimeError(msg) - - params_to_check = { - "thread_local": (thread_local, self.is_thread_local()), - "timeout": (timeout, self.timeout), - "mode": (mode, self.mode), - "blocking": (blocking, self.blocking), - } - - non_matching_params = { - name: (passed_param, set_param) - for name, (passed_param, set_param) in params_to_check.items() - if passed_param != set_param - } - if not non_matching_params: - return # bypass initialization because object is already initialized - - # parameters do not match; raise error - msg = "Singleton lock instances cannot be initialized with differing arguments" - msg += "\nNon-matching arguments: " - for param_name, (passed_param, set_param) in non_matching_params.items(): - msg += f"\n\t{param_name} (existing lock has {set_param} but {passed_param} was passed)" - raise ValueError(msg) - self._is_thread_local = thread_local self._is_singleton = is_singleton @@ -175,7 +177,8 @@ def __init__( # noqa: PLR0913 "mode": mode, "blocking": blocking, } - self._context: FileLockContext = (ThreadLocalFileContext if thread_local else FileLockContext)(**kwargs) + context_cls = ThreadLocalFileContext if thread_local else FileLockContext + self._context: FileLockContext = context_cls(**kwargs) def is_thread_local(self) -> bool: """:return: a flag indicating if this lock is thread local or not""" diff --git a/src/filelock/asyncio.py b/src/filelock/asyncio.py index feedd277..971f4cd3 100644 --- a/src/filelock/asyncio.py +++ b/src/filelock/asyncio.py @@ -11,7 +11,7 @@ from threading import local from typing import TYPE_CHECKING, Any, Callable, NoReturn -from ._api import BaseFileLock, FileLockContext +from ._api import BaseFileLock, FileLockContext, FileLockMeta from ._error import Timeout from ._soft import SoftFileLock from ._unix import UnixFileLock @@ -67,7 +67,34 @@ async def __aexit__( # noqa: D105 await self.lock.release() -class BaseAsyncFileLock(BaseFileLock): +class AsyncFileLockMeta(FileLockMeta): + def __call__( + cls, + lock_file: str | os.PathLike[str], + timeout: float = -1, # noqa: ARG003 + mode: int = 0o644, # noqa: ARG003 + thread_local: bool = False, # noqa: FBT001, FBT002, ARG003 + *, + blocking: bool = True, # noqa: ARG003 + is_singleton: bool = False, + loop: asyncio.AbstractEventLoop | None = None, + run_in_executor: bool = True, + executor: futures.Executor | None = None, + ) -> BaseAsyncFileLock: + return super().__call__( + lock_file=lock_file, + timeout=timeout, + mode=mode, + thread_local=thread_local, + blocking=blocking, + is_singleton=is_singleton, + loop=loop, + run_in_executor=run_in_executor, + executor=executor, + ) + + +class BaseAsyncFileLock(BaseFileLock, metaclass=AsyncFileLockMeta): """Base class for asynchronous file locks.""" def __init__( # noqa: PLR0913 diff --git a/tests/test_filelock.py b/tests/test_filelock.py index ecf423ae..c65261b0 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -798,7 +798,10 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: init_calls += 1 lock_path = tmp_path / "a" - MyFileLock(str(lock_path), is_singleton=True) - MyFileLock(str(lock_path), is_singleton=True) + lock1 = MyFileLock(str(lock_path), is_singleton=True) + lock2 = MyFileLock(str(lock_path), is_singleton=True) assert init_calls == 1 + + del lock1 + del lock2 From 305e005a9b31313c3563d9a0de3cddccfae03908 Mon Sep 17 00:00:00 2001 From: Kwist Date: Sun, 16 Jun 2024 21:34:37 +0300 Subject: [PATCH 3/7] fix lint --- src/filelock/_api.py | 12 ++++++------ src/filelock/asyncio.py | 12 ++++++------ tests/test_filelock.py | 3 +-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/filelock/_api.py b/src/filelock/_api.py index 1c5995a1..1e897093 100644 --- a/src/filelock/_api.py +++ b/src/filelock/_api.py @@ -78,16 +78,16 @@ class ThreadLocalFileContext(FileLockContext, local): class FileLockMeta(ABCMeta): - def __call__( + def __call__( # noqa: PLR0913 cls, lock_file: str | os.PathLike[str], - timeout: float = -1, # noqa: ARG003 - mode: int = 0o644, # noqa: ARG003 - thread_local: bool = True, # noqa: FBT001, FBT002, ARG003 + timeout: float = -1, + mode: int = 0o644, + thread_local: bool = True, # noqa: FBT001, FBT002 *, - blocking: bool = True, # noqa: ARG003 + blocking: bool = True, is_singleton: bool = False, - **kwargs: Any, # capture remaining kwargs for subclasses # noqa: ARG003, ANN401 + **kwargs: Any, # capture remaining kwargs for subclasses # noqa: ANN401 ) -> BaseFileLock: if is_singleton: instance = cls._instances.get(str(lock_file)) diff --git a/src/filelock/asyncio.py b/src/filelock/asyncio.py index 971f4cd3..750174bc 100644 --- a/src/filelock/asyncio.py +++ b/src/filelock/asyncio.py @@ -68,14 +68,14 @@ async def __aexit__( # noqa: D105 class AsyncFileLockMeta(FileLockMeta): - def __call__( - cls, + def __call__( # noqa: PLR0913 + cls, # noqa: N805 lock_file: str | os.PathLike[str], - timeout: float = -1, # noqa: ARG003 - mode: int = 0o644, # noqa: ARG003 - thread_local: bool = False, # noqa: FBT001, FBT002, ARG003 + timeout: float = -1, + mode: int = 0o644, + thread_local: bool = False, # noqa: FBT001, FBT002 *, - blocking: bool = True, # noqa: ARG003 + blocking: bool = True, is_singleton: bool = False, loop: asyncio.AbstractEventLoop | None = None, run_in_executor: bool = True, diff --git a/tests/test_filelock.py b/tests/test_filelock.py index c65261b0..e1c02a3b 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -788,11 +788,10 @@ class Lock2(lock_type): # type: ignore[valid-type, misc] def test_singleton_locks_when_inheriting_init_is_called_once(tmp_path: Path) -> None: - init_calls = 0 class MyFileLock(FileLock): - def __init__(self, *args: Any, **kwargs: Any) -> None: + def __init__(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401 super().__init__(*args, **kwargs) nonlocal init_calls init_calls += 1 From 725e9dbcbe16b201c6d717b6677b4fda4b8e447d Mon Sep 17 00:00:00 2001 From: Kwist Date: Mon, 17 Jun 2024 07:58:05 +0300 Subject: [PATCH 4/7] minor touch --- tests/test_filelock.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_filelock.py b/tests/test_filelock.py index e1c02a3b..69dc4209 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -800,7 +800,5 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401 lock1 = MyFileLock(str(lock_path), is_singleton=True) lock2 = MyFileLock(str(lock_path), is_singleton=True) + assert lock1 is lock2 assert init_calls == 1 - - del lock1 - del lock2 From c9aad371b254babd6e91364bdb580a6cf0663c8a Mon Sep 17 00:00:00 2001 From: Kwist Date: Mon, 17 Jun 2024 08:09:11 +0300 Subject: [PATCH 5/7] minor touch --- src/filelock/asyncio.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/filelock/asyncio.py b/src/filelock/asyncio.py index 750174bc..1ae203c1 100644 --- a/src/filelock/asyncio.py +++ b/src/filelock/asyncio.py @@ -81,6 +81,9 @@ def __call__( # noqa: PLR0913 run_in_executor: bool = True, executor: futures.Executor | None = None, ) -> BaseAsyncFileLock: + if thread_local and run_in_executor: + msg = "run_in_executor is not supported when thread_local is True" + raise ValueError(msg) return super().__call__( lock_file=lock_file, timeout=timeout, @@ -131,9 +134,6 @@ def __init__( # noqa: PLR0913 """ self._is_thread_local = thread_local self._is_singleton = is_singleton - if thread_local and run_in_executor: - msg = "run_in_executor is not supported when thread_local is True" - raise ValueError(msg) # Create the context. Note that external code should not work with the context directly and should instead use # properties of this class. From 7dcf256f56e75c2c87945a8e0fc3d35c46add611 Mon Sep 17 00:00:00 2001 From: Kwist Date: Mon, 17 Jun 2024 10:27:44 +0300 Subject: [PATCH 6/7] revert self._context --- src/filelock/_api.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/filelock/_api.py b/src/filelock/_api.py index 1e897093..e8ec4123 100644 --- a/src/filelock/_api.py +++ b/src/filelock/_api.py @@ -177,8 +177,7 @@ def __init__( # noqa: PLR0913 "mode": mode, "blocking": blocking, } - context_cls = ThreadLocalFileContext if thread_local else FileLockContext - self._context: FileLockContext = context_cls(**kwargs) + self._context: FileLockContext = (ThreadLocalFileContext if thread_local else FileLockContext)(**kwargs) def is_thread_local(self) -> bool: """:return: a flag indicating if this lock is thread local or not""" From 8a7972be17ef9fb6db83b2a241cee00ea9bd4701 Mon Sep 17 00:00:00 2001 From: Kwist Date: Tue, 18 Jun 2024 10:06:35 +0300 Subject: [PATCH 7/7] fix type check --- src/filelock/_api.py | 12 ++++++------ src/filelock/asyncio.py | 7 ++++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/filelock/_api.py b/src/filelock/_api.py index e8ec4123..96beffa4 100644 --- a/src/filelock/_api.py +++ b/src/filelock/_api.py @@ -8,7 +8,7 @@ from abc import ABCMeta, abstractmethod from dataclasses import dataclass from threading import local -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast from weakref import WeakValueDictionary from ._error import Timeout @@ -90,7 +90,7 @@ def __call__( # noqa: PLR0913 **kwargs: Any, # capture remaining kwargs for subclasses # noqa: ANN401 ) -> BaseFileLock: if is_singleton: - instance = cls._instances.get(str(lock_file)) + instance = cls._instances.get(str(lock_file)) # type: ignore[attr-defined] if instance: params_to_check = { "thread_local": (thread_local, instance.is_thread_local()), @@ -105,7 +105,7 @@ def __call__( # noqa: PLR0913 if passed_param != set_param } if not non_matching_params: - return instance + return cast(BaseFileLock, instance) # parameters do not match; raise error msg = "Singleton lock instances cannot be initialized with differing arguments" @@ -125,15 +125,15 @@ def __call__( # noqa: PLR0913 ) if is_singleton: - cls._instances[str(lock_file)] = instance + cls._instances[str(lock_file)] = instance # type: ignore[attr-defined] - return instance # type: ignore[return-value] # https://github.com/python/mypy/issues/15322 + return cast(BaseFileLock, instance) class BaseFileLock(contextlib.ContextDecorator, metaclass=FileLockMeta): """Abstract base class for a file lock object.""" - _instances: WeakValueDictionary[str, Self] + _instances: WeakValueDictionary[str, BaseFileLock] def __init_subclass__(cls, **kwargs: dict[str, Any]) -> None: """Setup unique state for lock subclasses.""" diff --git a/src/filelock/asyncio.py b/src/filelock/asyncio.py index 1ae203c1..f5848c89 100644 --- a/src/filelock/asyncio.py +++ b/src/filelock/asyncio.py @@ -9,7 +9,7 @@ import time from dataclasses import dataclass from threading import local -from typing import TYPE_CHECKING, Any, Callable, NoReturn +from typing import TYPE_CHECKING, Any, Callable, NoReturn, cast from ._api import BaseFileLock, FileLockContext, FileLockMeta from ._error import Timeout @@ -68,7 +68,7 @@ async def __aexit__( # noqa: D105 class AsyncFileLockMeta(FileLockMeta): - def __call__( # noqa: PLR0913 + def __call__( # type: ignore[override] # noqa: PLR0913 cls, # noqa: N805 lock_file: str | os.PathLike[str], timeout: float = -1, @@ -84,7 +84,7 @@ def __call__( # noqa: PLR0913 if thread_local and run_in_executor: msg = "run_in_executor is not supported when thread_local is True" raise ValueError(msg) - return super().__call__( + instance = super().__call__( lock_file=lock_file, timeout=timeout, mode=mode, @@ -95,6 +95,7 @@ def __call__( # noqa: PLR0913 run_in_executor=run_in_executor, executor=executor, ) + return cast(BaseAsyncFileLock, instance) class BaseAsyncFileLock(BaseFileLock, metaclass=AsyncFileLockMeta):