Skip to content

Commit

Permalink
#Refocus get /subjects filtering by tag working (#68)
Browse files Browse the repository at this point in the history
* filter GET /v1/subjects/ by tags

* Test format changed for feature flag
  • Loading branch information
pallavi2209 authored and shriramshankar committed Oct 24, 2016
1 parent eb5ae26 commit 85e8e2e
Show file tree
Hide file tree
Showing 9 changed files with 226 additions and 20 deletions.
1 change: 1 addition & 0 deletions api/v1/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ module.exports = {
SEQ_DEFAULT_SCOPE: 'defaultScope',
SEQ_DESC: 'DESC',
SEQ_LIKE: '$iLike',
SEQ_CONTAINS: '$contains',
SEQ_OR: '$or',
SEQ_WILDCARD: '%',
SLASH: '/',
Expand Down
1 change: 1 addition & 0 deletions api/v1/helpers/nouns/subjects.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,4 +304,5 @@ module.exports = {
fieldsWithJsonArrayType,
fieldsWithArrayType,
loggingEnabled,
tagFilterName: 'tags',
}; // exports
70 changes: 52 additions & 18 deletions api/v1/helpers/verbs/findUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
const u = require('./utils');
const constants = require('../../constants');
const defaults = require('../../../../config').api.defaults;
const conf = require('../../../../config');

const env = conf.environment[conf.nodeEnv];
const filterSubjByTags = env.filterSubjByTags;

/**
* Escapes all percent literals so they're not treated as wildcards.
Expand Down Expand Up @@ -52,10 +56,17 @@ function toSequelizeWildcards(val) {
* case-insensitive string matching
*
* @param {String} val - The value to transform into a Sequelize where clause
* @param {Object} props - The helpers/nouns module for the given DB model.
* @returns {Object} a Sequelize where clause using "$ilike" for
* case-insensitive string matching
*/
function toWhereClause(val) {
function toWhereClause(val, props) {
if (filterSubjByTags && Array.isArray(val) && props.tagFilterName) {
const containsClause = {};
containsClause[constants.SEQ_CONTAINS] = val;
return containsClause;
}

// TODO handle non-string data types like dates and numbers
if (typeof val !== 'string') {
return val;
Expand All @@ -72,9 +83,10 @@ function toWhereClause(val) {
* a Sequelize "where" object.
*
* @param {Object} filter
* @param {Object} props - The helpers/nouns module for the given DB model.
* @returns {Object} a Sequelize "where" object
*/
function toSequelizeWhere(filter) {
function toSequelizeWhere(filter, props) {
const where = {};
const keys = Object.keys(filter);
for (let i = 0; i < keys.length; i++) {
Expand All @@ -85,23 +97,34 @@ function toSequelizeWhere(filter) {
}

const values = [];
for (let j = 0; j < filter[key].length; j++) {
const v = filter[key][j];
if (typeof v === 'boolean') {
values.push(v);
} else if (typeof v === 'string') {
const arr = v.split(constants.COMMA);
for (let k = 0; k < arr.length; k++) {
values.push(toWhereClause(arr[k]));

// if tag filter is enabled and key is "tags", then create a contains
// clause and add it to where clause,
// like, { where : { '$contains': ['tag1', 'tag2'] } }
if (filterSubjByTags && props.tagFilterName &&
key === props.tagFilterName) {
const tagArr = filter[key];
values.push(toWhereClause(tagArr, props));
where[key] = values[0];
} else {
for (let j = 0; j < filter[key].length; j++) {
const v = filter[key][j];
if (typeof v === 'boolean') {
values.push(v);
} else if (typeof v === 'string') {
const arr = v.split(constants.COMMA);
for (let k = 0; k < arr.length; k++) {
values.push(toWhereClause(arr[k]));
}
}
}
}

if (values.length === 1) {
where[key] = values[0];
} else if (values.length > 1) {
where[key] = {};
where[key][constants.SEQ_OR] = values;
if (values.length === 1) {
where[key] = values[0];
} else if (values.length > 1) {
where[key] = {};
where[key][constants.SEQ_OR] = values;
}
}
}
}
Expand Down Expand Up @@ -165,14 +188,25 @@ function options(params, props) {
const keys = Object.keys(params);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
const isFilterField = constants.NOT_FILTER_FIELDS.indexOf(key) < 0;

let isFilterField;
if (filterSubjByTags) {
isFilterField = constants.NOT_FILTER_FIELDS.indexOf(key) < 0;
} else {
// if filterSubjByTags is disabled and we do v1/subjects?tags=mytag, we
// get error because isFilterField resolves to true becuase of swagger
// changes, and I cannot apply feature flag to swagger.
isFilterField = constants.NOT_FILTER_FIELDS.indexOf(key) < 0 &&
key !== props.tagFilterName;
}

if (isFilterField && params[key].value !== undefined) {
filter[key] = params[key].value;
}
}

if (filter) {
opts.where = toSequelizeWhere(filter);
opts.where = toSequelizeWhere(filter, props);
}

return opts;
Expand Down
11 changes: 11 additions & 0 deletions api/v1/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3094,6 +3094,17 @@ paths:
Filter by parentAbsolutePath; asterisk (*) wildcards ok.
required: false
type: string
-
name: tags
in: query
items:
type: string
maxLength: 60
pattern: ^[0-9A-Za-z_][0-9A-Za-z_\\-]{1,59}$
description: >-
Filter by tags.
required: false
type: array
responses:
200:
description: >-
Expand Down
27 changes: 26 additions & 1 deletion config.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ const optimizeUpsert = pe.OPTIMIZE_UPSERT === 'true' ||
// env variable to enable caching for /GET /v1/perspectives/{key}
const enableCachePerspective = pe.ENABLE_CACHE_PERSPECTIVE || false;

const filterSubjByTags = pe.FILTER_SUBJ_BY_TAGS === 'true' ||
pe.FILTER_SUBJ_BY_TAGS === true || false;

module.exports = {

api: {
Expand Down Expand Up @@ -111,6 +114,7 @@ module.exports = {
useAccessToken: pe.USE_ACCESS_TOKEN || false,
tokenSecret:
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags,
},
development: {
checkTimeoutIntervalMillis: pe.CHECK_TIMEOUT_INTERVAL_MILLIS ||
Expand All @@ -130,6 +134,7 @@ module.exports = {
useAccessToken: pe.USE_ACCESS_TOKEN || false,
tokenSecret:
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags,
},
production: {
checkTimeoutIntervalMillis: pe.CHECK_TIMEOUT_INTERVAL_MILLIS ||
Expand All @@ -148,6 +153,7 @@ module.exports = {
useAccessToken: pe.USE_ACCESS_TOKEN || false,
tokenSecret: pe.SECRET_TOKEN ||
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags,
},
test: {
checkTimeoutIntervalMillis: pe.CHECK_TIMEOUT_INTERVAL_MILLIS ||
Expand All @@ -166,6 +172,7 @@ module.exports = {
useAccessToken: pe.USE_ACCESS_TOKEN || false,
tokenSecret: pe.SECRET_TOKEN ||
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags,
},
testDisableHttp: {
checkTimeoutIntervalMillis: pe.CHECK_TIMEOUT_INTERVAL_MILLIS ||
Expand All @@ -179,6 +186,7 @@ module.exports = {
useAccessToken: 'true',
tokenSecret:
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags,
},
testWhitelistLocalhost: {
checkTimeoutIntervalMillis: pe.CHECK_TIMEOUT_INTERVAL_MILLIS ||
Expand All @@ -192,6 +200,7 @@ module.exports = {
ipWhitelist: iplist,
tokenSecret:
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags,
},
testBlockAllhosts: {
checkTimeoutIntervalMillis: pe.CHECK_TIMEOUT_INTERVAL_MILLIS ||
Expand All @@ -205,6 +214,7 @@ module.exports = {
ipWhitelist: [''],
tokenSecret:
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags,
},
testTokenReq: {
checkTimeoutIntervalMillis: pe.CHECK_TIMEOUT_INTERVAL_MILLIS ||
Expand All @@ -218,6 +228,7 @@ module.exports = {
useAccessToken: 'true',
tokenSecret:
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags,
},
testTokenNotReq: {
checkTimeoutIntervalMillis: pe.CHECK_TIMEOUT_INTERVAL_MILLIS ||
Expand All @@ -231,8 +242,22 @@ module.exports = {
useAccessToken: false,
tokenSecret:
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags,
},

testSubjTagFilter: {
checkTimeoutIntervalMillis: pe.CHECK_TIMEOUT_INTERVAL_MILLIS ||
DEFAULT_CHECK_TIMEOUT_INTERVAL_MILLIS,
dbLogging: false, // console.log | false | ...
dbUrl: defaultDbUrl,
disableHttp,
redisUrl: '//127.0.0.1:6379',
defaultNodePort: defaultPort,
host: '127.0.0.1',
useAccessToken: false,
tokenSecret:
'7265666f637573726f636b7377697468677265656e6f776c7373616e6672616e',
filterSubjByTags: true,
}
},

nodeEnv,
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@
"test-token-notreq": "NODE_ENV=testTokenNotReq mocha -R dot --recursive tests/tokenNotReq",
"test-db": "npm run resetdb && mocha -R dot --recursive tests/db",
"test-realtime": "mocha -R dot --recursive tests/realtime",
"test-subj-tag-filter": "NODE_ENV=testSubjTagFilter mocha -R dot --recursive tests/subjectTagFilter",
"test-view": "NODE_ENV=test mocha -R dot --recursive --compilers js:babel-core/register --require ./tests/view/setup.js tests/view",
"test": "istanbul cover ./node_modules/mocha/bin/_mocha --report lcovonly -- -R dot --recursive tests/api tests/db && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage && npm run test-view && npm run test-realtime && npm run test-token-req && npm run test-token-notreq && npm run test-disablehttp",
"test": "istanbul cover ./node_modules/mocha/bin/_mocha --report lcovonly -- -R dot --recursive tests/api tests/db && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage && npm run test-view && npm run test-realtime && npm run test-token-req && npm run test-token-notreq && npm run test-disablehttp && npm run test-subj-tag-filter",
"postinstall": "NODE_ENV=production gulp browserifyViews && gulp movecss && gulp movesocket && gulp movelensutil && npm run checkdb",
"prestart": "npm run checkprivatedb"
},
Expand Down
2 changes: 2 additions & 0 deletions tests/api/v1/subjects/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ describe(`api: GET ${path}`, () => {
const us = {
name: `${tu.namePrefix}UnitedStates`,
description: 'country',
tags: ['US'],
};
const vt = {
name: `${tu.namePrefix}Vermont`,
description: 'state',
tags: ['US', 'NE'],
};

before((done) => {
Expand Down
104 changes: 104 additions & 0 deletions tests/subjectTagFilter/subjectTagFilter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* Copyright (c) 2016, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or
* https://opensource.org/licenses/BSD-3-Clause
*/

/**
* tests/subjectTagFilter/subjectTagFilter.js
*/
'use strict';
const supertest = require('supertest');

const api = supertest(require('../../index').app);
const constants = require('../../api/v1/constants');
const tu = require('../testUtils');
const u = require('./utils');
const Subject = tu.db.Subject;
const path = '/v1/subjects';
const expect = require('chai').expect;

describe(`api: GET ${path}`, () => {
let token;

const na = {
name: `${tu.namePrefix}NorthAmerica`,
description: 'continent',
};
const us = {
name: `${tu.namePrefix}UnitedStates`,
description: 'country',
tags: ['US'],
};
const vt = {
name: `${tu.namePrefix}Vermont`,
description: 'state',
tags: ['US', 'NE'],
};

before((done) => {
tu.createToken()
.then((returnedToken) => {
token = returnedToken;
done();
})
.catch((err) => done(err));
});

before((done) => {
Subject.create(na)
.then((createdNa) => {
na.id = createdNa.id;
us.parentId = na.id;
return Subject.create(us);
})
.then((createdUs) => {
us.id = createdUs.id;
vt.parentId = us.id;
return Subject.create(vt);
})
.then((createdVt) => {
vt.id = createdVt.id;
done();
})
.catch((err) => done(err));
});

after(u.forceDelete);
after(tu.forceDeleteUser);

it('GET with tag filter :: one tag', (done) => {
api.get(`${path}?tags=US`)
.set('Authorization', token)
.expect(constants.httpStatus.OK)
.end((err, res) => {
if (err) {
return done(err);
}

expect(res.body.length).to.equal(2);
expect(res.body[0].tags).to.eql(['US']);
expect(res.body[1].tags).to.eql(['US', 'NE']);

done();
});
});

it('GET with tag filter :: multiple tags', (done) => {
api.get(`${path}?tags=NE,US`)
.set('Authorization', token)
.expect(constants.httpStatus.OK)
.end((err, res) => {
if (err) {
return done(err);
}

expect(res.body.length).to.equal(1);
expect(res.body[0].tags).to.eql(['US', 'NE']);

done();
});
});
});
Loading

0 comments on commit 85e8e2e

Please sign in to comment.