From 108abbabae273c05b95f91217547cf7677f61800 Mon Sep 17 00:00:00 2001 From: Tom Gasson Date: Thu, 19 May 2022 14:36:19 +1000 Subject: [PATCH] Allow including query text when persisting queries For safe migration from non-persisted use to persisted use --- compiler/crates/relay-codegen/src/ast.rs | 15 ++- .../crates/relay-codegen/src/build_ast.rs | 112 +++++++++--------- compiler/crates/relay-codegen/src/lib.rs | 1 + compiler/crates/relay-codegen/src/printer.rs | 13 +- .../src/artifact_content/content.rs | 29 ++++- .../crates/relay-config/src/project_config.rs | 8 +- .../relay-runtime/util/RelayConcreteNode.js | 36 ++---- 7 files changed, 121 insertions(+), 93 deletions(-) diff --git a/compiler/crates/relay-codegen/src/ast.rs b/compiler/crates/relay-codegen/src/ast.rs index 5064659cf209b..d20a946916ec0 100644 --- a/compiler/crates/relay-codegen/src/ast.rs +++ b/compiler/crates/relay-codegen/src/ast.rs @@ -155,15 +155,20 @@ pub enum QueryID { External(StringKey), } -pub struct RequestParameters<'a> { - pub id: &'a Option, +pub enum RequestFormat { + ID(QueryID), + Text(Option), + Migration(QueryID, Option), +} + +pub struct RequestParameters { pub name: StringKey, pub operation_kind: OperationKind, - pub text: Option, + pub request_format: Option, } -impl<'a> RequestParameters<'a> { +impl RequestParameters { pub fn is_client_request(&self) -> bool { - self.id.is_none() && self.text.is_none() + return self.request_format.is_none(); } } diff --git a/compiler/crates/relay-codegen/src/build_ast.rs b/compiler/crates/relay-codegen/src/build_ast.rs index 3b154fb650755..fb10144650fd1 100644 --- a/compiler/crates/relay-codegen/src/build_ast.rs +++ b/compiler/crates/relay-codegen/src/build_ast.rs @@ -73,6 +73,7 @@ use crate::ast::JSModuleDependency; use crate::ast::ObjectEntry; use crate::ast::Primitive; use crate::ast::QueryID; +use crate::ast::RequestFormat; use crate::ast::RequestParameters; use crate::constants::CODEGEN_CONSTANTS; use crate::object; @@ -80,7 +81,7 @@ use crate::top_level_statements::TopLevelStatements; pub fn build_request_params_ast_key( schema: &SDLSchema, - request_parameters: RequestParameters<'_>, + request_parameters: RequestParameters, ast_builder: &mut AstBuilder, operation: &OperationDefinition, top_level_statements: &TopLevelStatements, @@ -149,12 +150,11 @@ pub fn build_request( })) } -pub fn build_request_params(operation: &OperationDefinition) -> RequestParameters<'_> { +pub fn build_request_params(operation: &OperationDefinition) -> RequestParameters { RequestParameters { name: operation.name.item.0, operation_kind: operation.kind, - id: &None, - text: None, + request_format: None, } } @@ -1790,7 +1790,7 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> { fn build_request_parameters( &mut self, operation: &OperationDefinition, - request_parameters: RequestParameters<'_>, + request_parameters: RequestParameters, top_level_statements: &TopLevelStatements, ) -> AstKey { let mut metadata_items: Vec = operation @@ -1845,64 +1845,64 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> { }), }; - let id_prop = ObjectEntry { - key: CODEGEN_CONSTANTS.id, - value: match request_parameters.id { - Some(QueryID::Persisted { id, .. }) => Primitive::RawString(id.clone()), - Some(QueryID::External(module_name)) => { - Primitive::JSModuleDependency(JSModuleDependency { + let id_prop = if let Some(RequestFormat::ID(ref id) | RequestFormat::Migration(ref id, _)) = + request_parameters.request_format + { + match id { + QueryID::Persisted { id, .. } => ObjectEntry { + key: CODEGEN_CONSTANTS.id, + value: Primitive::RawString(id.clone()), + }, + QueryID::External(module_name) => ObjectEntry { + key: CODEGEN_CONSTANTS.id, + value: Primitive::JSModuleDependency(JSModuleDependency { path: *module_name, named_import: None, import_as: None, - }) - } - None => Primitive::Null, - }, - }; - - let mut params_object = if let Some(text) = request_parameters.text { - vec![ - ObjectEntry { - key: CODEGEN_CONSTANTS.cache_id, - value: Primitive::RawString(md5(&text)), - }, - id_prop, - metadata_prop, - name_prop, - operation_kind_prop, - ObjectEntry { - key: CODEGEN_CONSTANTS.text, - value: Primitive::RawString(text), - }, - ] - } else if request_parameters.id.is_some() { - vec![ - id_prop, - metadata_prop, - name_prop, - operation_kind_prop, - ObjectEntry { - key: CODEGEN_CONSTANTS.text, - value: Primitive::Null, + }), }, - ] + } } else { - vec![ - ObjectEntry { - key: CODEGEN_CONSTANTS.cache_id, - value: Primitive::RawString(md5(operation.name.item.0.lookup())), - }, - id_prop, - metadata_prop, - name_prop, - operation_kind_prop, - ObjectEntry { - key: CODEGEN_CONSTANTS.text, - value: Primitive::Null, - }, - ] + ObjectEntry { + key: CODEGEN_CONSTANTS.id, + value: Primitive::Null, + } }; + let mut params_object = vec![]; + + match request_parameters.request_format { + Some( + RequestFormat::Text(Some(ref text)) | RequestFormat::Migration(_, Some(ref text)), + ) => params_object.push(ObjectEntry { + key: CODEGEN_CONSTANTS.cache_id, + value: Primitive::RawString(md5(&text.clone())), + }), + None => params_object.push(ObjectEntry { + key: CODEGEN_CONSTANTS.cache_id, + value: Primitive::RawString(md5(operation.name.item.0.lookup())), + }), + _ => (), + } + + params_object.push(id_prop); + params_object.push(metadata_prop); + params_object.push(name_prop); + params_object.push(operation_kind_prop); + + match request_parameters.request_format { + Some( + RequestFormat::Text(Some(ref text)) | RequestFormat::Migration(_, Some(ref text)), + ) => params_object.push(ObjectEntry { + key: CODEGEN_CONSTANTS.text, + value: Primitive::RawString(text.to_string()), + }), + _ => params_object.push(ObjectEntry { + key: CODEGEN_CONSTANTS.text, + value: Primitive::Null, + }), + } + let provided_variables = if top_level_statements .contains(CODEGEN_CONSTANTS.provided_variables_definition.lookup()) { diff --git a/compiler/crates/relay-codegen/src/lib.rs b/compiler/crates/relay-codegen/src/lib.rs index ebf9c4ebfc487..7b6effee333f9 100644 --- a/compiler/crates/relay-codegen/src/lib.rs +++ b/compiler/crates/relay-codegen/src/lib.rs @@ -20,6 +20,7 @@ mod utils; pub use ast::AstBuilder; pub use ast::Primitive; pub use ast::QueryID; +pub use ast::RequestFormat; pub use ast::RequestParameters; pub use build_ast::build_request_params; pub use build_ast::is_static_storage_key_available; diff --git a/compiler/crates/relay-codegen/src/printer.rs b/compiler/crates/relay-codegen/src/printer.rs index c0800e30652ab..1b9e88cea14dd 100644 --- a/compiler/crates/relay-codegen/src/printer.rs +++ b/compiler/crates/relay-codegen/src/printer.rs @@ -30,6 +30,7 @@ use crate::ast::JSModuleDependency; use crate::ast::ObjectEntry; use crate::ast::Primitive; use crate::ast::QueryID; +use crate::ast::RequestFormat; use crate::ast::RequestParameters; use crate::build_ast::build_fragment; use crate::build_ast::build_operation; @@ -70,7 +71,7 @@ pub fn print_request( schema: &SDLSchema, operation: &OperationDefinition, fragment: &FragmentDefinition, - request_parameters: RequestParameters<'_>, + request_parameters: RequestParameters, project_config: &ProjectConfig, top_level_statements: &mut TopLevelStatements, ) -> String { @@ -91,7 +92,11 @@ pub fn print_request_params( top_level_statements: &mut TopLevelStatements, ) -> String { let mut request_parameters = build_request_params(operation); - request_parameters.id = query_id; + request_parameters.request_format = if let Some(query_id) = &query_id { + Some(RequestFormat::ID(query_id.clone())) + } else { + None + }; let mut builder = AstBuilder::default(); let request_parameters_ast_key = build_request_params_ast_key( @@ -179,7 +184,7 @@ impl<'p> Printer<'p> { schema: &SDLSchema, operation: &OperationDefinition, fragment: &FragmentDefinition, - request_parameters: RequestParameters<'_>, + request_parameters: RequestParameters, top_level_statements: &mut TopLevelStatements, ) -> String { let request_parameters = build_request_params_ast_key( @@ -242,7 +247,7 @@ impl<'p> Printer<'p> { pub fn print_request_params( &mut self, schema: &SDLSchema, - request_parameters: RequestParameters<'_>, + request_parameters: RequestParameters, operation: &OperationDefinition, top_level_statements: &mut TopLevelStatements, ) -> String { diff --git a/compiler/crates/relay-compiler/src/artifact_content/content.rs b/compiler/crates/relay-compiler/src/artifact_content/content.rs index b5c82b3360525..e61847541af91 100644 --- a/compiler/crates/relay-compiler/src/artifact_content/content.rs +++ b/compiler/crates/relay-compiler/src/artifact_content/content.rs @@ -18,8 +18,10 @@ use intern::Lookup; use relay_codegen::build_request_params; use relay_codegen::Printer; use relay_codegen::QueryID; +use relay_codegen::RequestFormat; use relay_codegen::TopLevelStatement; use relay_codegen::CODEGEN_CONSTANTS; +use relay_config::PersistConfig; use relay_transforms::is_operation_preloadable; use relay_transforms::ReactFlightLocalComponentsMetadata; use relay_transforms::RelayClientComponentMetadata; @@ -178,11 +180,25 @@ pub fn generate_operation( fragment_locations: &FragmentLocations, ) -> Result, FmtError> { let mut request_parameters = build_request_params(normalization_operation); - if id_and_text_hash.is_some() { - request_parameters.id = id_and_text_hash; + + request_parameters.request_format = if let Some(id_and_text_hash) = id_and_text_hash { + let safe_migration = match &project_config.persist { + Some(PersistConfig::Remote(config)) => config.safe_migration, + Some(PersistConfig::Local(config)) => config.safe_migration, + None => false, + }; + if safe_migration { + Some(RequestFormat::Migration( + id_and_text_hash.clone(), + text.to_owned(), + )) + } else { + Some(RequestFormat::ID(id_and_text_hash.clone())) + } } else { - request_parameters.text = text.clone(); + Some(RequestFormat::Text(text.to_owned())) }; + let operation_fragment = FragmentDefinition { name: reader_operation.name.map(|x| FragmentDefinitionName(x.0)), variable_definitions: reader_operation.variable_definitions.clone(), @@ -220,9 +236,14 @@ pub fn generate_operation( // -- Begin Metadata Annotations Section -- let mut section = CommentAnnotationsSection::default(); - if let Some(QueryID::Persisted { id, .. }) = &request_parameters.id { + if let Some( + RequestFormat::ID(QueryID::Persisted { id, .. }) + | RequestFormat::Migration(QueryID::Persisted { id, .. }, _), + ) = &request_parameters.request_format + { writeln!(section, "@relayRequestID {}", id)?; } + if project_config.variable_names_comment { write!(section, "@relayVariables")?; for variable_definition in &normalization_operation.variable_definitions { diff --git a/compiler/crates/relay-config/src/project_config.rs b/compiler/crates/relay-config/src/project_config.rs index 6520c878e92dd..44a639c9d6b13 100644 --- a/compiler/crates/relay-config/src/project_config.rs +++ b/compiler/crates/relay-config/src/project_config.rs @@ -42,7 +42,7 @@ type FnvIndexMap = IndexMap; pub type ProjectName = StringKey; #[derive(Clone, Debug, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] pub struct RemotePersistConfig { /// URL to send a POST request to to persist. pub url: String, @@ -61,6 +61,9 @@ pub struct RemotePersistConfig { deserialize_with = "deserialize_semaphore_permits" )] pub semaphore_permits: Option, + + #[serde(default)] + pub safe_migration: bool, } fn deserialize_semaphore_permits<'de, D>(d: D) -> Result, D::Error> @@ -97,6 +100,9 @@ pub struct LocalPersistConfig { #[serde(default)] pub algorithm: LocalPersistAlgorithm, + + #[serde(default)] + pub safe_migration: bool, } #[derive(Debug, Serialize, Clone)] diff --git a/packages/relay-runtime/util/RelayConcreteNode.js b/packages/relay-runtime/util/RelayConcreteNode.js index 97e25f8edb1d7..d71d761aaf0c1 100644 --- a/packages/relay-runtime/util/RelayConcreteNode.js +++ b/packages/relay-runtime/util/RelayConcreteNode.js @@ -43,30 +43,20 @@ export type ProvidedVariablesType = {+[key: string]: {get(): mixed}}; /** * Contains the parameters required for executing a GraphQL request. - * The operation can either be provided as a persisted `id` or `text`. If given - * in `text` format, a `cacheID` as a hash of the text should be set to be used - * for local caching. + * The operation can either be provided as a persisted `id` or `text` or both. + * If `text` format is provided, a `cacheID` as a hash of the text should be set + * to be used for local caching. */ -export type RequestParameters = - | { - +id: string, - +text: null, - // common fields - +name: string, - +operationKind: 'mutation' | 'query' | 'subscription', - +providedVariables?: ProvidedVariablesType, - +metadata: {[key: string]: mixed, ...}, - } - | { - +cacheID: string, - +id: null, - +text: string | null, - // common fields - +name: string, - +operationKind: 'mutation' | 'query' | 'subscription', - +providedVariables?: ProvidedVariablesType, - +metadata: {[key: string]: mixed, ...}, - }; +export type RequestParameters = { + +cacheID?: string, + +id: string | null, + +text: string | null, + // common fields + +name: string, + +operationKind: 'mutation' | 'query' | 'subscription', + +providedVariables?: ProvidedVariablesType, + +metadata: {[key: string]: mixed, ...}, +}; export type ClientRequestParameters = { +cacheID: string,