Skip to content

Commit

Permalink
🐛 Fixed all known filter limitations (#10159)
Browse files Browse the repository at this point in the history 
refs #10105, closes #10108, closes #9950, refs #9923, refs #9916, refs #9574, refs #6345, refs #6309, refs #6158, refs TryGhost/GQL#16

- removed GQL dependency
- replaced GQL with our brand new NQL implementation
- fixed all known filter limitations
- GQL suffered from some underlying filter bugs, which NQL tried to fix
- the bugs were mostly in how we query the database for relation filtering
- the underlying problem was caused by a too simple implementation of querying the relations
- mongo-knex has implemented a more robust and complex filtering mechanism for relations
- replaced logic in our bookshelf filter plugin
- we pass the custom, default and override filters from Ghost to NQL, which then are getting parsed and merged into a mongo JSON object. The mongo JSON is getting attached by mongo-knex.

NQL: https://github.com/NexesJS/NQL
mongo-knex: https://github.com/NexesJS/mongo-knex
  • Loading branch information
@kirrg001
kirrg001 authored Dec 11, 2018
1 parent 48923ac commit 9d7c3bd
Show file tree
Hide file tree
Showing 21 changed files with 623 additions and 1,165 deletions.
21 changes: 10 additions & 11 deletions core/server/api/v2/utils/serializers/input/pages.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,20 @@ module.exports = {
browse(apiConfig, frame) {
debug('browse');

// CASE: the content api endpoints for pages forces the model layer to return static pages only.
// we have to enforce the filter.
/**
* CASE:
*
* - the content api endpoints for pages forces the model layer to return static pages only
* - we have to enforce the filter
*
* @TODO: https://github.com/TryGhost/Ghost/issues/10268
*/
if (frame.options.filter) {
if (frame.options.filter.match(/page:\w+\+?/)) {
frame.options.filter = frame.options.filter.replace(/page:\w+\+?/, '');
}

if (frame.options.filter) {
frame.options.filter = frame.options.filter + '+page:true';
} else {
frame.options.filter = 'page:true';
}
frame.options.filter = `${frame.options.filter}+page:true`;
} else {
frame.options.filter = 'page:true';
}

removeMobiledocFormat(frame);

debug(frame.options);
Expand Down
23 changes: 12 additions & 11 deletions core/server/api/v2/utils/serializers/input/posts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,25 @@ module.exports = {
* - user exists? admin api access
*/
if (utils.isContentAPI(frame)) {
// CASE: the content api endpoints for posts should only return non page type resources
/**
* CASE:
*
* - the content api endpoints for posts should only return non page type resources
* - we have to enforce the filter
*
* @TODO: https://github.com/TryGhost/Ghost/issues/10268
*/
if (frame.options.filter) {
if (frame.options.filter.match(/page:\w+\+?/)) {
frame.options.filter = frame.options.filter.replace(/page:\w+\+?/, '');
}

if (frame.options.filter) {
frame.options.filter = frame.options.filter + '+page:false';
} else {
frame.options.filter = 'page:false';
}
frame.options.filter = `${frame.options.filter}+page:false`;
} else {
frame.options.filter = 'page:false';
}

// CASE: the content api endpoint for posts should not return mobiledoc
removeMobiledocFormat(frame);

// CASE: Members needs to have the tags to check if its allowed access
if (labs.isSet('members')) {
// CASE: Members needs to have the tags to check if its allowed access
includeTags(frame);
}
}
Expand Down
6 changes: 3 additions & 3 deletions core/server/models/base/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const _ = require('lodash'),
bookshelf = require('bookshelf'),
moment = require('moment'),
Promise = require('bluebird'),
gql = require('ghost-gql'),
ObjectId = require('bson-objectid'),
debug = require('ghost-ignition').debug('models:base'),
config = require('../../config'),
Expand Down Expand Up @@ -1012,6 +1011,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
fetchAll: function (options) {
options = options || {};

const nql = require('@nexes/nql');
const modelName = options.modelName;
const tableNames = {
Post: 'posts',
Expand Down Expand Up @@ -1072,8 +1072,8 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
query.select(toSelect);
}

// filter data
gql.knexify(query, gql.parse(filter));
// @NOTE: We can't use the filter plugin, because we are not using bookshelf.
nql(filter).querySQL(query);

return query.then((objects) => {
debug('fetched', modelName, filter);
Expand Down
271 changes: 78 additions & 193 deletions core/server/models/plugins/filter.js
View file
Original file line number Diff line number Diff line change
@@ -1,210 +1,95 @@
var _ = require('lodash'),
gql = require('ghost-gql'),
common = require('../../lib/common'),
filter,
filterUtils;

filterUtils = {
/**
* ## Combine Filters
* Util to combine the enforced, default and custom filters such that they behave accordingly
* @param {String|Object} enforced - filters which must ALWAYS be applied
* @param {String|Object} defaults - filters which must be applied if a matching filter isn't provided
* @param {...String|Object} [custom] - custom filters which are additional
* @returns {*}
*/
combineFilters: function combineFilters(enforced, defaults, custom /* ...custom */) {
custom = Array.prototype.slice.call(arguments, 2);

// Ensure everything has been run through the gql parser
try {
enforced = enforced ? (_.isString(enforced) ? gql.parse(enforced) : enforced) : null;
defaults = defaults ? (_.isString(defaults) ? gql.parse(defaults) : defaults) : null;
custom = _.map(custom, function (arg) {
return _.isString(arg) ? gql.parse(arg) : arg;
});
} catch (err) {
throw new common.errors.ValidationError({
err: err,
property: 'filter',
context: common.i18n.t('errors.models.plugins.filter.errorParsing'),
help: common.i18n.t('errors.models.plugins.filter.forInformationRead', {url: 'https://api.ghost.org/docs/filter'})
});
}

// Merge custom filter options into a single set of statements
custom = gql.json.mergeStatements.apply(this, custom);

// if there is no enforced or default statements, return just the custom statements;
if (!enforced && !defaults) {
return custom;
}

// Reduce custom filters based on enforced filters
if (custom && !_.isEmpty(custom.statements) && enforced && !_.isEmpty(enforced.statements)) {
custom.statements = gql.json.rejectStatements(custom.statements, function (customStatement) {
return gql.json.findStatement(enforced.statements, customStatement, 'prop');
});
}

// Reduce default filters based on custom filters
if (defaults && !_.isEmpty(defaults.statements) && custom && !_.isEmpty(custom.statements)) {
defaults.statements = gql.json.rejectStatements(defaults.statements, function (defaultStatement) {
return gql.json.findStatement(custom.statements, defaultStatement, 'prop');
});
}

// Merge enforced and defaults
enforced = gql.json.mergeStatements(enforced, defaults);

if (_.isEmpty(custom.statements)) {
return enforced;
}

if (_.isEmpty(enforced.statements)) {
return custom;
}

return {
statements: [
{group: enforced.statements},
{group: custom.statements, func: 'and'}
]
};
const debug = require('ghost-ignition').debug('models:plugins:filter');
const common = require('../../lib/common');

const RELATIONS = {
tags: {
tableName: 'tags',
type: 'manyToMany',
joinTable: 'posts_tags',
joinFrom: 'post_id',
joinTo: 'tag_id'
},
authors: {
tableName: 'users',
tableNameAs: 'authors',
type: 'manyToMany',
joinTable: 'posts_authors',
joinFrom: 'post_id',
joinTo: 'author_id'
}
};

filter = function filter(Bookshelf) {
var Model = Bookshelf.Model.extend({
const EXPANSIONS = [{
key: 'primary_tag',
replacement: 'tags.slug',
expansion: 'posts_tags.sort_order:0+tags.visibility:public'
}, {
key: 'primary_author',
replacement: 'authors.slug',
expansion: 'posts_authors.sort_order:0+authors.visibility:public'
}, {
key: 'authors',
replacement: 'authors.slug'
}, {
key: 'author',
replacement: 'authors.slug'
}, {
key: 'tag',
replacement: 'tags.slug'
}, {
key: 'tags',
replacement: 'tags.slug'
}];

const filter = function filter(Bookshelf) {
const Model = Bookshelf.Model.extend({
// Cached copy of the filters setup for this model instance
_filters: null,
// Override these on the various models
enforcedFilters: function enforcedFilters() {
},
defaultFilters: function defaultFilters() {
},
extraFilters: function extraFilters() {
},

preProcessFilters: function preProcessFilters() {
this._filters.statements = gql.json.replaceStatements(this._filters.statements, {prop: /primary_tag/}, function (statement) {
statement.prop = 'tags.slug';
return {
group: [
statement,
{prop: 'posts_tags.sort_order', op: '=', value: 0},
{prop: 'tags.visibility', op: '=', value: 'public'}
]
};
});

this._filters.statements = gql.json.replaceStatements(this._filters.statements, {prop: /primary_author/}, function (statement) {
statement.prop = 'authors.slug';
return {
group: [
statement,
{prop: 'posts_authors.sort_order', op: '=', value: 0},
{prop: 'authors.visibility', op: '=', value: 'public'}
]
};
});
},

/**
* ## Post process Filters
* Post Process filters looking for joins etc
* @TODO refactor this
* @param {object} options
*/
postProcessFilters: function postProcessFilters(options) {
var joinTables = this._filters.joins;

if (joinTables && joinTables.indexOf('tags') > -1) {
// We need to use leftOuterJoin to insure we still include posts which don't have tags in the result
// The where clause should restrict which items are returned
this
.query('leftOuterJoin', 'posts_tags', 'posts_tags.post_id', '=', 'posts.id')
.query('leftOuterJoin', 'tags', 'posts_tags.tag_id', '=', 'tags.id');

// We need to add a group by to counter the double left outer join
// TODO improve on the group by handling
options.groups = options.groups || [];
options.groups.push('posts.id');
}

if (joinTables && joinTables.indexOf('authors') > -1) {
// We need to use leftOuterJoin to insure we still include posts which don't have tags in the result
// The where clause should restrict which items are returned
this
.query('leftOuterJoin', 'posts_authors', 'posts_authors.post_id', '=', 'posts.id')
.query('leftOuterJoin', 'users as authors', 'posts_authors.author_id', '=', 'authors.id');

// We need to add a group by to counter the double left outer join
// TODO improve on the group by handling
options.groups = options.groups || [];
options.groups.push('posts.id');
}

/**
* @deprecated: `author`, will be removed in Ghost 3.0
*/
if (joinTables && joinTables.indexOf('author') > -1) {
this
.query('join', 'users as author', 'author.id', '=', 'posts.author_id');
}
},
enforcedFilters() {},
defaultFilters() {},
extraFilters() {},

/**
* ## fetchAndCombineFilters
* Helper method, uses the combineFilters util to apply filters to the current model instance
* based on options and the set enforced/default filters for this resource
* @param {Object} options
* @returns {Bookshelf.Model}
*/
fetchAndCombineFilters: function fetchAndCombineFilters(options) {
options = options || {};

this._filters = filterUtils.combineFilters(
this.enforcedFilters(options),
this.defaultFilters(options),
options.filter,
this.extraFilters(options)
);

return this;
},

/**
* ## Apply Filters
* Method which makes the necessary query builder calls (through knex) for the filters set
* on this model instance
* @param {Object} options
* @returns {Bookshelf.Model}
* Method which makes the necessary query builder calls (through knex) for the filters set on this model
* instance.
*/
applyDefaultAndCustomFilters: function applyDefaultAndCustomFilters(options) {
var self = this;

// @TODO figure out a better place/way to trigger loading filters
if (!this._filters) {
this.fetchAndCombineFilters(options);
}

if (this._filters) {
if (this.debug) {
gql.json.printStatements(this._filters.statements);
const nql = require('@nexes/nql');

let custom = options.filter;
let extra = this.extraFilters(options);
let overrides = this.enforcedFilters(options);
let defaults = this.defaultFilters(options);

debug('custom', custom);
debug('extra', extra);
debug('enforced', overrides);
debug('default', defaults);

if (extra) {
if (custom) {
custom = `${custom}+${extra}`;
} else {
custom = extra;
}
}

this.preProcessFilters(options);

this.query(function (qb) {
gql.knexify(qb, self._filters);
try {
this.query((qb) => {
nql(custom, {
relations: RELATIONS,
expansions: EXPANSIONS,
overrides: overrides,
defaults: defaults
}).querySQL(qb);
});
} catch (err) {
throw new common.errors.BadRequestError({
message: common.i18n.t('errors.models.plugins.filter.errorParsing'),
err: err
});

// Replaces processGQLResult
this.postProcessFilters(options);
}

return this;
}
});

Expand Down
Loading

0 comments on commit 9d7c3bd

Please sign in to comment.