Merge remote-tracking branch 'origin/master' into copilot/add-cache-analytics-framework

Author: Borda

AGENTS.md

@@ -439,10 +439,13 @@ ______________________________________________________________________
 - `make test-all-local` - Test all backends with Docker
 - `make test-external` - Test all external backends
 - `make test-mongo-local` - Test MongoDB only
+- `make test-mongo-inmemory` - Test MongoDB marker set with local/in-memory setup
+- `make test-mongo-also-local` - Test MongoDB together with local core tests
 - `make test-redis-local` - Test Redis only
 - `make test-sql-local` - Test SQL only
 - `make services-start` - Start all Docker containers
 - `make services-stop` - Stop all Docker containers
+- `make services-logs` - Tail logs for Dockerized test services
 
 **Available Cores:**
 
@@ -462,8 +465,11 @@ ______________________________________________________________________
 - `-k, --keep-running` - Keep containers running after tests
 - `-h, --html-coverage` - Generate HTML coverage report
 - `-f, --files` - Run only specific test files
+- `-p, --parallel` - Run tests with `pytest-xdist`
+- `-w, --workers` - Set number of parallel workers (default: `auto`)
 
-**Note:** External backends (MongoDB, Redis, SQL) require Docker. S3, memory, and pickle backends work without Docker.
+**Note:** Redis and SQL backends require Docker. MongoDB tests run in-memory by default (no Docker needed) when invoked directly (for example, `pytest -m mongo` or `make test-mongo-inmemory` without `CACHIER_TEST_VS_DOCKERIZED_MONGO` set). When using `./scripts/test-local.sh mongo` or including `mongo` in the core list, MongoDB is always run via a Docker container and requires Docker. S3, memory, and pickle backends work without Docker.
+You can also set cores with `CACHIER_TEST_CORES="mongo redis" ./scripts/test-local.sh`, in which case both MongoDB and Redis will run via Docker.
 
 ______________________________________________________________________
 

Makefile

@@ -69,13 +69,13 @@ test-mongo-inmemory:
 
 test-mongo-docker:
 	@echo "Running MongoDB tests against Docker MongoDB..."
-	./scripts/test-mongo-local.sh
+	./scripts/test-local.sh mongo
 
 test-mongo-local: test-mongo-docker
 
 test-mongo-also-local:
 	@echo "Running MongoDB tests with local core tests..."
-	./scripts/test-mongo-local.sh --mode also-local
+	./scripts/test-local.sh mongo memory pickle
 
 # New unified testing targets
 test-local:

README.rst

@@ -95,7 +95,7 @@ You can add a default, pickle-based, persistent cache to your function - meaning
     """Your function now has a persistent cache mapped by argument values!"""
     return {'arg1': arg1, 'arg2': arg2}
 
-Class and object methods can also be cached. Cachier will automatically ignore the `self` parameter when determining the cache key for an object method. **This means that methods will be cached across all instances of an object, which may not be what you want.**
+Class and object methods can also be cached. Cachier will automatically ignore the ``self`` parameter when determining the cache key for an object method. **This means that methods will be cached across all instances of an object, which may not be what you want.** Because this is a common source of bugs, ``@cachier`` raises a ``TypeError`` by default when applied to an instance method (a function whose first parameter is named ``self``). This error is raised when ``@cachier`` is applied (at class definition time), not when the method is called. To opt in to cross-instance cache sharing, pass ``allow_non_static_methods=True``.
 
 .. code-block:: python
 
@@ -108,17 +108,18 @@ Class and object methods can also be cached. Cachier will automatically ignore t
       return arg_1 + arg_2
 
     # Instance method does not depend on object's internal state, so good to cache
-    @cachier()
+    @cachier(allow_non_static_methods=True)
     def good_usage_1(self, arg_1, arg_2):
       return arg_1 + arg_2
 
     # Instance method is calling external service, probably okay to cache
-    @cachier()
+    @cachier(allow_non_static_methods=True)
     def good_usage_2(self, arg_1, arg_2):
       result = self.call_api(arg_1, arg_2)
       return result
 
     # Instance method relies on object attribute, NOT good to cache
+    # @cachier() would raise TypeError here -- this is intentional
     @cachier()
     def bad_usage(self, arg_1, arg_2):
       return arg_1 + arg_2 + self.arg_3
