From e6c3113a43230304361417ebd322b77c16b52e31 Mon Sep 17 00:00:00 2001 From: Daniel Harvey Date: Fri, 4 Nov 2022 13:08:40 +0000 Subject: [PATCH] [server]: feature flag to remove `_stream` fields from schema PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6698 GitOrigin-RevId: d2b80900d06353647505256fc351a07e6f7cd5f7 --- .../graphql-engine-flags/reference.mdx | 4 +- server/lib/api-tests/api-tests.cabal | 1 + .../Test/Regression/StreamConflictSpec.hs | 60 +++++++++++++++++++ .../Backends/Postgres/Instances/Schema.hs | 2 +- server/src-lib/Hasura/GraphQL/Schema.hs | 6 +- server/src-lib/Hasura/GraphQL/Schema/Build.hs | 26 ++++---- .../src-lib/Hasura/GraphQL/Schema/Options.hs | 12 +++- server/src-lib/Hasura/Server/Types.hs | 2 + server/src-test/Test/Parser/Monad.hs | 1 + 9 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 server/lib/api-tests/test/Test/Regression/StreamConflictSpec.hs diff --git a/docs/docs/deployment/graphql-engine-flags/reference.mdx b/docs/docs/deployment/graphql-engine-flags/reference.mdx index cf509c9d08f..a901c645afb 100644 --- a/docs/docs/deployment/graphql-engine-flags/reference.mdx +++ b/docs/docs/deployment/graphql-engine-flags/reference.mdx @@ -624,7 +624,9 @@ Interval in milliseconds to sleep before trying to fetch events again after a fe List of experimental features to be enabled. A comma separated value is expected. Options: `inherited_roles`, -`naming_convention`, `streaming_subscriptions`, `hide_update_many_fields`. +`optimise_permission_filters`, `naming_convention`, `streaming_subscriptions`, `apollo_federation`, `hide_update_many_fields`, +`bigquery_string_numeric_input`, `hide_aggregation_predicates`, +`hide_stream_fields`. _(Available for versions > v2.0.0)_ diff --git a/server/lib/api-tests/api-tests.cabal b/server/lib/api-tests/api-tests.cabal index 3fe5d716edd..95a3545bdc7 100644 --- a/server/lib/api-tests/api-tests.cabal +++ b/server/lib/api-tests/api-tests.cabal @@ -140,6 +140,7 @@ executable api-tests Test.Regression.NullRemoteRelationship8345Spec Test.Regression.NullsOrderParsing8780Spec Test.Regression.ObjectRelationshipsLimit7936Spec + Test.Regression.StreamConflictSpec Test.Regression.UsingTheSameFunctionForRootFieldAndComputedField8643Spec Test.RemoteRelationship.FromRemoteSchemaSpec Test.RemoteRelationship.MetadataAPI.ClearMetadataSpec diff --git a/server/lib/api-tests/test/Test/Regression/StreamConflictSpec.hs b/server/lib/api-tests/test/Test/Regression/StreamConflictSpec.hs new file mode 100644 index 00000000000..78c311fa94d --- /dev/null +++ b/server/lib/api-tests/test/Test/Regression/StreamConflictSpec.hs @@ -0,0 +1,60 @@ +-- | Test the schema names conflict of _stream +module Test.Regression.StreamConflictSpec (spec) where + +import Data.List.NonEmpty qualified as NE +import Harness.Backend.Postgres qualified as Postgres +import Harness.Test.Fixture qualified as Fixture +import Harness.Test.Schema (Table (..), table) +import Harness.Test.Schema qualified as Schema +import Harness.TestEnvironment (TestEnvironment) +import Hasura.Prelude +import Test.Hspec (SpecWith, it) + +spec :: SpecWith TestEnvironment +spec = + Fixture.run + ( NE.fromList + [ (Fixture.fixture $ Fixture.Backend Fixture.Postgres) + { Fixture.setupTeardown = \(testEnv, _) -> + [ Postgres.setupTablesAction schema testEnv + ], + Fixture.customOptions = + Just $ + Fixture.defaultOptions + { Fixture.skipTests = + Just "Disabled until we can dynamically change server settings per test. To test, add EFHideStreamFields to soSubscriptions in Harness.Constants -> serveOptions" + } + } + ] + ) + tests + +-------------------------------------------------------------------------------- +-- Schema + +schema :: [Schema.Table] +schema = + [ (table "author") + { tableColumns = + [ Schema.column "id" Schema.defaultSerialType, + Schema.column "name" Schema.TStr + ], + tablePrimaryKey = ["id"] + }, + (table "author_stream") + { tableColumns = + [ Schema.column "id" Schema.defaultSerialType, + Schema.column "name" Schema.TStr + ], + tablePrimaryKey = ["id"] + } + ] + +-------------------------------------------------------------------------------- +-- Tests + +tests :: Fixture.Options -> SpecWith TestEnvironment +tests _ = + -- All of the testing is done during setup. + -- If setup succeeds and we have no conflicts, and this test will pass. + it "Creates a schema without conflicts" \_ -> pure @IO () diff --git a/server/src-lib/Hasura/Backends/Postgres/Instances/Schema.hs b/server/src-lib/Hasura/Backends/Postgres/Instances/Schema.hs index e0482c52e22..76a3039ef0e 100644 --- a/server/src-lib/Hasura/Backends/Postgres/Instances/Schema.hs +++ b/server/src-lib/Hasura/Backends/Postgres/Instances/Schema.hs @@ -409,7 +409,7 @@ pgkBuildTableUpdateMutationFields mkRootFieldName scenario sourceInfo tableName pure $ case soIncludeUpdateManyFields of Options.IncludeUpdateManyFields -> singleUpdates ++ maybeToList multiUpdate - Options.DontIncludeUpdateManyFields -> + Options.Don'tIncludeUpdateManyFields -> singleUpdates -- | Create a parser for 'update_table_many'. This function is very similar to diff --git a/server/src-lib/Hasura/GraphQL/Schema.hs b/server/src-lib/Hasura/GraphQL/Schema.hs index 5419891ecee..1b3be0eb4df 100644 --- a/server/src-lib/Hasura/GraphQL/Schema.hs +++ b/server/src-lib/Hasura/GraphQL/Schema.hs @@ -189,12 +189,16 @@ buildSchemaOptions soOptimizePermissionFilters = optimizePermissionFilters, soIncludeUpdateManyFields = if EFHideUpdateManyFields `Set.member` expFeatures - then Options.DontIncludeUpdateManyFields + then Options.Don'tIncludeUpdateManyFields else Options.IncludeUpdateManyFields, soIncludeAggregationPredicates = if EFHideAggregationPredicates `Set.member` expFeatures then Options.Don'tIncludeAggregationPredicates else Options.IncludeAggregationPredicates, + soIncludeStreamFields = + if EFHideStreamFields `Set.member` expFeatures + then Options.Don'tIncludeStreamFields + else Options.IncludeStreamFields, soBigQueryStringNumericInput = bigqueryStringNumericInput } diff --git a/server/src-lib/Hasura/GraphQL/Schema/Build.hs b/server/src-lib/Hasura/GraphQL/Schema/Build.hs index 67e4491928c..f695ad5993d 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/Build.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/Build.hs @@ -212,16 +212,22 @@ buildTableStreamingSubscriptionFields :: C.GQLNameIdentifier -> SchemaT r m [FieldParser n (QueryDB b (RemoteRelationshipField UnpreparedValue) (UnpreparedValue b))] buildTableStreamingSubscriptionFields mkRootFieldName sourceInfo tableName tableInfo tableIdentifier = do - tCase <- asks getter - let customRootFields = _tcCustomRootFields $ _tciCustomConfig $ _tiCoreInfo tableInfo - selectDesc = Just $ G.Description $ "fetch data from the table in a streaming manner: " <>> tableName - selectStreamName = - runMkRootFieldName mkRootFieldName $ - setFieldNameCase tCase tableInfo (_tcrfSelectStream customRootFields) mkSelectStreamField tableIdentifier - catMaybes - <$> sequenceA - [ optionalFieldParser QDBStreamMultipleRows $ selectStreamTable sourceInfo tableInfo selectStreamName selectDesc - ] + -- Check in schema options whether we should include streaming subscription + -- fields + include <- retrieve Options.soIncludeStreamFields + case include of + Options.Don'tIncludeStreamFields -> pure mempty + Options.IncludeStreamFields -> do + tCase <- asks getter + let customRootFields = _tcCustomRootFields $ _tciCustomConfig $ _tiCoreInfo tableInfo + selectDesc = Just $ G.Description $ "fetch data from the table in a streaming manner: " <>> tableName + selectStreamName = + runMkRootFieldName mkRootFieldName $ + setFieldNameCase tCase tableInfo (_tcrfSelectStream customRootFields) mkSelectStreamField tableIdentifier + catMaybes + <$> sequenceA + [ optionalFieldParser QDBStreamMultipleRows $ selectStreamTable sourceInfo tableInfo selectStreamName selectDesc + ] buildTableInsertMutationFields :: forall b r m n. diff --git a/server/src-lib/Hasura/GraphQL/Schema/Options.hs b/server/src-lib/Hasura/GraphQL/Schema/Options.hs index 99951d51c95..20dbc21106d 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/Options.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/Options.hs @@ -8,6 +8,7 @@ module Hasura.GraphQL.Schema.Options RemoteSchemaPermissions (..), OptimizePermissionFilters (..), IncludeAggregationPredicates (..), + IncludeStreamFields (..), IncludeUpdateManyFields (..), BigQueryStringNumericInput (..), ) @@ -25,6 +26,7 @@ data SchemaOptions = SchemaOptions soOptimizePermissionFilters :: OptimizePermissionFilters, soIncludeUpdateManyFields :: IncludeUpdateManyFields, soIncludeAggregationPredicates :: IncludeAggregationPredicates, + soIncludeStreamFields :: IncludeStreamFields, soBigQueryStringNumericInput :: BigQueryStringNumericInput } @@ -42,7 +44,15 @@ data StringifyNumbers -- any tables that this may conflict with if needed data IncludeUpdateManyFields = IncludeUpdateManyFields - | DontIncludeUpdateManyFields + | Don'tIncludeUpdateManyFields + deriving (Eq, Show) + +-- | Should we include `TABLE_stream` fields in schemas +-- This is a toggle so that users can opt-in, and so that we can rename +-- any tables that this may conflict with if needed +data IncludeStreamFields + = IncludeStreamFields + | Don'tIncludeStreamFields deriving (Eq, Show) -- | Should we include aggregation functions in where clauses? diff --git a/server/src-lib/Hasura/Server/Types.hs b/server/src-lib/Hasura/Server/Types.hs index 8ab72e33d38..9202657a720 100644 --- a/server/src-lib/Hasura/Server/Types.hs +++ b/server/src-lib/Hasura/Server/Types.hs @@ -82,6 +82,7 @@ data ExperimentalFeature | EFHideUpdateManyFields | EFBigQueryStringNumericInput | EFHideAggregationPredicates + | EFHideStreamFields deriving (Bounded, Enum, Eq, Generic, Show) experimentalFeatureKey :: ExperimentalFeature -> Text @@ -94,6 +95,7 @@ experimentalFeatureKey = \case EFHideUpdateManyFields -> "hide_update_many_fields" EFBigQueryStringNumericInput -> "bigquery_string_numeric_input" EFHideAggregationPredicates -> "hide_aggregation_predicates" + EFHideStreamFields -> "hide_stream_fields" instance Hashable ExperimentalFeature diff --git a/server/src-test/Test/Parser/Monad.hs b/server/src-test/Test/Parser/Monad.hs index fb83e413f17..96392794ef9 100644 --- a/server/src-test/Test/Parser/Monad.hs +++ b/server/src-test/Test/Parser/Monad.hs @@ -64,6 +64,7 @@ defaultSchemaOptions = soOptimizePermissionFilters = Options.Don'tOptimizePermissionFilters, soIncludeUpdateManyFields = Options.IncludeUpdateManyFields, soIncludeAggregationPredicates = Options.IncludeAggregationPredicates, + soIncludeStreamFields = Options.IncludeStreamFields, soBigQueryStringNumericInput = Options.EnableBigQueryStringNumericInput }