From 46a2d291b654b289a5b1a5206f607130a8313ca8 Mon Sep 17 00:00:00 2001 From: Ilya Semenov Date: Sat, 16 Apr 2022 16:08:48 +0700 Subject: [PATCH] fix: select fields with ref() Prevent naming clash on fields in joins. Automatically select pagination key fields. Fix https://github.com/IlyaSemenov/objection-graphql-resolver/issues/7 --- src/paginators/cursor.ts | 10 ++-- src/resolver/field.ts | 6 +- src/resolver/model.ts | 19 +++--- src/shim.d.ts | 8 +++ tests/auto-select-pagination-keys.ts | 83 ++++++++++++++++++++++++++ tests/issue-1.ts | 2 + tests/issue-7.ts | 4 +- tests/select-only-virtual-attribute.ts | 63 +++++++++++++++++++ 8 files changed, 180 insertions(+), 15 deletions(-) create mode 100644 src/shim.d.ts create mode 100644 tests/auto-select-pagination-keys.ts create mode 100644 tests/select-only-virtual-attribute.ts diff --git a/src/paginators/cursor.ts b/src/paginators/cursor.ts index b421ba3..b05f024 100644 --- a/src/paginators/cursor.ts +++ b/src/paginators/cursor.ts @@ -1,4 +1,4 @@ -import { Model, QueryBuilder, raw } from "objection" +import { Model, QueryBuilder, raw, ref } from "objection" import { PaginatorFn } from "." @@ -84,9 +84,11 @@ export function CursorPaginator( // Set query order query.clearOrder() - fields.forEach((field) => - query.orderBy(field.name, field.desc ? "desc" : "asc") - ) + fields.forEach((field) => { + const f = ref(field.name).from(query.tableRef()) + // TODO: prevent potential name clash with aliases like .as(`_${table_ref}_order_key_0`) + query.select(f).orderBy(f, field.desc ? "desc" : "asc") + }) if (cursor) { set_query_cursor(query, cursor) diff --git a/src/resolver/field.ts b/src/resolver/field.ts index be8dcbf..39bbd2c 100644 --- a/src/resolver/field.ts +++ b/src/resolver/field.ts @@ -22,7 +22,11 @@ export function FieldResolver( if (select) { select(query, resolve_opts) } else { - query.select(model_field ? ref(model_field).as(field) : field) + query.select( + ref(model_field || field) + .from(query.tableRef()) + .as(field) + ) } if (clean) { const context = query.context() diff --git a/src/resolver/model.ts b/src/resolver/model.ts index 6a90411..b76002c 100644 --- a/src/resolver/model.ts +++ b/src/resolver/model.ts @@ -4,6 +4,7 @@ import { ModelClass, ModelConstructor, QueryBuilder, + ref, RelationMappings, } from "objection" @@ -127,16 +128,16 @@ export function ModelResolver( r(query, { field, tree: subtree, resolve_tree }) } - if ( - !query.has( - ((op: any) => - op.name === "select" && op.args[0] === ThisModel.idColumn) as any + // Performance: select ID if nothing was selected. + // This avoid automatic "select *" generated by objection. + if (!query.has((QueryBuilder as any).SelectSelector)) { + query.select( + ref( + typeof ThisModel.idColumn === "string" + ? ThisModel.idColumn + : ThisModel.idColumn[0] + ).from(query.tableRef()) ) - ) { - // Always select ID: - // 1. This is useful for potential $query() - // 2. This avoid automatic "select *" when not a single normal field was selected - query.select(ThisModel.idColumn) } const clean_instance = model_options.clean diff --git a/src/shim.d.ts b/src/shim.d.ts new file mode 100644 index 0000000..5d3552b --- /dev/null +++ b/src/shim.d.ts @@ -0,0 +1,8 @@ +import "objection" + +declare module "objection" { + interface QueryBuilder { + // Not sure why it's missing in official typings + tableRef(): string + } +} diff --git a/tests/auto-select-pagination-keys.ts b/tests/auto-select-pagination-keys.ts new file mode 100644 index 0000000..b662ab6 --- /dev/null +++ b/tests/auto-select-pagination-keys.ts @@ -0,0 +1,83 @@ +import gql from "graphql-tag" +import { Model } from "objection" +import { + CursorPaginator, + GraphResolver, + ModelResolver, +} from "objection-graphql-resolver" +import tap from "tap" + +import { Resolvers, setup } from "./setup" + +class BookModel extends Model { + static tableName = "book" + + id?: number + title?: string + author?: string +} + +const schema = gql` + type Book { + id: Int! + title: String! + author: String! + } + + type BookPage { + nodes: [Book!]! + cursor: String + } + + type Query { + library: BookPage! + } +` + +const resolve_graph = GraphResolver({ + Book: ModelResolver(BookModel), +}) + +const resolvers: Resolvers = { + Query: { + library: (_parent, _args, ctx, info) => + resolve_graph(ctx, info, BookModel.query(), { + paginate: CursorPaginator({ take: 1, fields: ["author", "id"] }), + }), + }, +} + +tap.test("auto select pagination key", async (tap) => { + const { client, knex } = await setup(tap, { typeDefs: schema, resolvers }) + + await knex.schema.createTable("book", (book) => { + book.increments("id").notNullable().primary() + book.string("title").notNullable() + book.string("author").notNullable() + }) + + await BookModel.query().insertGraph([ + { title: "1984", author: "George Orwell" }, + { title: "Tom Sawyer", author: "Mark Twain" }, + ]) + + tap.strictSame( + await client.request( + gql` + { + library { + nodes { + title + } + } + } + ` + ), + { + library: { + nodes: [{ title: "1984" }], + }, + }, + "pagination works without selecting author explicitly" + ) +}) diff --git a/tests/issue-1.ts b/tests/issue-1.ts index ea6cf56..d265561 100644 --- a/tests/issue-1.ts +++ b/tests/issue-1.ts @@ -1,3 +1,5 @@ +// Regression test for https://github.com/IlyaSemenov/objection-graphql-resolver/issues/1 + import gql from "graphql-tag" import { Model } from "objection" import { GraphResolver, ModelResolver } from "objection-graphql-resolver" diff --git a/tests/issue-7.ts b/tests/issue-7.ts index 90cc7e5..18c7f8d 100644 --- a/tests/issue-7.ts +++ b/tests/issue-7.ts @@ -1,3 +1,5 @@ +// Regression test for https://github.com/IlyaSemenov/objection-graphql-resolver/issues/7 + import gql from "graphql-tag" import { Model } from "objection" import { GraphResolver, ModelResolver } from "objection-graphql-resolver" @@ -85,7 +87,7 @@ const resolvers: Resolvers = { }, } -tap.todo("m2m with extra columns in relation table", async (tap) => { +tap.test("m2m: naming clash with column in relation table", async (tap) => { const { client, knex } = await setup(tap, { typeDefs: schema, resolvers }) await knex.schema.createTable("author", (author) => { diff --git a/tests/select-only-virtual-attribute.ts b/tests/select-only-virtual-attribute.ts new file mode 100644 index 0000000..51faf8f --- /dev/null +++ b/tests/select-only-virtual-attribute.ts @@ -0,0 +1,63 @@ +import gql from "graphql-tag" +import { Model } from "objection" +import { GraphResolver, ModelResolver } from "objection-graphql-resolver" +import tap from "tap" + +import { Resolvers, setup } from "./setup" + +class BookModel extends Model { + static tableName = "book" + + id?: number + + get foo() { + return "whatever" + } +} + +const schema = gql` + type Book { + id: Int! + foo: String! + } + + type Query { + library: [Book!]! + } +` + +const resolve_graph = GraphResolver({ + Book: ModelResolver(BookModel), +}) + +const resolvers: Resolvers = { + Query: { + library: (_parent, _args, ctx, info) => + resolve_graph(ctx, info, BookModel.query()), + }, +} + +tap.test("select virtual attribute only", async (tap) => { + const { client, knex } = await setup(tap, { typeDefs: schema, resolvers }) + + await knex.schema.createTable("book", (book) => { + book.integer("id").notNullable().primary() + }) + + await BookModel.query().insert({ id: 1 }) + + tap.strictSame( + await client.request( + gql` + { + library { + foo + } + } + ` + ), + { + library: [{ foo: "whatever" }], + } + ) +})