Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a JSON file query persister strategy to relay-compiler #3666

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 13 additions & 16 deletions compiler/crates/relay-compiler/src/build_project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,22 +292,19 @@ pub async fn commit_project(
return Err(BuildProjectFailure::Cancelled);
}

if let Some(ref operation_persister) = config.operation_persister {
if let Some(ref persist_config) = project_config.persist {
let persist_operations_timer = log_event.start("persist_operations_time");
persist_operations::persist_operations(
&mut artifacts,
&config.root_dir,
persist_config,
config,
project_config,
operation_persister.as_ref(),
&log_event,
&programs,
)
.await?;
log_event.stop(persist_operations_timer);
}
if let Some(ref operation_persister) = project_config.operation_persister {
let persist_operations_timer = log_event.start("persist_operations_time");
persist_operations::persist_operations(
&mut artifacts,
&config.root_dir,
config,
project_config,
operation_persister.as_ref(),
&log_event,
&programs,
)
.await?;
log_event.stop(persist_operations_timer);
}

if source_control_update_status.is_started() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use crate::{
config::{Config, ProjectConfig},
config::{OperationPersister, PersistConfig},
config::{OperationPersister},
errors::BuildProjectError,
Artifact, ArtifactContent,
};
Expand All @@ -29,7 +29,6 @@ lazy_static! {
pub async fn persist_operations(
artifacts: &mut [Artifact],
root_dir: &PathBuf,
persist_config: &PersistConfig,
config: &Config,
project_config: &ProjectConfig,
operation_persister: &'_ (dyn OperationPersister + Send + Sync),
Expand Down Expand Up @@ -68,7 +67,7 @@ pub async fn persist_operations(
let text = text.clone();
Some(async move {
operation_persister
.persist_artifact(text, persist_config)
.persist_artifact(text)
.await
.map(|id| {
*id_and_text_hash = Some(QueryID::Persisted { id, text_hash });
Expand Down
69 changes: 40 additions & 29 deletions compiler/crates/relay-compiler/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use crate::build_project::{
};
use crate::compiler_state::{ProjectName, SourceSet};
use crate::errors::{ConfigValidationError, Error, Result};
use crate::remote_persister::{RemotePersistConfig, RemotePersister};
use crate::file_persister::{FilePersistConfig, FilePersister};
use crate::saved_state::SavedStateLoader;
use crate::status_reporter::{ConsoleStatusReporter, StatusReporter};
use async_trait::async_trait;
Expand Down Expand Up @@ -89,9 +91,6 @@ pub struct Config {
pub saved_state_loader: Option<Box<dyn SavedStateLoader + Send + Sync>>,
pub saved_state_version: String,

/// Function that is called to save operation text (e.g. to a database) and to generate an id.
pub operation_persister: Option<Box<dyn OperationPersister + Send + Sync>>,

pub post_artifacts_write: Option<PostArtifactsWriter>,

/// Validations that can be added to the config that will be called in addition to default
Expand Down Expand Up @@ -202,6 +201,14 @@ impl Config {
projects,
..
} = config_file;

let config_file_dir = config_path.parent().unwrap();
let root_dir = if let Some(config_root) = config_file.root {
config_file_dir.join(config_root).canonicalize().unwrap()
} else {
config_file_dir.to_owned()
};

let projects = projects
.into_iter()
.map(|(project_name, config_file_project)| {
Expand Down Expand Up @@ -245,6 +252,19 @@ impl Config {
}],
})?;

let operation_persister: Option<Box<dyn OperationPersister + Send + Sync>> =
match config_file_project.persist {
Some(config) => match config {
PersistConfig::Remote(remote_config) => {
Some(Box::new(RemotePersister::new(remote_config)))
},
PersistConfig::File(file_config) => {
Some(Box::new(FilePersister::new(file_config.file_path, root_dir.clone())))
}
},
_ => None,
};

let project_config = ProjectConfig {
name: project_name,
base: config_file_project.base,
Expand All @@ -256,7 +276,7 @@ impl Config {
shard_strip_regex,
schema_location,
typegen_config: config_file_project.typegen_config,
persist: config_file_project.persist,
operation_persister: operation_persister,
variable_names_comment: config_file_project.variable_names_comment,
extra: config_file_project.extra,
test_path_regex,
Expand All @@ -274,13 +294,6 @@ impl Config {
})
.collect::<Result<FnvIndexMap<_, _>>>()?;

let config_file_dir = config_path.parent().unwrap();
let root_dir = if let Some(config_root) = config_file.root {
config_file_dir.join(config_root).canonicalize().unwrap()
} else {
config_file_dir.to_owned()
};

let config = Self {
name: config_file.name,
artifact_writer: Box::new(ArtifactFileWriter::new(None, root_dir.clone())),
Expand All @@ -298,7 +311,6 @@ impl Config {
saved_state_loader: None,
saved_state_version: hex::encode(hash.result()),
connection_interface: config_file.connection_interface,
operation_persister: None,
compile_everything: false,
repersist_operations: false,
post_artifacts_write: None,
Expand Down Expand Up @@ -449,7 +461,6 @@ impl fmt::Debug for Config {
saved_state_loader,
connection_interface,
saved_state_version,
operation_persister,
post_artifacts_write,
..
} = self;
Expand All @@ -470,10 +481,6 @@ impl fmt::Debug for Config {
.field("codegen_command", codegen_command)
.field("load_saved_state_file", load_saved_state_file)
.field("saved_state_config", saved_state_config)
.field(
"operation_persister",
&option_fn_to_string(operation_persister),
)
.field(
"generate_extra_artifacts",
&option_fn_to_string(generate_extra_artifacts),
Expand Down Expand Up @@ -503,7 +510,7 @@ pub struct ProjectConfig {
pub enabled: bool,
pub schema_location: SchemaLocation,
pub typegen_config: TypegenConfig,
pub persist: Option<PersistConfig>,
pub operation_persister: Option<Box<dyn OperationPersister + Send + Sync>>,
pub variable_names_comment: bool,
pub extra: serde_json::Value,
pub feature_flags: Arc<FeatureFlags>,
Expand All @@ -528,7 +535,7 @@ impl Debug for ProjectConfig {
enabled,
schema_location,
typegen_config,
persist,
operation_persister,
variable_names_comment,
extra,
feature_flags,
Expand All @@ -538,6 +545,11 @@ impl Debug for ProjectConfig {
rollout,
js_module_format,
} = self;

fn option_fn_to_string<T>(option: &Option<T>) -> &'static str {
if option.is_some() { "Some(Fn)" } else { "None" }
}

f.debug_struct("ProjectConfig")
.field("name", name)
.field("base", base)
Expand All @@ -549,7 +561,10 @@ impl Debug for ProjectConfig {
.field("enabled", enabled)
.field("schema_location", schema_location)
.field("typegen_config", typegen_config)
.field("persist", persist)
.field(
"operation_persister",
&option_fn_to_string(operation_persister),
)
.field("variable_names_comment", variable_names_comment)
.field("extra", extra)
.field("feature_flags", feature_flags)
Expand Down Expand Up @@ -845,14 +860,11 @@ struct ConfigFileProject {
js_module_format: JsModuleFormat,
}

#[derive(Debug, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct PersistConfig {
/// URL to send a POST request to to persist.
pub url: String,
/// The document will be in a POST parameter `text`. This map can contain
/// additional parameters to send.
pub params: FnvIndexMap<String, String>,
#[derive(Serialize, Deserialize, Debug)]
#[serde(untagged)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this serde feature, the only problem with it: obscure error messages if both/mixed values are provided.

pub enum PersistConfig {
Remote(RemotePersistConfig),
File(FilePersistConfig),
}

type PersistId = String;
Expand All @@ -862,7 +874,6 @@ pub trait OperationPersister {
async fn persist_artifact(
&self,
artifact_text: String,
project_config: &PersistConfig,
) -> std::result::Result<PersistId, PersistError>;

fn worker_count(&self) -> usize;
Expand Down
67 changes: 67 additions & 0 deletions compiler/crates/relay-compiler/src/file_persister.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

use crate::OperationPersister;
use async_trait::async_trait;
use persist_query::PersistError;
use serde::{Deserialize, Serialize};
use sha1::{Digest, Sha1};
use std::collections::HashMap;
use std::path::PathBuf;

#[derive(Debug, Serialize, Deserialize)]
#[serde(deny_unknown_fields, rename_all = "camelCase")]
pub struct FilePersistConfig {
/// Path to a file to persist the document to.
pub file_path: PathBuf,
}

pub struct FilePersister {
file_path: PathBuf,
}

impl FilePersister {
pub fn new(path: PathBuf, root_path: PathBuf) -> Self {
// Append the incoming path to the project's root.
let mut file_path = root_path.clone();
file_path.push(path);
Self { file_path }
}

fn hash_operation(&self, operation_text: String) -> String {
let mut hash = Sha1::new();
hash.input(&operation_text);
hex::encode(hash.result())
}
}

#[async_trait]
impl OperationPersister for FilePersister {
async fn persist_artifact(&self, operation_text: String) -> Result<String, PersistError> {
let mut map: HashMap<String, String> = if self.file_path.exists() {
let existing_content = std::fs::read_to_string(&self.file_path);
match existing_content {
Ok(content) => serde_json::from_str(&content).unwrap(),
Err(_e) => HashMap::new(),
}
} else {
HashMap::new()
};

let op_hash = self.hash_operation(operation_text.clone());

map.insert(op_hash.clone(), operation_text);
let content = serde_json::to_string_pretty(&map).unwrap();
std::fs::write(&self.file_path, content).unwrap();

Ok(op_hash)
}

fn worker_count(&self) -> usize {
0
}
}
2 changes: 2 additions & 0 deletions compiler/crates/relay-compiler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod file_source;
mod graphql_asts;
mod red_to_green;
mod remote_persister;
mod file_persister;
pub mod saved_state;
pub mod status_reporter;

Expand All @@ -40,3 +41,4 @@ pub use file_source::{
};
pub use graphql_asts::GraphQLAsts;
pub use remote_persister::RemotePersister;
pub use file_persister::FilePersister;
3 changes: 1 addition & 2 deletions compiler/crates/relay-compiler/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use log::{error, info, Level};
use relay_compiler::{
compiler::Compiler,
config::{Config, SingleProjectConfigFile, TypegenLanguage},
FileSourceKind, RemotePersister,
FileSourceKind,
};
use std::io::Write;
use std::{
Expand Down Expand Up @@ -101,7 +101,6 @@ async fn main() {
error!("{}", err);
std::process::exit(1);
});
config.operation_persister = Some(Box::new(RemotePersister));
config.file_source_config = if should_use_watchman() {
FileSourceKind::Watchman
} else {
Expand Down
37 changes: 28 additions & 9 deletions compiler/crates/relay-compiler/src/remote_persister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,40 @@
* LICENSE file in the root directory of this source tree.
*/

use crate::{OperationPersister, PersistConfig};
use crate::OperationPersister;
use async_trait::async_trait;
use fnv::FnvBuildHasher;
use indexmap::IndexMap;
use persist_query::{persist, PersistError};
use serde::{Deserialize, Serialize};

pub struct RemotePersister;
type FnvIndexMap<K, V> = IndexMap<K, V, FnvBuildHasher>;

#[derive(Debug, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct RemotePersistConfig {
/// URL to send a POST request to to persist.
pub url: String,
/// The document will be in a POST parameter `text`. This map can contain
/// additional parameters to send.
pub params: FnvIndexMap<String, String>,
}

pub struct RemotePersister {
config: RemotePersistConfig,
}

impl RemotePersister {
pub fn new(config: RemotePersistConfig) -> Self {
Self { config }
}
}
Comment on lines +19 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!


#[async_trait]
impl OperationPersister for RemotePersister {
async fn persist_artifact(
&self,
operation_text: String,
persist_config: &PersistConfig,
) -> Result<String, PersistError> {
let params = &persist_config.params;
let url = &persist_config.url;
async fn persist_artifact(&self, operation_text: String) -> Result<String, PersistError> {
let params = &self.config.params;
let url = &self.config.url;
persist(&operation_text, url, params).await
}

Expand Down
5 changes: 5 additions & 0 deletions packages/relay-compiler/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ use [glob](https://docs.rs/glob/latest/glob/) to query the filesystem for files.
- `params` The document will be in a `POST`
parameter `text`. This map can contain additional
parameters to send. [object]
- `filePath` Relative path to JSON file where you want queries to be
locally persisted to. Note that if you set this
property you cannot also have the `url` or `params`
properties set.
[string]

### CLI configuration

Expand Down