From 6d153c0c832f5a03fc572312fb23f46f9404db03 Mon Sep 17 00:00:00 2001 From: Mithun Kamath Date: Tue, 30 Jun 2020 21:12:34 +0530 Subject: [PATCH 1/2] #179 - Support v5 challenge ids but ensure that we store legacy in db --- README.md | 4 --- config/default.js | 1 - package.json | 1 - scripts/ESloadHelper.js | 10 ++++---- scripts/createIndex.js | 4 +-- scripts/createNewIndex.js | 36 -------------------------- scripts/deleteIndex.js | 5 +--- scripts/migrateFromDBToES.js | 2 +- src/common/helper.js | 39 +++++++++++++---------------- src/services/SubmissionService.js | 20 +++++++++++++-- test/unit/SubmissionService.test.js | 2 +- 11 files changed, 46 insertions(+), 78 deletions(-) delete mode 100644 scripts/createNewIndex.js diff --git a/README.md b/README.md index 6a581ce3..8deeab04 100755 --- a/README.md +++ b/README.md @@ -116,10 +116,6 @@ This script will load the data from `scripts/data` directory into ES npm run start ``` -#### Duplicating the ES Index - -To duplicate the existing ES Index (from the `ES_INDEX` to `ES_INDEX_V2` based on the configs in `config/default.js`) run `npm run create-new-index` - #### Linting JS files ``` diff --git a/config/default.js b/config/default.js index 7fa37340..d5df5d96 100755 --- a/config/default.js +++ b/config/default.js @@ -32,7 +32,6 @@ module.exports = { HOST: process.env.ES_HOST || 'localhost:9200', API_VERSION: process.env.ES_API_VERSION || '6.3', ES_INDEX: process.env.ES_INDEX || 'submission', - ES_INDEX_V2: process.env.ES_INDEX_V2 || 'new_submission', ES_TYPE: process.env.ES_TYPE || '_doc' // ES 6.x accepts only 1 Type per index and it's mandatory to define it }, PAGE_SIZE: process.env.PAGE_SIZE || 20, diff --git a/package.json b/package.json index 21b9dafc..42516c16 100755 --- a/package.json +++ b/package.json @@ -11,7 +11,6 @@ "create-tables": "node scripts/createTables.js", "init-db": "node scripts/importData.js", "create-index": "node scripts/createIndex.js", - "create-new-index": "node scripts/createNewIndex.js", "delete-index": "node scripts/deleteIndex.js", "init-es": "node scripts/loadES.js", "db-to-es": "node scripts/migrateFromDBToES.js", diff --git a/scripts/ESloadHelper.js b/scripts/ESloadHelper.js index 2237f9e1..b28e348b 100644 --- a/scripts/ESloadHelper.js +++ b/scripts/ESloadHelper.js @@ -20,7 +20,7 @@ const esClient = helper.getEsClient() function deleteDatafromES () { logger.info('Clear data from ES if any') const filter = { - index: config.get('esConfig.ES_INDEX_V2'), + index: config.get('esConfig.ES_INDEX'), type: config.get('esConfig.ES_TYPE'), q: '*' } @@ -34,7 +34,7 @@ function * loadReviewTypes () { const promises = [] reviewTypes.forEach((reviewType) => { const record = { - index: config.get('esConfig.ES_INDEX_V2'), + index: config.get('esConfig.ES_INDEX'), type: config.get('esConfig.ES_TYPE'), id: reviewType.id, body: _.extend({ resource: 'reviewType' }, reviewType) @@ -51,7 +51,7 @@ function * loadSubmissions () { const promises = [] submissions.forEach((submission) => { const record = { - index: config.get('esConfig.ES_INDEX_V2'), + index: config.get('esConfig.ES_INDEX'), type: config.get('esConfig.ES_TYPE'), id: submission.id, body: _.extend({ resource: 'submission' }, submission) @@ -68,7 +68,7 @@ function * loadReviews () { const promises = [] reviews.forEach((review) => { const record = { - index: config.get('esConfig.ES_INDEX_V2'), + index: config.get('esConfig.ES_INDEX'), type: config.get('esConfig.ES_TYPE'), id: review.id, body: _.extend({ resource: 'review' }, review) @@ -85,7 +85,7 @@ function * loadReviewSummations () { const promises = [] reviewSummations.forEach((reviewSummation) => { const record = { - index: config.get('esConfig.ES_INDEX_V2'), + index: config.get('esConfig.ES_INDEX'), type: config.get('esConfig.ES_TYPE'), id: reviewSummation.id, body: _.extend({ resource: 'reviewSummation' }, reviewSummation) diff --git a/scripts/createIndex.js b/scripts/createIndex.js index 9957be4e..f25ff1de 100644 --- a/scripts/createIndex.js +++ b/scripts/createIndex.js @@ -17,7 +17,7 @@ co(function * createIndex () { // fields not specified below will be 'text' by default properties: { resource: { type: 'keyword' }, - challengeId: { type: 'keyword' }, + challengeId: { type: 'long' }, memberId: { type: 'keyword' }, type: { type: 'keyword' }, isFileSubmission: { type: 'boolean' }, @@ -44,7 +44,7 @@ co(function * createIndex () { } } yield esClient.indices.create({ - index: config.get('esConfig.ES_INDEX_V2'), + index: config.get('esConfig.ES_INDEX'), body }) logger.info('ES Index creation succeeded!') diff --git a/scripts/createNewIndex.js b/scripts/createNewIndex.js deleted file mode 100644 index 166cfe41..00000000 --- a/scripts/createNewIndex.js +++ /dev/null @@ -1,36 +0,0 @@ -/** - * Copies the existing index to a new index and updates the type of challengeId - * to be keyword (It is long in the source index) - */ - -const co = require('co') -const config = require('config') -const logger = require('../src/common/logger') -const helper = require('../src/common/helper') - -co(function * createIndex () { - logger.info('ES Index creation started!') - const esClient = helper.getEsClient() - const indices = yield esClient.indices.get({ - index: config.get('esConfig.ES_INDEX') - }) - const existingIndex = indices[config.get('esConfig.ES_INDEX')] - const existingMappings = existingIndex.mappings[config.get('esConfig.ES_TYPE')] - const body = { mappings: {} } - body.mappings[config.get('esConfig.ES_TYPE')] = { - properties: { - ...existingMappings.properties, - challengeId: { type: 'keyword' }, - memberId: { type: 'keyword' } - } - } - yield esClient.indices.create({ - index: config.get('esConfig.ES_INDEX_V2'), - body - }) - logger.info('ES Index creation succeeded!') - process.exit(0) -}).catch((err) => { - logger.logFullError(err) - process.exit(1) -}) diff --git a/scripts/deleteIndex.js b/scripts/deleteIndex.js index 082e5ea2..9e3b37ab 100644 --- a/scripts/deleteIndex.js +++ b/scripts/deleteIndex.js @@ -10,11 +10,8 @@ const helper = require('../src/common/helper') co(function * deleteIndex () { logger.info('ES Index deletion started!') const esClient = helper.getEsClient() - // yield esClient.indices.delete({ - // index: config.get('esConfig.ES_INDEX') - // }) yield esClient.indices.delete({ - index: config.get('esConfig.ES_INDEX_V2') + index: config.get('esConfig.ES_INDEX') }) logger.info('ES Index deletion succeeded!') process.exit(0) diff --git a/scripts/migrateFromDBToES.js b/scripts/migrateFromDBToES.js index 3dc9e7ed..7cd762de 100644 --- a/scripts/migrateFromDBToES.js +++ b/scripts/migrateFromDBToES.js @@ -30,7 +30,7 @@ function * migrateRecords (tableName) { logger.debug(`Number of ${tableName}s fetched from DB - ` + totalRecords) for (let i = 0; i < totalRecords; i++) { const record = { - index: config.get('esConfig.ES_INDEX_V2'), + index: config.get('esConfig.ES_INDEX'), type: config.get('esConfig.ES_TYPE'), id: records.Items[i].id, body: { diff --git a/src/common/helper.js b/src/common/helper.js index 30c43ca4..5b015d5e 100755 --- a/src/common/helper.js +++ b/src/common/helper.js @@ -178,7 +178,7 @@ function prepESFilter (query, actResource) { } const searchCriteria = { - index: config.get('esConfig.ES_INDEX_V2'), + index: config.get('esConfig.ES_INDEX'), type: config.get('esConfig.ES_TYPE'), size: pageSize, from: (page - 1) * pageSize, // Es Index starts from 0 @@ -345,14 +345,18 @@ function * getM2Mtoken (parentSpan) { * @param {Object} parentSpan the parent Span object * @returns {String} Legacy Challenge ID of the given challengeId */ -function * getlegacyChallengeId (challengeId, token, parentSpan) { +function * getLegacyChallengeId (challengeId, parentSpan) { if (/^[0-9a-f]{8}-[0-9a-f]{4}-[0-5][0-9a-f]{3}-[089ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(challengeId)) { + logger.debug(`${challengeId} detected as uuid. Fetching legacy challenge id`) const getlegacyChallengeIdSpan = tracer.startChildSpans('helper.getlegacyChallengeId', parentSpan) + const token = yield getM2Mtoken(getlegacyChallengeIdSpan) try { const response = yield request.get(`${config.CHALLENGEAPI_V5_URL}/${challengeId}`) .set('Authorization', `Bearer ${token}`) .set('Content-Type', 'application/json') - return response.body.legacyId + const legacyId = parseInt(response.body.legacyId, 10) + logger.debug(`Legacy challenge id is ${legacyId} for v5 challenge id ${challengeId}`) + return legacyId } catch (err) { getlegacyChallengeIdSpan.setTag('error', true) logger.error(`Error while accessing ${config.CHALLENGEAPI_V5_URL}/${challengeId}`) @@ -381,19 +385,17 @@ function * getSubmissionPhaseId (challengeId, parentSpan) { try { let phaseId = null let response - let legacyChallengeId const token = yield getM2Mtoken(getSubmissionPhaseIdSpan) const getChallengePhasesSpan = tracer.startChildSpans('getChallengePhases', getSubmissionPhaseIdSpan) getChallengePhasesSpan.setTag('challengeId', challengeId) try { - legacyChallengeId = yield getlegacyChallengeId(challengeId, token, getChallengePhasesSpan) - response = yield request.get(`${config.CHALLENGEAPI_URL}/${legacyChallengeId}/phases`) + response = yield request.get(`${config.CHALLENGEAPI_URL}/${challengeId}/phases`) .set('Authorization', `Bearer ${token}`) .set('Content-Type', 'application/json') } catch (ex) { - logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}/${legacyChallengeId}/phases`) + logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}/${challengeId}/phases`) logger.debug('Setting submissionPhaseId to Null') response = null // log error @@ -437,7 +439,6 @@ function * checkCreateAccess (authUser, subEntity, parentSpan) { try { let response - let legacyChallengeId // User can only create submission for themselves if (authUser.userId !== subEntity.memberId) { @@ -449,12 +450,11 @@ function * checkCreateAccess (authUser, subEntity, parentSpan) { const getChallengeDetailSpan = tracer.startChildSpans('getChallengeDetail', checkCreateAccessSpan) getChallengeDetailSpan.setTag('challengeId', subEntity.challengeId) try { - legacyChallengeId = yield getlegacyChallengeId(subEntity.challengeId, token, getChallengeDetailSpan) - response = yield request.get(`${config.CHALLENGEAPI_URL}?filter=id=${legacyChallengeId}`) + response = yield request.get(`${config.CHALLENGEAPI_URL}?filter=id=${subEntity.challengeId}`) .set('Authorization', `Bearer ${token}`) .set('Content-Type', 'application/json') } catch (ex) { - logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}?filter=id=${legacyChallengeId}`) + logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}?filter=id=${subEntity.challengeId}`) logger.error(ex) // log error getChallengeDetailSpan.log({ @@ -506,7 +506,6 @@ function * checkGetAccess (authUser, submission, parentSpan) { try { let resources let challengeDetails - let legacyChallengeId // Allow downloading Own submission if (submission.memberId === authUser.userId) { return true @@ -517,12 +516,11 @@ function * checkGetAccess (authUser, submission, parentSpan) { const getChallengeResourcesSpan = tracer.startChildSpans('getChallengeResources', checkGetAccessSpan) getChallengeResourcesSpan.setTag('challengeId', submission.challengeId) try { - legacyChallengeId = yield getlegacyChallengeId(submission.challengeId, token, getChallengeResourcesSpan) - resources = yield request.get(`${config.CHALLENGEAPI_URL}/${legacyChallengeId}/resources`) + resources = yield request.get(`${config.CHALLENGEAPI_URL}/${submission.challengeId}/resources`) .set('Authorization', `Bearer ${token}`) .set('Content-Type', 'application/json') } catch (ex) { - logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}/${legacyChallengeId}/resources`) + logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}/${submission.challengeId}/resources`) logger.error(ex) // log error getChallengeResourcesSpan.log({ @@ -538,11 +536,11 @@ function * checkGetAccess (authUser, submission, parentSpan) { const getChallengeDetailSpan = tracer.startChildSpans('getChallengeDetail', checkGetAccessSpan) getChallengeDetailSpan.setTag('challengeId', submission.challengeId) try { - challengeDetails = yield request.get(`${config.CHALLENGEAPI_URL}?filter=id=${legacyChallengeId}`) + challengeDetails = yield request.get(`${config.CHALLENGEAPI_URL}?filter=id=${submission.challengeId}`) .set('Authorization', `Bearer ${token}`) .set('Content-Type', 'application/json') } catch (ex) { - logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}?filter=id=${legacyChallengeId}`) + logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}?filter=id=${submission.challengeId}`) logger.error(ex) // log error getChallengeDetailSpan.log({ @@ -636,18 +634,16 @@ function * checkReviewGetAccess (authUser, submission, parentSpan) { try { let challengeDetails - let legacyChallengeId const token = yield getM2Mtoken(checkReviewGetAccessSpan) const getChallengeDetailSpan = tracer.startChildSpans('getChallengeDetail', checkReviewGetAccessSpan) getChallengeDetailSpan.setTag('challengeId', submission.challengeId) try { - legacyChallengeId = yield getlegacyChallengeId(submission.challengeId, token, getChallengeDetailSpan) - challengeDetails = yield request.get(`${config.CHALLENGEAPI_URL}?filter=id=${legacyChallengeId}`) + challengeDetails = yield request.get(`${config.CHALLENGEAPI_URL}?filter=id=${submission.challengeId}`) .set('Authorization', `Bearer ${token}`) .set('Content-Type', 'application/json') } catch (ex) { - logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}?filter=id=${legacyChallengeId}`) + logger.error(`Error while accessing ${config.CHALLENGEAPI_URL}?filter=id=${submission.challengeId}`) // log error getChallengeDetailSpan.log({ @@ -800,6 +796,7 @@ module.exports = { fetchFromES, camelize, setPaginationHeaders, + getLegacyChallengeId, getSubmissionPhaseId, checkCreateAccess, checkGetAccess, diff --git a/src/services/SubmissionService.js b/src/services/SubmissionService.js index 1398dc87..c53042f7 100755 --- a/src/services/SubmissionService.js +++ b/src/services/SubmissionService.js @@ -250,6 +250,12 @@ function * listSubmissions (authUser, query, span) { const listSubmissionsSpan = tracer.startChildSpans('SubmissionService.listSubmissions', span) let data = [] + if (query.challengeId) { + // Submission api only works with legacy challenge id + // If it is a v5 challenge id, get the associated legacy challenge id + query.challengeId = yield helper.getLegacyChallengeId(query.challengeId, listSubmissionsSpan) + } + try { data = yield helper.fetchFromES(query, helper.camelize(table), listSubmissionsSpan) logger.info(`listSubmissions: returning ${data.rows.length} submissions for query: ${JSON.stringify(query)}`) @@ -341,6 +347,10 @@ function * createSubmission (authUser, files, entity, span) { throw new errors.HttpStatusError(400, 'The file should be uploaded under the "submission" attribute') } + // Submission api only works with legacy challenge id + // If it is a v5 challenge id, get the associated legacy challenge id + const challengeId = yield helper.getLegacyChallengeId(entity.challengeId, createSubmissionSpan) + const currDate = (new Date()).toISOString() const item = { @@ -348,7 +358,7 @@ function * createSubmission (authUser, files, entity, span) { type: entity.type, url: url, memberId: entity.memberId, - challengeId: entity.challengeId, + challengeId: challengeId, created: currDate, updated: currDate, createdBy: authUser.handle || authUser.sub, @@ -366,7 +376,7 @@ function * createSubmission (authUser, files, entity, span) { if (entity.submissionPhaseId) { item.submissionPhaseId = entity.submissionPhaseId } else { - item.submissionPhaseId = yield helper.getSubmissionPhaseId(entity.challengeId, createSubmissionSpan) + item.submissionPhaseId = yield helper.getSubmissionPhaseId(challengeId, createSubmissionSpan) } if (entity.fileType) { @@ -457,6 +467,12 @@ function * _updateSubmission (authUser, submissionId, entity, parentSpan) { throw new errors.HttpStatusError(404, `Submission with ID = ${submissionId} is not found`) } + if (entity.challengeId) { + // Submission api only works with legacy challenge id + // If it is a v5 challenge id, get the associated legacy challenge id + entity.challengeId = yield helper.getLegacyChallengeId(entity.challengeId, updateSubmissionSpan) + } + const currDate = (new Date()).toISOString() // Record used for updating in Database const record = { diff --git a/test/unit/SubmissionService.test.js b/test/unit/SubmissionService.test.js index 7943b113..92c9e885 100644 --- a/test/unit/SubmissionService.test.js +++ b/test/unit/SubmissionService.test.js @@ -304,7 +304,7 @@ describe('Submission Service tests', () => { res.should.have.status(200) res.body.should.have.keys(Object.keys(_.extend({ fileType: 'zip' }, testSubmission.Item))) res.body.id.should.not.be.eql(null) - res.body.challengeId.should.be.eql(testChallengeV5APIResponse.id) + res.body.challengeId.should.be.eql(parseInt(testChallengeV5APIResponse.legacyId, 10)) res.body.type.should.be.eql(testSubmission.Item.type) res.body.url.should.be.eql(testSubmission.Item.url) res.body.fileType.should.be.eql('zip') From b86c0454e00b13cdf3dccd37bbb82fc639479f82 Mon Sep 17 00:00:00 2001 From: Mithun Kamath Date: Tue, 30 Jun 2020 21:47:04 +0530 Subject: [PATCH 2/2] Fix test --- test/e2e/SubmissionService.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/SubmissionService.test.js b/test/e2e/SubmissionService.test.js index 554d91e3..2912f209 100644 --- a/test/e2e/SubmissionService.test.js +++ b/test/e2e/SubmissionService.test.js @@ -111,7 +111,7 @@ describe('Submission Service tests', () => { .attach('submission', './test/common/fileToUpload.zip', 'fileToUpload.zip') .end((err, res) => { res.should.have.status(400) - res.body.message.should.be.eql('Either file to be uploaded or URL should be present') + res.body.message.should.be.eql('Either file to be uploaded or URL should be present. Not both.') done() }) })