From 0438b60bbe4c456c506100978dc4892adec952ea Mon Sep 17 00:00:00 2001 From: whereishammer Date: Wed, 16 Apr 2025 20:51:49 +0800 Subject: [PATCH 1/4] fix Attachment tests --- mock-api/app.js | 12 ++- package.json | 1 + src/common/challenge-helper.js | 19 +++-- src/common/helper.js | 15 +++- src/services/AttachmentService.js | 9 +- test/testHelper.js | 26 +++++- test/unit/AttachmentService.test.js | 126 +++++++++------------------- test/unit/ChallengeService.test.js | 4 + 8 files changed, 112 insertions(+), 100 deletions(-) diff --git a/mock-api/app.js b/mock-api/app.js index adca0c31..08530a92 100755 --- a/mock-api/app.js +++ b/mock-api/app.js @@ -61,6 +61,8 @@ app.get('/v5/resources', (req, res) => { const challengeId = req.query.challengeId winston.info(`Get resources of challenge id ${challengeId}`) + const memberId = req.query.memberId + const resources = [{ id: '22ba038e-48da-487b-96e8-8d3b99b6d181', challengeId, @@ -82,8 +84,16 @@ app.get('/v5/resources', (req, res) => { roleId: '732339e7-8e30-49d7-9198-cccf9451e221' }] + let ret + if (memberId) { + // filter with memberId + ret = _.filter(resources, r => r.memberId === memberId) + } else { + ret = resources + } + winston.info(`Challenge resources: ${JSON.stringify(resources, null, 4)}`) - res.json(resources) + res.json(ret) }) // get challenges member can access to diff --git a/package.json b/package.json index 55999041..20c12a40 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "license": "MIT", "repository": "https://github.com/topcoder-platform/challenge-api", "devDependencies": { + "aws-sdk-mock": "^6.2.1", "chai": "^4.2.0", "chai-http": "^4.2.1", "mocha": "^11.1.0", diff --git a/src/common/challenge-helper.js b/src/common/challenge-helper.js index 90573058..f524eb8f 100644 --- a/src/common/challenge-helper.js +++ b/src/common/challenge-helper.js @@ -392,15 +392,16 @@ class ChallengeHelper { if (type) { challenge.type = type.name; } - - challenge.metadata = challenge.metadata.map((m) => { - try { - m.value = JSON.stringify(JSON.parse(m.value)); // when we update how we index data, make this a JSON field - } catch (err) { - // do nothing - } - return m; - }); + if (challenge.metadata) { + challenge.metadata = challenge.metadata.map((m) => { + try { + m.value = JSON.stringify(JSON.parse(m.value)); // when we update how we index data, make this a JSON field + } catch (err) { + // do nothing + } + return m; + }); + } } static convertDateToISOString(startDate) { diff --git a/src/common/helper.js b/src/common/helper.js index fec537d3..00be19e9 100644 --- a/src/common/helper.js +++ b/src/common/helper.js @@ -29,7 +29,16 @@ AWS.config.update({ // secretAccessKey: config.AMAZON.AWS_SECRET_ACCESS_KEY, region: config.AMAZON.AWS_REGION, }); -const s3 = new AWS.S3(); + +let s3 + +// lazy initialization of S3 instance +function getS3() { + if (!s3) { + s3 = new AWS.S3(); + } + return s3 +} /** * Wrap async function to standard express function @@ -194,7 +203,7 @@ async function downloadFromFileStack(url) { * @return {Promise} promise resolved to downloaded data */ async function downloadFromS3(bucket, key) { - const file = await s3.getObject({ Bucket: bucket, Key: key }).promise(); + const file = await getS3().getObject({ Bucket: bucket, Key: key }).promise(); return { data: file.Body, mimetype: file.ContentType, @@ -208,7 +217,7 @@ async function downloadFromS3(bucket, key) { * @return {Promise} promise resolved to deleted data */ async function deleteFromS3(bucket, key) { - return s3.deleteObject({ Bucket: bucket, Key: key }).promise(); + return getS3().deleteObject({ Bucket: bucket, Key: key }).promise(); } /** diff --git a/src/services/AttachmentService.js b/src/services/AttachmentService.js index 96c2dfac..93b3e0e5 100644 --- a/src/services/AttachmentService.js +++ b/src/services/AttachmentService.js @@ -9,6 +9,10 @@ const helper = require("../common/helper"); const s3ParseUrl = require("../common/s3ParseUrl"); const logger = require("../common/logger"); const constants = require("../../app-constants"); +const { + enrichChallengeForResponse +} = require("../common/challenge-helper"); +const prismaHelper = require('../common/prisma-helper'); const bucketWhitelist = config.AMAZON.BUCKET_WHITELIST.split(",").map((bucketName) => bucketName.trim() @@ -43,8 +47,11 @@ async function _getChallengeAttachment(challengeId, attachmentId) { const challenge = await prisma.challenge.findUnique({ where: { id: challengeId } }) const attachment = await prisma.attachment.findUnique({ where: { id: attachmentId } }) if (!challenge || !challenge.id || !attachment || attachment.challengeId !== challengeId) { - throw errors.NotFoundError(`Attachment ${attachmentId} not found in challenge ${challengeId}`) + throw new errors.NotFoundError(`Attachment ${attachmentId} not found in challenge ${challengeId}`) } + // convert challenge data + enrichChallengeForResponse(challenge) + prismaHelper.convertModelToResponse(challenge) return { challenge, attachment }; } diff --git a/test/testHelper.js b/test/testHelper.js index 1c08e980..4b8fa4a7 100644 --- a/test/testHelper.js +++ b/test/testHelper.js @@ -12,6 +12,7 @@ let phase let phase2 let timelineTemplate let challenge +let taskChallenge /** * function to deeply compare arrays regardeless of the order @@ -30,6 +31,7 @@ const phase1Id = uuid() const phase2Id = uuid() const timelineTemplateId = uuid() const challengeId = uuid() +const taskChallengeId = uuid() /** * Create test data @@ -129,6 +131,27 @@ async function createData () { updatedBy: 'admin' } challenge = await prisma.challenge.create({ data: challengeData }) + + taskChallenge = await prisma.challenge.create({ data: { + id: taskChallengeId, + taskIsTask: true, + taskIsAssigned: true, + name: 'Task', + description: 'desc', + privateDescription: 'private description', + descriptionFormat: 'html', + timelineTemplate: { connect: { id: timelineTemplate.id } }, + type: { connect: { id: challengeTypeId } }, + track: { connect: { id: challengeTrackId } }, + tags: ['tag1'], + projectId: 111, + legacyId: 222, + startDate: new Date(), + status: constants.challengeStatuses.Completed.toUpperCase(), + createdAt: new Date(), + createdBy: 'admin', + updatedBy: 'admin' + }}) } const defaultProjectTerms = [ @@ -163,7 +186,7 @@ const additionalTerm = { */ async function clearData () { await prisma.challenge.deleteMany({ - where: { id: { in: [challengeId] } } + where: { id: { in: [challengeId, taskChallengeId] } } }) await prisma.timelineTemplate.deleteMany({ where: { id: timelineTemplateId } }) await prisma.phase.deleteMany({ where: { id: { in: [phase1Id, phase2Id] } } }) @@ -182,6 +205,7 @@ function getData () { phase2, timelineTemplate, challenge, + taskChallenge, defaultProjectTerms, additionalTerm, mockTerms diff --git a/test/unit/AttachmentService.test.js b/test/unit/AttachmentService.test.js index 86954f11..f4e262b0 100644 --- a/test/unit/AttachmentService.test.js +++ b/test/unit/AttachmentService.test.js @@ -7,106 +7,64 @@ const fs = require('fs') const path = require('path') const uuid = require('uuid/v4') const chai = require('chai') -const service = require('../../src/services/AttachmentService') +const awsMock = require('aws-sdk-mock') const testHelper = require('../testHelper') +const prisma = require('../../src/common/prisma').getClient() const should = chai.should() const attachmentContent = fs.readFileSync(path.join(__dirname, '../attachment.txt')) -/* +let service + describe('attachment service unit tests', () => { // created attachment id let id // generated data let data + // attachment for task challenge + let id2 const notFoundId = uuid() before(async () => { + // mock S3 before creating S3 instance + awsMock.mock('S3', 'getObject', (params, callback) => { + callback(null, { Body: Buffer.from(attachmentContent) }); + }); + // import service after setting up S3 mock + service = require('../../src/services/AttachmentService') await testHelper.createData() data = testHelper.getData() - }) - - after(async () => { - await testHelper.clearData() - }) - - describe('upload attachment tests', () => { - it('upload attachment successfully', async () => { - const result = await service.uploadAttachment({ - isMachine: true - }, data.challenge.id, { - attachment: { - data: attachmentContent, - mimetype: 'text/plain', - name: 'attachment.txt', - size: attachmentContent.length - } - }) - should.exist(result.id) - id = result.id - should.equal(result.fileSize, attachmentContent.length) - should.equal(result.fileName, 'attachment.txt') - should.equal(result.challengeId, data.challenge.id) - }) - - it('upload attachment - forbidden', async () => { - try { - await service.uploadAttachment({ - roles: ['user'] - }, data.challenge.id, { - attachment: { - data: attachmentContent, - mimetype: 'text/plain', - name: 'attachment.txt', - size: attachmentContent.length - } - }) - } catch (e) { - should.equal(e.message, 'You are not allowed to upload attachment of the challenge.') - return + // create attachment + const createdAttachment = await prisma.attachment.create({ + data: { + name: 'attachment.txt', + url: 'http://s3.amazonaws.com/topcoder_01/attachment.txt', + fileSize: 1024, + createdBy: 'testdata', + updatedBy: 'testdata', + challenge: { connect: { id: data.challenge.id } } } - throw new Error('should not reach here') }) - - it('upload attachment - file too large', async () => { - try { - await service.uploadAttachment({ - isMachine: true - }, data.challenge.id, { - attachment: { - truncated: true, - data: attachmentContent, - mimetype: 'text/plain', - name: 'attachment.txt', - size: attachmentContent.length - } - }) - } catch (e) { - should.equal(e.message.indexOf('attachment is too large') >= 0, true) - return + id = createdAttachment.id + const taskAttachment = await prisma.attachment.create({ + data: { + name: 'attachment.txt', + url: 'http://s3.amazonaws.com/topcoder_01/attachment.txt', + fileSize: 1024, + createdBy: 'testdata', + updatedBy: 'testdata', + challenge: { connect: { id: data.taskChallenge.id } } } - throw new Error('should not reach here') }) + id2 = taskAttachment.id + }) - it('upload attachment - challenge not found', async () => { - try { - await service.uploadAttachment({ - isMachine: true - }, notFoundId, { - attachment: { - data: attachmentContent, - mimetype: 'text/plain', - name: 'attachment.txt', - size: attachmentContent.length - } - }) - } catch (e) { - should.equal(e.message, `Challenge with id: ${notFoundId} doesn't exist`) - return - } - throw new Error('should not reach here') - }) + after(async () => { + await testHelper.clearData() + await prisma.attachment.deleteMany({ where: { id }}) + // restore S3 + awsMock.restore('S3'); }) describe('download attachment tests', () => { @@ -118,9 +76,9 @@ describe('attachment service unit tests', () => { it('download attachment - forbidden', async () => { try { - await service.downloadAttachment({ roles: ['user'], userId: 678678 }, data.challenge.id, id) + await service.downloadAttachment({ roles: ['user'], userId: 678678 }, data.taskChallenge.id, id2) } catch (e) { - should.equal(e.message, 'You are not allowed to download attachment of the challenge.') + should.equal(e.message, 'You don\'t have access to view this challenge') return } throw new Error('should not reach here') @@ -130,7 +88,7 @@ describe('attachment service unit tests', () => { try { await service.downloadAttachment({ isMachine: true }, data.challenge.id, notFoundId) } catch (e) { - should.equal(e.message, `Attachment with id: ${notFoundId} doesn't exist`) + should.equal(e.message, `Attachment ${notFoundId} not found in challenge ${data.challenge.id}`) return } throw new Error('should not reach here') @@ -140,7 +98,7 @@ describe('attachment service unit tests', () => { try { await service.downloadAttachment({ isMachine: true }, notFoundId, id) } catch (e) { - should.equal(e.message, 'The attachment challengeId does not match the path challengeId.') + should.equal(e.message, `Attachment ${id} not found in challenge ${notFoundId}`) return } throw new Error('should not reach here') @@ -167,5 +125,3 @@ describe('attachment service unit tests', () => { }) }) }) - -*/ diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js index 748075ae..a69e022c 100644 --- a/test/unit/ChallengeService.test.js +++ b/test/unit/ChallengeService.test.js @@ -373,7 +373,11 @@ describe('challenge service unit tests', () => { page: 1, perPage: 10, id: id, +<<<<<<< HEAD +======= + +>>>>>>> bf8ac4b (fix Attachment tests) typeId: testChallengeData.typeId, name: testChallengeData.name.substring(2).trim(), description: testChallengeData.description, From 84693e5343e57fc8f92ee444544e10f086ade205 Mon Sep 17 00:00:00 2001 From: whereishammer Date: Wed, 16 Apr 2025 21:17:46 +0800 Subject: [PATCH 2/4] fix merge conflict in ChallengeService test --- test/unit/ChallengeService.test.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js index a69e022c..a22dfedb 100644 --- a/test/unit/ChallengeService.test.js +++ b/test/unit/ChallengeService.test.js @@ -373,11 +373,6 @@ describe('challenge service unit tests', () => { page: 1, perPage: 10, id: id, -<<<<<<< HEAD - -======= - ->>>>>>> bf8ac4b (fix Attachment tests) typeId: testChallengeData.typeId, name: testChallengeData.name.substring(2).trim(), description: testChallengeData.description, From 74dad09ad73b95f80592de87f4ce27f7a372d2b8 Mon Sep 17 00:00:00 2001 From: whereishammer Date: Wed, 16 Apr 2025 21:22:36 +0800 Subject: [PATCH 3/4] fix typo in ChallengeService test --- test/unit/ChallengeService.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js index a22dfedb..748075ae 100644 --- a/test/unit/ChallengeService.test.js +++ b/test/unit/ChallengeService.test.js @@ -373,6 +373,7 @@ describe('challenge service unit tests', () => { page: 1, perPage: 10, id: id, + typeId: testChallengeData.typeId, name: testChallengeData.name.substring(2).trim(), description: testChallengeData.description, From 257eb8fedba3f41d6148288672842990f3ec62aa Mon Sep 17 00:00:00 2001 From: whereishammer Date: Wed, 16 Apr 2025 21:24:35 +0800 Subject: [PATCH 4/4] revert changes in AttachmentService test --- test/unit/AttachmentService.test.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/unit/AttachmentService.test.js b/test/unit/AttachmentService.test.js index f4e262b0..4673c078 100644 --- a/test/unit/AttachmentService.test.js +++ b/test/unit/AttachmentService.test.js @@ -8,6 +8,7 @@ const path = require('path') const uuid = require('uuid/v4') const chai = require('chai') const awsMock = require('aws-sdk-mock') +const service = require('../../src/services/AttachmentService') const testHelper = require('../testHelper') const prisma = require('../../src/common/prisma').getClient() @@ -15,8 +16,6 @@ const should = chai.should() const attachmentContent = fs.readFileSync(path.join(__dirname, '../attachment.txt')) -let service - describe('attachment service unit tests', () => { // created attachment id let id @@ -31,8 +30,6 @@ describe('attachment service unit tests', () => { awsMock.mock('S3', 'getObject', (params, callback) => { callback(null, { Body: Buffer.from(attachmentContent) }); }); - // import service after setting up S3 mock - service = require('../../src/services/AttachmentService') await testHelper.createData() data = testHelper.getData() // create attachment