Skip to content

Commit

Permalink
Allow persisting to also include the query text for safe migration (#…
Browse files Browse the repository at this point in the history
…3917)

Summary:
Hi folks.

We're attempting to start using persisted queries. However, we're not 100% confident in our ability to switch over to them in one big step safely.

Fetching the persisted documents is a new capability for our GraphQL server. If something goes wrong fetching the document, or if it introduces unexpected latency to our existing production queries we want to be able to fallback to the query text until we're more confident in its maturity.

Relay currently supports outputting the query text, or outputting the persisted document id. This PR introduces a middle state, where the text and id are both output. When both are sent with the GraphQL request from the client to the server, the server is able to control the rollout of persisted queries via a feature flag.

This PR introduces a `safeMigration` bool to the remote persist config, which when set will also output the query text in the operation params as well as the id
```js
persistConfig: {
  url: ...,
  params: ...,
  includeQueryText: true
}
```

The benefits of persisted queries (saving the network bytes) won't come until we've progressed past the migration stage, but it makes it possible to do the move in the first place.

Pull Request resolved: #3917

Reviewed By: tyao1

Differential Revision: D46075478

Pulled By: captbaritone

fbshipit-source-id: 528ce0cec6fcee50ea80f416edb2e4568c487b77
  • Loading branch information
tomgasson authored and facebook-github-bot committed Jun 2, 2023
1 parent c530888 commit 95c54b4
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 66 deletions.
98 changes: 38 additions & 60 deletions compiler/crates/relay-codegen/src/build_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1874,24 +1874,21 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
metadata_items.sort_unstable_by_key(|entry| entry.key);

// Construct metadata object
let metadata_prop = ObjectEntry {
key: CODEGEN_CONSTANTS.metadata,
value: Primitive::Key(self.object(metadata_items)),
};
let name_prop = ObjectEntry {
key: CODEGEN_CONSTANTS.name,
value: Primitive::String(request_parameters.name),
};
let operation_kind_prop = ObjectEntry {
key: CODEGEN_CONSTANTS.operation_kind,
value: Primitive::String(match request_parameters.operation_kind {
OperationKind::Query => CODEGEN_CONSTANTS.query,
OperationKind::Mutation => CODEGEN_CONSTANTS.mutation,
OperationKind::Subscription => CODEGEN_CONSTANTS.subscription,
}),
};
let mut params_object = vec![];

let id_prop = ObjectEntry {
if let Some(ref text) = &request_parameters.text {
params_object.push(ObjectEntry {
key: CODEGEN_CONSTANTS.cache_id,
value: Primitive::RawString(md5(text)),
});
} else if request_parameters.id.is_none() {
params_object.push(ObjectEntry {
key: CODEGEN_CONSTANTS.cache_id,
value: Primitive::RawString(md5(operation.name.item.0.lookup())),
});
}

params_object.push(ObjectEntry {
key: CODEGEN_CONSTANTS.id,
value: match request_parameters.id {
Some(QueryID::Persisted { id, .. }) => Primitive::RawString(id.clone()),
Expand All @@ -1903,50 +1900,31 @@ impl<'schema, 'builder, 'config> CodegenBuilder<'schema, 'builder, 'config> {
}
None => Primitive::Null,
},
};
});
params_object.push(ObjectEntry {
key: CODEGEN_CONSTANTS.metadata,
value: Primitive::Key(self.object(metadata_items)),
});
params_object.push(ObjectEntry {
key: CODEGEN_CONSTANTS.name,
value: Primitive::String(request_parameters.name),
});
params_object.push(ObjectEntry {
key: CODEGEN_CONSTANTS.operation_kind,
value: Primitive::String(match request_parameters.operation_kind {
OperationKind::Query => CODEGEN_CONSTANTS.query,
OperationKind::Mutation => CODEGEN_CONSTANTS.mutation,
OperationKind::Subscription => CODEGEN_CONSTANTS.subscription,
}),
});

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,
},
]
};
params_object.push(ObjectEntry {
key: CODEGEN_CONSTANTS.text,
value: match request_parameters.text {
Some(text) => Primitive::RawString(text),
None => Primitive::Null,
},
});

let provided_variables = if top_level_statements
.contains(CODEGEN_CONSTANTS.provided_variables_definition.lookup())
Expand Down
11 changes: 10 additions & 1 deletion compiler/crates/relay-compiler/src/artifact_content/content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,20 @@ pub fn generate_operation(
fragment_locations: &FragmentLocations,
) -> Result<Vec<u8>, FmtError> {
let mut request_parameters = build_request_params(normalization_operation);

if id_and_text_hash.is_some() {
request_parameters.id = id_and_text_hash;
if project_config
.persist
.as_ref()
.map_or(false, |config| config.include_query_text())
{
request_parameters.text = text.clone();
}
} else {
request_parameters.text = text.clone();
};
}

let operation_fragment = FragmentDefinition {
name: reader_operation.name.map(|x| FragmentDefinitionName(x.0)),
variable_definitions: reader_operation.variable_definitions.clone(),
Expand Down
17 changes: 16 additions & 1 deletion compiler/crates/relay-config/src/project_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type FnvIndexMap<K, V> = IndexMap<K, V, FnvBuildHasher>;
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,
Expand All @@ -62,6 +62,9 @@ pub struct RemotePersistConfig {
deserialize_with = "deserialize_semaphore_permits"
)]
pub semaphore_permits: Option<usize>,

#[serde(default)]
pub include_query_text: bool,
}

fn deserialize_semaphore_permits<'de, D>(d: D) -> Result<Option<usize>, D::Error>
Expand Down Expand Up @@ -98,6 +101,9 @@ pub struct LocalPersistConfig {

#[serde(default)]
pub algorithm: LocalPersistAlgorithm,

#[serde(default)]
pub include_query_text: bool,
}

#[derive(Debug, Serialize, Clone)]
Expand All @@ -107,6 +113,15 @@ pub enum PersistConfig {
Local(LocalPersistConfig),
}

impl PersistConfig {
pub fn include_query_text(&self) -> bool {
match self {
PersistConfig::Remote(remote_config) => remote_config.include_query_text,
PersistConfig::Local(local_config) => local_config.include_query_text,
}
}
}

impl<'de> Deserialize<'de> for PersistConfig {
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> std::result::Result<Self, D::Error> {
let value = Value::deserialize(deserializer)?;
Expand Down
8 changes: 4 additions & 4 deletions packages/relay-runtime/util/RelayConcreteNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ 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,
+text: string | null,
// common fields
+name: string,
+operationKind: 'mutation' | 'query' | 'subscription',
Expand Down

0 comments on commit 95c54b4

Please sign in to comment.