From a268a3dc2f0092c304b3e91b24c03de62c7bfa31 Mon Sep 17 00:00:00 2001
From: Karthikeyan Chinnakonda <karthikeyan@hasura.io>
Date: Tue, 14 Sep 2021 17:32:13 +0530
Subject: [PATCH] server: fix bug which allowed metadata to be inconsistent in
 the `replace_metadata` API call

https://github.com/hasura/graphql-engine-mono/pull/2288

GitOrigin-RevId: 93b181c957a5c38748c419a5146f0590605957ce
---
 CHANGELOG.md                                  |  2 +
 server/src-lib/Hasura/RQL/DDL/Metadata.hs     |  9 +-
 .../Hasura/RQL/Types/SchemaCache/Build.hs     |  2 +-
 server/tests-py/test_metadata.py              | 90 +++++++++++++++++++
 4 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1e64ea5ef30..f699f50def4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,8 @@
 
 - server: update `create_scheduled_event` API to return `event_id` in response
 
+- server: fix bug which allowed inconsistent metadata to exist after the `replace_metadata` API even though `allow_inconsistent_object` is set to `false`.
+
 ## v2.0.9
 
 - server: disable mutation for materialised views (#6688)
diff --git a/server/src-lib/Hasura/RQL/DDL/Metadata.hs b/server/src-lib/Hasura/RQL/DDL/Metadata.hs
index 334cc048088..341d0469b1b 100644
--- a/server/src-lib/Hasura/RQL/DDL/Metadata.hs
+++ b/server/src-lib/Hasura/RQL/DDL/Metadata.hs
@@ -393,7 +393,14 @@ runDropInconsistentMetadata _ = do
   metadataModifier <- execWriterT $ mapM_ (tell . purgeMetadataObj) (reverse inconsSchObjs)
   metadata <- getMetadata
   putMetadata $ unMetadataModifier metadataModifier metadata
-  buildSchemaCacheStrict
+  buildSchemaCache noMetadataModify
+  -- after building the schema cache, we need to check the inconsistent metadata, if any
+  -- are only those which are not droppable
+  newInconsistentObjects <- scInconsistentObjs <$> askSchemaCache
+  let droppableInconsistentObjects = filter droppableInconsistentMetadata newInconsistentObjects
+  unless (null droppableInconsistentObjects) $
+    throwError (err400 Unexpected "cannot continue due to new inconsistent metadata")
+      { qeInternal = Just $ toJSON newInconsistentObjects }
   return successMsg
 
 purgeMetadataObj :: MetadataObjId -> MetadataModifier
diff --git a/server/src-lib/Hasura/RQL/Types/SchemaCache/Build.hs b/server/src-lib/Hasura/RQL/Types/SchemaCache/Build.hs
index 8a67db8c08e..4c6a7431f6e 100644
--- a/server/src-lib/Hasura/RQL/Types/SchemaCache/Build.hs
+++ b/server/src-lib/Hasura/RQL/Types/SchemaCache/Build.hs
@@ -246,7 +246,7 @@ buildSchemaCacheStrict = do
   buildSchemaCache noMetadataModify
   sc <- askSchemaCache
   let inconsObjs = scInconsistentObjs sc
-  when (any droppableInconsistentMetadata inconsObjs) $ do
+  unless (null inconsObjs) $ do
     let err = err400 Unexpected "cannot continue due to inconsistent metadata"
     throwError err{ qeInternal = Just $ toJSON inconsObjs }
 
diff --git a/server/tests-py/test_metadata.py b/server/tests-py/test_metadata.py
index 385622742e1..f2f2fd52bee 100644
--- a/server/tests-py/test_metadata.py
+++ b/server/tests-py/test_metadata.py
@@ -43,6 +43,96 @@ class TestMetadata:
         check_query_f(hge_ctx, self.dir() +
                       '/replace_metadata_allow_inconsistent.yaml')
 
+    def test_replace_metadata_disallow_inconsistent_metadata(self, hge_ctx):
+        st_code, resp = hge_ctx.v1metadataq({"type": "export_metadata", "args": {}})
+        assert st_code == 200, resp
+        default_source_config = {}
+        default_source = list(filter(lambda source: (source["name"] == "default"), resp["sources"]))
+        if default_source:
+            default_source_config = default_source[0]["configuration"]
+        else:
+            assert False, "default source config not found"
+            return
+        st_code, resp = hge_ctx.v1metadataq({
+               "type": "replace_metadata",
+               "version": 2,
+               "args": {
+                 "metadata": {
+                   "version": 3,
+                   "sources": [
+                     {
+                       "name": "default",
+                       "kind": "postgres",
+                       "tables": [
+                         {
+                           "table": {
+                             "schema": "public",
+                             "name": "author"
+                           },
+                           "insert_permissions": [
+                             {
+                               "role": "user1",
+                               "permission": {
+                                 "check": {},
+                                 "columns": [
+                                   "id",
+                                   "name"
+                                 ],
+                                 "backend_only": False
+                               }
+                             },
+                             {
+                               "role": "user2",
+                               "permission": {
+                                 "check": {
+                                   "id": {
+                                     "_eq": "X-Hasura-User-Id"
+                                   }
+                                 },
+                                 "columns": [
+                                   "id",
+                                   "name"
+                                 ],
+                                 "backend_only": False
+                               }
+                             }
+                           ]
+                         }
+                       ],
+                       "configuration": default_source_config
+                     }
+                   ],
+                   "inherited_roles": [
+                     {
+                       "role_name": "users",
+                       "role_set": [
+                         "user2",
+                         "user1"
+                       ]
+                     }
+                   ]
+                 }
+               }
+             })
+        assert st_code == 400, resp
+        assert resp == {
+            "internal": [
+                {
+                    "reason": "Could not inherit permission for the role 'users' for the entity: 'insert permission, table: author, source: 'default''",
+                    "name": "users",
+                    "type": "inherited role permission inconsistency",
+                    "entity": {
+                        "permission_type": "insert",
+                        "source": "default",
+                        "table": "author"
+                    }
+                }
+            ],
+            "path": "$.args",
+            "error": "cannot continue due to inconsistent metadata",
+            "code": "unexpected"
+        }
+
     def test_dump_internal_state(self, hge_ctx):
         check_query_f(hge_ctx, self.dir() + '/dump_internal_state.yaml')