From af51715637433bcdd2538835c98ac71a8eb86122 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Thu, 23 Jul 2020 16:36:09 +1000 Subject: [PATCH] Move mongoose specific code into mongo-join-builder (#3283) --- .changeset/bright-badgers-relate.md | 7 + .../tests/list-adapter.test.js | 2 - .../src/types/Relationship/Implementation.js | 11 -- packages/keystone/lib/adapters/index.js | 6 - packages/mongo-join-builder/lib/tokenizers.js | 10 +- .../mongo-join-builder/tests/index.test.js | 1 + .../tests/relationship-path.test.js | 133 ++++++++++-------- .../tests/relationship.test.js | 9 +- packages/mongo-join-builder/tests/utils.js | 25 ++-- 9 files changed, 103 insertions(+), 101 deletions(-) create mode 100644 .changeset/bright-badgers-relate.md diff --git a/.changeset/bright-badgers-relate.md b/.changeset/bright-badgers-relate.md new file mode 100644 index 00000000000..22a270d7d1f --- /dev/null +++ b/.changeset/bright-badgers-relate.md @@ -0,0 +1,7 @@ +--- +'@keystonejs/fields': major +'@keystonejs/keystone': major +'@keystonejs/mongo-join-builder': patch +--- + +Removed `BaseListAdapter.findFieldAdapterForQuerySegment()` and `MongoRelationshipInterface.supportsRelationshipQuery()`. diff --git a/packages/adapter-mongoose/tests/list-adapter.test.js b/packages/adapter-mongoose/tests/list-adapter.test.js index f75a961d0fc..56194b453e9 100644 --- a/packages/adapter-mongoose/tests/list-adapter.test.js +++ b/packages/adapter-mongoose/tests/list-adapter.test.js @@ -54,7 +54,6 @@ describe('MongooseListAdapter', () => { { isRelationship: true, getQueryConditions: () => {}, - supportsRelationshipQuery: query => query === 'posts_some', getRefListAdapter: () => postListAdapter, field: { config: { many: true }, many: true }, path: 'posts', @@ -131,7 +130,6 @@ describe('MongooseListAdapter', () => { { isRelationship: true, getQueryConditions: () => {}, - supportsRelationshipQuery: query => query === 'posts_some', getRefListAdapter: () => postListAdapter, field: { config: { many: true }, many: true }, path: 'posts', diff --git a/packages/fields/src/types/Relationship/Implementation.js b/packages/fields/src/types/Relationship/Implementation.js index 2ebd7cc482f..1cb75d583d0 100644 --- a/packages/fields/src/types/Relationship/Implementation.js +++ b/packages/fields/src/types/Relationship/Implementation.js @@ -344,12 +344,6 @@ export class MongoRelationshipInterface extends MongooseFieldAdapter { }), }; } - - supportsRelationshipQuery(query) { - return [this.path, `${this.path}_every`, `${this.path}_some`, `${this.path}_none`].includes( - query - ); - } } export class KnexRelationshipInterface extends KnexFieldAdapter { @@ -416,9 +410,4 @@ export class KnexRelationshipInterface extends KnexFieldAdapter { value ? b.whereNull(dbPath) : b.whereNotNull(dbPath), }; } - supportsRelationshipQuery(query) { - return [this.path, `${this.path}_every`, `${this.path}_some`, `${this.path}_none`].includes( - query - ); - } } diff --git a/packages/keystone/lib/adapters/index.js b/packages/keystone/lib/adapters/index.js index 23e12dda6bc..1baef3af09c 100644 --- a/packages/keystone/lib/adapters/index.js +++ b/packages/keystone/lib/adapters/index.js @@ -144,12 +144,6 @@ class BaseListAdapter { return this.itemsQuery(args, { meta: true }); } - findFieldAdapterForQuerySegment(segment) { - return this.fieldAdapters - .filter(adapter => adapter.isRelationship) - .find(adapter => adapter.supportsRelationshipQuery(segment)); - } - getFieldAdapterByPath(path) { return this.fieldAdaptersByPath[path]; } diff --git a/packages/mongo-join-builder/lib/tokenizers.js b/packages/mongo-join-builder/lib/tokenizers.js index 227c7541159..a7e7028d0b4 100644 --- a/packages/mongo-join-builder/lib/tokenizers.js +++ b/packages/mongo-join-builder/lib/tokenizers.js @@ -24,7 +24,11 @@ const getRelatedListAdapterFromQueryPath = (listAdapter, queryPath) => { } // Eg; search for which field adapter handles `posts_some`, and return that one - const fieldAdapter = foundListAdapter.findFieldAdapterForQuerySegment(segment); + const fieldAdapter = foundListAdapter.fieldAdapters + .filter(adapter => adapter.isRelationship) + .find(({ path }) => + [path, `${path}_every`, `${path}_some`, `${path}_none`].includes(segment) + ); if (!fieldAdapter) { // prettier-ignore @@ -54,7 +58,9 @@ const getRelatedListAdapterFromQueryPath = (listAdapter, queryPath) => { const relationshipTokenizer = (listAdapter, queryKey, path, getUID = cuid) => { const refListAdapter = getRelatedListAdapterFromQueryPath(listAdapter, path); - const fieldAdapter = refListAdapter.findFieldAdapterForQuerySegment(queryKey); + const fieldAdapter = refListAdapter.fieldAdapters + .filter(adapter => adapter.isRelationship) + .find(({ path }) => [path, `${path}_every`, `${path}_some`, `${path}_none`].includes(queryKey)); // Nothing found, return an empty operation // TODO: warn? diff --git a/packages/mongo-join-builder/tests/index.test.js b/packages/mongo-join-builder/tests/index.test.js index 70fca2de341..38ed0863d46 100644 --- a/packages/mongo-join-builder/tests/index.test.js +++ b/packages/mongo-join-builder/tests/index.test.js @@ -43,6 +43,7 @@ describe('Test main export', () => { { $match: { $expr: { $eq: [`$_id`, '$$tmpVar'] } } }, { $match: { name: { $eq: 'foo' } } }, { $addFields: { id: '$_id' } }, + { $project: { posts: 0 } }, ], }, }, diff --git a/packages/mongo-join-builder/tests/relationship-path.test.js b/packages/mongo-join-builder/tests/relationship-path.test.js index 28d965589ab..038d9fe9b62 100644 --- a/packages/mongo-join-builder/tests/relationship-path.test.js +++ b/packages/mongo-join-builder/tests/relationship-path.test.js @@ -3,18 +3,14 @@ const { getRelatedListAdapterFromQueryPath } = require('../lib/tokenizers'); describe('Relationship Path parser', () => { describe('paths', () => { test('Handles simple paths correctly', () => { - let fooListAdapter; - let barListAdapter; - let zipListAdapter; + const fieldAdapters = [ + { path: 'bar', isRelationship: true, getRefListAdapter: jest.fn(() => barListAdapter) }, + { path: 'zip', isRelationship: true, getRefListAdapter: jest.fn(() => zipListAdapter) }, + ]; - const fieldAdapters = { - bar: { getRefListAdapter: jest.fn(() => barListAdapter) }, - zip: { getRefListAdapter: jest.fn(() => zipListAdapter) }, - }; - - fooListAdapter = { findFieldAdapterForQuerySegment: jest.fn(key => fieldAdapters[key]) }; - barListAdapter = { findFieldAdapterForQuerySegment: jest.fn(key => fieldAdapters[key]) }; - zipListAdapter = {}; + const fooListAdapter = { fieldAdapters }; + const barListAdapter = { fieldAdapters }; + const zipListAdapter = { fieldAdapters: [] }; expect(getRelatedListAdapterFromQueryPath(fooListAdapter, ['bar', 'zip', 'ignore'])).toEqual( zipListAdapter @@ -22,9 +18,12 @@ describe('Relationship Path parser', () => { }); test('Handles circular paths correctly', () => { - let listAdapter; - const fieldAdapter = { getRefListAdapter: jest.fn(() => listAdapter) }; - listAdapter = { findFieldAdapterForQuerySegment: jest.fn(() => fieldAdapter) }; + const fieldAdapter = { + path: 'foo', + isRelationship: true, + getRefListAdapter: jest.fn(() => listAdapter), + }; + const listAdapter = { fieldAdapters: [fieldAdapter] }; expect( getRelatedListAdapterFromQueryPath(listAdapter, ['foo', 'foo', 'foo', 'ignore']) @@ -32,18 +31,22 @@ describe('Relationship Path parser', () => { }); test('Handles arbitrary path strings correctly', () => { - let fooListAdapter; - let barListAdapter; - let zipListAdapter; - - const fieldAdapters = { - bar_koodle: { getRefListAdapter: jest.fn(() => barListAdapter) }, - 'zip-boom_zap': { getRefListAdapter: jest.fn(() => zipListAdapter) }, - }; - - fooListAdapter = { findFieldAdapterForQuerySegment: jest.fn(key => fieldAdapters[key]) }; - barListAdapter = { findFieldAdapterForQuerySegment: jest.fn(key => fieldAdapters[key]) }; - zipListAdapter = {}; + const fieldAdapters = [ + { + path: 'bar_koodle', + isRelationship: true, + getRefListAdapter: jest.fn(() => barListAdapter), + }, + { + path: 'zip-boom_zap', + isRelationship: true, + getRefListAdapter: jest.fn(() => zipListAdapter), + }, + ]; + + const fooListAdapter = { fieldAdapters }; + const barListAdapter = { fieldAdapters }; + const zipListAdapter = { fieldAdapters: [] }; expect( getRelatedListAdapterFromQueryPath(fooListAdapter, ['bar_koodle', 'zip-boom_zap', 'ignore']) @@ -51,18 +54,22 @@ describe('Relationship Path parser', () => { }); test('Handles paths with AND correctly', () => { - let fooListAdapter; - let barListAdapter; - let zipListAdapter; - - const fieldAdapters = { - bar_koodle: { getRefListAdapter: jest.fn(() => barListAdapter) }, - 'zip-boom_zap': { getRefListAdapter: jest.fn(() => zipListAdapter) }, - }; - - fooListAdapter = { findFieldAdapterForQuerySegment: jest.fn(key => fieldAdapters[key]) }; - barListAdapter = { findFieldAdapterForQuerySegment: jest.fn(key => fieldAdapters[key]) }; - zipListAdapter = {}; + const fieldAdapters = [ + { + path: 'bar_koodle', + isRelationship: true, + getRefListAdapter: jest.fn(() => barListAdapter), + }, + { + path: 'zip-boom_zap', + isRelationship: true, + getRefListAdapter: jest.fn(() => zipListAdapter), + }, + ]; + + const fooListAdapter = { fieldAdapters }; + const barListAdapter = { fieldAdapters }; + const zipListAdapter = { fieldAdapters: [] }; expect( getRelatedListAdapterFromQueryPath(fooListAdapter, [ @@ -76,18 +83,22 @@ describe('Relationship Path parser', () => { }); test('Handles paths with OR correctly', () => { - let fooListAdapter; - let barListAdapter; - let zipListAdapter; - - const fieldAdapters = { - bar_koodle: { getRefListAdapter: jest.fn(() => barListAdapter) }, - 'zip-boom_zap': { getRefListAdapter: jest.fn(() => zipListAdapter) }, - }; - - fooListAdapter = { findFieldAdapterForQuerySegment: jest.fn(key => fieldAdapters[key]) }; - barListAdapter = { findFieldAdapterForQuerySegment: jest.fn(key => fieldAdapters[key]) }; - zipListAdapter = {}; + const fieldAdapters = [ + { + path: 'bar_koodle', + isRelationship: true, + getRefListAdapter: jest.fn(() => barListAdapter), + }, + { + path: 'zip-boom_zap', + isRelationship: true, + getRefListAdapter: jest.fn(() => zipListAdapter), + }, + ]; + + const fooListAdapter = { fieldAdapters }; + const barListAdapter = { fieldAdapters }; + const zipListAdapter = { fieldAdapters: [] }; expect( getRelatedListAdapterFromQueryPath(fooListAdapter, [ @@ -103,19 +114,17 @@ describe('Relationship Path parser', () => { describe('errors', () => { test('Throws error when field not found', () => { - let fooListAdapter; - let zipListAdapter; - - const fieldAdapters = { + const fieldAdapters = [ // NOTE: bar_koodle is missing to make the test fail - 'zip-boom_zap': { getRefListAdapter: jest.fn(() => zipListAdapter) }, - }; - - fooListAdapter = { - key: 'foo', - findFieldAdapterForQuerySegment: jest.fn(key => fieldAdapters[key]), - }; - zipListAdapter = {}; + { + path: 'zip-boom_zap', + isRelationship: true, + getRefListAdapter: jest.fn(() => zipListAdapter), + }, + ]; + + const fooListAdapter = { key: 'foo', fieldAdapters }; + const zipListAdapter = { fieldAdapters: [] }; expect(() => getRelatedListAdapterFromQueryPath(fooListAdapter, ['bar_koodle', 'zip-boom_zap', 'ignore']) diff --git a/packages/mongo-join-builder/tests/relationship.test.js b/packages/mongo-join-builder/tests/relationship.test.js index a29dceb25a4..b8ba00e7b5f 100644 --- a/packages/mongo-join-builder/tests/relationship.test.js +++ b/packages/mongo-join-builder/tests/relationship.test.js @@ -3,13 +3,13 @@ const { relationshipTokenizer } = require('../lib/tokenizers'); describe('Relationship tokenizer', () => { test('Uses correct conditions', () => { const relationshipConditions = { + isRelationship: true, getRefListAdapter: () => ({ key: 'Bar', model: { collection: { name: 'name' } } }), field: { many: false }, path: 'name', rel: {}, }; - const findFieldAdapterForQuerySegment = jest.fn(() => relationshipConditions); - const listAdapter = { findFieldAdapterForQuerySegment, key: 'Foo' }; + const listAdapter = { key: 'Foo', fieldAdapters: [relationshipConditions] }; expect(relationshipTokenizer(listAdapter, 'name', ['name'], () => 'abc123')).toMatchObject({ matchTerm: { $expr: { $eq: [{ $size: '$abc123_name' }, 1] } }, @@ -22,15 +22,12 @@ describe('Relationship tokenizer', () => { farCollection: 'name', }, }); - expect(findFieldAdapterForQuerySegment).toHaveBeenCalledTimes(1); }); test('returns empty array when no matches found', () => { - const findFieldAdapterForQuerySegment = jest.fn(() => {}); - const listAdapter = { findFieldAdapterForQuerySegment }; + const listAdapter = { fieldAdapters: [] }; const result = relationshipTokenizer(listAdapter, 'name', ['name'], () => {}); expect(result).toMatchObject({}); - expect(findFieldAdapterForQuerySegment).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/mongo-join-builder/tests/utils.js b/packages/mongo-join-builder/tests/utils.js index f53d709e177..c8f337152e3 100644 --- a/packages/mongo-join-builder/tests/utils.js +++ b/packages/mongo-join-builder/tests/utils.js @@ -1,6 +1,3 @@ -const findFieldAdapterForQuerySegment = ({ fieldAdapters }) => segment => - fieldAdapters.find(({ path }) => path === segment || path === segment.split('_')[0]); - const tagsAdapter = { key: 'Tag', model: { collection: { name: 'tags' } }, @@ -29,7 +26,9 @@ const postsAdapter = { }, { path: 'tags', - field: { many: true }, + dbPath: 'tags', + isRelationship: true, + field: { many: true, config: { many: true } }, rel: { cardinality: 'N:N', columnNames: { @@ -71,7 +70,8 @@ const listAdapter = { }, { path: 'company', - field: { many: false }, + isRelationship: true, + field: { many: false, config: { many: false } }, rel: { columnNames: { User: {}, Company: {} } }, getQueryConditions: () => {}, getRefListAdapter: () => ({ @@ -90,7 +90,9 @@ const listAdapter = { listAdapter.fieldAdapters.push({ getQueryConditions: () => {}, path: 'posts', - field: { many: true }, + dbPath: 'posts', + isRelationship: true, + field: { many: true, config: { many: true } }, rel: { cardinality: '1:N', columnNames: { Tag: {}, Post: {} }, @@ -102,7 +104,9 @@ listAdapter.fieldAdapters.push({ tagsAdapter.fieldAdapters.push({ path: 'posts', - field: { many: true }, + dbPath: 'posts', + isRelationship: true, + field: { many: true, config: { many: true } }, rel: { cardinality: 'N:N', columnNames: { @@ -118,7 +122,8 @@ tagsAdapter.fieldAdapters.push({ postsAdapter.fieldAdapters.push({ getQueryConditions: () => {}, path: 'author', - field: { many: false }, + isRelationship: true, + field: { many: false, config: { many: false } }, rel: { cardinality: '1:N', columnNames: { Tag: {}, Post: {} }, @@ -128,8 +133,4 @@ postsAdapter.fieldAdapters.push({ getRefListAdapter: () => listAdapter, }); -postsAdapter.findFieldAdapterForQuerySegment = findFieldAdapterForQuerySegment(postsAdapter); -tagsAdapter.findFieldAdapterForQuerySegment = findFieldAdapterForQuerySegment(tagsAdapter); -listAdapter.findFieldAdapterForQuerySegment = findFieldAdapterForQuerySegment(listAdapter); - module.exports = { tagsAdapter, postsAdapter, listAdapter };