Skip to content

Commit

Permalink
Fix a bunch of unnecessary clones in diesel_cli config parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
@Ten0
Ten0 committed Mar 3, 2025
1 parent 7c72d21 commit 34ca931
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 78 deletions.
138 changes: 62 additions & 76 deletions diesel_cli/src/config.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use super::find_project_root;
use crate::infer_schema_internals::TableName;
use crate::print_schema::ColumnSorting;
use crate::print_schema::{self, DocConfig};
use crate::print_schema::{self, ColumnSorting, DocConfig};
use clap::ArgMatches;
use serde::de::{self, MapAccess, Visitor};
use serde::{Deserialize, Deserializer};
Expand All @@ -10,8 +9,7 @@ use std::collections::btree_map::Entry;
use std::collections::BTreeMap;
use std::ops::Bound;
use std::path::{Path, PathBuf};
use std::{env, fmt};
use std::{fs, iter};
use std::{env, fmt, fs, iter};

#[derive(Deserialize, Default, Debug)]
#[serde(deny_unknown_fields)]
Expand All @@ -22,18 +20,13 @@ pub struct Config {
pub migrations_directory: Option<MigrationsDirectory>,
}

fn get_values_with_indices<T: Clone + Send + Sync + 'static>(
matches: &ArgMatches,
fn get_values_with_indices<'a, T: Clone + Send + Sync + 'static>(
matches: &'a ArgMatches,
id: &str,
) -> Result<Option<BTreeMap<usize, T>>, crate::errors::Error> {
) -> Result<Option<BTreeMap<usize, &'a T>>, crate::errors::Error> {
match matches.indices_of(id) {
Some(indices) => match matches.try_get_many::<T>(id) {
Ok(Some(values)) => Ok(Some(
indices
.zip(values)
.map(|(index, value)| (index, value.clone()))
.collect(),
)),
Ok(Some(values)) => Ok(Some(indices.zip(values).collect())),
Ok(None) => {
unreachable!("`ids` only reports what is present")
}
Expand Down Expand Up @@ -86,15 +79,15 @@ impl Config {
let except_tables_with_indices =
get_values_with_indices::<bool>(matches, "except-tables")?;

for (key, boundary) in selected_schema_keys.values().cloned().zip(
for (key, boundary) in selected_schema_keys.values().map(|k| k.as_str()).zip(
selected_schema_keys
.keys()
.cloned()
.copied()
.map(Bound::Included)
.zip(
selected_schema_keys
.keys()
.cloned()
.copied()
.skip(1)
.map(Bound::Excluded)
.chain(iter::once(Bound::Unbounded)),
Expand All @@ -103,56 +96,50 @@ impl Config {
let print_schema = self
.print_schema
.all_configs
.get_mut(&key)
.ok_or(crate::errors::Error::NoSchemaKeyFound(key))?;
if let Some(table_names_with_indices) = table_names_with_indices.clone() {
.get_mut(key)
.ok_or_else(|| crate::errors::Error::NoSchemaKeyFound(key.to_owned()))?;
if let Some(table_names_with_indices) = &table_names_with_indices {
let table_names = table_names_with_indices
.range(boundary)
.map(|(_, v)| v.clone())
.map(|(_, v)| v.as_str())
.map(|table_name_regex| {
regex::Regex::new(&table_name_regex).map(Into::into)
})
.collect::<Result<Vec<Regex>, _>>()?;
if table_names.is_empty() {
continue;
}
if only_tables_with_indices
.clone()
.and_then(|only_tables_with_indices| {
only_tables_with_indices
if except_tables_with_indices
.as_ref()
.and_then(|except_tables_with_indices| {
except_tables_with_indices
.range(boundary)
.nth(0)
.map(|v| *v.1)
.map(|v| **v.1)
})
.unwrap_or(false)
{
print_schema.filter = Filtering::OnlyTables(table_names.clone());
}
if except_tables_with_indices
.clone()
.and_then(|except_tables_with_indices| {
except_tables_with_indices
print_schema.filter = Filtering::ExceptTables(table_names);
} else if only_tables_with_indices
.as_ref()
.and_then(|only_tables_with_indices| {
only_tables_with_indices
.range(boundary)
.nth(0)
.map(|v| *v.1)
.map(|v| **v.1)
})
.unwrap_or(false)
{
print_schema.filter = Filtering::ExceptTables(table_names);
print_schema.filter = Filtering::OnlyTables(table_names);
}
}
}
} else {
let print_schema = self
.print_schema
.all_configs
.entry("default".to_string())
.or_insert(PrintSchema::default().set_filter(matches)?);
let print_schema = print_schema.clone().set_filter(matches)?;
self.print_schema
.all_configs
.entry("default".to_string())
.and_modify(|v| *v = print_schema);
.or_insert_with(|| PrintSchema::default())
.set_filter(matches)?;
}
Ok(self)
}
Expand Down Expand Up @@ -192,35 +179,34 @@ impl Config {
"except-custom-type-definitions",
)?;

for (key, boundary) in selected_schema_keys.values().cloned().zip(
for (key, boundary) in selected_schema_keys.values().map(|k| k.as_str()).zip(
selected_schema_keys
.keys()
.cloned()
.copied()
.map(Bound::Included)
.zip(
selected_schema_keys
.keys()
.cloned()
.copied()
.skip(1)
.map(Bound::Excluded)
.chain(iter::once(Bound::Unbounded)),
),
) {
let print_schema = self
.print_schema
.all_configs
.get_mut(&key)
.ok_or(crate::errors::Error::NoSchemaKeyFound(key))?;
let print_schema =
self.print_schema.all_configs.get_mut(key).ok_or_else(|| {
crate::errors::Error::NoSchemaKeyFound(key.to_owned())
})?;
if let Some(schema) = schema_with_indices
.clone()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.clone()))
.as_ref()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.as_str()))
{
print_schema.schema = Some(schema)
print_schema.schema = Some(schema.to_owned());
}
if with_docs_with_indices
.clone()
.as_ref()
.and_then(|with_docs_with_indices| {
with_docs_with_indices.range(boundary).nth(0).map(|v| *v.1)
with_docs_with_indices.range(boundary).nth(0).map(|v| **v.1)
})
.unwrap_or(false)
{
Expand All @@ -229,8 +215,8 @@ impl Config {
}

if let Some(doc_config) = with_docs_config_with_indices
.clone()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.clone()))
.as_ref()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.as_str()))
{
print_schema.with_docs = doc_config.parse().map_err(|_| {
crate::errors::Error::UnsupportedFeature(format!(
Expand All @@ -241,8 +227,8 @@ impl Config {

if let Some(allow_tables_to_appear_in_same_query_config) =
allow_tables_to_appear_in_same_query_config_with_indices
.clone()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.clone()))
.as_ref()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.as_str()))
{
print_schema.allow_tables_to_appear_in_same_query_config =
allow_tables_to_appear_in_same_query_config
Expand All @@ -256,10 +242,10 @@ impl Config {
}

if let Some(sorting) = column_sorting_with_indices
.clone()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.clone()))
.as_ref()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.as_str()))
{
match sorting.as_str() {
match sorting {
"ordinal_position" => {
print_schema.column_sorting = ColumnSorting::OrdinalPosition
}
Expand All @@ -273,27 +259,27 @@ impl Config {
}

if let Some(patch_file) = patch_file_with_indices
.clone()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.clone()))
.as_ref()
.and_then(|v| v.range(boundary).nth(0).map(|v| v.1.as_path()))
{
print_schema.patch_file = Some(patch_file);
print_schema.patch_file = Some(patch_file.to_owned());
}

let import_types = import_types_with_indices
.clone()
.map(|v| v.range(boundary).map(|v| v.1.clone()).collect())
.as_ref()
.map(|v| v.range(boundary).map(|v| v.1.as_str().to_owned()).collect())
.unwrap_or(vec![]);
if !import_types.is_empty() {
print_schema.import_types = Some(import_types);
}

if generate_custom_type_definitions_with_indices
.clone()
.as_ref()
.and_then(|generate_custom_type_definitions_with_indices| {
generate_custom_type_definitions_with_indices
.range(boundary)
.nth(0)
.map(|v| *v.1)
.map(|v| **v.1)
})
.unwrap_or(false)
{
Expand All @@ -309,17 +295,17 @@ impl Config {
}

let custom_type_derives = custom_type_derives_with_indices
.clone()
.map(|v| v.range(boundary).map(|v| v.1.clone()).collect())
.as_ref()
.map(|v| v.range(boundary).map(|v| v.1.as_str().to_owned()).collect())
.unwrap_or(vec![]);
if !custom_type_derives.is_empty() {
print_schema.custom_type_derives = Some(custom_type_derives);
}
if let Some(sqlite_integer_primary_key_is_bigint) =
sqlite_integer_primary_key_is_bigint_with_indices
.clone()
.as_ref()
.and_then(|with_docs_with_indices| {
with_docs_with_indices.range(boundary).nth(0).map(|v| *v.1)
with_docs_with_indices.range(boundary).nth(0).map(|v| **v.1)
})
{
print_schema.sqlite_integer_primary_key_is_bigint =
Expand All @@ -333,7 +319,7 @@ impl Config {
Entry::Occupied(entry) => entry.into_mut(),
};
if let Some(schema_name) = matches.get_one::<String>("schema") {
config.schema = Some(schema_name.clone())
config.schema = Some(schema_name.to_owned())
}
if matches.get_flag("with-docs") {
config.with_docs = DocConfig::DatabaseCommentsFallbackToAutoGeneratedDocComment;
Expand Down Expand Up @@ -530,7 +516,7 @@ impl PrintSchema {
}
}

pub fn set_filter(mut self, matches: &ArgMatches) -> Result<Self, crate::errors::Error> {
pub fn set_filter(&mut self, matches: &ArgMatches) -> Result<(), crate::errors::Error> {
let table_names = matches
.get_many::<String>("table-name")
.unwrap_or_default()
Expand All @@ -539,18 +525,18 @@ impl PrintSchema {

if matches
.try_get_one::<bool>("only-tables")?
.cloned()
.copied()
.unwrap_or(false)
{
self.filter = Filtering::OnlyTables(table_names)
} else if matches
.try_get_one::<bool>("except-tables")?
.cloned()
.copied()
.unwrap_or(false)
{
self.filter = Filtering::ExceptTables(table_names)
}
Ok(self)
Ok(())
}
}

Expand Down
4 changes: 2 additions & 2 deletions diesel_cli/src/migrations/diff_schema.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ fn compatible_type_list() -> HashMap<&'static str, Vec<&'static str>> {

#[tracing::instrument]
pub fn generate_sql_based_on_diff_schema(
config: PrintSchema,
mut config: PrintSchema,
matches: &ArgMatches,
schema_file_path: &Path,
) -> Result<(String, String), crate::errors::Error> {
let mut config = config.set_filter(matches)?;
config.set_filter(matches)?;

let project_root = crate::find_project_root()?;

Expand Down

0 comments on commit 34ca931

Please sign in to comment.