From 20fb9fc255384c82793a89b153689382c04a47e4 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Thu, 10 Jan 2019 15:03:24 +0530 Subject: [PATCH 1/2] accept null for relational insert input, fix #1352 -> Handle empty array for insert objects and array insert data --- .../Hasura/GraphQL/Resolve/InputValue.hs | 6 ++ .../src-lib/Hasura/GraphQL/Resolve/Insert.hs | 100 +++++++++++------- 2 files changed, 68 insertions(+), 38 deletions(-) diff --git a/server/src-lib/Hasura/GraphQL/Resolve/InputValue.hs b/server/src-lib/Hasura/GraphQL/Resolve/InputValue.hs index 21171f02b316d..3e661301c8405 100644 --- a/server/src-lib/Hasura/GraphQL/Resolve/InputValue.hs +++ b/server/src-lib/Hasura/GraphQL/Resolve/InputValue.hs @@ -7,6 +7,7 @@ module Hasura.GraphQL.Resolve.InputValue , withObject , asObject , withObjectM + , asObjectM , withArray , asArray , withArrayM @@ -87,6 +88,11 @@ withObjectM fn v = case v of AGObject nt objM -> fn nt objM _ -> tyMismatch "object" v +asObjectM + :: (MonadError QErr m) + => AnnGValue -> m (Maybe AnnGObject) +asObjectM = withObjectM (\_ o -> return o) + withArrayM :: (MonadError QErr m) => (G.ListType -> Maybe [AnnGValue] -> m a) -> AnnGValue -> m a diff --git a/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs b/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs index c578d3724121b..dd8d8df9a957e 100644 --- a/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs +++ b/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs @@ -2,6 +2,7 @@ module Hasura.GraphQL.Resolve.Insert (convertInsert) where +import Control.Arrow (second) import Data.Has import Hasura.Prelude import Hasura.Server.Utils @@ -94,7 +95,7 @@ traverseInsObj -> (G.Name, AnnGValue) -> AnnInsObj -> m AnnInsObj -traverseInsObj rim (gName, annVal) (AnnInsObj cols objRels arrRels) = +traverseInsObj rim (gName, annVal) defVal@(AnnInsObj cols objRels arrRels) = case annVal of AGScalar colty mColVal -> do let col = PGCol $ G.unName gName @@ -102,35 +103,42 @@ traverseInsObj rim (gName, annVal) (AnnInsObj cols objRels arrRels) = return (AnnInsObj ((col, colty, colVal):cols) objRels arrRels) _ -> do - obj <- asObject annVal - let relName = RelName $ G.unName gName - onConflictM = OMap.lookup "on_conflict" obj - dataVal <- onNothing (OMap.lookup "data" obj) $ - throw500 "\"data\" object not found" - relInfo <- onNothing (Map.lookup relName rim) $ - throw500 $ "relation " <> relName <<> " not found" - - let rTable = riRTable relInfo - InsCtx rtView rtCols rtDefVals rtRelInfoMap rtUpdPerm <- getInsCtx rTable - - withPathK (G.unName gName) $ case riType relInfo of - ObjRel -> do - dataObj <- asObject dataVal - annDataObj <- mkAnnInsObj rtRelInfoMap dataObj - ccM <- forM onConflictM $ parseOnConflict rTable rtUpdPerm - let singleObjIns = AnnIns annDataObj ccM rtView rtCols rtDefVals - objRelIns = RelIns singleObjIns relInfo - return (AnnInsObj cols (objRelIns:objRels) arrRels) - - ArrRel -> do - arrDataVals <- asArray dataVal - annDataObjs <- forM arrDataVals $ \arrDataVal -> do - dataObj <- asObject arrDataVal - mkAnnInsObj rtRelInfoMap dataObj - ccM <- forM onConflictM $ parseOnConflict rTable rtUpdPerm - let multiObjIns = AnnIns annDataObjs ccM rtView rtCols rtDefVals - arrRelIns = RelIns multiObjIns relInfo - return (AnnInsObj cols objRels (arrRelIns:arrRels)) + objM <- asObjectM annVal + -- if relational insert input is 'null' then ignore + -- return default value + fmap (fromMaybe defVal) $ forM objM $ \obj -> do + let relName = RelName $ G.unName gName + onConflictM = OMap.lookup "on_conflict" obj + dataVal <- onNothing (OMap.lookup "data" obj) $ + throw500 "\"data\" object not found" + relInfo <- onNothing (Map.lookup relName rim) $ + throw500 $ "relation " <> relName <<> " not found" + + let rTable = riRTable relInfo + InsCtx rtView rtCols rtDefVals rtRelInfoMap rtUpdPerm <- getInsCtx rTable + + withPathK (G.unName gName) $ case riType relInfo of + ObjRel -> do + dataObj <- asObject dataVal + annDataObj <- mkAnnInsObj rtRelInfoMap dataObj + ccM <- forM onConflictM $ parseOnConflict rTable rtUpdPerm + let singleObjIns = AnnIns annDataObj ccM rtView rtCols rtDefVals + objRelIns = RelIns singleObjIns relInfo + return (AnnInsObj cols (objRelIns:objRels) arrRels) + + ArrRel -> do + arrDataVals <- asArray dataVal + let withNonEmptyArrData = do + annDataObjs <- forM arrDataVals $ \arrDataVal -> do + dataObj <- asObject arrDataVal + mkAnnInsObj rtRelInfoMap dataObj + ccM <- forM onConflictM $ parseOnConflict rTable rtUpdPerm + let multiObjIns = AnnIns annDataObjs ccM rtView rtCols rtDefVals + arrRelIns = RelIns multiObjIns relInfo + return (AnnInsObj cols objRels (arrRelIns:arrRels)) + -- if array relation insert input data has empty objects + -- then ignore and return default value + bool withNonEmptyArrData (return defVal) $ null arrDataVals parseOnConflict :: (MonadError QErr m) @@ -437,6 +445,17 @@ insertMultipleObjects role tn multiObjIns addCols mutFlds errP = getRet (t, r@(RR.MRet _)) = Just (t, r) getRet _ = Nothing +-- | build mutation response for empty objects +withEmptyObjs :: RR.MutFlds -> Convert RespTx +withEmptyObjs = return . mkTx + where + mkTx = return . J.encode . OMap.fromList . map (second convMutFld) + -- generate empty mutation response + convMutFld = \case + RR.MCount -> J.toJSON (0 :: Int) + RR.MExp e -> J.toJSON e + RR.MRet _ -> J.toJSON ([] :: [J.Value]) + prefixErrPath :: (MonadError QErr m) => Field -> m a -> m a prefixErrPath fld = withPathK "selectionSet" . fieldAsPath fld . withPathK "args" @@ -447,16 +466,21 @@ convertInsert -> Field -- the mutation field -> Convert RespTx convertInsert role tn fld = prefixErrPath fld $ do - InsCtx vn tableCols defValMap relInfoMap updPerm <- getInsCtx tn - annVals <- withArg arguments "objects" asArray - annObjs <- mapM asObject annVals - annInsObjs <- forM annObjs $ mkAnnInsObj relInfoMap - conflictClauseM <- forM onConflictM $ parseOnConflict tn updPerm mutFlds <- convertMutResp (_fType fld) $ _fSelSet fld - let multiObjIns = AnnIns annInsObjs conflictClauseM vn tableCols defValMap - return $ prefixErrPath fld $ insertMultipleObjects role tn - multiObjIns [] mutFlds "objects" + annVals <- withArg arguments "objects" asArray + -- if insert input objects is empty array then + -- do not perform insert and return mutation response + bool (withNonEmptyObjs annVals mutFlds) (withEmptyObjs mutFlds) $ null annVals where + withNonEmptyObjs annVals mutFlds = do + InsCtx vn tableCols defValMap relInfoMap updPerm <- getInsCtx tn + annObjs <- mapM asObject annVals + annInsObjs <- forM annObjs $ mkAnnInsObj relInfoMap + conflictClauseM <- forM onConflictM $ parseOnConflict tn updPerm + let multiObjIns = AnnIns annInsObjs conflictClauseM vn tableCols defValMap + return $ prefixErrPath fld $ insertMultipleObjects role tn + multiObjIns [] mutFlds "objects" + arguments = _fArguments fld onConflictM = Map.lookup "on_conflict" arguments From 3690b64b57fc62956687852b66baeece2e983f60 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Thu, 10 Jan 2019 15:30:04 +0530 Subject: [PATCH 2/2] add test --- .../insert/basic/author_article.yaml | 25 ++++++++++++- .../nested/author_with_articles_empty.yaml | 37 +++++++++++++++++++ .../nested/author_with_articles_null.yaml | 35 ++++++++++++++++++ server/tests-py/test_graphql_mutations.py | 6 +++ 4 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 server/tests-py/queries/graphql_mutation/insert/nested/author_with_articles_empty.yaml create mode 100644 server/tests-py/queries/graphql_mutation/insert/nested/author_with_articles_null.yaml diff --git a/server/tests-py/queries/graphql_mutation/insert/basic/author_article.yaml b/server/tests-py/queries/graphql_mutation/insert/basic/author_article.yaml index 4be1580abccad..2ff771d37ef64 100644 --- a/server/tests-py/queries/graphql_mutation/insert/basic/author_article.yaml +++ b/server/tests-py/queries/graphql_mutation/insert/basic/author_article.yaml @@ -28,7 +28,30 @@ name } } - } + } + +#Inserting empty objects +- description: Inserts empty objects via GraphQL mutation + url: /v1alpha1/graphql + status: 200 + response: + data: + insert_author: + affected_rows: 0 + returning: [] + query: + query: | + mutation insert_empty_objects { + insert_author( + objects: [] + ){ + affected_rows + returning{ + id + name + } + } + } #Inserting article data - description: Inserts article data via GraphQL mutation diff --git a/server/tests-py/queries/graphql_mutation/insert/nested/author_with_articles_empty.yaml b/server/tests-py/queries/graphql_mutation/insert/nested/author_with_articles_empty.yaml new file mode 100644 index 0000000000000..e24109e93c569 --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/insert/nested/author_with_articles_empty.yaml @@ -0,0 +1,37 @@ +description: Insert author with empty articles +url: /v1alpha1/graphql +status: 200 +query: + query: | + mutation { + insert_author( + objects: [ + { + id: 4 + name: "Author 4" + articles: { + data: [] + } + } + ] + ){ + affected_rows + returning{ + id + name + articles{ + id + title + content + } + } + } + } +response: + data: + insert_author: + affected_rows: 1 + returning: + - id: 4 + name: Author 4 + articles: [] diff --git a/server/tests-py/queries/graphql_mutation/insert/nested/author_with_articles_null.yaml b/server/tests-py/queries/graphql_mutation/insert/nested/author_with_articles_null.yaml new file mode 100644 index 0000000000000..56b24e63b5892 --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/insert/nested/author_with_articles_null.yaml @@ -0,0 +1,35 @@ +description: Insert author with empty articles +url: /v1alpha1/graphql +status: 200 +query: + query: | + mutation { + insert_author( + objects: [ + { + id: 4 + name: "Author 4" + articles: null + } + ] + ){ + affected_rows + returning{ + id + name + articles{ + id + title + content + } + } + } + } +response: + data: + insert_author: + affected_rows: 1 + returning: + - id: 4 + name: Author 4 + articles: [] diff --git a/server/tests-py/test_graphql_mutations.py b/server/tests-py/test_graphql_mutations.py index 69a4cfedc5a0c..447ffb39844c5 100644 --- a/server/tests-py/test_graphql_mutations.py +++ b/server/tests-py/test_graphql_mutations.py @@ -205,6 +205,12 @@ class TestGraphqlNestedInserts(DefaultTestQueries): def test_author_with_articles(self, hge_ctx): check_query_f(hge_ctx, self.dir() + "/author_with_articles.yaml") + def test_author_with_articles_empty(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + "/author_with_articles_empty.yaml") + + def test_author_with_articles_null(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + "/author_with_articles_null.yaml") + def test_author_with_articles_author_id_fail(self, hge_ctx): check_query_f(hge_ctx, self.dir() + "/author_with_articles_author_id_fail.yaml")