From 5ba0191e52eb163a69e54348a12887ac721dd9a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=20Houl=C3=A9?= Date: Wed, 26 Apr 2023 13:29:32 +0200 Subject: [PATCH] qe/dmmf: take QuerySchema by ref and rename IntoRenderer - We now take QuerySchema by reference. That binds a handy lifetime into the rendering context for everything derived from Queryschema. - IntoRenderer is renamed to AsRenderer to make clippy and people who care about the rust api guidelines happy (myself included), and the signature changes a little so that lifetimes are clarified, This PR is a simple cleanup with ulterior motives re: the API exposed by `schema` with https://github.com/prisma/client-planning/issues/344. --- .../ast_builders/schema_ast_builder/mod.rs | 39 +++++++------- .../schema_ast_builder/object_renderer.rs | 2 +- .../schema_ast_builder/schema_renderer.rs | 10 ++-- query-engine/dmmf/src/lib.rs | 6 +-- .../query-engine-node-api/src/engine.rs | 2 +- .../query-engine-node-api/src/functions.rs | 5 +- query-engine/query-engine/src/cli.rs | 9 ++-- query-engine/query-engine/src/main.rs | 2 +- query-engine/query-engine/src/server/mod.rs | 2 +- query-engine/query-engine/src/tests/dmmf.rs | 2 +- query-engine/request-handlers/src/dmmf/mod.rs | 4 +- .../graphql/schema_renderer/field_renderer.rs | 6 +-- .../protocols/graphql/schema_renderer/mod.rs | 51 ++++++++----------- .../schema_renderer/object_renderer.rs | 4 +- .../graphql/schema_renderer/type_renderer.rs | 8 +-- 15 files changed, 68 insertions(+), 84 deletions(-) diff --git a/query-engine/dmmf/src/ast_builders/schema_ast_builder/mod.rs b/query-engine/dmmf/src/ast_builders/schema_ast_builder/mod.rs index ceba7292eb84..6fbb032ff4ad 100644 --- a/query-engine/dmmf/src/ast_builders/schema_ast_builder/mod.rs +++ b/query-engine/dmmf/src/ast_builders/schema_ast_builder/mod.rs @@ -11,11 +11,11 @@ use indexmap::map::Entry; use object_renderer::*; use schema::*; use schema_renderer::*; -use std::{collections::HashSet, sync::Arc}; +use std::collections::HashSet; use type_renderer::*; -pub(crate) fn render(query_schema: QuerySchemaRef) -> (DmmfSchema, DmmfOperationMappings) { - let mut ctx = RenderContext::new(&query_schema); +pub(crate) fn render(query_schema: &QuerySchema) -> (DmmfSchema, DmmfOperationMappings) { + let mut ctx = RenderContext::new(query_schema); ctx.mark_to_be_rendered(&query_schema); while !ctx.next_pass.is_empty() { @@ -44,7 +44,7 @@ pub(crate) struct RenderContext<'a> { /// The child objects to render next. Rendering is considered complete when /// this is empty. - next_pass: Vec>, + next_pass: Vec>, } impl<'a> RenderContext<'a> { @@ -149,9 +149,9 @@ impl<'a> RenderContext<'a> { } } - fn mark_to_be_rendered(&mut self, into_renderer: &dyn IntoRenderer) { + fn mark_to_be_rendered(&mut self, into_renderer: &dyn AsRenderer<'a>) { if !into_renderer.is_already_rendered(self) { - let renderer: Box = into_renderer.into_renderer(); + let renderer: Box = into_renderer.as_renderer(); self.next_pass.push(renderer) } } @@ -161,18 +161,16 @@ pub(crate) trait Renderer { fn render(&self, ctx: &mut RenderContext); } -trait IntoRenderer { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&self) -> Box; +trait AsRenderer<'a> { + fn as_renderer(&self) -> Box; /// Returns whether the item still needs to be rendered. - fn is_already_rendered(&self, ctx: &RenderContext) -> bool; + fn is_already_rendered(&self, ctx: &RenderContext<'_>) -> bool; } -impl IntoRenderer for QuerySchemaRef { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&self) -> Box { - Box::new(DmmfSchemaRenderer::new(Arc::clone(self))) +impl<'a> AsRenderer<'a> for &'a QuerySchema { + fn as_renderer(&self) -> Box { + Box::new(DmmfSchemaRenderer::new(self)) } fn is_already_rendered(&self, _ctx: &RenderContext) -> bool { @@ -180,9 +178,8 @@ impl IntoRenderer for QuerySchemaRef { } } -impl<'a> IntoRenderer for &'a EnumType { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&self) -> Box { +impl<'a> AsRenderer<'a> for &'a EnumType { + fn as_renderer(&self) -> Box { Box::new(DmmfEnumRenderer::new(self)) } @@ -191,8 +188,8 @@ impl<'a> IntoRenderer for &'a EnumType { } } -impl IntoRenderer for InputObjectTypeId { - fn into_renderer(&self) -> Box { +impl AsRenderer<'_> for InputObjectTypeId { + fn as_renderer(&self) -> Box { Box::new(DmmfObjectRenderer::Input(*self)) } @@ -201,8 +198,8 @@ impl IntoRenderer for InputObjectTypeId { } } -impl IntoRenderer for OutputObjectTypeId { - fn into_renderer(&self) -> Box { +impl AsRenderer<'_> for OutputObjectTypeId { + fn as_renderer(&self) -> Box { Box::new(DmmfObjectRenderer::Output(*self)) } diff --git a/query-engine/dmmf/src/ast_builders/schema_ast_builder/object_renderer.rs b/query-engine/dmmf/src/ast_builders/schema_ast_builder/object_renderer.rs index aac450f026d4..14fbf5c13f17 100644 --- a/query-engine/dmmf/src/ast_builders/schema_ast_builder/object_renderer.rs +++ b/query-engine/dmmf/src/ast_builders/schema_ast_builder/object_renderer.rs @@ -1,7 +1,7 @@ use super::*; #[derive(Debug)] -pub enum DmmfObjectRenderer { +pub(crate) enum DmmfObjectRenderer { Input(InputObjectTypeId), Output(OutputObjectTypeId), } diff --git a/query-engine/dmmf/src/ast_builders/schema_ast_builder/schema_renderer.rs b/query-engine/dmmf/src/ast_builders/schema_ast_builder/schema_renderer.rs index 8f3530e9fe06..b77b8e8b0379 100644 --- a/query-engine/dmmf/src/ast_builders/schema_ast_builder/schema_renderer.rs +++ b/query-engine/dmmf/src/ast_builders/schema_ast_builder/schema_renderer.rs @@ -1,10 +1,10 @@ use super::*; -pub struct DmmfSchemaRenderer { - query_schema: QuerySchemaRef, +pub(crate) struct DmmfSchemaRenderer<'a> { + query_schema: &'a QuerySchema, } -impl Renderer for DmmfSchemaRenderer { +impl<'a> Renderer for DmmfSchemaRenderer<'a> { fn render(&self, ctx: &mut RenderContext) { // This ensures that all enums are rendered, even if not reached by the output and input types. render_enum_types(ctx, self.query_schema.enum_types()); @@ -13,8 +13,8 @@ impl Renderer for DmmfSchemaRenderer { } } -impl DmmfSchemaRenderer { - pub fn new(query_schema: QuerySchemaRef) -> DmmfSchemaRenderer { +impl<'a> DmmfSchemaRenderer<'a> { + pub(crate) fn new(query_schema: &'a QuerySchema) -> DmmfSchemaRenderer<'a> { DmmfSchemaRenderer { query_schema } } } diff --git a/query-engine/dmmf/src/lib.rs b/query-engine/dmmf/src/lib.rs index 4a4af661ad61..079f2deaed2e 100644 --- a/query-engine/dmmf/src/lib.rs +++ b/query-engine/dmmf/src/lib.rs @@ -7,7 +7,7 @@ mod tests; pub use serialization_ast::DataModelMetaFormat; use ast_builders::schema_to_dmmf; -use schema::QuerySchemaRef; +use schema::QuerySchema; use std::sync::Arc; pub fn dmmf_json_from_schema(schema: &str) -> String { @@ -18,10 +18,10 @@ pub fn dmmf_json_from_schema(schema: &str) -> String { pub fn dmmf_from_schema(schema: &str) -> DataModelMetaFormat { let schema = Arc::new(psl::parse_schema(schema).unwrap()); let internal_data_model = prisma_models::convert(schema); - from_precomputed_parts(Arc::new(schema::build(internal_data_model, true))) + from_precomputed_parts(&schema::build(internal_data_model, true)) } -pub fn from_precomputed_parts(query_schema: QuerySchemaRef) -> DataModelMetaFormat { +pub fn from_precomputed_parts(query_schema: &QuerySchema) -> DataModelMetaFormat { let data_model = schema_to_dmmf(&query_schema.internal_data_model.schema); let (schema, mappings) = ast_builders::render(query_schema); diff --git a/query-engine/query-engine-node-api/src/engine.rs b/query-engine/query-engine-node-api/src/engine.rs index a7f1158a1704..4338b19e7873 100644 --- a/query-engine/query-engine-node-api/src/engine.rs +++ b/query-engine/query-engine-node-api/src/engine.rs @@ -416,7 +416,7 @@ impl QueryEngine { async_panic_to_js_error(async { let inner = self.inner.read().await; let engine = inner.as_engine()?; - let dmmf = dmmf::render_dmmf(engine.query_schema.clone()); + let dmmf = dmmf::render_dmmf(&engine.query_schema); Ok(serde_json::to_string(&dmmf)?) }) diff --git a/query-engine/query-engine-node-api/src/functions.rs b/query-engine/query-engine-node-api/src/functions.rs index e26461cbee00..5ac5f1320793 100644 --- a/query-engine/query-engine-node-api/src/functions.rs +++ b/query-engine/query-engine-node-api/src/functions.rs @@ -1,7 +1,6 @@ use crate::error::ApiError; use napi::{bindgen_prelude::*, JsUnknown}; use napi_derive::napi; -use query_core::schema::{self, QuerySchemaRef}; use request_handlers::dmmf; use std::{ collections::{BTreeMap, HashMap}, @@ -33,8 +32,8 @@ pub fn dmmf(datamodel_string: String) -> napi::Result { .map_err(|errors| ApiError::conversion(errors, schema.db.source()))?; let internal_data_model = prisma_models::convert(Arc::new(schema)); - let query_schema: QuerySchemaRef = Arc::new(schema::build(internal_data_model, true)); - let dmmf = dmmf::render_dmmf(query_schema); + let query_schema = query_core::schema::build(internal_data_model, true); + let dmmf = dmmf::render_dmmf(&query_schema); Ok(serde_json::to_string(&dmmf)?) } diff --git a/query-engine/query-engine/src/cli.rs b/query-engine/query-engine/src/cli.rs index 671a32c8c753..0dd455d4cfb7 100644 --- a/query-engine/query-engine/src/cli.rs +++ b/query-engine/query-engine/src/cli.rs @@ -4,10 +4,7 @@ use crate::{ opt::{CliOpt, PrismaOpt, Subcommand}, PrismaResult, }; -use query_core::{ - protocol::EngineProtocol, - schema::{self, QuerySchemaRef}, -}; +use query_core::{protocol::EngineProtocol, schema}; use request_handlers::{dmmf, RequestBody, RequestHandler}; use std::{env, sync::Arc}; @@ -93,8 +90,8 @@ impl CliCommand { async fn dmmf(request: DmmfRequest) -> PrismaResult<()> { let internal_data_model = prisma_models::convert(Arc::new(request.schema)); - let query_schema: QuerySchemaRef = Arc::new(schema::build(internal_data_model, request.enable_raw_queries)); - let dmmf = dmmf::render_dmmf(query_schema); + let query_schema = schema::build(internal_data_model, request.enable_raw_queries); + let dmmf = dmmf::render_dmmf(&query_schema); let serialized = serde_json::to_string_pretty(&dmmf)?; println!("{serialized}"); diff --git a/query-engine/query-engine/src/main.rs b/query-engine/query-engine/src/main.rs index d9724451143e..fa5822e146f4 100644 --- a/query-engine/query-engine/src/main.rs +++ b/query-engine/query-engine/src/main.rs @@ -1,4 +1,4 @@ -#![allow(clippy::wrong_self_convention, clippy::upper_case_acronyms, clippy::needless_borrow)] +#![allow(clippy::upper_case_acronyms, clippy::needless_borrow)] #[macro_use] extern crate tracing; diff --git a/query-engine/query-engine/src/server/mod.rs b/query-engine/query-engine/src/server/mod.rs index 053d67dfa21c..6557ee2e2bc5 100644 --- a/query-engine/query-engine/src/server/mod.rs +++ b/query-engine/query-engine/src/server/mod.rs @@ -72,7 +72,7 @@ pub async fn routes(cx: Arc, req: Request) -> Result { - let schema = dmmf::render_dmmf(Arc::clone(cx.query_schema())); + let schema = dmmf::render_dmmf(cx.query_schema()); Response::builder() .status(StatusCode::OK) diff --git a/query-engine/query-engine/src/tests/dmmf.rs b/query-engine/query-engine/src/tests/dmmf.rs index d74b94cee81d..e2f29bccae5a 100644 --- a/query-engine/query-engine/src/tests/dmmf.rs +++ b/query-engine/query-engine/src/tests/dmmf.rs @@ -29,7 +29,7 @@ fn must_not_fail_on_missing_env_vars_in_a_datasource() { } "#; let query_schema = get_query_schema(dm); - let dmmf = request_handlers::dmmf::render_dmmf(Arc::new(query_schema)); + let dmmf = request_handlers::dmmf::render_dmmf(&query_schema); let inputs = &dmmf.schema.input_object_types; assert!(!inputs.is_empty()); diff --git a/query-engine/request-handlers/src/dmmf/mod.rs b/query-engine/request-handlers/src/dmmf/mod.rs index 9402eae1749d..3b1927cf700d 100644 --- a/query-engine/request-handlers/src/dmmf/mod.rs +++ b/query-engine/request-handlers/src/dmmf/mod.rs @@ -1,6 +1,6 @@ use dmmf_crate::DataModelMetaFormat; -use query_core::schema::QuerySchemaRef; +use query_core::schema::QuerySchema; -pub fn render_dmmf(query_schema: QuerySchemaRef) -> DataModelMetaFormat { +pub fn render_dmmf(query_schema: &QuerySchema) -> DataModelMetaFormat { dmmf_crate::from_precomputed_parts(query_schema) } diff --git a/query-engine/request-handlers/src/protocols/graphql/schema_renderer/field_renderer.rs b/query-engine/request-handlers/src/protocols/graphql/schema_renderer/field_renderer.rs index 45049f8e315e..38cce46e3ecf 100644 --- a/query-engine/request-handlers/src/protocols/graphql/schema_renderer/field_renderer.rs +++ b/query-engine/request-handlers/src/protocols/graphql/schema_renderer/field_renderer.rs @@ -18,7 +18,7 @@ impl Renderer for GqlFieldRenderer<'_> { impl GqlFieldRenderer<'_> { fn render_input_field(&self, input_field: &InputField, ctx: &mut RenderContext) -> String { let rendered_type = pick_input_type(input_field.field_types(ctx.query_schema)) - .into_renderer() + .as_renderer() .render(ctx); let required = if input_field.is_required { "!" } else { "" }; @@ -45,7 +45,7 @@ impl GqlFieldRenderer<'_> { format!("({})", rendered_args.join(", ")) }; - let rendered_type = field.field_type.into_renderer().render(ctx); + let rendered_type = field.field_type.as_renderer().render(ctx); let bang = if !field.is_nullable { "!" } else { "" }; format!("{}{}: {}{}", field.name, rendered_args, rendered_type, bang) } @@ -62,7 +62,7 @@ impl GqlFieldRenderer<'_> { fn render_argument(&self, arg: &InputField, ctx: &mut RenderContext) -> String { let rendered_type = pick_input_type(arg.field_types(ctx.query_schema)) - .into_renderer() + .as_renderer() .render(ctx); let required = if arg.is_required { "!" } else { "" }; diff --git a/query-engine/request-handlers/src/protocols/graphql/schema_renderer/mod.rs b/query-engine/request-handlers/src/protocols/graphql/schema_renderer/mod.rs index 038a01d76933..60a9e2fce811 100644 --- a/query-engine/request-handlers/src/protocols/graphql/schema_renderer/mod.rs +++ b/query-engine/request-handlers/src/protocols/graphql/schema_renderer/mod.rs @@ -17,8 +17,8 @@ struct GqlSchemaRenderer { impl Renderer for GqlSchemaRenderer { fn render(&self, ctx: &mut RenderContext) -> String { - let _ = self.query_schema.query.into_renderer().render(ctx); - self.query_schema.mutation.into_renderer().render(ctx) + let _ = self.query_schema.query.as_renderer().render(ctx); + self.query_schema.mutation.as_renderer().render(ctx) } } @@ -30,7 +30,7 @@ impl GqlSchemaRenderer { pub fn render_graphql_schema(query_schema: QuerySchemaRef) -> String { let mut context = RenderContext::new(&query_schema); - query_schema.into_renderer().render(&mut context); + query_schema.as_renderer().render(&mut context); // Add custom scalar types (required for graphql.js implementations) format!( @@ -116,63 +116,54 @@ impl<'a> Renderer for GqlRenderer<'a> { } } -trait IntoRenderer<'a> { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&'a self) -> GqlRenderer<'a>; +trait AsRenderer<'a> { + fn as_renderer(&'a self) -> GqlRenderer<'a>; } -impl<'a> IntoRenderer<'a> for QuerySchemaRef { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&self) -> GqlRenderer<'a> { +impl<'a> AsRenderer<'a> for QuerySchemaRef { + fn as_renderer(&self) -> GqlRenderer<'a> { GqlRenderer::Schema(GqlSchemaRenderer::new(Arc::clone(self))) } } -impl<'a> IntoRenderer<'a> for &'a InputType { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&self) -> GqlRenderer<'a> { +impl<'a> AsRenderer<'a> for &'a InputType { + fn as_renderer(&self) -> GqlRenderer<'a> { GqlRenderer::Type(GqlTypeRenderer::Input(self)) } } -impl<'a> IntoRenderer<'a> for OutputType { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&'a self) -> GqlRenderer<'a> { +impl<'a> AsRenderer<'a> for OutputType { + fn as_renderer(&'a self) -> GqlRenderer<'a> { GqlRenderer::Type(GqlTypeRenderer::Output(self)) } } -impl<'a> IntoRenderer<'a> for InputField { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&'a self) -> GqlRenderer<'a> { +impl<'a> AsRenderer<'a> for InputField { + fn as_renderer(&'a self) -> GqlRenderer<'a> { GqlRenderer::Field(GqlFieldRenderer::Input(self)) } } -impl<'a> IntoRenderer<'a> for OutputField { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&'a self) -> GqlRenderer<'a> { +impl<'a> AsRenderer<'a> for OutputField { + fn as_renderer(&'a self) -> GqlRenderer<'a> { GqlRenderer::Field(GqlFieldRenderer::Output(self)) } } -impl<'a> IntoRenderer<'a> for EnumType { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&'a self) -> GqlRenderer<'a> { +impl<'a> AsRenderer<'a> for EnumType { + fn as_renderer(&'a self) -> GqlRenderer<'a> { GqlRenderer::Enum(GqlEnumRenderer::new(self)) } } -impl<'a> IntoRenderer<'a> for InputObjectTypeId { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&self) -> GqlRenderer<'a> { +impl<'a> AsRenderer<'a> for InputObjectTypeId { + fn as_renderer(&self) -> GqlRenderer<'a> { GqlRenderer::Object(GqlObjectRenderer::Input(*self)) } } -impl<'a> IntoRenderer<'a> for OutputObjectTypeId { - #[allow(clippy::wrong_self_convention)] - fn into_renderer(&self) -> GqlRenderer<'a> { +impl<'a> AsRenderer<'a> for OutputObjectTypeId { + fn as_renderer(&self) -> GqlRenderer<'a> { GqlRenderer::Object(GqlObjectRenderer::Output(*self)) } } diff --git a/query-engine/request-handlers/src/protocols/graphql/schema_renderer/object_renderer.rs b/query-engine/request-handlers/src/protocols/graphql/schema_renderer/object_renderer.rs index 3ea29342efbc..81bdc00f78f6 100644 --- a/query-engine/request-handlers/src/protocols/graphql/schema_renderer/object_renderer.rs +++ b/query-engine/request-handlers/src/protocols/graphql/schema_renderer/object_renderer.rs @@ -29,7 +29,7 @@ impl GqlObjectRenderer { let mut rendered_fields = Vec::with_capacity(fields.len()); for field in fields { - rendered_fields.push(field.into_renderer().render(ctx)) + rendered_fields.push(field.as_renderer().render(ctx)) } let indented: Vec = rendered_fields @@ -60,7 +60,7 @@ impl GqlObjectRenderer { let mut rendered_fields = Vec::with_capacity(fields.len()); for field in fields { - rendered_fields.push(field.into_renderer().render(ctx)) + rendered_fields.push(field.as_renderer().render(ctx)) } let indented: Vec = rendered_fields diff --git a/query-engine/request-handlers/src/protocols/graphql/schema_renderer/type_renderer.rs b/query-engine/request-handlers/src/protocols/graphql/schema_renderer/type_renderer.rs index 80e2b3af8a0d..646cd4cc4433 100644 --- a/query-engine/request-handlers/src/protocols/graphql/schema_renderer/type_renderer.rs +++ b/query-engine/request-handlers/src/protocols/graphql/schema_renderer/type_renderer.rs @@ -20,13 +20,13 @@ impl<'a> GqlTypeRenderer<'a> { fn render_input_type(&self, i: &InputType, ctx: &mut RenderContext) -> String { match i { InputType::Object(ref obj) => { - let _ = obj.into_renderer().render(ctx); + let _ = obj.as_renderer().render(ctx); ctx.query_schema.db[*obj].identifier.name() } InputType::Enum(et) => { let et = &ctx.query_schema.db[*et]; - et.into_renderer().render(ctx); + et.as_renderer().render(ctx); et.identifier().name() } @@ -60,13 +60,13 @@ impl<'a> GqlTypeRenderer<'a> { fn render_output_type(&self, o: &OutputType, ctx: &mut RenderContext) -> String { match o { OutputType::Object(obj) => { - let _ = obj.into_renderer().render(ctx); + let _ = obj.as_renderer().render(ctx); ctx.query_schema.db[*obj].identifier.name() } OutputType::Enum(et) => { let et = &ctx.query_schema.db[*et]; - et.into_renderer().render(ctx); + et.as_renderer().render(ctx); et.identifier().name() }