hasura/graphql-engine
1
0
Fork 0
mirror of https://github.com/hasura/graphql-engine.git synced 2024-12-15 09:22:43 +03:00
Code Issues Projects Releases Wiki Activity

Pass Configuration by reference, for clearer intent (#1010)

Browse Source 
### What

Passing `Configuration` by reference signals intent (there is only ever
one configuration that is given at the toplevel which is never altered)
better than owning and copying values.

While in its current state it requires slightly more memory to pass a
reference to a `Configuration` rather than a value, this does not seem
like the thing to optimize for when it inhibits readability.

V3_GIT_ORIGIN_REV_ID: e5e05ad2e7ee41dfb49173ec9575de17552c63ae
This commit is contained in:
Philip Lykke Carlsen 2024-08-23 16:00:43 +02:00 committed by hasura-bot
parent 32a369b1e3
commit 5f861736c4
9 changed files with 19 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
v3/Cargo.toml
View File

@ -48,6 +48,7 @@ must_use_candidate = "allow"
return_self_not_must_use = "allow"
struct_field_names = "allow"
wildcard_imports = "allow"
trivially_copy_pass_by_ref = "allow"
# disable these for now, but we should probably fix them
result_large_err = "allow"
similar_names = "allow"

4
v3/crates/engine/bin/engine/main.rs
View File

@ -363,7 +363,7 @@ async fn start_engine(server: &ServerOptions) -> Result<(), StartupError> {
expose_internal_errors,
&server.authn_config_path,
&server.metadata_path,
metadata_resolve_configuration,
&metadata_resolve_configuration,
)
.map_err(StartupError::ReadSchema)?;
@ -749,7 +749,7 @@ fn build_state(
expose_internal_errors: execute::ExposeInternalErrors,
authn_config_path: &PathBuf,
metadata_path: &PathBuf,
metadata_resolve_configuration: metadata_resolve::configuration::Configuration,
metadata_resolve_configuration: &metadata_resolve::configuration::Configuration,
) -> Result<Arc<EngineState>, anyhow::Error> {
// Auth Config
let raw_auth_config = std::fs::read_to_string(authn_config_path)?;

2
v3/crates/engine/src/build.rs
View File

@ -20,7 +20,7 @@ pub struct BuildSchemaResponse {
/// this function is used by Metadata Build Service
pub fn build_schema(
metadata: open_dds::Metadata,
metadata_resolve_configuration: metadata_resolve::configuration::Configuration,
metadata_resolve_configuration: &metadata_resolve::configuration::Configuration,
) -> Result<BuildSchemaResponse, BuildError> {
let (resolved_metadata, warnings) =
metadata_resolve::resolve(metadata, metadata_resolve_configuration)?;

10
v3/crates/engine/tests/common.rs
View File

@ -83,7 +83,7 @@ pub fn test_execution_expectation_legacy(
metadata
);
let gds = GDS::new(metadata, test_metadata_resolve_configuration())?;
let gds = GDS::new(metadata, &test_metadata_resolve_configuration())?;
let schema = GDS::build_schema(&gds)?;
// Ensure schema is serialized successfully.
@ -169,7 +169,7 @@ pub(crate) fn test_introspection_expectation(
metadata
);
let gds = GDS::new(metadata, test_metadata_resolve_configuration())?;
let gds = GDS::new(metadata, &test_metadata_resolve_configuration())?;
let schema = GDS::build_schema(&gds)?;
@ -312,7 +312,7 @@ pub fn test_execution_expectation_for_multiple_ndc_versions(
metadata
);
let gds = GDS::new(metadata, test_metadata_resolve_configuration())?;
let gds = GDS::new(metadata, &test_metadata_resolve_configuration())?;
let schema = GDS::build_schema(&gds)?;
// Verify successful serialization and deserialization of the schema.
@ -491,7 +491,7 @@ pub fn test_execute_explain(
};
let gds = GDS::new(
open_dds::traits::OpenDd::deserialize(metadata)?,
configuration,
&configuration,
)?;
let schema = GDS::build_schema(&gds)?;
@ -575,7 +575,7 @@ pub(crate) fn test_sql(test_path_string: &str) -> anyhow::Result<()> {
metadata
);
let gds = GDS::new(metadata, test_metadata_resolve_configuration())?;
let gds = GDS::new(metadata, &test_metadata_resolve_configuration())?;
let schema = GDS::build_schema(&gds)?;
// Ensure schema is serialized successfully.

4
v3/crates/metadata-resolve/src/stages/mod.rs
View File

@ -30,7 +30,7 @@ use crate::types::error::Error;
/// This is where we take the input metadata and attempt to resolve a working `Metadata` object.
pub fn resolve(
metadata: open_dds::Metadata,
configuration: Configuration,
configuration: &Configuration,
) -> Result<(Metadata, Vec<Warning>), Error> {
// all warnings raised throughout metadata-resolve
let mut all_warnings = vec![];
@ -47,7 +47,7 @@ pub fn resolve(
let data_connectors::DataConnectorsOutput {
data_connectors,
issues,
} = data_connectors::resolve(&metadata_accessor, &configuration)?;
} = data_connectors::resolve(&metadata_accessor, configuration)?;
all_warnings.extend(issues.into_iter().map(Warning::from));

6
v3/crates/metadata-resolve/src/stages/relationships/mod.rs
View File

@ -35,7 +35,7 @@ pub use types::{
/// returns updated `types` value
pub fn resolve(
metadata_accessor: &open_dds::accessor::MetadataAccessor,
configuration: Configuration,
configuration: &Configuration,
data_connectors: &data_connectors::DataConnectors,
data_connector_scalars: &BTreeMap<
Qualified<DataConnectorName>,
@ -684,7 +684,7 @@ fn resolve_command_relationship_field(
}
fn resolve_relationships(
configuration: Configuration,
configuration: &Configuration,
relationship: &RelationshipV1,
subgraph: &open_dds::identifier::SubgraphName,
known_subgraphs: &HashSet<open_dds::identifier::SubgraphName>,
@ -756,7 +756,7 @@ fn resolve_relationships(
// the same functionality to that stage of metadata resolution, and perhaps think about creating an
// abstraction for that purpose.
fn should_skip(
configuration: Configuration,
configuration: &Configuration,
known_subgraphs: &HashSet<open_dds::identifier::SubgraphName>,
target_subgraph: Option<&SubgraphName>,
) -> bool {

2
v3/crates/metadata-resolve/src/types/configuration.rs
View File

@ -1,7 +1,7 @@
/// Configuration for the metadata-resolve step.
///
/// Deserialization is only intended to be used for testing and is not reliable.
#[derive(Debug, Clone, Copy, Default, serde::Deserialize)]
#[derive(Debug, Clone, Default, serde::Deserialize)]
#[serde(default, deny_unknown_fields, rename_all = "camelCase")]
pub struct Configuration {
pub allow_unknown_subgraphs: bool,

4
v3/crates/metadata-resolve/tests/metadata_golden_tests.rs
View File

@ -27,7 +27,7 @@ fn test_passing_metadata() {
let metadata = open_dds::traits::OpenDd::deserialize(metadata_json_value)
.unwrap_or_else(|error| panic!("{}: Could not deserialize metadata: {error}", directory.display()));
let resolved = metadata_resolve::resolve(metadata, configuration)
let resolved = metadata_resolve::resolve(metadata, &configuration)
.unwrap_or_else(|error| panic!("{}: Could not resolve metadata: {error}",directory.display()));
insta::assert_debug_snapshot!("resolved", resolved);
@ -54,7 +54,7 @@ fn test_failing_metadata() {
Ok(metadata_json_value) => {
match open_dds::traits::OpenDd::deserialize(metadata_json_value) {
Ok(metadata) => {
match metadata_resolve::resolve(metadata, configuration) {
match metadata_resolve::resolve(metadata, &configuration) {
Ok(_) => {
panic!("{}: Unexpected success when resolving {path:?}.", directory.display());
}

4
v3/crates/schema/src/lib.rs
View File

@ -89,7 +89,7 @@ pub struct GDS {
impl GDS {
pub fn new(
user_metadata: open_dds::Metadata,
metadata_resolve_configuration: metadata_resolve::configuration::Configuration,
metadata_resolve_configuration: &metadata_resolve::configuration::Configuration,
) -> Result<Self, Error> {
let (resolved_metadata, _) =
metadata_resolve::resolve(user_metadata, metadata_resolve_configuration)?;
@ -101,7 +101,7 @@ impl GDS {
pub fn new_with_default_flags(user_metadata: open_dds::Metadata) -> Result<Self, Error> {
let (resolved_metadata, _) = metadata_resolve::resolve(
user_metadata,
metadata_resolve::configuration::Configuration::default(),
&metadata_resolve::configuration::Configuration::default(),
)?;
Ok(GDS {
metadata: Arc::new(resolved_metadata),