From 32394d928e9ccb9b6ce1d6b4b15071f3a280870d Mon Sep 17 00:00:00 2001 From: Natoandro Date: Thu, 7 Dec 2023 22:08:58 +0300 Subject: [PATCH] fix: Fix default argument computation for objects (#510) ### Describe your change Fix the default argument computation. Make non-optional objects optional if all of its fields are optional. ### Motivation and context [MET-295](https://metatype.atlassian.net/browse/MET-295) ### Migration notes *N/A* ### Checklist - [x] The change come with new or modified tests - [x] Hard-to-understand functions have explanatory comments - [x] ~End-user documentation is updated to reflect the change~: *N/A* --- meta-cli/src/typegraph/loader/mod.rs | 6 +- typegate/src/engine/planner/args.ts | 27 ++-- typegate/tests/e2e/cli/deploy1_test.ts | 130 ++++++++++++++++++ typegate/tests/planner/default_args_test.ts | 37 +++++ .../runtimes/prisma/full_prisma_mapping.py | 11 ++ .../prisma/full_prisma_mapping_test.ts | 6 +- 6 files changed, 201 insertions(+), 16 deletions(-) create mode 100644 typegate/tests/e2e/cli/deploy1_test.ts create mode 100644 typegate/tests/planner/default_args_test.ts diff --git a/meta-cli/src/typegraph/loader/mod.rs b/meta-cli/src/typegraph/loader/mod.rs index 525ae6dd5d..bb8df9f9e4 100644 --- a/meta-cli/src/typegraph/loader/mod.rs +++ b/meta-cli/src/typegraph/loader/mod.rs @@ -141,12 +141,12 @@ impl Loader { fn get_load_command(module_type: ModuleType, path: &Path) -> Result { match module_type { ModuleType::Python => { - // TODO cache result? ensure_venv(path).map_err(|e| LoaderError::PythonVenvNotFound { path: path.to_owned().into(), error: e, })?; let vars: HashMap<_, _> = env::vars().collect(); + // TODO cache result? let mut command = Command::new("python3"); command .arg(path.to_str().unwrap()) @@ -231,9 +231,7 @@ impl ToString for LoaderError { format!("module file not found: {path:?}") } Self::PythonVenvNotFound { path, error } => { - format!( - "python venv (.venv) not found in parent directories of {path:?}: {error:?}" - ) + format!("python venv (.venv) not found in parent directories of {path:?}: {error}",) } } } diff --git a/typegate/src/engine/planner/args.ts b/typegate/src/engine/planner/args.ts index 25ac541710..86ea7f8669 100644 --- a/typegate/src/engine/planner/args.ts +++ b/typegate/src/engine/planner/args.ts @@ -571,13 +571,21 @@ class ArgumentCollector { // no injection for the effect // fallthrough } - if (typ.type != Type.OPTIONAL) { - throw new MandatoryArgumentError(this.currentNodeDetails); - } else { - this.addPoliciesFrom(typ.item); + + switch (typ.type) { + case Type.OPTIONAL: { + this.addPoliciesFrom(typ.item); + const { default_value: defaultValue = null } = typ; + return () => defaultValue; + } + + case Type.OBJECT: { + return this.collectDefaults(typ.properties); + } + + default: + throw new MandatoryArgumentError(this.currentNodeDetails); } - const { default_value: defaultValue } = typ; - return () => defaultValue; } /** Collect the value of an injected parameter. */ @@ -673,11 +681,8 @@ class ArgumentCollector { return typ.default_value; } - const suggestions = `available fields from parent are: ${ - Object.keys( - parent, - ).join(", ") - }`; + const keys = Object.keys(parent).join(", "); + const suggestions = `available fields from parent are: ${keys}`; throw new Error( `non-optional injected argument ${name} is missing from parent: ${suggestions}`, ); diff --git a/typegate/tests/e2e/cli/deploy1_test.ts b/typegate/tests/e2e/cli/deploy1_test.ts new file mode 100644 index 0000000000..1ca72057ed --- /dev/null +++ b/typegate/tests/e2e/cli/deploy1_test.ts @@ -0,0 +1,130 @@ +// Copyright Metatype OÜ, licensed under the Elastic License 2.0. +// SPDX-License-Identifier: Elastic-2.0 + +import { gql, Meta } from "test-utils/mod.ts"; +import { TestModule } from "test-utils/test_module.ts"; +import { removeMigrations } from "test-utils/migrations.ts"; +import { assertStringIncludes } from "std/assert/mod.ts"; +import pg from "npm:pg"; + +const m = new TestModule(import.meta); + +const tgName = "migration-failure-test"; + +/** + * These tests use different ports for the virtual typegate instance to avoid + * conflicts with one another when running in parallel. + */ + +async function writeTypegraph(version: number | null) { + if (version == null) { + await m.shell([ + "bash", + "-c", + "cp ./templates/migration.py migration.py", + ]); + } else { + await m.shell([ + "bash", + "select.sh", + "templates/migration.py", + `${version}`, + "migration.py", + ]); + } +} + +async function deploy(port: number | null, noMigration = false) { + const migrationOpts = noMigration ? [] : ["--create-migration"]; + + try { + const out = await m.cli( + {}, + "deploy", + "--target", + port == null ? "dev" : `dev${port}`, + "-f", + "migration.py", + "--allow-dirty", + ...migrationOpts, + "--allow-destructive", + ); + if (out.stdout.length > 0) { + console.log( + `-- deploy STDOUT start --\n${out.stdout}-- deploy STDOUT end --`, + ); + } + if (out.stderr.length > 0) { + console.log( + `-- deploy STDERR start --\n${out.stderr}-- deploy STDERR end --`, + ); + } + } catch (e) { + console.log(e.toString()); + throw e; + } +} + +async function reset(schema: string) { + await removeMigrations(tgName); + + // remove the database schema + const client = new pg.Client({ + connectionString: "postgres://postgres:password@localhost:5432/db", + }); + await client.connect(); + await client.query(`DROP SCHEMA IF EXISTS ${schema} CASCADE`); + await client.end(); +} + +Meta.test( + "meta deploy: fails migration for new columns without default value", + async (t) => { + await t.should("load first version of the typegraph", async () => { + await reset("e2e7895alt"); + await writeTypegraph(null); + }); + + const port = 7895; + + // `deploy` must be run outside of the `should` block, + // otherwise this would fail by leaking ops. + // That is expected since it creates new engine that persists beyond the + // `should` block. + await deploy(port); + + await t.should("insert records", async () => { + const e = t.getTypegraphEngine(tgName); + if (!e) { + throw new Error("typegraph not found"); + } + await gql` + mutation { + createRecord(data: {}) { + id + } + } + ` + .expectData({ + createRecord: { + id: 1, + }, + }) + .on(e); + }); + + await t.should("load second version of the typegraph", async () => { + await writeTypegraph(1); + }); + + try { + await deploy(port); + } catch (e) { + assertStringIncludes( + e.message, + 'column "age" of relation "Record" contains null values: set a default value:', + ); + } + }, + { port: 7895, systemTypegraphs: true }, +); diff --git a/typegate/tests/planner/default_args_test.ts b/typegate/tests/planner/default_args_test.ts new file mode 100644 index 0000000000..a8857421f8 --- /dev/null +++ b/typegate/tests/planner/default_args_test.ts @@ -0,0 +1,37 @@ +// Copyright Metatype OÜ, licensed under the Elastic License 2.0. +// SPDX-License-Identifier: Elastic-2.0 + +import { dropSchemas, recreateMigrations } from "test-utils/migrations.ts"; +import { gql, Meta } from "test-utils/mod.ts"; + +Meta.test("prisma", async (t) => { + const e = await t.engine("runtimes/prisma/full_prisma_mapping.py", { + secrets: { + POSTGRES: + "postgresql://postgres:password@localhost:5432/db?schema=prisma", + }, + }); + + await dropSchemas(e); + await recreateMigrations(e); + + await t.should("fail", async () => { + await gql` + query CommentedAuthors { + findCommentersOfCurrentUser { + id + name + age + city + } + } + ` + .withContext({ + "user_id": 12, + }) + .expectData({ + findCommentersOfCurrentUser: [], + }) + .on(e); + }); +}); diff --git a/typegate/tests/runtimes/prisma/full_prisma_mapping.py b/typegate/tests/runtimes/prisma/full_prisma_mapping.py index 68f43febeb..a3b52b3c72 100644 --- a/typegate/tests/runtimes/prisma/full_prisma_mapping.py +++ b/typegate/tests/runtimes/prisma/full_prisma_mapping.py @@ -20,6 +20,7 @@ def prisma(g: Graph): "city": t.string(), "posts": t.list(g.ref("Post")), "extended_profile": g.ref("ExtendedProfile").optional(), + "comments": t.list(g.ref("Comment")), }, name="User", ) @@ -42,6 +43,7 @@ def prisma(g: Graph): "id": t.integer(as_id=True), "content": t.string(), "related_post": g.ref("Post"), + "author": g.ref("User"), }, name="Comment", ) @@ -65,6 +67,15 @@ def prisma(g: Graph): updateUser=db.update(user), upsertOneUser=db.upsert(user), findManyPosts=db.find_many(post), + findCommentersOfCurrentUser=db.find_many(user).reduce( + { + "where": { + "comments": { + "some": {"author": {"id": g.inherit().from_context("user_id")}} + } + } + } + ), findFirstPost=db.find_first(post), findFirstComment=db.find_first(comment), findUniquePost=db.find_unique(post), diff --git a/typegate/tests/runtimes/prisma/full_prisma_mapping_test.ts b/typegate/tests/runtimes/prisma/full_prisma_mapping_test.ts index 5c7118b24b..7081fecb84 100644 --- a/typegate/tests/runtimes/prisma/full_prisma_mapping_test.ts +++ b/typegate/tests/runtimes/prisma/full_prisma_mapping_test.ts @@ -678,7 +678,11 @@ Meta.test("prisma full mapping", async (t) => { await gql` mutation { createOneComment( - data: { id: 50001, content: "good", related_post: {connect: {id: 10001}} } + data: { + id: 50001, + content: "good", + related_post: { connect: { id: 10001 }}, + author: { connect: { id: 1 } } } ) { id content