From 67ac12c1967dc2206b5d125b9a66b2d4c5aa7b35 Mon Sep 17 00:00:00 2001 From: Marc Scholten Date: Tue, 18 Oct 2022 09:27:29 +0200 Subject: [PATCH] When dropping a column, also drop related policies Fixes #1550 --- IHP/IDE/SchemaDesigner/SchemaOperations.hs | 32 +++++++++++++++++++ Test/IDE/CodeGeneration/MigrationGenerator.hs | 27 +++++++++++++++- .../SchemaDesigner/SchemaOperationsSpec.hs | 29 +++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/IHP/IDE/SchemaDesigner/SchemaOperations.hs b/IHP/IDE/SchemaDesigner/SchemaOperations.hs index 4e0986129..93fdc964c 100644 --- a/IHP/IDE/SchemaDesigner/SchemaOperations.hs +++ b/IHP/IDE/SchemaDesigner/SchemaOperations.hs @@ -552,11 +552,43 @@ deleteColumn DeleteColumnOptions { .. } schema = then deleteTriggerIfExists (updatedAtTriggerName tableName) else \schema -> schema ) + |> filter deletePolicyReferencingPolicy where deleteColumnInTable :: Statement -> Statement deleteColumnInTable (StatementCreateTable table@CreateTable { name, columns }) | name == tableName = StatementCreateTable $ table { columns = delete (columns !! columnId) columns} deleteColumnInTable statement = statement + deletePolicyReferencingPolicy :: Statement -> Bool + deletePolicyReferencingPolicy CreatePolicy { tableName = policyTable, using, check } | policyTable == tableName = + case (using, check) of + (Just using, Nothing) -> not (isRef using) + (Nothing, Just check) -> not (isRef check) + (Just using, Just check) -> not (isRef using && isRef check) + where + isRef :: Expression -> Bool + isRef (TextExpression {}) = False + isRef (VarExpression var) = var == columnName + isRef (CallExpression _ args) = foldl' (||) False (map isRef args) + isRef (NotEqExpression a b) = isRef a || isRef b + isRef (EqExpression a b) = isRef a || isRef b + isRef (AndExpression a b) = isRef a || isRef b + isRef (IsExpression a b) = isRef a || isRef b + isRef (InExpression a b) = isRef a || isRef b + isRef (NotExpression a) = isRef a + isRef (ExistsExpression a) = isRef a + isRef (OrExpression a b) = isRef a || isRef b + isRef (LessThanExpression a b) = isRef a || isRef b + isRef (LessThanOrEqualToExpression a b) = isRef a || isRef b + isRef (GreaterThanExpression a b) = isRef a || isRef b + isRef (GreaterThanOrEqualToExpression a b) = isRef a || isRef b + isRef (DoubleExpression _) = False + isRef (IntExpression _) = False + isRef (TypeCastExpression a _) = isRef a + isRef (SelectExpression _) = False + isRef (DotExpression a _) = isRef a + isRef (ConcatenationExpression a b) = isRef a || isRef b + deletePolicyReferencingPolicy otherwise = True + -- | Returns True if a CreateIndex statement references a specific column -- -- E.g. given a schema like this: diff --git a/Test/IDE/CodeGeneration/MigrationGenerator.hs b/Test/IDE/CodeGeneration/MigrationGenerator.hs index 3cce645d1..bc988b620 100644 --- a/Test/IDE/CodeGeneration/MigrationGenerator.hs +++ b/Test/IDE/CodeGeneration/MigrationGenerator.hs @@ -1204,8 +1204,33 @@ CREATE POLICY "Users can read and edit their own record" ON public.users USING ( |] diffSchemas targetSchema actualSchema `shouldBe` migration + + it "should delete policies when the column is deleted" do + -- https://github.com/digitallyinduced/ihp/issues/1480 + let targetSchema = sql $ cs [plain| + CREATE TABLE artefacts ( + id UUID DEFAULT uuid_generate_v4() PRIMARY KEY NOT NULL, + created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL, + updated_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL + ); + ALTER TABLE artefacts ENABLE ROW LEVEL SECURITY; + |] + let actualSchema = sql $ cs [plain| + CREATE TABLE artefacts ( + id UUID DEFAULT uuid_generate_v4() PRIMARY KEY NOT NULL, + created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL, + updated_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL, + user_id UUID DEFAULT ihp_user_id() NOT NULL + ); + ALTER TABLE artefacts ENABLE ROW LEVEL SECURITY; + CREATE POLICY "Users can manage their artefacts" ON artefacts USING (user_id = ihp_user_id()) WITH CHECK (user_id = ihp_user_id()); + |] + let migration = sql [i| + ALTER TABLE artefacts DROP COLUMN user_id; + DROP POLICY "Users can manage their artefacts" ON artefacts; + |] - + diffSchemas targetSchema actualSchema `shouldBe` migration sql :: Text -> [Statement] sql code = case Megaparsec.runParser Parser.parseDDL "" code of Left parsingFailed -> error (cs $ Megaparsec.errorBundlePretty parsingFailed) diff --git a/Test/IDE/SchemaDesigner/SchemaOperationsSpec.hs b/Test/IDE/SchemaDesigner/SchemaOperationsSpec.hs index 630920a4e..de9ed7c59 100644 --- a/Test/IDE/SchemaDesigner/SchemaOperationsSpec.hs +++ b/Test/IDE/SchemaDesigner/SchemaOperationsSpec.hs @@ -409,6 +409,35 @@ tests = do } (SchemaOperations.deleteColumn options inputSchema) `shouldBe` expectedSchema + + it "should delete an referenced policy" do + let tableAWithUserId = StatementCreateTable CreateTable + { name = "a" + , columns = [ + Column + { name = "user_id" + , columnType = PUUID + , defaultValue = Just (CallExpression "ihp_user_id" []) + , notNull = True + , isUnique = False + , generator = Nothing + } + ] + , primaryKeyConstraint = PrimaryKeyConstraint [] + , constraints = [] + } + let policy = CreatePolicy { name = "a_policy", tableName = "a", action = Nothing, using = Just (EqExpression (VarExpression "user_id") (CallExpression "ihp_user_id" [])), check = Nothing } + + let inputSchema = [tableAWithUserId, policy] + let expectedSchema = [tableA] + + let options = SchemaOperations.DeleteColumnOptions + { tableName = "a" + , columnName = "user_id" + , columnId = 0 + } + + (SchemaOperations.deleteColumn options inputSchema) `shouldBe` expectedSchema describe "update" do it "update a column's name, type, default value and not null" do let tableAWithCreatedAt = StatementCreateTable CreateTable