server: Fix fine-grained incremental cache invalidation (fix #3759)

CHANGELOG.md

@@ -67,6 +67,10 @@ This release contains the [PDV refactor (#4111)](https://github.com/hasura/graph
 
 (Add entries here in the order of: server, console, cli, docs, others)
 
+- server: Fix fine-grained incremental cache invalidation (fix #3759)
+
+  This issue could cause enum table values to sometimes not be properly reloaded without restarting `graphql-engine`. Now a `reload_metadata` API call (or clicking “Reload enum values” in the console) should consistently force a reload of all enum table values.
+
 - server: add `--websocket-compression` command-line flag for enabling websocket compression (fix #3292)
 - server: some mutations that cannot be performed will no longer be in the schema (for instance, `delete_by_pk` mutations won't be shown to users that do not have select permissions on all primary keys) (#4111)
 - server: miscellaneous description changes (#4111)

server/src-lib/Hasura/Incremental/Internal/Cache.hs

@@ -70,10 +70,12 @@ instance (MonadUnique m) => ArrowCache m (Rule m) where
         (k $! (s <> s')) (b, s') (listenAccesses r')
 
       cached accesses a b (Rule r) = Rule \s a' k -> if
-        | unchanged accesses a a' -> k s b (cached accesses a b (Rule r))
+        | unchanged accesses a a' -> (k $! (s <> accesses)) b (cached accesses a b (Rule r))
         | otherwise -> r s a' \s' (b', accesses') r' -> k s' b' (cached accesses' a' b' r')
 
-  newDependency = arrM \a -> newUniqueS <&> \u -> Dependency (DependencyRoot u) a
+  newDependency = Rule \s a k -> do
+    key <- DependencyRoot <$> newUniqueS
+    k s (Dependency key a) (arr (Dependency key))
   {-# INLINABLE newDependency #-}
 
   dependOn = Rule \s (Dependency key v) k -> (k $! recordAccess key AccessedAll s) v dependOn

server/src-test/Hasura/IncrementalSpec.hs

@@ -31,6 +31,29 @@ spec = do
       (_, state3) <- runStateT (Inc.rebuild result2 (True, True)) 0
       state3 `shouldBe` 2
 
+    it "tracks dependencies within nested uses of cache across multiple executions" do
+      let rule :: (MonadWriter String m, MonadUnique m)
+               => Inc.Rule m (Inc.InvalidationKey, Inc.InvalidationKey) ()
+          rule = proc (key1, key2) -> do
+            dep1 <- Inc.newDependency -< key2
+            (key1, dep1) >- Inc.cache (proc (_, dep2) ->
+              dep2 >- Inc.cache (proc dep3 -> do
+                Inc.dependOn -< dep3
+                arrM tell -< "executed"))
+            returnA -< ()
+
+      let key1 = Inc.initialInvalidationKey
+          key2 = Inc.invalidate key1
+
+      (result1, log1) <- runWriterT $ Inc.build rule (key1, key1)
+      log1 `shouldBe` "executed"
+
+      (result2, log2) <- runWriterT $ Inc.rebuild result1 (key2, key1)
+      log2 `shouldBe` ""
+
+      (_, log3) <- runWriterT $ Inc.rebuild result2 (key2, key2)
+      log3 `shouldBe` "executed"
+
   describe "keyed" $ do
     it "preserves incrementalization when entries don’t change" $ do
       let rule :: (MonadWriter (S.HashSet (String, Integer)) m, MonadUnique m)

server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/introspect_enum_values.yaml

@@ -0,0 +1,44 @@
+- description: Insert a new enum value without reloading
+  url: /v1/query
+  status: 200
+  query:
+    type: run_sql
+    args:
+      sql: |
+        INSERT INTO crust VALUES ('thick');
+
+- description: Check that the previous enum values are cached
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      crust:
+        enumValues:
+        - name: thin
+  query: &crust_query
+    query: |
+      {
+        crust: __type(name: "crust_enum") {
+          enumValues {
+            name
+          }
+        }
+      }
+
+- description: Reload the metadata
+  url: /v1/query
+  status: 200
+  query:
+    type: reload_metadata
+    args: {}
+
+- description: Check that the enum values are updated
+  url: /v1/graphql
+  status: 200
+  response:
+    data:
+      crust:
+        enumValues:
+        - name: thick
+        - name: thin
+  query: *crust_query

server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/setup.yaml

@@ -0,0 +1,24 @@
+type: bulk
+args:
+- type: run_sql
+  args:
+    sql: |
+      CREATE TABLE crust (name text NOT NULL PRIMARY KEY);
+      INSERT INTO crust VALUES ('thin');
+- type: add_existing_table_or_view
+  args: crust
+- type: set_table_is_enum
+  args:
+    table: crust
+    is_enum: true
+- type: run_sql
+  args:
+    sql: |
+      CREATE TABLE pizza
+        ( id serial NOT NULL PRIMARY KEY
+        , name text NOT NULL
+        , crust text NOT NULL REFERENCES crust );
+
+      INSERT INTO pizza (name, crust) VALUES ('margarita', 'thin');
+- type: add_existing_table_or_view
+  args: pizza

server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/teardown.yaml

@@ -0,0 +1,8 @@
+type: bulk
+args:
+- type: run_sql
+  args:
+    sql: |
+      DROP TABLE pizza;
+      DROP TABLE crust;
+    cascade: true

server/tests-py/test_v1_queries.py

@@ -729,6 +729,16 @@ def test_add_test_schema_enum_table(self, hge_ctx):
     def test_relationship_with_inconsistent_enum_table(self, hge_ctx):
         check_query_f(hge_ctx, self.dir() + '/relationship_with_inconsistent_enum_table.yaml')
 
+# regression test for issue #3759
+@usefixtures('per_method_tests_db_state')
+class TestSetTableIsEnumSetAndDelayedReload:
+    @classmethod
+    def dir(cls):
+        return 'queries/v1/set_table_is_enum/set_and_delayed_reload'
+
+    def test_introspect_enum_values(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/introspect_enum_values.yaml')
+
 @usefixtures('per_method_tests_db_state')
 class TestSetTableCustomFields: