Skip to content

[#919710] Ensure only published makes are returned from general search, ... #151

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.

Sign up for GitHub

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

Open
ScottDowne wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[#919710] Ensure only published makes are returned from general search, ... #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e7bd0e2
[#919710] Ensure only published makes are returned from general searc…
ScottDowne Sep 24, 2013
ed2adec
[#919710] Updating tests.
ScottDowne Sep 27, 2013
df118fc
[#919710] Removed authenticated search and going to do that in anothe…
ScottDowne Sep 30, 2013
fe94780
review
ScottDowne Sep 30, 2013
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
2 changes: 2 additions & 0 deletions lib/models/make.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ module.exports = function( environment, mongoInstance ) {
published: {
type: Boolean,
"default": true,
es_type: "boolean",
es_indexed: true,
es_index: "not_analyzed"
},
tags: {
Expand Down
267 changes: 142 additions & 125 deletions lib/queryBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,147 +136,164 @@ module.exports = function( loginApi ) {
}
};

// capture valid query generator keys
GENERATOR_KEYS = Object.keys( generators );

return {
build: function( query, callback ) {
if ( !( query && query.constructor === Object ) || !( callback && typeof callback === "function" ) ) {
throw new Error( "Check your arguments." );
}
function buildQuery( query, customFilter, callback ) {
if ( typeof customFilter === "function" ) {
callback = customFilter;
customFilter = {};
}
if ( !( query && query.constructor === Object ) || !( callback && typeof callback === "function" ) ) {
Copy link
Contributor

@cadecairos cadecairos Sep 30, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check that customFilter exists, or default it

Copy link
Contributor

@cadecairos cadecairos Sep 30, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you're defaulting it below when it's referenced for baseQuery - that should be fine, but perhaps you could shorthand it so that buildQuery( query, callback ) works alongside the three argument version. Worth it?

Copy link
Contributor Author

@ScottDowne ScottDowne Sep 30, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree with this.

Why burden the calls that don't care about filters to have to worry about filters.

throw new Error( "Check your arguments." );
}

query.limit = +query.limit;
query.page = +query.page;

// baseQuery is the most basic query we can make.
// advancedQuery is used if we need to generate filters, and wraps around baseQuery.
var baseQuery = {
query: {
filtered: {
query: {
match_all: {}
},
filter: {
missing: {
field: "deletedAt",
null_value: true
}
}
}
}
},
advancedQuery = {
query: {
filtered: {
filter: {
bool: {
must: [],
should: []
}
},
query: baseQuery.query
}
query.limit = +query.limit;
query.page = +query.page;

// baseQuery is the most basic query we can make.
// advancedQuery is used if we need to generate filters, and wraps around baseQuery.
var baseQuery = {
query: {
filtered: {
query: {
match_all: {}
},
filter: customFilter
}
},
searchQuery = {},
size = query.limit && isFinite( query.limit ) ? query.limit : DEFAULT_SEARCH_SIZE,
page = query.page && isFinite( query.page ) ? query.page : 1,
user = query.user,
sort = query.sortByField,
filterOccurence = query.or ? "should" : "must",
sortObj,
notRegexMatch;

// If the request contains any of the filter generating keys, or defines a user search, use the advancedQuery object
if ( Object.keys( query ).some( hasGeneratorKey ) || user ) {
searchQuery = advancedQuery;
Object.keys( query ).forEach(function( key ){
value = query[ key ];
if ( generators[ key ] ) {
notRegexMatch = NOT_REGEX.exec( value );
if ( notRegexMatch ) {
searchQuery.query.filtered.filter.bool[ filterOccurence ].push( generators[ key ]( notRegexMatch[ 1 ], true ) );
} else {
searchQuery.query.filtered.filter.bool[ filterOccurence ].push( generators[ key ]( value ) );
}
},
advancedQuery = {
query: {
filtered: {
filter: {
bool: {
must: [],
should: []
}
},
query: baseQuery.query
}
}
});
} else {
searchQuery = baseQuery;
}
},
searchQuery = {},
size = query.limit && isFinite( query.limit ) ? query.limit : DEFAULT_SEARCH_SIZE,
page = query.page && isFinite( query.page ) ? query.page : 1,
user = query.user,
sort = query.sortByField,
filterOccurence = query.or ? "should" : "must",
sortObj,
notRegexMatch;

// If the request contains any of the filter generating keys, or defines a user search, use the advancedQuery object
if ( Object.keys( query ).some( hasGeneratorKey ) || user ) {
searchQuery = advancedQuery;
Object.keys( query ).forEach(function( key ){
value = query[ key ];
if ( generators[ key ] ) {
notRegexMatch = NOT_REGEX.exec( value );
if ( notRegexMatch ) {
searchQuery.query.filtered.filter.bool[ filterOccurence ].push( generators[ key ]( notRegexMatch[ 1 ], true ) );
} else {
searchQuery.query.filtered.filter.bool[ filterOccurence ].push( generators[ key ]( value ) );
}
}
});
} else {
searchQuery = baseQuery;
}

// set size and from and sort
if ( size > MAX_SEARCH_SIZE ) {
size = MAX_SEARCH_SIZE;
} else if ( size < 1 ) {
size = 1;
}
searchQuery.size = size;
// set size and from and sort
if ( size > MAX_SEARCH_SIZE ) {
size = MAX_SEARCH_SIZE;
} else if ( size < 1 ) {
size = 1;
}
searchQuery.size = size;

if ( page < 1 ) {
page = 1;
}
searchQuery.from = ( page - 1 ) * size;
if ( page < 1 ) {
page = 1;
}
searchQuery.from = ( page - 1 ) * size;

if ( sort ) {
sort = ( Array.isArray( sort ) ? sort : [ sort ] ).filter(function( pair ) {
return typeof pair === "string" && pair.length && VALID_SORT_FIELDS.indexOf( pair.split( "," )[ 0 ] ) !== -1;
});
if ( sort ) {
sort = ( Array.isArray( sort ) ? sort : [ sort ] ).filter(function( pair ) {
return typeof pair === "string" && pair.length && VALID_SORT_FIELDS.indexOf( pair.split( "," )[ 0 ] ) !== -1;
});

if ( sort.length ) {
searchQuery.sort = [];
sort.forEach(function( pair ){
pair = pair.split( "," );
sortObj = {};
if ( pair[ 0 ] === "likes" ) {
sortObj._script = {
lang: "js",
order: pair[ 1 ],
script: "doc['likes.userId'].values.length",
type: "number"
};
} else {
sortObj[ pair[ 0 ] ] = pair[ 1 ] || "desc";
}
searchQuery.sort.push( sortObj );
});
}
if ( sort.length ) {
searchQuery.sort = [];
sort.forEach(function( pair ){
pair = pair.split( "," );
sortObj = {};
if ( pair[ 0 ] === "likes" ) {
sortObj._script = {
lang: "js",
order: pair[ 1 ],
script: "doc['likes.userId'].values.length",
type: "number"
};
} else {
sortObj[ pair[ 0 ] ] = pair[ 1 ] || "desc";
}
searchQuery.sort.push( sortObj );
});
}
}

if ( user ) {
notRegexMatch = NOT_REGEX.exec( user );
if ( notRegexMatch ) {
user = notRegexMatch[ 1 ];
if ( user ) {
notRegexMatch = NOT_REGEX.exec( user );
if ( notRegexMatch ) {
user = notRegexMatch[ 1 ];
}
loginApi.getUser( user, function( err, userData ) {
if ( err ) {
callback({
error: err,
code: 500
});
return;
}
loginApi.getUser( user, function( err, userData ) {
if ( err ) {
callback({
error: err,
code: 500
});
return;
}

if ( !userData ) {
if ( searchQuery.query.filtered.filter.bool.should.length ) {
// If this is an OR filtered query, ignore the undefined user
callback( null, searchQuery );
} else {
callback( { code: 404 } );
}
return;
if ( !userData ) {
if ( searchQuery.query.filtered.filter.bool.should.length ) {
// If this is an OR filtered query, ignore the undefined user
callback( null, searchQuery );
} else {
callback( { code: 404 } );
}
return;
}

var filter = generateFilter( "term", {
email: userData.email
}, !!notRegexMatch );
var filter = generateFilter( "term", {
email: userData.email
}, !!notRegexMatch );

searchQuery.query.filtered.filter.bool[ filterOccurence ].push( filter );
callback( null, searchQuery );
});
} else {
searchQuery.query.filtered.filter.bool[ filterOccurence ].push( filter );
callback( null, searchQuery );
}
});
} else {
callback( null, searchQuery );
}
}

// capture valid query generator keys
GENERATOR_KEYS = Object.keys( generators );

return {
build: function( query, callback ) {
buildQuery( query, {
and: [
{
missing: {
field: "deletedAt",
null_value: true
}
},
{
term: {
published: true
}
}
]
}, callback );
}
};
};
17 changes: 13 additions & 4 deletions test/queryBuilder/core.unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,19 @@ module.exports = function( qb ){
match_all: {}
},
filter: {
missing: {
field: "deletedAt",
null_value: true
}
and: [
{
missing: {
field: "deletedAt",
null_value: true
}
},
{
term: {
published: true
}
}
]
}
}
},
Expand Down
17 changes: 13 additions & 4 deletions test/queryBuilder/sort.unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,19 @@ module.exports = function( qb ) {
match_all: {}
},
filter: {
missing: {
field: "deletedAt",
null_value: true
}
and: [
{
missing: {
field: "deletedAt",
null_value: true
}
},
{
term: {
published: true
}
}
]
}
}
},
Expand Down