From db19cf965a72422ead9af461cc0d7b8f425d76a6 Mon Sep 17 00:00:00 2001 From: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com> Date: Fri, 18 Oct 2024 14:28:35 +0530 Subject: [PATCH] subscriptions: Unhide OpenDd Metadata and lift from unstability (#1225) ### What - Remove `enable-subscriptions` from unstable features. - Expose the subscriptions related opendd metadata in the json schema ### How - Update unstable feature related types by dropping subscriptions. - Remove `hidden=true` opendd attribute and other related on subscription opendd metadata. - Update json schema files V3_GIT_ORIGIN_REV_ID: 0aa763f516d394aab2e375da0817d0e60228c9b2 --- v3/crates/engine/src/internal_flags.rs | 4 - v3/crates/engine/tests/common.rs | 2 - v3/crates/graphql-ws/tests/common.rs | 7 +- .../jsonapi/tests/jsonapi_golden_tests.rs | 1 - .../src/stages/graphql_config/mod.rs | 1 - v3/crates/metadata-resolve/src/stages/mod.rs | 1 - .../src/stages/models_graphql/graphql.rs | 12 +- .../src/stages/models_graphql/mod.rs | 3 - .../src/types/configuration.rs | 1 - v3/crates/metadata-resolve/src/types/error.rs | 2 - .../tests/metadata_golden_tests.rs | 1 - v3/crates/open-dds/metadata.jsonschema | 113 ++++++++++++++++++ v3/crates/open-dds/src/graphql_config.rs | 2 - v3/crates/open-dds/src/models.rs | 5 +- v3/crates/open-dds/src/permissions.rs | 8 +- v3/justfile | 2 - 16 files changed, 120 insertions(+), 45 deletions(-) diff --git a/v3/crates/engine/src/internal_flags.rs b/v3/crates/engine/src/internal_flags.rs index 839506a18d0..7334952e5d7 100644 --- a/v3/crates/engine/src/internal_flags.rs +++ b/v3/crates/engine/src/internal_flags.rs @@ -19,7 +19,6 @@ pub enum UnstableFeature { EnableOrderByExpressions, EnableNdcV02Support, - EnableSubscriptions, EnableJsonApi, EnableAggregationPredicates, } @@ -37,9 +36,6 @@ pub fn resolve_unstable_features( UnstableFeature::EnableNdcV02Support => { features.enable_ndc_v02_support = true; } - UnstableFeature::EnableSubscriptions => { - features.enable_subscriptions = true; - } UnstableFeature::EnableJsonApi => { features.enable_jsonapi = true; } diff --git a/v3/crates/engine/tests/common.rs b/v3/crates/engine/tests/common.rs index e656905aed1..056574ff7aa 100644 --- a/v3/crates/engine/tests/common.rs +++ b/v3/crates/engine/tests/common.rs @@ -456,7 +456,6 @@ pub fn test_execute_explain( unstable_features: metadata_resolve::configuration::UnstableFeatures { enable_order_by_expressions: false, enable_ndc_v02_support: true, - enable_subscriptions: false, enable_jsonapi: false, ..Default::default() }, @@ -518,7 +517,6 @@ pub(crate) fn test_metadata_resolve_configuration() -> metadata_resolve::configu unstable_features: metadata_resolve::configuration::UnstableFeatures { enable_order_by_expressions: false, enable_ndc_v02_support: true, - enable_subscriptions: true, enable_jsonapi: false, ..Default::default() }, diff --git a/v3/crates/graphql-ws/tests/common.rs b/v3/crates/graphql-ws/tests/common.rs index 4c7c5703384..2901e4b3a7b 100644 --- a/v3/crates/graphql-ws/tests/common.rs +++ b/v3/crates/graphql-ws/tests/common.rs @@ -59,12 +59,7 @@ pub(crate) async fn start_websocket_server() -> TestServer { let metadata_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(METADATA_PATH); let raw_metadata = std::fs::read_to_string(metadata_path).unwrap(); let metadata = open_dds::Metadata::from_json_str(&raw_metadata).unwrap(); - let metadata_resolve_configuration = metadata_resolve::configuration::Configuration { - unstable_features: metadata_resolve::configuration::UnstableFeatures { - enable_subscriptions: true, - ..Default::default() - }, - }; + let metadata_resolve_configuration = metadata_resolve::configuration::Configuration::default(); let (resolved_metadata, _warnings) = metadata_resolve::resolve(metadata, &metadata_resolve_configuration).unwrap(); diff --git a/v3/crates/jsonapi/tests/jsonapi_golden_tests.rs b/v3/crates/jsonapi/tests/jsonapi_golden_tests.rs index 2e067c210b3..07621baa751 100644 --- a/v3/crates/jsonapi/tests/jsonapi_golden_tests.rs +++ b/v3/crates/jsonapi/tests/jsonapi_golden_tests.rs @@ -179,7 +179,6 @@ fn get_metadata_resolve_configuration() -> metadata_resolve::configuration::Conf let unstable_features = metadata_resolve::configuration::UnstableFeatures { enable_order_by_expressions: false, enable_ndc_v02_support: false, - enable_subscriptions: true, enable_jsonapi: true, enable_aggregation_predicates: false, }; diff --git a/v3/crates/metadata-resolve/src/stages/graphql_config/mod.rs b/v3/crates/metadata-resolve/src/stages/graphql_config/mod.rs index 4bff1b1d3d0..291d691852a 100644 --- a/v3/crates/metadata-resolve/src/stages/graphql_config/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/graphql_config/mod.rs @@ -262,7 +262,6 @@ fn fallback_graphql_config() -> &'static graphql_config::GraphqlConfig { mutation: graphql_config::MutationGraphqlConfig { root_operation_type_name: GraphQlTypeName::from("Mutation"), }, - // TODO: Subscriptions are still unsupported. No need to consider them for now. subscription: None, apollo_federation: None, }) diff --git a/v3/crates/metadata-resolve/src/stages/mod.rs b/v3/crates/metadata-resolve/src/stages/mod.rs index 93334d6a71b..afd9faf6c91 100644 --- a/v3/crates/metadata-resolve/src/stages/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/mod.rs @@ -271,7 +271,6 @@ pub fn resolve( &order_by_expressions, &graphql_types, &graphql_config, - configuration, )?; all_issues.extend(issues); diff --git a/v3/crates/metadata-resolve/src/stages/models_graphql/graphql.rs b/v3/crates/metadata-resolve/src/stages/models_graphql/graphql.rs index 2370689aa70..ae637bac98d 100644 --- a/v3/crates/metadata-resolve/src/stages/models_graphql/graphql.rs +++ b/v3/crates/metadata-resolve/src/stages/models_graphql/graphql.rs @@ -11,7 +11,6 @@ use super::types::{ SelectAggregateGraphQlDefinition, SelectManyGraphQlDefinition, SelectUniqueGraphQlDefinition, SubscriptionGraphQlDefinition, UniqueIdentifierField, }; -use crate::configuration::Configuration; use crate::helpers::types::{mk_name, store_new_graphql_type}; use crate::stages::order_by_expressions::OrderByExpressions; use crate::stages::{data_connector_scalar_types, graphql_config, models, object_types}; @@ -36,7 +35,6 @@ pub(crate) fn resolve_model_graphql_api( aggregate_expression_name: &Option>, order_by_expressions: &OrderByExpressions, graphql_config: &graphql_config::GraphqlConfig, - configuration: &Configuration, issues: &mut Vec, ) -> Result { let model_name = &model.name; @@ -94,7 +92,7 @@ pub(crate) fn resolve_model_graphql_api( let subscription = select_unique .subscription .as_ref() - .map(|s| resolve_subscription_graphql_api(s, configuration)) + .map(resolve_subscription_graphql_api) .transpose()?; graphql_api .select_uniques @@ -171,7 +169,7 @@ pub(crate) fn resolve_model_graphql_api( let subscription = gql_definition .subscription .as_ref() - .map(|s| resolve_subscription_graphql_api(s, configuration)) + .map(resolve_subscription_graphql_api) .transpose()?; mk_name(gql_definition.query_root_field.as_str()).map(|f: ast::Name| { let select_many_description = if gql_definition.description.is_some() { @@ -242,7 +240,7 @@ pub(crate) fn resolve_model_graphql_api( let subscription = graphql_aggregate .subscription .as_ref() - .map(|s| resolve_subscription_graphql_api(s, configuration)) + .map(resolve_subscription_graphql_api) .transpose()?; Some(SelectAggregateGraphQlDefinition { @@ -336,12 +334,8 @@ fn is_model_used_in_any_aggregate_relationship( fn resolve_subscription_graphql_api( subscription: &open_dds::models::SubscriptionGraphQlDefinition, - configuration: &Configuration, ) -> Result { // Subscriptions are currently unstable. - if !configuration.unstable_features.enable_subscriptions { - return Err(Error::UnstableFeatureSubscriptions); - } let open_dds::models::SubscriptionGraphQlDefinition { root_field, description, diff --git a/v3/crates/metadata-resolve/src/stages/models_graphql/mod.rs b/v3/crates/metadata-resolve/src/stages/models_graphql/mod.rs index 3a0f015fd11..4deab199b35 100644 --- a/v3/crates/metadata-resolve/src/stages/models_graphql/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/models_graphql/mod.rs @@ -9,7 +9,6 @@ use std::collections::{BTreeMap, BTreeSet}; use lang_graphql::ast::common as ast; use open_dds::{data_connector::DataConnectorName, models::ModelName, types::CustomTypeName}; -use crate::configuration::Configuration; use crate::stages::{ boolean_expressions, data_connector_scalar_types, graphql_config, models, object_boolean_expressions, object_relationships, @@ -45,7 +44,6 @@ pub fn resolve( order_by_expressions: &order_by_expressions::OrderByExpressions, existing_graphql_types: &BTreeSet, graphql_config: &graphql_config::GraphqlConfig, - configuration: &Configuration, ) -> Result { let mut output = ModelsWithGraphqlOutput { models_with_graphql: IndexMap::new(), @@ -99,7 +97,6 @@ pub fn resolve( &model.aggregate_expression, order_by_expressions, graphql_config, - configuration, &mut output.issues, )?, None => types::ModelGraphQlApi::default(), diff --git a/v3/crates/metadata-resolve/src/types/configuration.rs b/v3/crates/metadata-resolve/src/types/configuration.rs index 9c3253d3e96..9a8d62388bb 100644 --- a/v3/crates/metadata-resolve/src/types/configuration.rs +++ b/v3/crates/metadata-resolve/src/types/configuration.rs @@ -16,7 +16,6 @@ pub struct Configuration { pub struct UnstableFeatures { pub enable_order_by_expressions: bool, pub enable_ndc_v02_support: bool, - pub enable_subscriptions: bool, pub enable_jsonapi: bool, pub enable_aggregation_predicates: bool, } diff --git a/v3/crates/metadata-resolve/src/types/error.rs b/v3/crates/metadata-resolve/src/types/error.rs index 786d449bb0e..38882a1c850 100644 --- a/v3/crates/metadata-resolve/src/types/error.rs +++ b/v3/crates/metadata-resolve/src/types/error.rs @@ -345,8 +345,6 @@ pub enum Error { CompatibilityError { warnings_as_errors: SeparatedBy, }, - #[error("Subscriptions are currently unstable. Please add 'subscriptions' to unstable features to enable them.")] - UnstableFeatureSubscriptions, } pub trait ShouldBeAnError { diff --git a/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs b/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs index 92103ccc60a..ad293b70bef 100644 --- a/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs +++ b/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs @@ -83,7 +83,6 @@ fn read_test_configuration( let unstable_features = configuration::UnstableFeatures { enable_order_by_expressions: false, enable_ndc_v02_support: false, - enable_subscriptions: false, enable_jsonapi: false, enable_aggregation_predicates: true, }; diff --git a/v3/crates/open-dds/metadata.jsonschema b/v3/crates/open-dds/metadata.jsonschema index a3fd4b98a4c..6d019a359c2 100644 --- a/v3/crates/open-dds/metadata.jsonschema +++ b/v3/crates/open-dds/metadata.jsonschema @@ -1948,6 +1948,16 @@ "mutation": { "$ref": "#/definitions/MutationGraphqlConfig" }, + "subscription": { + "anyOf": [ + { + "$ref": "#/definitions/SubscriptionGraphqlConfig" + }, + { + "type": "null" + } + ] + }, "apolloFederation": { "anyOf": [ { @@ -2322,6 +2332,17 @@ "type": "null" } ] + }, + "subscription": { + "description": "Enable subscription on this aggregate root field.", + "anyOf": [ + { + "$ref": "#/definitions/SubscriptionGraphQlDefinition" + }, + { + "type": "null" + } + ] } }, "additionalProperties": false @@ -5238,6 +5259,17 @@ "type": "null" } ] + }, + "subscription": { + "description": "Enable subscription on this select many root field.", + "anyOf": [ + { + "$ref": "#/definitions/SubscriptionGraphQlDefinition" + }, + { + "type": "null" + } + ] } }, "additionalProperties": false @@ -5266,6 +5298,11 @@ "items": { "$ref": "#/definitions/ArgumentPreset" } + }, + "allowSubscriptions": { + "description": "Whether the role is allowed to subscribe to the root fields of this model.", + "default": false, + "type": "boolean" } }, "additionalProperties": false @@ -5312,6 +5349,82 @@ "type": "null" } ] + }, + "subscription": { + "description": "Enable subscription on this select unique root field.", + "anyOf": [ + { + "$ref": "#/definitions/SubscriptionGraphQlDefinition" + }, + { + "type": "null" + } + ] + } + }, + "additionalProperties": false + }, + "SubscriptionGraphQlDefinition": { + "$id": "https://hasura.io/jsonschemas/metadata/SubscriptionGraphQlDefinition", + "title": "SubscriptionGraphQlDefinition", + "description": "The definition of the GraphQL API for enabling subscription on query root fields.", + "type": "object", + "required": [ + "rootField" + ], + "properties": { + "rootField": { + "description": "The name of the subscription root field.", + "allOf": [ + { + "$ref": "#/definitions/GraphQlFieldName" + } + ] + }, + "description": { + "description": "The description of the subscription graphql definition. Gets added to the description of the subscription root field in the graphql schema.", + "type": [ + "string", + "null" + ] + }, + "deprecated": { + "description": "Whether this subscription root field is deprecated. If set, the deprecation status is added to the subscription root field's graphql schema.", + "anyOf": [ + { + "$ref": "#/definitions/Deprecated" + }, + { + "type": "null" + } + ] + }, + "pollingIntervalMs": { + "description": "Polling interval in milliseconds for the subscription.", + "default": 1000, + "type": "integer", + "format": "uint64", + "minimum": 0.0 + } + }, + "additionalProperties": false + }, + "SubscriptionGraphqlConfig": { + "$id": "https://hasura.io/jsonschemas/metadata/SubscriptionGraphqlConfig", + "title": "SubscriptionGraphqlConfig", + "description": "Configuration for the GraphQL schema of Hasura features for subscriptions.", + "type": "object", + "required": [ + "rootOperationTypeName" + ], + "properties": { + "rootOperationTypeName": { + "description": "The name of the root operation type name for subscriptions. Usually `subscription`.", + "allOf": [ + { + "$ref": "#/definitions/GraphQlTypeName" + } + ] } }, "additionalProperties": false diff --git a/v3/crates/open-dds/src/graphql_config.rs b/v3/crates/open-dds/src/graphql_config.rs index cc47bcbf2ea..3c47251dbba 100644 --- a/v3/crates/open-dds/src/graphql_config.rs +++ b/v3/crates/open-dds/src/graphql_config.rs @@ -30,8 +30,6 @@ pub enum GraphqlConfig { pub struct GraphqlConfigV1 { pub query: QueryGraphqlConfig, pub mutation: MutationGraphqlConfig, - #[opendd(hidden = true)] - #[serde(skip_serializing_if = "Option::is_none")] pub subscription: Option, pub apollo_federation: Option, } diff --git a/v3/crates/open-dds/src/models.rs b/v3/crates/open-dds/src/models.rs index fce1e45daf5..252914c58a7 100644 --- a/v3/crates/open-dds/src/models.rs +++ b/v3/crates/open-dds/src/models.rs @@ -385,7 +385,6 @@ pub struct SelectUniqueGraphQlDefinition { /// If set, the deprecation status is added to the select unique root field's graphql schema. pub deprecated: Option, /// Enable subscription on this select unique root field. - #[opendd(hidden = true)] pub subscription: Option, } @@ -403,11 +402,10 @@ pub struct SelectManyGraphQlDefinition { /// If set, the deprecation status is added to the select many root field's graphql schema. pub deprecated: Option, /// Enable subscription on this select many root field. - #[opendd(hidden = true)] pub subscription: Option, } -/// The definition of the GraphQL API for enabling subscription on select_many or select_uniques root fields. +/// The definition of the GraphQL API for enabling subscription on query root fields. #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, opendds_derive::OpenDd)] #[serde(rename_all = "camelCase")] #[opendd(json_schema(title = "SubscriptionGraphQlDefinition"))] @@ -502,6 +500,5 @@ pub struct ModelAggregateGraphQlDefinition { /// If set, the deprecation status is added to the aggregate root field's graphql schema. pub deprecated: Option, /// Enable subscription on this aggregate root field. - #[opendd(hidden = true)] pub subscription: Option, } diff --git a/v3/crates/open-dds/src/permissions.rs b/v3/crates/open-dds/src/permissions.rs index 41cd67db40a..6491e613fb4 100644 --- a/v3/crates/open-dds/src/permissions.rs +++ b/v3/crates/open-dds/src/permissions.rs @@ -342,12 +342,8 @@ pub struct SelectPermission { /// Preset values for arguments for this role #[opendd(default, json_schema(default_exp = "serde_json::json!([])"))] pub argument_presets: Vec, - /// Whether to allow subscriptions for this role. - #[opendd( - hidden = true, - default, - json_schema(default_exp = "serde_json::json!(false)") - )] + /// Whether the role is allowed to subscribe to the root fields of this model. + #[opendd(default, json_schema(default_exp = "serde_json::json!(false)"))] pub allow_subscriptions: bool, } diff --git a/v3/justfile b/v3/justfile index b9c2599be75..4c12a01447c 100644 --- a/v3/justfile +++ b/v3/justfile @@ -85,7 +85,6 @@ watch: start-docker-test-deps start-docker-run-deps --otlp-endpoint http://localhost:4317 \ --authn-config-path static/auth/auth_config.json \ --metadata-path static/sample-schema.json \ - --unstable-feature enable-subscriptions \ --unstable-feature enable-json-api \ --expose-internal-errors' @@ -175,6 +174,5 @@ run METADATA_PATH="static/sample-schema.json": start-docker-test-deps start-dock --otlp-endpoint http://localhost:4317 \ --authn-config-path static/auth/auth_config.json \ --metadata-path {{ METADATA_PATH }} \ - --unstable-feature enable-subscriptions \ --unstable-feature enable-json-api \ --expose-internal-errors