From 85ab047beb628d9ad6bf0c4cc6a3d017b242c489 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Tue, 19 Feb 2019 17:48:00 +0800 Subject: [PATCH 1/2] winning submission for challenge 30083089 - Topcoder Connect - Filter project table by columns, fix for issue #245 --- migrations/elasticsearch_sync.js | 3 +- postman.json | 116 +++++++++++++++++++- src/routes/projects/list.js | 142 ++++++++++++++++++++---- src/routes/projects/list.spec.js | 182 ++++++++++++++++++++++++++++++- src/util.js | 2 +- swagger.yaml | 6 +- 6 files changed, 425 insertions(+), 26 deletions(-) mode change 100755 => 100644 swagger.yaml diff --git a/migrations/elasticsearch_sync.js b/migrations/elasticsearch_sync.js index 349be0b6..b116082d 100644 --- a/migrations/elasticsearch_sync.js +++ b/migrations/elasticsearch_sync.js @@ -209,7 +209,8 @@ function getRequestBody(indexName) { }, }, id: { - type: 'long', + type: 'string', + index: 'not_analyzed', }, members: { type: 'nested', diff --git a/postman.json b/postman.json index 999689af..86172fb6 100644 --- a/postman.json +++ b/postman.json @@ -1485,6 +1485,120 @@ }, "response": [] }, + { + "name": "List projects with filters - name, code, customer, manager", + "request": { + "url": { + "raw": "{{api-url}}/v4/projects?filter=id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", + "host": [ + "{{api-url}}" + ], + "path": [ + "v4", + "projects" + ], + "query": [ + { + "key": "filter", + "value": "id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", + "equals": true, + "description": "" + } + ], + "variable": [] + }, + "method": "GET", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}", + "description": "" + } + ], + "body": { + "mode": "raw", + "raw": "" + }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, + { + "name": "List projects with filters - id", + "request": { + "url": { + "raw": "{{api-url}}/v4/projects?filter=id%3D2", + "host": [ + "{{api-url}}" + ], + "path": [ + "v4", + "projects" + ], + "query": [ + { + "key": "filter", + "value": "id%3D2", + "equals": true, + "description": "" + } + ], + "variable": [] + }, + "method": "GET", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}", + "description": "" + } + ], + "body": { + "mode": "raw", + "raw": "" + }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, + { + "name": "List projects with filters - code", + "request": { + "url": { + "raw": "{{api-url}}/v4/projects?filter=code%3Dtest*", + "host": [ + "{{api-url}}" + ], + "path": [ + "v4", + "projects" + ], + "query": [ + { + "key": "filter", + "value": "code%3Dtest*", + "equals": true, + "description": "" + } + ], + "variable": [] + }, + "method": "GET", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}", + "description": "" + } + ], + "body": { + "mode": "raw", + "raw": "" + }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, { "name": "List projects with sort applied", "request": { @@ -5395,4 +5509,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/src/routes/projects/list.js b/src/routes/projects/list.js index 1e490f57..691d7d81 100755 --- a/src/routes/projects/list.js +++ b/src/routes/projects/list.js @@ -102,6 +102,87 @@ const buildEsFullTextQuery = (keyword, matchType, singleFieldName) => { }; }; +/** + * Build ES query search request body based on value, keyword, matchType and fieldName + * + * @param {String} value the value to build request body for + * @param {String} keyword the keyword to query + * @param {String} matchType wildcard match or exact match + * @param {Array} fieldName the fieldName + * @return {Object} search request body that can be passed to .search api call + */ +const buildEsQueryWithFilter = (value, keyword, matchType, fieldName) => { + let should = []; + if (value !== 'details' && value !== 'customer' && value !== 'manager') { + should = _.concat(should, { + query_string: { + query: keyword, + analyze_wildcard: (matchType === MATCH_TYPE_WILDCARD), + fields: fieldName, + }, + }); + } + + if (value === 'details') { + should = _.concat(should, { + nested: { + path: 'details', + query: { + nested: { + path: 'details.utm', + query: { + query_string: { + query: keyword, + analyze_wildcard: (matchType === MATCH_TYPE_WILDCARD), + fields: fieldName, + }, + }, + }, + }, + }, + }); + } + + if (value === 'customer' || value === 'manager') { + should = _.concat(should, { + nested: { + path: 'members', + query: { + bool: { + must: [ + { match: { 'members.role': value } }, + { + query_string: { + query: keyword, + analyze_wildcard: (matchType === MATCH_TYPE_WILDCARD), + fields: fieldName, + }, + }, + ], + }, + }, + }, + }); + } + + return should; +}; + +/** + * Prepare search request body based on wildcard query + * + * @param {String} value the value to build request body for + * @param {String} keyword the keyword to query + * @param {Array} fieldName the fieldName + * @return {Object} search request body that can be passed to .search api call + */ +const setFilter = (value, keyword, fieldName) => { + if (keyword.indexOf('*') > -1) { + return buildEsQueryWithFilter(value, keyword, MATCH_TYPE_WILDCARD, fieldName); + } + return buildEsQueryWithFilter(value, keyword, MATCH_TYPE_EXACT_PHRASE, fieldName); +}; + /** * Parse the ES search criteria and prepare search request body * @@ -152,6 +233,7 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { } // prepare the elasticsearch filter criteria const boolQuery = []; + let mustQuery = []; let fullTextQuery; if (_.has(criteria, 'filters.id.$in')) { boolQuery.push({ @@ -159,6 +241,34 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { values: criteria.filters.id.$in, }, }); + } else if (_.has(criteria, 'filters.id') && criteria.filters.id.indexOf('*') > -1) { + mustQuery = _.concat(mustQuery, buildEsQueryWithFilter('id', criteria.filters.id, MATCH_TYPE_WILDCARD, ['id'])); + } else if (_.has(criteria, 'filters.id')) { + boolQuery.push({ + term: { + id: criteria.filters.id, + }, + }); + } + + if (_.has(criteria, 'filters.name')) { + mustQuery = _.concat(mustQuery, setFilter('name', criteria.filters.name, ['name'])); + } + + if (_.has(criteria, 'filters.code')) { + mustQuery = _.concat(mustQuery, setFilter('details', criteria.filters.code, ['details.utm.code'])); + } + + if (_.has(criteria, 'filters.customer')) { + mustQuery = _.concat(mustQuery, setFilter('customer', + criteria.filters.customer, + ['members.firstName', 'members.lastName'])); + } + + if (_.has(criteria, 'filters.manager')) { + mustQuery = _.concat(mustQuery, setFilter('manager', + criteria.filters.manager, + ['members.firstName', 'members.lastName'])); } if (_.has(criteria, 'filters.status.$in')) { @@ -177,21 +287,6 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { }); } - if (_.has(criteria, 'filters.type.$in')) { - // type is an array - boolQuery.push({ - terms: { - type: criteria.filters.type.$in, - }, - }); - } else if (_.has(criteria, 'filters.type')) { - // type is simple string - boolQuery.push({ - term: { - type: criteria.filters.type, - }, - }); - } if (_.has(criteria, 'filters.keyword')) { // keyword is a full text search // escape special fields from keyword search @@ -222,7 +317,7 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { if (!keyword) { // Not a specific field search nor an exact phrase search, do a wildcard match - keyword = escapeEsKeyword(criteria.filters.keyword); + keyword = criteria.filters.keyword; matchType = MATCH_TYPE_WILDCARD; } @@ -234,6 +329,12 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { filter: boolQuery, }; } + + if (mustQuery.length > 0) { + body.query.bool = _.merge(body.query.bool, { + must: mustQuery, + }); + } if (fullTextQuery) { body.query = _.merge(body.query, fullTextQuery); if (body.query.bool) { @@ -241,10 +342,9 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { } } - if (fullTextQuery || boolQuery.length > 0) { + if (fullTextQuery || boolQuery.length > 0 || mustQuery.length > 0) { searchCriteria.body = body; } - return searchCriteria; }; @@ -267,8 +367,7 @@ const retrieveProjects = (req, criteria, sort, ffields) => { fields.projects.push('id'); } - const searchCriteria = parseElasticSearchCriteria(criteria, fields, order); - + const searchCriteria = parseElasticSearchCriteria(criteria, fields, order) || {}; return new Promise((accept, reject) => { const es = util.getElasticSearchClient(); es.search(searchCriteria).then((docs) => { @@ -300,7 +399,8 @@ module.exports = [ 'name', 'name asc', 'name desc', 'type', 'type asc', 'type desc', ]; - if (!util.isValidFilter(filters, ['id', 'status', 'type', 'memberOnly', 'keyword']) || + if (!util.isValidFilter(filters, + ['id', 'status', 'memberOnly', 'keyword', 'name', 'code', 'customer', 'manager']) || (sort && _.indexOf(sortableProps, sort) < 0)) { return util.handleError('Invalid filters or sort', null, req, next); } diff --git a/src/routes/projects/list.spec.js b/src/routes/projects/list.spec.js index ef1bb7c6..8eee10df 100644 --- a/src/routes/projects/list.spec.js +++ b/src/routes/projects/list.spec.js @@ -37,6 +37,8 @@ const data = [ userId: 40051331, projectId: 1, role: 'customer', + firstName: 'Firstname', + lastName: 'Lastname', handle: 'test_tourist_handle', isPrimary: true, createdBy: 1, @@ -101,6 +103,19 @@ const data = [ updatedBy: 1, lastActivityAt: 3, lastActivityUserId: '1', + members: [{ + id: 5, + userId: 40051334, + projectId: 2, + role: 'manager', + firstName: 'first', + lastName: 'last', + handle: 'manager_handle', + isPrimary: true, + createdBy: 1, + updatedBy: 1, + }, + ], }, ]; @@ -193,7 +208,18 @@ describe('LIST Project', () => { lastActivityUserId: '1', }).then((p) => { project3 = p; - return Promise.resolve(); + // create members + return models.ProjectMember.create({ + userId: 40051334, + projectId: project3.id, + role: 'manager', + firstName: 'first', + lastName: 'last', + handle: 'manager_handle', + isPrimary: true, + createdBy: 1, + updatedBy: 1, + }); }); return Promise.all([p1, p2, p3]).then(() => { @@ -487,6 +513,160 @@ describe('LIST Project', () => { }); }); + it('should return project that match when filtering by id', (done) => { + request(server) + .get('/v4/projects/?filter=id%3D1*') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].id.should.equal(1); + resJson[0].name.should.equal('test1'); + done(); + } + }); + }); + + it('should return project that match when filtering by name', (done) => { + request(server) + .get('/v4/projects/?filter=name%3Dtest1') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + done(); + } + }); + }); + + it('should return project that match when filtering by name\'s substring', (done) => { + request(server) + .get('/v4/projects/?filter=name%3D*st1') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + done(); + } + }); + }); + + it('should return all projects that match when filtering by details code', (done) => { + request(server) + .get('/v4/projects/?filter=code%3Dcode1') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + resJson[0].details.utm.code.should.equal('code1'); + done(); + } + }); + }); + + it('should return all projects that match when filtering by details code\'s substring', (done) => { + request(server) + .get('/v4/projects/?filter=code%3D*de1') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + resJson[0].details.utm.code.should.equal('code1'); + done(); + } + }); + }); + + it('should return all projects that match when filtering by customer', (done) => { + request(server) + .get('/v4/projects/?filter=customer%3Dfirst*') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test1'); + resJson[0].members.should.have.deep.property('[0].role', 'customer'); + resJson[0].members[0].userId.should.equal(40051331); + done(); + } + }); + }); + + it('should return all projects that match when filtering by manager', (done) => { + request(server) + .get('/v4/projects/?filter=manager%3D*ast') + .set({ + Authorization: `Bearer ${testUtil.jwts.admin}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body.result.content; + should.exist(resJson); + resJson.should.have.lengthOf(1); + resJson[0].name.should.equal('test3'); + resJson[0].members.should.have.deep.property('[0].role', 'manager'); + resJson[0].members[0].userId.should.equal(40051334); + done(); + } + }); + }); + it('should return list of projects ordered ascending by lastActivityAt when sort column is "lastActivityAt"', (done) => { request(server) .get('/v4/projects/?sort=lastActivityAt') diff --git a/src/util.js b/src/util.js index c1852c79..5fe2b511 100644 --- a/src/util.js +++ b/src/util.js @@ -180,7 +180,7 @@ _.assignIn(util, { } return val; }); - if (queryFilter.id) { + if (queryFilter.id && queryFilter.id.$in) { queryFilter.id.$in = _.map(queryFilter.id.$in, _.parseInt); } return queryFilter; diff --git a/swagger.yaml b/swagger.yaml old mode 100755 new mode 100644 index 2d7e7c8b..eb8d063f --- a/swagger.yaml +++ b/swagger.yaml @@ -55,6 +55,10 @@ paths: - type - memberOnly - keyword + - name + - code + - customer + - manager - name: sort required: false description: | @@ -4129,4 +4133,4 @@ definitions: metadata: $ref: "#/definitions/ResponseMetadata" content: - $ref: "#/definitions/ProjectMemberInvite" + $ref: "#/definitions/ProjectMemberInvite" \ No newline at end of file From 281af4a9ca524d59e82ade4c039eb3ddfa1be69f Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Tue, 19 Feb 2019 19:11:55 +0800 Subject: [PATCH 2/2] remove filtering by id substring (only exact match support for now) --- migrations/elasticsearch_sync.js | 3 +- postman.json | 94 ++++++++++++++------------------ src/routes/projects/list.js | 19 ++++++- src/routes/projects/list.spec.js | 4 +- 4 files changed, 60 insertions(+), 60 deletions(-) diff --git a/migrations/elasticsearch_sync.js b/migrations/elasticsearch_sync.js index b116082d..349be0b6 100644 --- a/migrations/elasticsearch_sync.js +++ b/migrations/elasticsearch_sync.js @@ -209,8 +209,7 @@ function getRequestBody(indexName) { }, }, id: { - type: 'string', - index: 'not_analyzed', + type: 'long', }, members: { type: 'nested', diff --git a/postman.json b/postman.json index 86172fb6..96995389 100644 --- a/postman.json +++ b/postman.json @@ -1,6 +1,6 @@ { "info": { - "_postman_id": "97085cd7-b298-4f1c-9629-24af14ff5f13", + "_postman_id": "4fc2b7cf-404a-4fd7-b6d2-4828a3994859", "name": "tc-project-service", "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json" }, @@ -1452,7 +1452,7 @@ "response": [] }, { - "name": "List projects with filters applied", + "name": "List projects with filters - type (exact)", "request": { "method": "GET", "header": [ @@ -1466,7 +1466,7 @@ "raw": "" }, "url": { - "raw": "{{api-url}}/v4/projects?filter=type%3Dgeneric", + "raw": "{{api-url}}/v4/projects?filter=type%3Dapp", "host": [ "{{api-url}}" ], @@ -1477,7 +1477,7 @@ "query": [ { "key": "filter", - "value": "type%3Dgeneric" + "value": "type%3Dapp" } ] }, @@ -1486,10 +1486,21 @@ "response": [] }, { - "name": "List projects with filters - name, code, customer, manager", + "name": "List projects with filters - id (exact)", "request": { + "method": "GET", + "header": [ + { + "key": "Authorization", + "value": "Bearer {{jwt-token}}" + } + ], + "body": { + "mode": "raw", + "raw": "" + }, "url": { - "raw": "{{api-url}}/v4/projects?filter=id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", + "raw": "{{api-url}}/v4/projects?filter=id%3D1", "host": [ "{{api-url}}" ], @@ -1500,34 +1511,30 @@ "query": [ { "key": "filter", - "value": "id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", - "equals": true, - "description": "" + "value": "id%3D1" } - ], - "variable": [] + ] }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, + { + "name": "List projects with filters - name, code, customer, manager", + "request": { "method": "GET", "header": [ { "key": "Authorization", - "value": "Bearer {{jwt-token}}", - "description": "" + "value": "Bearer {{jwt-token}}" } ], "body": { "mode": "raw", "raw": "" }, - "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" - }, - "response": [] - }, - { - "name": "List projects with filters - id", - "request": { "url": { - "raw": "{{api-url}}/v4/projects?filter=id%3D2", + "raw": "{{api-url}}/v4/projects?filter=id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*", "host": [ "{{api-url}}" ], @@ -1538,32 +1545,28 @@ "query": [ { "key": "filter", - "value": "id%3D2", - "equals": true, - "description": "" + "value": "id%3D1*%26name%3Dtes*%26code=test*%26customer%3DDiya*%26manager=first*" } - ], - "variable": [] + ] }, + "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" + }, + "response": [] + }, + { + "name": "List projects with filters - code", + "request": { "method": "GET", "header": [ { "key": "Authorization", - "value": "Bearer {{jwt-token}}", - "description": "" + "value": "Bearer {{jwt-token}}" } ], "body": { "mode": "raw", "raw": "" }, - "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" - }, - "response": [] - }, - { - "name": "List projects with filters - code", - "request": { "url": { "raw": "{{api-url}}/v4/projects?filter=code%3Dtest*", "host": [ @@ -1576,24 +1579,9 @@ "query": [ { "key": "filter", - "value": "code%3Dtest*", - "equals": true, - "description": "" + "value": "code%3Dtest*" } - ], - "variable": [] - }, - "method": "GET", - "header": [ - { - "key": "Authorization", - "value": "Bearer {{jwt-token}}", - "description": "" - } - ], - "body": { - "mode": "raw", - "raw": "" + ] }, "description": "List all the project with filters applied. The filter string should be url encoded. Default limit and offset is applicable" }, @@ -5509,4 +5497,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/src/routes/projects/list.js b/src/routes/projects/list.js index 691d7d81..4b841f60 100755 --- a/src/routes/projects/list.js +++ b/src/routes/projects/list.js @@ -241,8 +241,6 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { values: criteria.filters.id.$in, }, }); - } else if (_.has(criteria, 'filters.id') && criteria.filters.id.indexOf('*') > -1) { - mustQuery = _.concat(mustQuery, buildEsQueryWithFilter('id', criteria.filters.id, MATCH_TYPE_WILDCARD, ['id'])); } else if (_.has(criteria, 'filters.id')) { boolQuery.push({ term: { @@ -287,6 +285,21 @@ const parseElasticSearchCriteria = (criteria, fields, order) => { }); } + if (_.has(criteria, 'filters.type.$in')) { + // type is an array + boolQuery.push({ + terms: { + type: criteria.filters.type.$in, + }, + }); + } else if (_.has(criteria, 'filters.type')) { + // type is simple string + boolQuery.push({ + term: { + type: criteria.filters.type, + }, + }); + } if (_.has(criteria, 'filters.keyword')) { // keyword is a full text search // escape special fields from keyword search @@ -400,7 +413,7 @@ module.exports = [ 'type', 'type asc', 'type desc', ]; if (!util.isValidFilter(filters, - ['id', 'status', 'memberOnly', 'keyword', 'name', 'code', 'customer', 'manager']) || + ['id', 'status', 'memberOnly', 'keyword', 'type', 'name', 'code', 'customer', 'manager']) || (sort && _.indexOf(sortableProps, sort) < 0)) { return util.handleError('Invalid filters or sort', null, req, next); } diff --git a/src/routes/projects/list.spec.js b/src/routes/projects/list.spec.js index 8eee10df..dfecec90 100644 --- a/src/routes/projects/list.spec.js +++ b/src/routes/projects/list.spec.js @@ -513,9 +513,9 @@ describe('LIST Project', () => { }); }); - it('should return project that match when filtering by id', (done) => { + it('should return project that match when filtering by id (exact)', (done) => { request(server) - .get('/v4/projects/?filter=id%3D1*') + .get('/v4/projects/?filter=id%3D1') .set({ Authorization: `Bearer ${testUtil.jwts.admin}`, })