fix: schema cache loading before the in-db config (#3670)

Fixes #3660. Load the config after getting the pg version but before loading the schema.

The regression happened on f09655b.

Also remove schema cache load wrapper and separate db queries in different functions.

Co-authored-by: Laurence Isla <lau.isla.c@gmail.com>

src/PostgREST/AppState.hs

@@ -161,7 +161,7 @@ initWithPool (sock, adminSock) pool conf loggerState metricsState observer = do
   deb <-
     let decisecond = 100000 in
     mkDebounce defaultDebounceSettings
-       { debounceAction = internalSchemaCacheLoad appState
+       { debounceAction = retryingSchemaCacheLoad appState
        , debounceFreq = decisecond
        , debounceEdge = leadingEdge -- runs the worker at the start and the end
        }
@@ -355,13 +355,6 @@ putSCacheStatus = atomicWriteIORef . stateSCacheStatus
 getObserver :: AppState -> ObservationHandler
 getObserver = stateObserver
 
-internalSchemaCacheLoad :: AppState -> IO ()
-internalSchemaCacheLoad appState = do
-  AppConfig{..} <- getConfig appState
-  void $ retryingSchemaCacheLoad appState
-  -- We cannot retry reading the in-db config after it fails immediately, because it could have user errors. We just report the error and continue.
-  when configDbConfig $ readInDbConfig False appState
-
 -- | Try to load the schema cache and retry if it fails.
 --
 -- This is done by repeatedly: 1) flushing the pool, 2) querying the version and validating that the postgres version is supported by us, and 3) loading the schema cache.
@@ -369,16 +362,17 @@ internalSchemaCacheLoad appState = do
 --
 -- + Because connections cache the pg catalog(see #2620)
 -- + For rapid recovery. Otherwise, the pool idle or lifetime timeout would have to be reached for new healthy connections to be acquired.
-retryingSchemaCacheLoad :: AppState -> IO (Maybe PgVersion, Maybe SchemaCache)
+retryingSchemaCacheLoad :: AppState -> IO ()
 retryingSchemaCacheLoad appState@AppState{stateObserver=observer, stateMainThreadId=mainThreadId} =
-  retrying retryPolicy shouldRetry (\RetryStatus{rsIterNumber, rsPreviousDelay} -> do
+  void $ retrying retryPolicy shouldRetry (\RetryStatus{rsIterNumber, rsPreviousDelay} -> do
     when (rsIterNumber > 0) $ do
       let delay = fromMaybe 0 rsPreviousDelay `div` oneSecondInUs
       observer $ ConnectionRetryObs delay
       putNextListenerDelay appState delay
 
     flushPool appState
-    (,) <$> qPgVersion <*> qSchemaCache
+
+    (,) <$> qPgVersion <*> (qInDbConfig *> qSchemaCache)
   )
   where
     qPgVersion :: IO (Maybe PgVersion)
@@ -400,6 +394,11 @@ retryingSchemaCacheLoad appState@AppState{stateObserver=observer, stateMainThrea
           putPgVersion appState actualPgVersion
           return $ Just actualPgVersion
 
+    qInDbConfig :: IO ()
+    qInDbConfig = do
+      AppConfig{..} <- getConfig appState
+      when configDbConfig $ readInDbConfig False appState
+
     qSchemaCache :: IO (Maybe SchemaCache)
     qSchemaCache = do
       conf@AppConfig{..} <- getConfig appState
@@ -438,6 +437,7 @@ retryingSchemaCacheLoad appState@AppState{stateObserver=observer, stateMainThrea
     oneSecondInUs = 1000000 -- one second in microseconds
 
 -- | Reads the in-db config and reads the config file again
+-- | We don't retry reading the in-db config after it fails immediately, because it could have user errors. We just report the error and continue.
 readInDbConfig :: Bool -> AppState -> IO ()
 readInDbConfig startingUp appState@AppState{stateObserver=observer} = do
   AppConfig{..} <- getConfig appState

test/io/__snapshots__/test_cli/test_schema_cache_snapshot[dbRoutines].yaml

@@ -218,6 +218,40 @@
       pdSchema: public
       pdVolatility: Volatile
 
+- - qiName: reset_db_schemas_config
+    qiSchema: public
+  - - pdDescription: null
+      pdFuncSettings: []
+      pdHasVariadic: false
+      pdName: reset_db_schemas_config
+      pdParams: []
+      pdReturnType:
+        contents:
+          contents:
+            qiName: void
+            qiSchema: pg_catalog
+          tag: Scalar
+        tag: Single
+      pdSchema: public
+      pdVolatility: Volatile
+
+- - qiName: change_db_schemas_config
+    qiSchema: public
+  - - pdDescription: null
+      pdFuncSettings: []
+      pdHasVariadic: false
+      pdName: change_db_schemas_config
+      pdParams: []
+      pdReturnType:
+        contents:
+          contents:
+            qiName: void
+            qiSchema: pg_catalog
+          tag: Scalar
+        tag: Single
+      pdSchema: public
+      pdVolatility: Volatile
+
 - - qiName: change_db_schema_and_full_reload
     qiSchema: public
   - - pdDescription: null

test/io/fixtures.sql

@@ -33,6 +33,9 @@ GRANT
 CREATE SCHEMA v1;
 GRANT USAGE ON SCHEMA v1 TO postgrest_test_anonymous;
 
+CREATE SCHEMA test;
+GRANT USAGE ON SCHEMA test TO postgrest_test_anonymous;
+
 CREATE TABLE authors_only ();
 GRANT SELECT ON authors_only TO postgrest_test_author;
 
@@ -226,3 +229,17 @@ $$ language sql;
 create function get_statement_timeout(items) returns text as $$
   select current_setting('statement_timeout', true) as statement_timeout
 $$ language sql;
+
+create function change_db_schemas_config() returns void as $_$
+begin
+  alter role postgrest_test_authenticator set pgrst.db_schemas = 'test';
+end $_$ volatile security definer language plpgsql;
+
+create function reset_db_schemas_config() returns void as $_$
+begin
+  alter role postgrest_test_authenticator reset pgrst.db_schemas;
+end $_$ volatile security definer language plpgsql ;
+
+create function test.get_current_schema() returns text as $$
+  select current_schema()::text;
+$$ language sql;

test/io/test_io.py

@@ -1618,3 +1618,20 @@ def test_admin_metrics(defaultenv):
         assert "pgrst_db_pool_waiting" in response.text
         assert "pgrst_db_pool_available" in response.text
         assert "pgrst_db_pool_timeouts_total" in response.text
+
+
+def test_schema_cache_startup_load_with_in_db_config(defaultenv, metapostgrest):
+    "verify that the Schema Cache loads correctly at startup, using the in-db `pgrst.db_schemas` config"
+
+    response = metapostgrest.session.post("/rpc/change_db_schemas_config")
+    assert response.text == ""
+    assert response.status_code == 204
+
+    with run(env=defaultenv) as postgrest:
+        response = postgrest.session.get("/rpc/get_current_schema")
+        assert response.text == '"test"'
+        assert response.status_code == 200
+
+    response = metapostgrest.session.post("/rpc/reset_db_schemas_config")
+    assert response.text == ""
+    assert response.status_code == 204