Merge pull request lightspeed-core#964 from tisnik/lcore-1150-schema-namespace-used-by-conversation-cache

LCORE-1150: Schema namespace used by conversation cache

Author: tisnik

src/cache/postgres_cache.py

@@ -2,6 +2,7 @@
 
 import json
 import psycopg2
+from psycopg2.extensions import AsIs
 
 from cache.cache import Cache
 from cache.cache_error import CacheError
@@ -38,6 +39,10 @@ class PostgresCache(Cache):
     ```
     """
 
+    CREATE_SCHEMA = """
+       CREATE SCHEMA IF NOT EXISTS %s;
+       """
+
     CREATE_CACHE_TABLE = """
         CREATE TABLE IF NOT EXISTS cache (
             user_id              text NOT NULL,
@@ -133,6 +138,18 @@ def connect(self) -> None:
         # even if PostgreSQL is not alive
         self.connection = None
         config = self.postgres_config
+        namespace = "public"
+        if config.namespace is not None:
+            namespace = config.namespace
+            # LCORE-1158 need to be implemented too!
+            # validate namespace contains only valid PostgreSQL identifier characters
+            if not namespace.replace("_", "").replace("$", "").isalnum():
+                raise ValueError(f"Invalid namespace: {namespace}")
+            if len(namespace) > 63:
+                raise ValueError(
+                    f"Invalid namespace: {namespace}. "
+                    "Maximum length is 63 characters."
+                )
         try:
             self.connection = psycopg2.connect(
                 host=config.host,
@@ -143,8 +160,9 @@ def connect(self) -> None:
                 sslmode=config.ssl_mode,
                 sslrootcert=config.ca_cert_path,
                 gssencmode=config.gss_encmode,
+                options=f"-c search_path={namespace}",
             )
-            self.initialize_cache()
+            self.initialize_cache(namespace)
         except Exception as e:
             if self.connection is not None:
                 self.connection.close()
@@ -166,8 +184,16 @@ def connected(self) -> bool:
             logger.error("Disconnected from storage: %s", e)
             return False
 
-    def initialize_cache(self) -> None:
-        """Initialize cache - clean it up etc."""
+    def initialize_cache(self, namespace: str) -> None:
+        """Initialize cache and create schema if needed.
+
+        Parameters:
+            namespace: PostgreSQL schema namespace to use for cache tables.
+                      If not "public", the schema will be created if it doesn't exist.
+
+        Raises:
+            CacheError: If the cache is disconnected.
+        """
         if self.connection is None:
             logger.error("Cache is disconnected")
             raise CacheError("Initialize_cache: cache is disconnected")
@@ -177,6 +203,10 @@ def initialize_cache(self) -> None:
         # and it should not interfere with other statements
         cursor = self.connection.cursor()
 
+        logger.info("Initializing schema")
+        if namespace != "public":
+            cursor.execute(PostgresCache.CREATE_SCHEMA, (AsIs(namespace),))
+
         logger.info("Initializing table for cache")
         cursor.execute(PostgresCache.CREATE_CACHE_TABLE)
 

tests/unit/cache/test_postgres_cache.py

@@ -109,6 +109,56 @@ def postgres_cache_config() -> PostgreSQLDatabaseConfiguration:
     )
 
 
+@pytest.fixture(scope="module", name="postgres_cache_config_fixture_wrong_namespace")
+def postgres_cache_config_wrong_namespace() -> PostgreSQLDatabaseConfiguration:
+    """Fixture with invalid namespace containing spaces for validation testing.
+
+    Create a PostgreSQLDatabaseConfiguration with an invalid namespace ("foo bar baz")
+    to verify that the PostgresCache constructor properly rejects namespaces with spaces.
+
+    Returns:
+        PostgreSQLDatabaseConfiguration: A configuration object with host,
+        port, db, user, SecretStr password, and an invalid namespace containing
+        spaces. Values are placeholders and not intended for real database
+        connections.
+    """
+    # can be any configuration, becuase tests won't really try to
+    # connect to database
+    return PostgreSQLDatabaseConfiguration(
+        host="localhost",
+        port=1234,
+        db="database",
+        user="user",
+        password=SecretStr("password"),
+        namespace="foo bar baz",
+    )
+
+
+@pytest.fixture(scope="module", name="postgres_cache_config_fixture_too_long_namespace")
+def postgres_cache_config_too_long_namespace() -> PostgreSQLDatabaseConfiguration:
+    """Fixture with namespace exceeding PostgreSQL's 63-character limit.
+
+    Create a PostgreSQLDatabaseConfiguration with an overly long namespace
+    to verify that the PostgresCache constructor enforces the maximum length constraint.
+
+    Returns:
+        PostgreSQLDatabaseConfiguration: A configuration object with host,
+        port, db, user, SecretStr password, and a namespace exceeding 63
+        characters. Values are placeholders and not intended for real database
+        connections.
+    """
+    # can be any configuration, becuase tests won't really try to
+    # connect to database
+    return PostgreSQLDatabaseConfiguration(
+        host="localhost",
+        port=1234,
+        db="database",
+        user="user",
+        password=SecretStr("password"),
+        namespace="too long namespace that is longer than allowed 63 characters limit",
+    )
+
+
 def test_cache_initialization(
     postgres_cache_config_fixture: PostgreSQLDatabaseConfiguration,
     mocker: MockerFixture,
@@ -220,7 +270,7 @@ def test_initialize_cache_when_connected(
     mocker.patch("psycopg2.connect")
     cache = PostgresCache(postgres_cache_config_fixture)
     # should not fail
-    cache.initialize_cache()
+    cache.initialize_cache("public")
 
 
 def test_initialize_cache_when_disconnected(
@@ -234,7 +284,7 @@ def test_initialize_cache_when_disconnected(
     cache.connection = None
 
     with pytest.raises(CacheError, match="cache is disconnected"):
-        cache.initialize_cache()
+        cache.initialize_cache("public")
 
 
 def test_ready_method(
@@ -619,3 +669,44 @@ def test_insert_and_get_without_referenced_documents(
     assert len(retrieved_entries) == 1
     assert retrieved_entries[0] == entry_without_docs
     assert retrieved_entries[0].referenced_documents is None
+
+
+def test_initialize_cache_with_custom_namespace(
+    postgres_cache_config_fixture: PostgreSQLDatabaseConfiguration,
+    mocker: MockerFixture,
+) -> None:
+    """Test the initialize_cache() with a custom namespace."""
+    mock_connect = mocker.patch("psycopg2.connect")
+    cache = PostgresCache(postgres_cache_config_fixture)
+
+    mock_connection = mock_connect.return_value
+    mock_cursor = mock_connection.cursor.return_value
+
+    # should not fail and should execute CREATE SCHEMA
+    cache.initialize_cache("custom_schema")
+
+    # Verify CREATE SCHEMA was called for non-public namespace
+    create_schema_calls = [
+        call
+        for call in mock_cursor.execute.call_args_list
+        if "CREATE SCHEMA" in str(call)
+    ]
+    assert len(create_schema_calls) > 0
+
+
+def test_connect_to_cache_with_improper_namespace(
+    postgres_cache_config_fixture_wrong_namespace: PostgreSQLDatabaseConfiguration,
+) -> None:
+    """Test that PostgresCache constructor raises ValueError for invalid namespace."""
+    # should fail due to invalid namespace containing spaces
+    with pytest.raises(ValueError, match="Invalid namespace: foo bar baz"):
+        PostgresCache(postgres_cache_config_fixture_wrong_namespace)
+
+
+def test_connect_to_cache_with_too_long_namespace(
+    postgres_cache_config_fixture_too_long_namespace: PostgreSQLDatabaseConfiguration,
+) -> None:
+    """Test that PostgresCache constructor raises ValueError for invalid namespace."""
+    # should fail due to invalid namespace containing spaces
+    with pytest.raises(ValueError, match="Invalid namespace: too long namespace"):
+        PostgresCache(postgres_cache_config_fixture_too_long_namespace)