Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a $project pipeline step, rather than post-processing results. #2251

Merged
merged 1 commit into from
Jan 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cool-bees-joke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystonejs/adapter-mongoose': patch
---

Uses the new `mongo-join-builder` API.
5 changes: 5 additions & 0 deletions .changeset/olive-mails-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystonejs/mongo-join-builder': major
---

Removed the `mutationBuilder` function in favour of using `$project` operations in the main pipeline.
3 changes: 1 addition & 2 deletions packages/adapter-mongoose/lib/adapter-mongoose.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const {
} = require('@keystonejs/utils');

const { BaseKeystoneAdapter, BaseListAdapter, BaseFieldAdapter } = require('@keystonejs/keystone');
const { queryParser, pipelineBuilder, mutationBuilder } = require('@keystonejs/mongo-join-builder');
const { queryParser, pipelineBuilder } = require('@keystonejs/mongo-join-builder');
const logger = require('@keystonejs/logger').logger('mongoose');

const slugify = require('@sindresorhus/slugify');
Expand Down Expand Up @@ -249,7 +249,6 @@ class MongooseListAdapter extends BaseListAdapter {
return this.model
.aggregate(pipelineBuilder(queryTree))
.exec()
.then(mutationBuilder(queryTree.relationships))
.then(foundItems => {
if (meta) {
// When there are no items, we get undefined back, so we simulate the
Expand Down
1 change: 0 additions & 1 deletion packages/adapter-mongoose/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"@keystonejs/mongo-join-builder": "^5.0.2",
"@keystonejs/utils": "^5.1.3",
"@sindresorhus/slugify": "^0.6.0",
"lodash.omitby": "^4.6.0",
"mongoose": "^5.8.4",
"p-settle": "^3.1.0",
"pluralize": "^7.0.0"
Expand Down
8 changes: 4 additions & 4 deletions packages/adapter-mongoose/tests/list-adapter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe('MongooseListAdapter', () => {
},
{ $match: { $and: [expect.any(Object), { title: { $eq: 'bar' } }] } },
{ $addFields: { id: '$_id' } },
{ $project: { posts: 0 } },
{ $project: expect.any(Object) },
]);

await userListAdapter.itemsQuery({
Expand All @@ -104,7 +104,7 @@ describe('MongooseListAdapter', () => {
},
{ $match: { $or: [expect.any(Object), { title: { $eq: 'bar' } }] } },
{ $addFields: { id: '$_id' } },
{ $project: { posts: 0 } },
{ $project: expect.any(Object) },
]);
});

Expand Down Expand Up @@ -167,7 +167,7 @@ describe('MongooseListAdapter', () => {
},
{ $match: expect.any(Object) },
{ $addFields: { id: '$_id' } },
{ $project: { posts: 0 } },
{ $project: expect.any(Object) },
]);

await userListAdapter.itemsQuery({
Expand All @@ -190,7 +190,7 @@ describe('MongooseListAdapter', () => {
},
{ $match: expect.any(Object) },
{ $addFields: { id: '$_id' } },
{ $project: { posts: 0 } },
{ $project: expect.any(Object) },
]);
});
});
14 changes: 7 additions & 7 deletions packages/fields/src/types/DateTime/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ keystone.createList('User', {

### Config

| Option | Type | Default | Description |
| ---------------- | --------- | ---------------------- | -------------------------------------------------------------------------- |
| `format` | `String` | `--` | Defines the format of the string that the component generates |
| `yearRangeFrom` | `String` | The current year - 100 | Defines the starting point of the year range, e.g. `1918` |
| Option | Type | Default | Description |
| ---------------- | --------- | ---------------------- | --------------------------------------------------------------------------- |
| `format` | `String` | `--` | Defines the format of the string that the component generates |
| `yearRangeFrom` | `String` | The current year - 100 | Defines the starting point of the year range, e.g. `1918` |
| `yearRangeTo` | `String` | The current year | Defines the ending point of the range in the yearSelect field , e.g. `2018` |
| `yearPickerType` | `String` | `auto` | Defines the input type for the year selector |
| `isRequired` | `Boolean` | `false` | Does this field require a value? |
| `isUnique` | `Boolean` | `false` | Adds a unique index that allows only unique values to be stored |
| `yearPickerType` | `String` | `auto` | Defines the input type for the year selector |
| `isRequired` | `Boolean` | `false` | Does this field require a value? |
| `isUnique` | `Boolean` | `false` | Adds a unique index that allows only unique values to be stored |

#### `format`

Expand Down
18 changes: 9 additions & 9 deletions packages/file-adapters/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,15 @@ const fileAdapter = new S3Adapter({
});
```

| Option | Type | Default | Description |
| ----------------- | ----------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `accessKeyId` | `String` | Required | AWS access key ID |
| `secretAccessKey` | `String` | Required | AWS secret access key |
| `region` | `String` | Required | AWS region |
| `bucket` | `String` | Required | S3 bucket name |
| `folder` | `String` | Required | Upload folder from root of bucket |
| `publicUrl` | `Function` | | By default the publicUrl returns a url for the S3 bucket in the form `https://{bucket}.s3.amazonaws.com/{key}/{filename}`. This will only work if the bucket is configured to allow public access. |
| `s3Options` | `Object` | `undefined` | For available options refer to the [AWS S3 API](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html) |
| Option | Type | Default | Description |
| ----------------- | ----------------- | ----------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `accessKeyId` | `String` | Required | AWS access key ID |
| `secretAccessKey` | `String` | Required | AWS secret access key |
| `region` | `String` | Required | AWS region |
| `bucket` | `String` | Required | S3 bucket name |
| `folder` | `String` | Required | Upload folder from root of bucket |
| `publicUrl` | `Function` | | By default the publicUrl returns a url for the S3 bucket in the form `https://{bucket}.s3.amazonaws.com/{key}/{filename}`. This will only work if the bucket is configured to allow public access. |
| `s3Options` | `Object` | `undefined` | For available options refer to the [AWS S3 API](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html) |
| `uploadParams` | `Object|Function` | `{}` | A config object or function returning a config object to be passed with each call to S3.upload. For available options refer to the [AWS S3 upload API](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#upload-property). |

### Methods
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
const { queryParser, pipelineBuilder, mutationBuilder } = require('../../');
const { queryParser, pipelineBuilder } = require('../../');
const getDatabase = require('../database');

const builder = async (query, aggregate, listAdapter) => {
const queryTree = queryParser({ listAdapter }, query);
const pipeline = pipelineBuilder(queryTree);
const postQueryMutations = mutationBuilder(queryTree.relationships);
// Run the query against the given database and collection
return await aggregate(pipeline).then(postQueryMutations);
return await aggregate(pipeline);
};

// Get all unfulfilled orders that have some out of stock items
Expand Down
5 changes: 2 additions & 3 deletions packages/mongo-join-builder/examples/relationship/index.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
const { queryParser, pipelineBuilder, mutationBuilder } = require('../../');
const { queryParser, pipelineBuilder } = require('../../');
const getDatabase = require('../database');
const builder = async (query, aggregate, listAdapter) => {
const queryTree = queryParser({ listAdapter }, query);
const pipeline = pipelineBuilder(queryTree);
const postQueryMutations = mutationBuilder(queryTree.relationships);
// Run the query against the given database and collection
return await aggregate(pipeline).then(postQueryMutations);
return await aggregate(pipeline);
};

const query = {
Expand Down
8 changes: 2 additions & 6 deletions packages/mongo-join-builder/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
const { queryParser } = require('./lib/query-parser');
const { pipelineBuilder, mutationBuilder } = require('./lib/join-builder');
const { pipelineBuilder } = require('./lib/join-builder');

module.exports = {
queryParser,
pipelineBuilder,
mutationBuilder,
};
module.exports = { queryParser, pipelineBuilder };
49 changes: 4 additions & 45 deletions packages/mongo-join-builder/lib/join-builder.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const omitBy = require('lodash.omitby');
const { flatten, compose, defaultObj } = require('@keystonejs/utils');
const { flatten, defaultObj } = require('@keystonejs/utils');

/**
* Format of input object:
Expand Down Expand Up @@ -43,47 +42,6 @@ const { flatten, compose, defaultObj } = require('@keystonejs/utils');
],
}
*/
function mutation(uid, lookupPath) {
return queryResult => {
function mutate(arrayToMutate, lookupPathFragment, pathSoFar = []) {
const [keyToMutate, ...restOfLookupPath] = lookupPathFragment;

return arrayToMutate.map((value, index) => {
if (!(keyToMutate in value)) {
return value;
}

if (restOfLookupPath.length === 0) {
// Now we can execute the mutation
return omitBy(value, (_, keyToOmit) => keyToOmit.startsWith(uid));
}

// Recurse
return {
...value,
[keyToMutate]: mutate(value[keyToMutate], restOfLookupPath, [
...pathSoFar,
index,
keyToMutate,
]),
};
});
}

return mutate(queryResult, lookupPath);
};
}

function mutationBuilder(relationships, path = []) {
return compose(
Object.entries(relationships).map(([uid, { relationshipInfo, relationships }]) => {
const { uniqueField } = relationshipInfo;
const postQueryMutations = mutationBuilder(relationships, [...path, uniqueField]);
// NOTE: Order is important. We want depth first, so we perform the related mutators first.
return compose([postQueryMutations, mutation(uid, [...path, uniqueField])]);
})
);
}

function relationshipPipeline(relationship) {
const { field, many, from, uniqueField } = relationship.relationshipInfo;
Expand All @@ -108,13 +66,14 @@ function relationshipPipeline(relationship) {
}

function pipelineBuilder({ relationships, matchTerm, excludeFields, postJoinPipeline }) {
excludeFields.push(...relationships.map(({ relationshipInfo }) => relationshipInfo.uniqueField));
return [
...flatten(Object.values(relationships).map(relationshipPipeline)),
...flatten(relationships.map(relationshipPipeline)),
matchTerm && { $match: matchTerm },
{ $addFields: { id: '$_id' } },
excludeFields && excludeFields.length && { $project: defaultObj(excludeFields, 0) },
...postJoinPipeline, // $search / $orderBy / $skip / $first / $count
].filter(i => i);
}

module.exports = { pipelineBuilder, mutationBuilder };
module.exports = { pipelineBuilder };
13 changes: 5 additions & 8 deletions packages/mongo-join-builder/lib/query-parser.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const cuid = require('cuid');
const { getType, flatten, objMerge } = require('@keystonejs/utils');
const { getType, flatten } = require('@keystonejs/utils');

const { simpleTokenizer, relationshipTokenizer, modifierTokenizer } = require('./tokenizers');

Expand All @@ -13,7 +13,7 @@ const flattenQueries = (parsedQueries, joinOp) => ({
joinOp
),
postJoinPipeline: flatten(parsedQueries.map(q => q.postJoinPipeline || [])).filter(pipe => pipe),
relationships: objMerge(parsedQueries.map(q => q.relationships || {})),
relationships: flatten(parsedQueries.map(q => q.relationships || [])),
});

function queryParser({ listAdapter, getUID = cuid }, query, pathSoFar = [], include) {
Expand Down Expand Up @@ -44,29 +44,26 @@ function queryParser({ listAdapter, getUID = cuid }, query, pathSoFar = [], incl
}
} else if (getType(value) === 'Object') {
// A relationship query component
const uid = getUID(key);
const { matchTerm, relationshipInfo } = relationshipTokenizer(
listAdapter,
query,
key,
path,
uid
getUID(key)
);
return {
// matchTerm is our filtering expression. This determines if the
// parent item is included in the final list
matchTerm,
relationships: {
[uid]: { relationshipInfo, ...queryParser({ listAdapter, getUID }, value, path) },
},
relationships: [{ relationshipInfo, ...queryParser({ listAdapter, getUID }, value, path) }],
};
} else {
// A simple field query component
return { matchTerm: simpleTokenizer(listAdapter, query, key, path) };
}
});
const flatQueries = flattenQueries(parsedQueries, '$and');
const includeFields = Object.values(flatQueries.relationships).map(({ field }) => field);
const includeFields = flatQueries.relationships.map(({ field }) => field);
if (include) includeFields.push(include);

return {
Expand Down
3 changes: 1 addition & 2 deletions packages/mongo-join-builder/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
},
"dependencies": {
"@keystonejs/utils": "^5.1.3",
"cuid": "^2.1.6",
"lodash.omitby": "^4.6.0"
"cuid": "^2.1.6"
},
"devDependencies": {
"mongodb": "^3.4.1",
Expand Down
7 changes: 4 additions & 3 deletions packages/mongo-join-builder/tests/index.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { queryParser, pipelineBuilder, mutationBuilder } = require('../');
const { queryParser, pipelineBuilder } = require('../');
const { listAdapter } = require('./utils');

describe('Test main export', () => {
Expand Down Expand Up @@ -56,10 +56,9 @@ describe('Test main export', () => {
},
];
const pipeline = pipelineBuilder(queryTree);
const postQueryMutations = mutationBuilder(queryTree.relationships);

const aggregate = jest.fn(() => Promise.resolve(aggregateResponse));
const result = await aggregate(pipeline).then(postQueryMutations);
const result = await aggregate(pipeline);
expect(pipeline).toMatchObject([
{
$lookup: {
Expand Down Expand Up @@ -89,6 +88,7 @@ describe('Test main export', () => {
},
},
{ $addFields: { id: '$_id' } },
{ $project: { tags_some_tags: 0 } },
],
},
},
Expand All @@ -106,6 +106,7 @@ describe('Test main export', () => {
},
},
{ $addFields: { id: '$_id' } },
{ $project: { posts_every_posts: 0 } },
]);

expect(result).toMatchObject([
Expand Down
Loading