From ca830c05fbf11625eb83d06e06b61bb4cc909df0 Mon Sep 17 00:00:00 2001 From: Samir Talwar Date: Wed, 5 Jun 2024 16:25:34 +0300 Subject: [PATCH] Be more explicit about clones, and remove a few. (#671) Calling `.to_owned()` on a reference, `.to_vec()` on a vector reference, etc. are just synonyms for `.clone()` which are less explicit about cloning. Let's be explicit. This also removes some unnecessary clones. V3_GIT_ORIGIN_REV_ID: 1bc00c4106f0346303d73e4268c89030c0ce93fc --- v3/Cargo.toml | 2 - v3/crates/execute/src/plan.rs | 2 +- v3/crates/execute/src/remote_joins/collect.rs | 2 +- .../src/stages/boolean_expressions/object.rs | 2 +- .../src/stages/boolean_expressions/scalar.rs | 4 +- .../stages/data_connector_scalar_types/mod.rs | 2 +- .../metadata-resolve/src/stages/models/mod.rs | 2 +- .../stages/object_boolean_expressions/mod.rs | 10 ++--- .../src/stages/relationships/mod.rs | 42 ++++++++----------- .../src/stages/type_permissions/mod.rs | 2 +- .../metadata-resolve/src/types/subgraph.rs | 4 +- v3/crates/schema/src/types/output_type.rs | 2 +- 12 files changed, 32 insertions(+), 44 deletions(-) diff --git a/v3/Cargo.toml b/v3/Cargo.toml index 9da5e10de30..641452a2cbc 100644 --- a/v3/Cargo.toml +++ b/v3/Cargo.toml @@ -31,9 +31,7 @@ must_use_candidate = "allow" struct_field_names = "allow" wildcard_imports = "allow" # disable these for now, but we should probably fix them -implicit_clone = "allow" implicit_hasher = "allow" -inefficient_to_string = "allow" result_large_err = "allow" return_self_not_must_use = "allow" semicolon_if_nothing_returned = "allow" diff --git a/v3/crates/execute/src/plan.rs b/v3/crates/execute/src/plan.rs index 5a9b461d683..5a901d97d82 100644 --- a/v3/crates/execute/src/plan.rs +++ b/v3/crates/execute/src/plan.rs @@ -446,7 +446,7 @@ fn assign_join_ids<'s, 'ir>( } }; let new_location = Location { - join_node: new_node.to_owned(), + join_node: new_node.clone(), rest: assign_join_ids(&location.rest, state), }; (key.to_string(), new_location) diff --git a/v3/crates/execute/src/remote_joins/collect.rs b/v3/crates/execute/src/remote_joins/collect.rs index d6d67a8d331..3a415008c9c 100644 --- a/v3/crates/execute/src/remote_joins/collect.rs +++ b/v3/crates/execute/src/remote_joins/collect.rs @@ -305,7 +305,7 @@ fn resolve_command_response_row( // the array and use that as the value for the relationship otherwise // we return the array of objects. let array_values: Vec> = - json::from_value(json::Value::Array(values.to_vec()))?; + json::from_value(json::Value::Array(values.clone()))?; if type_container.is_list(){ Ok(array_values) diff --git a/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs b/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs index 70dfd12b581..7c0dce94378 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/object.rs @@ -35,7 +35,7 @@ pub(crate) fn resolve_object_boolean_expression_type( ) -> Result { let qualified_object_type_name = Qualified::new( subgraph.to_string(), - object_boolean_expression_operand.r#type.to_owned(), + object_boolean_expression_operand.r#type.clone(), ); let object_type_representation = diff --git a/v3/crates/metadata-resolve/src/stages/boolean_expressions/scalar.rs b/v3/crates/metadata-resolve/src/stages/boolean_expressions/scalar.rs index 03175179003..6b43a37fc64 100644 --- a/v3/crates/metadata-resolve/src/stages/boolean_expressions/scalar.rs +++ b/v3/crates/metadata-resolve/src/stages/boolean_expressions/scalar.rs @@ -29,9 +29,7 @@ pub(crate) fn resolve_scalar_boolean_expression_type( // scope the data connector to the current subgraph let qualified_data_connector_name = Qualified::new( subgraph.to_string(), - data_connector_operator_mapping - .data_connector_name - .to_owned(), + data_connector_operator_mapping.data_connector_name.clone(), ); // lookup the data connector we are referring to diff --git a/v3/crates/metadata-resolve/src/stages/data_connector_scalar_types/mod.rs b/v3/crates/metadata-resolve/src/stages/data_connector_scalar_types/mod.rs index 7febf5785dd..56075a8d08b 100644 --- a/v3/crates/metadata-resolve/src/stages/data_connector_scalar_types/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/data_connector_scalar_types/mod.rs @@ -44,7 +44,7 @@ pub fn resolve<'a>( let qualified_data_connector_name = Qualified::new( subgraph.to_string(), - scalar_type_representation.data_connector_name.to_owned(), + scalar_type_representation.data_connector_name.clone(), ); let scalars = data_connector_scalars diff --git a/v3/crates/metadata-resolve/src/stages/models/mod.rs b/v3/crates/metadata-resolve/src/stages/models/mod.rs index f4fd27eb3e9..754fdf2011b 100644 --- a/v3/crates/metadata-resolve/src/stages/models/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/models/mod.rs @@ -231,7 +231,7 @@ fn resolve_model( >, ) -> Result { let qualified_object_type_name = - Qualified::new(subgraph.to_string(), model.object_type.to_owned()); + Qualified::new(subgraph.to_string(), model.object_type.clone()); let qualified_model_name = Qualified::new(subgraph.to_string(), model.name.clone()); let object_type_representation = get_model_object_type_representation( object_types, diff --git a/v3/crates/metadata-resolve/src/stages/object_boolean_expressions/mod.rs b/v3/crates/metadata-resolve/src/stages/object_boolean_expressions/mod.rs index a41407dfa46..be6d98a784c 100644 --- a/v3/crates/metadata-resolve/src/stages/object_boolean_expressions/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/object_boolean_expressions/mod.rs @@ -80,14 +80,12 @@ pub(crate) fn resolve_object_boolean_expression_type( graphql_config: &graphql_config::GraphqlConfig, ) -> Result { // name of the boolean expression - let qualified_name = Qualified::new( - subgraph.to_string(), - object_boolean_expression.name.to_owned(), - ); + let qualified_name = + Qualified::new(subgraph.to_string(), object_boolean_expression.name.clone()); // name of the object type backing the boolean expression let qualified_object_type_name = Qualified::new( subgraph.to_string(), - object_boolean_expression.object_type.to_owned(), + object_boolean_expression.object_type.clone(), ); let object_type_representation = object_types @@ -102,7 +100,7 @@ pub(crate) fn resolve_object_boolean_expression_type( let qualified_data_connector_name = Qualified::new( subgraph.to_string(), - object_boolean_expression.data_connector_name.to_owned(), + object_boolean_expression.data_connector_name.clone(), ); // validate data connector name diff --git a/v3/crates/metadata-resolve/src/stages/relationships/mod.rs b/v3/crates/metadata-resolve/src/stages/relationships/mod.rs index b5f63342508..bc6414cc780 100644 --- a/v3/crates/metadata-resolve/src/stages/relationships/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/relationships/mod.rs @@ -1,30 +1,28 @@ mod types; -pub use types::{ - ObjectTypeWithRelationships, Relationship, RelationshipCapabilities, - RelationshipCommandMapping, RelationshipExecutionCategory, RelationshipModelMapping, - RelationshipTarget, RelationshipTargetName, -}; use std::collections::BTreeMap; +use std::collections::BTreeSet; use indexmap::IndexMap; +use open_dds::relationships::{self, FieldAccess, RelationshipName, RelationshipV1}; use open_dds::{ commands::CommandName, data_connector::DataConnectorName, models::ModelName, types::CustomTypeName, }; -use crate::types::error::{Error, RelationshipError}; -use crate::types::subgraph::Qualified; - use crate::helpers::types::mk_name; use crate::stages::{ commands, data_connector_scalar_types, data_connectors, models, object_types, type_permissions, }; +use crate::types::error::{Error, RelationshipError}; +use crate::types::subgraph::Qualified; -use open_dds::relationships::{self, FieldAccess, RelationshipName, RelationshipV1}; - -use std::collections::BTreeSet; +pub use types::{ + ObjectTypeWithRelationships, Relationship, RelationshipCapabilities, + RelationshipCommandMapping, RelationshipExecutionCategory, RelationshipModelMapping, + RelationshipTarget, RelationshipTargetName, +}; /// resolve relationships /// returns updated `types` value @@ -70,7 +68,7 @@ pub fn resolve( } in &metadata_accessor.relationships { let qualified_relationship_source_type_name = - Qualified::new(subgraph.to_string(), relationship.source_type.to_owned()); + Qualified::new(subgraph.to_string(), relationship.source_type.clone()); let object_representation = object_types_with_relationships .get_mut(&qualified_relationship_source_type_name) .ok_or_else(|| Error::RelationshipDefinedOnUnknownType { @@ -352,11 +350,11 @@ fn resolve_relationship_mappings_command( fn get_relationship_capabilities( type_name: &Qualified, relationship_name: &RelationshipName, - source_data_connector: &Option, + source_data_connector: Option<&data_connectors::DataConnectorLink>, target_name: &RelationshipTargetName, data_connectors: &data_connectors::DataConnectors, ) -> Result, Error> { - let Some(data_connector) = &source_data_connector else { + let Some(data_connector) = source_data_connector else { return Ok(None); }; @@ -413,12 +411,8 @@ pub fn resolve_relationship( let (relationship_target, source_data_connector, target_name) = match &relationship.target { relationships::RelationshipTarget::Model(target_model) => { let qualified_target_model_name = Qualified::new( - target_model - .subgraph() - .to_owned() - .unwrap_or(subgraph) - .to_string(), - target_model.name.to_owned(), + target_model.subgraph().unwrap_or(subgraph).to_string(), + target_model.name.clone(), ); let resolved_target_model = models.get(&qualified_target_model_name).ok_or_else(|| { @@ -431,7 +425,7 @@ pub fn resolve_relationship( let source_data_connector = resolved_target_model .source .as_ref() - .map(|source| source.data_connector.clone()); + .map(|source| &source.data_connector); ( RelationshipTarget::Model { model_name: qualified_target_model_name, @@ -456,7 +450,7 @@ pub fn resolve_relationship( .as_deref() .unwrap_or(subgraph) .to_string(), - target_command.name.to_owned(), + target_command.name.clone(), ); let resolved_target_command = commands @@ -470,7 +464,7 @@ pub fn resolve_relationship( let source_data_connector = resolved_target_command .source .as_ref() - .map(|source| source.data_connector.clone()); + .map(|source| &source.data_connector); ( RelationshipTarget::Command { command_name: qualified_target_command_name, @@ -491,7 +485,7 @@ pub fn resolve_relationship( let target_capabilities = get_relationship_capabilities( &source_type_name, &relationship.name, - &source_data_connector, + source_data_connector, &target_name, data_connectors, )?; diff --git a/v3/crates/metadata-resolve/src/stages/type_permissions/mod.rs b/v3/crates/metadata-resolve/src/stages/type_permissions/mod.rs index 1788ad0af22..be29e3d7d69 100644 --- a/v3/crates/metadata-resolve/src/stages/type_permissions/mod.rs +++ b/v3/crates/metadata-resolve/src/stages/type_permissions/mod.rs @@ -58,7 +58,7 @@ pub fn resolve( { let qualified_type_name = Qualified::new( subgraph.to_string(), - output_type_permission.type_name.to_owned(), + output_type_permission.type_name.clone(), ); match object_types_with_permissions.get_mut(&qualified_type_name) { None => { diff --git a/v3/crates/metadata-resolve/src/types/subgraph.rs b/v3/crates/metadata-resolve/src/types/subgraph.rs index 47bd3c26eb4..42873b5baf9 100644 --- a/v3/crates/metadata-resolve/src/types/subgraph.rs +++ b/v3/crates/metadata-resolve/src/types/subgraph.rs @@ -100,7 +100,7 @@ where let map: BTreeMap = Deserialize::deserialize(deserializer)?; let mut result = BTreeMap::new(); for (key, value) in map { - let qualified = serde_json::from_str(&key.to_owned()).map_err(serde::de::Error::custom)?; + let qualified = serde_json::from_str(&key).map_err(serde::de::Error::custom)?; result.insert(qualified, value); } Ok(result) @@ -135,7 +135,7 @@ where let map: BTreeMap = Deserialize::deserialize(deserializer)?; let mut result = BTreeMap::new(); for (key, value) in map { - let qualified = serde_json::from_str(&key.to_owned()).map_err(serde::de::Error::custom)?; + let qualified = serde_json::from_str(&key).map_err(serde::de::Error::custom)?; result.insert(qualified, value); } Ok(result) diff --git a/v3/crates/schema/src/types/output_type.rs b/v3/crates/schema/src/types/output_type.rs index e85d52b306f..64dfd48c2de 100644 --- a/v3/crates/schema/src/types/output_type.rs +++ b/v3/crates/schema/src/types/output_type.rs @@ -454,7 +454,7 @@ pub fn output_type_schema( global_id_fields: object_type_representation .object_type .global_id_fields - .to_vec(), + .clone(), }), get_output_type(gds, builder, &ID_TYPE_REFERENCE)?, BTreeMap::new(),