@@ -149,6 +150,7 @@ The following parameters will only be applied to decorators defined after `set_d
 *  `pickle_reload`
 *  `separate_files`
 *  `entry_size_limit`
+*  `allow_non_static_methods`
 
 These parameters can be changed at any time and they will apply to all decorators:
 

src/cachier/config.py

@@ -66,6 +66,7 @@ class Params:
     cleanup_stale: bool = False
     cleanup_interval: timedelta = timedelta(days=1)
     entry_size_limit: Optional[int] = None
+    allow_non_static_methods: bool = False
 
 
 _global_params = Params()
@@ -130,7 +131,14 @@ def set_global_params(**params: Any) -> None:
       'cleanup_interval', and 'caching_enabled'. In some cores, if the
       decorator was created without concrete value for 'wait_for_calc_timeout',
       calls that check calculation timeouts will fall back to the global
-      'wait_for_calc_timeout' as well.
+      'wait_for_calc_timeout' as well. 'allow_non_static_methods'
+      (decoration-time only) controls whether instance methods are
+      permitted; it is read once when @cachier is applied, not on each call.
+
+    Note that ``allow_non_static_methods`` is a **decoration-time**
+    parameter: it is checked once when the ``@cachier`` decorator is
+    applied and is not re-read on each function call. Changing it via
+    ``set_global_params`` only affects decorators created after the call.
 
     """
     import cachier

src/cachier/core.py

@@ -218,6 +218,7 @@ def cachier(
     cleanup_stale: Optional[bool] = None,
     cleanup_interval: Optional[timedelta] = None,
     entry_size_limit: Optional[Union[int, str]] = None,
+    allow_non_static_methods: Optional[bool] = None,
     enable_metrics: bool = False,
     metrics_sampling_rate: float = 1.0,
 ):
@@ -307,6 +308,13 @@ def cachier(
         Maximum serialized size of a cached value. Values exceeding the limit
         are returned but not cached. Human readable strings like ``"10MB"`` are
         allowed.
+    allow_non_static_methods : bool, optional
+        If True, allows ``@cachier`` to decorate instance methods (functions
+        whose first parameter is named ``self``). By default, decorating an
+        instance method raises ``TypeError`` because the ``self`` argument is
+        ignored for cache-key computation, meaning all instances share the
+        same cache -- which is rarely the intended behaviour. Set this to
+        ``True`` only when cross-instance cache sharing is intentional.
     enable_metrics: bool, optional
         Enable metrics collection for this cached function. When enabled,
         cache hits, misses, latencies, and other performance metrics are tracked. Defaults to False.
@@ -394,6 +402,23 @@ def cachier(
 
     def _cachier_decorator(func):
         core.set_func(func)
+
+        # Guard: raise TypeError when decorating an instance method unless
+        # explicitly opted in.  The 'self' parameter is ignored for cache-key
+        # computation, so all instances share the same cache.
+        if core.func_is_method:
+            _allow_methods = _update_with_defaults(allow_non_static_methods, "allow_non_static_methods")
+            if not _allow_methods:
+                raise TypeError(
+                    f"@cachier cannot decorate instance method "
+                    f"'{func.__qualname__}' because the 'self' parameter is "
+                    "excluded from cache-key computation and all instances "
+                    "would share a single cache. Pass allow_non_static_methods=True "
+                    "to the decorator or call "
+                    "set_global_params(allow_non_static_methods=True) if "
+                    "cross-instance cache sharing is intentional."
+                )
+
         is_coroutine = inspect.iscoroutinefunction(func)
 
         if backend == "mongo":

src/cachier/cores/base.py

@@ -52,12 +52,23 @@ def __init__(
         self.wait_for_calc_timeout = wait_for_calc_timeout
         self.lock = threading.RLock()
         self.entry_size_limit = entry_size_limit
+        self.func_is_method: bool = False
         self.metrics = metrics
 
     def set_func(self, func):
         """Set the function this core will use.
 
-        This has to be set before any method is called. Also determine if the function is an object method.
+        This must be called before any other method is invoked.  In addition
+        to storing ``func`` on the instance, this method inspects the
+        function's signature and sets ``self.func_is_method`` to ``True``
+        when the first parameter is named ``"self"``.
+
+        Notes
+        -----
+        Detection is name-based: only ``func_params[0] == "self"`` is
+        checked.  ``@classmethod`` functions whose first parameter is
+        conventionally named ``cls`` are not detected as methods --
+        this is a known gap.
 
         """
         # unwrap if the function is functools.partial

tests/conftest.py

@@ -3,103 +3,119 @@
 import logging
 import os
 import re
+from typing import Optional
 from urllib.parse import parse_qs, unquote, urlencode, urlparse, urlunparse
 
 import pytest
 
 logger = logging.getLogger(__name__)
 
 
-def _worker_schema_name(worker_id: str) -> str | None:
+def _worker_schema_name(worker_id: str) -> Optional[str]:
     """Return a safe SQL schema name for an xdist worker ID."""
     match = re.fullmatch(r"gw(\d+)", worker_id)
     if match is None:
         return None
     return f"test_worker_{match.group(1)}"
 
 
-@pytest.fixture(autouse=True)
-def inject_worker_schema_for_sql_tests(monkeypatch, request):
-    """Automatically inject worker-specific schema into SQL connection string.
+def _build_worker_url(original_url: str, schema_name: str) -> str:
+    """Return a copy of original_url with search_path set to schema_name."""
+    parsed = urlparse(original_url)
+    query_params = parse_qs(parsed.query)
+
+    if "options" in query_params:
+        current_options = unquote(query_params["options"][0])
+        new_options = f"{current_options} -csearch_path={schema_name}"
+    else:
+        new_options = f"-csearch_path={schema_name}"
+
+    query_params["options"] = [new_options]
+    new_query = urlencode(query_params, doseq=True)
+    return urlunparse(
+        (
+            parsed.scheme,
+            parsed.netloc,
+            parsed.path,
+            parsed.params,
+            new_query,
+            parsed.fragment,
+        )
+    )
+
 
-    This fixture enables parallel SQL test execution by giving each pytest-xdist worker its own PostgreSQL schema,
-    preventing table creation conflicts.
+@pytest.fixture(scope="session")
+def worker_sql_connection(request: pytest.FixtureRequest) -> Optional[str]:
+    """Create the worker-specific PostgreSQL schema once per xdist worker session.
+
+    Returns the worker-specific connection URL, or None when schema isolation is not
+    needed (serial run or non-PostgreSQL backend). The schema is created with
+    ``CREATE SCHEMA IF NOT EXISTS`` so this fixture is safe to run even if the schema
+    already exists from a previous interrupted run.
+
+    A non-None return value means "use this URL"; schema creation is attempted but may
+    fail silently (e.g. if SQLAlchemy is not installed or the DB is unreachable). Tests
+    that depend on the schema will fail at the DB level with a diagnostic error.
 
     """
-    # Only apply to SQL tests
-    if "sql" not in request.node.keywords:
-        yield
-        return
+    # Avoid touching SQL backends entirely when no SQL tests are collected.
+    has_sql_tests = any("sql" in item.keywords for item in request.session.items)
+    if not has_sql_tests:
+        return None
 
     worker_id = os.environ.get("PYTEST_XDIST_WORKER", "master")
-
     if worker_id == "master":
-        # Not running in parallel, no schema isolation needed
-        yield
-        return
+        return None
 
-    # Get the original SQL connection string
     original_url = os.environ.get("SQLALCHEMY_DATABASE_URL", "sqlite:///:memory:")
+    if "postgresql" not in original_url:
+        return None
 
-    if "postgresql" in original_url:
-        # Create worker-specific schema name
-        schema_name = _worker_schema_name(worker_id)
-        if schema_name is None:
-            logger.warning("Unexpected worker ID for SQL schema isolation: %s", worker_id)
-            yield
-            return
-
-        # Parse the URL
-        parsed = urlparse(original_url)
-
-        # Get existing query parameters
-        query_params = parse_qs(parsed.query)
-
-        # Add or update the options parameter to set search_path
-        if "options" in query_params:
-            # Append to existing options
-            current_options = unquote(query_params["options"][0])
-            new_options = f"{current_options} -csearch_path={schema_name}"
-        else:
-            # Create new options
-            new_options = f"-csearch_path={schema_name}"
-
-        query_params["options"] = [new_options]
-
-        # Rebuild the URL with updated query parameters
-        new_query = urlencode(query_params, doseq=True)
-        new_url = urlunparse(
-            (
-                parsed.scheme,
-                parsed.netloc,
-                parsed.path,
-                parsed.params,
-                new_query,
-                parsed.fragment,
-            )
-        )
+    schema_name = _worker_schema_name(worker_id)
+    if schema_name is None:
+        logger.warning("Unexpected worker ID for SQL schema isolation: %s", worker_id)
+        return None
+
+    new_url = _build_worker_url(original_url, schema_name)
 
-        # Override both the environment variable and the module constant
-        monkeypatch.setenv("SQLALCHEMY_DATABASE_URL", new_url)
+    engine = None
+    try:
+        from sqlalchemy import create_engine, text
 
-        # Also patch the SQL_CONN_STR constant used in tests
-        import tests.sql_tests.test_sql_core
+        engine = create_engine(original_url)
+        with engine.connect() as conn:
+            conn.execute(text(f"CREATE SCHEMA IF NOT EXISTS {schema_name}"))
+            conn.commit()
+    except Exception as e:
+        logger.debug("Failed to create schema %s: %s", schema_name, e)
+    finally:
+        if engine is not None:
+            engine.dispose()
 
-        monkeypatch.setattr(tests.sql_tests.test_sql_core, "SQL_CONN_STR", new_url)
+    return new_url
 
-        # Ensure schema creation by creating it before tests run
-        try:
-            from sqlalchemy import create_engine, text
 
-            # Use original URL to create schema (without search_path)
-            engine = create_engine(original_url)
-            with engine.connect() as conn:
-                conn.execute(text(f"CREATE SCHEMA IF NOT EXISTS {schema_name}"))
-                conn.commit()
-            engine.dispose()
-        except Exception as e:
-            # If we can't create the schema, the test will fail anyway
-            logger.debug(f"Failed to create schema {schema_name}: {e}")
+@pytest.fixture(autouse=True)
+def inject_worker_schema_for_sql_tests(monkeypatch, request, worker_sql_connection):
+    """Automatically inject worker-specific schema into SQL connection string.
+
+    This fixture enables parallel SQL test execution by giving each pytest-xdist worker
+    its own PostgreSQL schema, preventing table creation conflicts.
+
+    Schema creation is handled once per worker session by
+    :func:`worker_sql_connection`. This fixture only performs lightweight
+    per-test monkeypatching of the environment variable and module constant.
+
+    """
+    if "sql" not in request.node.keywords or worker_sql_connection is None:
+        yield
+        return
+
+    monkeypatch.setenv("SQLALCHEMY_DATABASE_URL", worker_sql_connection)
+
+    import tests.sql_tests.test_sql_core
+
+    monkeypatch.setattr(tests.sql_tests.test_sql_core, "SQL_CONN_STR", worker_sql_connection)
 
     yield
 
@@ -193,4 +209,4 @@ def cleanup_test_schemas(request):
                 engine.dispose()
             except Exception as e:
                 # If cleanup fails, it's not critical
-                logger.debug(f"Failed to cleanup schema {schema_name}: {e}")
+                logger.debug("Failed to cleanup schema %s: %s", schema_name, e)

tests/mongo_tests/test_async_mongo_core.py

@@ -76,7 +76,7 @@ async def async_mongetter():
     call_count = 0
 
     class _MongoMethods:
-        @cachier(mongetter=async_mongetter)
+        @cachier(mongetter=async_mongetter, allow_non_static_methods=True)
         async def async_cached_mongo_method_args_kwargs(self, x: int, y: int) -> int:
             nonlocal call_count
             call_count += 1

tests/redis_tests/test_async_redis_core.py

@@ -84,7 +84,7 @@ async def get_redis_client():
     call_count = 0
 
     class _RedisMethods:
-        @cachier(backend="redis", redis_client=get_redis_client)
+        @cachier(backend="redis", redis_client=get_redis_client, allow_non_static_methods=True)
         async def async_cached_redis_method_args_kwargs(self, x: int, y: int) -> int:
             nonlocal call_count
             call_count += 1