Skip to content

Commit

Permalink
Move mongoose specific code into mongo-join-builder (#3283)
Browse files Browse the repository at this point in the history
  • Loading branch information
timleslie authored Jul 23, 2020
1 parent ef70749 commit af51715
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 101 deletions.
7 changes: 7 additions & 0 deletions .changeset/bright-badgers-relate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@keystonejs/fields': major
'@keystonejs/keystone': major
'@keystonejs/mongo-join-builder': patch
---

Removed `BaseListAdapter.findFieldAdapterForQuerySegment()` and `MongoRelationshipInterface.supportsRelationshipQuery()`.
2 changes: 0 additions & 2 deletions packages/adapter-mongoose/tests/list-adapter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ describe('MongooseListAdapter', () => {
{
isRelationship: true,
getQueryConditions: () => {},
supportsRelationshipQuery: query => query === 'posts_some',
getRefListAdapter: () => postListAdapter,
field: { config: { many: true }, many: true },
path: 'posts',
Expand Down Expand Up @@ -131,7 +130,6 @@ describe('MongooseListAdapter', () => {
{
isRelationship: true,
getQueryConditions: () => {},
supportsRelationshipQuery: query => query === 'posts_some',
getRefListAdapter: () => postListAdapter,
field: { config: { many: true }, many: true },
path: 'posts',
Expand Down
11 changes: 0 additions & 11 deletions packages/fields/src/types/Relationship/Implementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
);
}
}
6 changes: 0 additions & 6 deletions packages/keystone/lib/adapters/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
10 changes: 8 additions & 2 deletions packages/mongo-join-builder/lib/tokenizers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?
Expand Down
1 change: 1 addition & 0 deletions packages/mongo-join-builder/tests/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('Test main export', () => {
{ $match: { $expr: { $eq: [`$_id`, '$$tmpVar'] } } },
{ $match: { name: { $eq: 'foo' } } },
{ $addFields: { id: '$_id' } },
{ $project: { posts: 0 } },
],
},
},
Expand Down
133 changes: 71 additions & 62 deletions packages/mongo-join-builder/tests/relationship-path.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,66 +3,73 @@ 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
);
});

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'])
).toEqual(listAdapter);
});

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'])
).toEqual(zipListAdapter);
});

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, [
Expand All @@ -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, [
Expand All @@ -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'])
Expand Down
9 changes: 3 additions & 6 deletions packages/mongo-join-builder/tests/relationship.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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] } },
Expand All @@ -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);
});
});
25 changes: 13 additions & 12 deletions packages/mongo-join-builder/tests/utils.js
Original file line number Diff line number Diff line change
@@ -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' } },
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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: () => ({
Expand All @@ -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: {} },
Expand All @@ -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: {
Expand All @@ -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: {} },
Expand All @@ -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 };

0 comments on commit af51715

Please sign in to comment.