Skip to content

Commit 51167cf

Browse files
jmgasperjmgasper
authored
Merge pull request #857 from topcoder-platform/develop
HOTFIX - better handling of large uploads
2 parents 0b058d1 + ccfcc6e commit 51167cf

File tree

5 files changed

+103
-118
lines changed

5 files changed

+103
-118
lines changed

‎.circleci/config.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ workflows:
149149
context : org-global
150150
filters:
151151
branches:
152-
only: ['develop', 'migration-setup', 'pm-1611_1', 'pm-1836_prod']
152+
only: ['develop', 'migration-setup', 'PM-1612', 'fix-project-exposing']
153153
- deployProd:
154154
context : org-global
155155
filters:

‎src/routes/attachments/create.js‎

Lines changed: 78 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ module.exports = [
3939
* Add project attachment
4040
* In development mode we have to mock the ec2 file transfer and file service calls
4141
*/
42-
(req, res, next) => {
42+
async (req, res, next) => {
4343
const data = req.body;
4444
// default values
4545
const projectId = req.params.projectId;
@@ -53,64 +53,22 @@ module.exports = [
5353
// extract file name
5454
const fileName = Path.parse(data.path).base;
5555
// create file path
56-
const path = _.join([
56+
const attachmentPath = _.join([
5757
config.get('projectAttachmentPathPrefix'),
5858
data.projectId,
5959
config.get('projectAttachmentPathPrefix'),
6060
fileName,
6161
], '/');
62-
let newAttachment = null;
6362

6463
const sourceBucket = data.s3Bucket;
6564
const sourceKey = data.path;
6665
const destBucket = config.get('attachmentsS3Bucket');
67-
const destKey = path;
66+
const destKey = attachmentPath;
6867

69-
if (data.type === ATTACHMENT_TYPES.LINK) {
70-
// We create the record in the db and return (i.e. no need to handle transferring file between S3 buckets)
71-
Promise.resolve(models.ProjectAttachment.create({
72-
projectId,
73-
allowedUsers,
74-
createdBy: req.authUser.userId,
75-
updatedBy: req.authUser.userId,
76-
title: data.title,
77-
size: data.size,
78-
category: data.category || null,
79-
description: data.description,
80-
contentType: data.contentType,
81-
path: data.path,
82-
type: data.type,
83-
tags: data.tags,
84-
})).then((_link) => {
85-
const link = _link.get({ plain: true });
86-
req.log.debug('New Link Attachment record: ', link);
87-
88-
// emit the Kafka event
89-
util.sendResourceToKafkaBus(
90-
req,
91-
EVENT.ROUTING_KEY.PROJECT_ATTACHMENT_ADDED,
92-
RESOURCES.ATTACHMENT,
93-
link);
94-
95-
res.status(201).json(link);
96-
return Promise.resolve();
97-
})
98-
.catch((error) => {
99-
req.log.error('Error adding link attachment', error);
100-
const rerr = error;
101-
rerr.status = rerr.status || 500;
102-
next(rerr);
103-
});
104-
} else {
105-
// don't actually transfer file in development mode if file uploading is disabled, so we can test this endpoint
106-
const fileTransferPromise = (process.env.NODE_ENV !== 'development' || config.get('enableFileUpload') === 'true')
107-
? util.s3FileTransfer(req, sourceBucket, sourceKey, destBucket, destKey)
108-
: Promise.resolve();
109-
110-
fileTransferPromise.then(() => {
111-
// file copied to final destination, create DB record
112-
req.log.debug('creating db file record');
113-
return models.ProjectAttachment.create({
68+
try {
69+
if (data.type === ATTACHMENT_TYPES.LINK) {
70+
// Create the record and return immediately (no file transfer needed)
71+
const linkInstance = await models.ProjectAttachment.create({
11472
projectId,
11573
allowedUsers,
11674
createdBy: req.authUser.userId,
@@ -120,60 +78,84 @@ module.exports = [
12078
category: data.category || null,
12179
description: data.description,
12280
contentType: data.contentType,
123-
path,
81+
path: data.path,
12482
type: data.type,
12583
tags: data.tags,
12684
});
127-
}).then((_newAttachment) => {
128-
newAttachment = _newAttachment.get({ plain: true });
129-
req.log.debug('New Attachment record: ', newAttachment);
130-
if (process.env.NODE_ENV !== 'development' || config.get('enableFileUpload') === 'true') {
131-
// retrieve download url for the response
132-
req.log.debug('retrieving download url');
133-
return getDownloadUrl(destBucket, path);
134-
}
135-
return Promise.resolve();
136-
}).then((url) => {
137-
if (
138-
process.env.NODE_ENV !== 'development' ||
139-
config.get('enableFileUpload') === 'true'
140-
) {
141-
req.log.debug('Retreiving Presigned Url resp: ', url);
142-
let response = _.cloneDeep(newAttachment);
143-
response = _.omit(response, ['path', 'deletedAt']);
144-
145-
response.downloadUrl = url;
146-
147-
// emit the event
148-
util.sendResourceToKafkaBus(
149-
req,
150-
EVENT.ROUTING_KEY.PROJECT_ATTACHMENT_ADDED,
151-
RESOURCES.ATTACHMENT,
152-
newAttachment,
153-
);
154-
res.status(201).json(response);
155-
return Promise.resolve();
156-
}
157-
let response = _.cloneDeep(newAttachment);
158-
response = _.omit(response, ['path', 'deletedAt']);
159-
// only in development mode
160-
response.downloadUrl = path;
161-
// emit the event
85+
const link = linkInstance.get({ plain: true });
86+
req.log.debug('New Link Attachment record: ', link);
87+
16288
util.sendResourceToKafkaBus(
16389
req,
16490
EVENT.ROUTING_KEY.PROJECT_ATTACHMENT_ADDED,
16591
RESOURCES.ATTACHMENT,
166-
newAttachment);
167-
168-
res.status(201).json(response);
169-
return Promise.resolve();
170-
})
171-
.catch((error) => {
172-
req.log.error('Error adding file attachment', error);
173-
const rerr = error;
174-
rerr.status = rerr.status || 500;
175-
next(rerr);
176-
});
92+
link,
93+
);
94+
95+
res.status(201).json(link);
96+
return;
97+
}
98+
99+
const shouldTransfer = process.env.NODE_ENV !== 'development' || config.get('enableFileUpload') === 'true';
100+
const downloadUrlPromise = shouldTransfer
101+
? getDownloadUrl(destBucket, destKey)
102+
: Promise.resolve(destKey);
103+
104+
req.log.debug('creating db file record');
105+
const attachmentInstance = await models.ProjectAttachment.create({
106+
projectId,
107+
allowedUsers,
108+
createdBy: req.authUser.userId,
109+
updatedBy: req.authUser.userId,
110+
title: data.title,
111+
size: data.size,
112+
category: data.category || null,
113+
description: data.description,
114+
contentType: data.contentType,
115+
path: destKey,
116+
type: data.type,
117+
tags: data.tags,
118+
});
119+
120+
const newAttachment = attachmentInstance.get({ plain: true });
121+
req.log.debug('New Attachment record: ', newAttachment);
122+
123+
const downloadUrl = await downloadUrlPromise;
124+
req.log.debug('Retrieved presigned url for new attachment');
125+
126+
let response = _.cloneDeep(newAttachment);
127+
response = _.omit(response, ['path', 'deletedAt']);
128+
response.downloadUrl = downloadUrl;
129+
130+
util.sendResourceToKafkaBus(
131+
req,
132+
EVENT.ROUTING_KEY.PROJECT_ATTACHMENT_ADDED,
133+
RESOURCES.ATTACHMENT,
134+
newAttachment,
135+
);
136+
137+
res.status(201).json(response);
138+
139+
if (shouldTransfer) {
140+
util.s3FileTransfer(req, sourceBucket, sourceKey, destBucket, destKey)
141+
.then(() => {
142+
req.log.debug('File attachment copied asynchronously', { attachmentId: newAttachment.id });
143+
})
144+
.catch((error) => {
145+
req.log.error('Async S3 file transfer failed', {
146+
error: error.message,
147+
stack: error.stack,
148+
attachmentId: newAttachment.id,
149+
source: `${sourceBucket}/${sourceKey}`,
150+
destination: `${destBucket}/${destKey}`,
151+
});
152+
});
153+
}
154+
} catch (error) {
155+
req.log.error('Error adding attachment', error);
156+
const rerr = error;
157+
rerr.status = rerr.status || 500;
158+
next(rerr);
177159
}
178160
},
179161
];

‎src/routes/copilotOpportunity/get.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ module.exports = [
1010
}
1111

1212
const isAdminOrManager = util.hasRoles(req, [USER_ROLE.CONNECT_ADMIN, USER_ROLE.TOPCODER_ADMIN, USER_ROLE.PROJECT_MANAGER]);
13+
1314
return models.CopilotOpportunity.findOne({
1415
where: { id },
1516
include: isAdminOrManager ? [
@@ -43,6 +44,7 @@ module.exports = [
4344
if (req.authUser) {
4445
canApplyAsCopilot = !memberIds.includes(req.authUser.userId)
4546
}
47+
4648
if (plainOpportunity.project) {
4749
// This shouldn't be exposed to the clientside
4850
delete plainOpportunity.project.members;

‎src/routes/copilotOpportunity/list.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ module.exports = [
4343
baseOrder.push([sortParams[0], sortParams[1]]);
4444

4545
return models.CopilotOpportunity.findAll({
46-
include: isAdminOrManager ? [
46+
include: isAdminOrManager ?[
4747
{
4848
model: models.CopilotRequest,
4949
as: 'copilotRequest',
@@ -57,7 +57,7 @@ module.exports = [
5757
{
5858
model: models.CopilotRequest,
5959
as: 'copilotRequest',
60-
},
60+
}
6161
],
6262
order: baseOrder,
6363
limit,

‎src/routes/copilotOpportunityApply/list.js‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,12 @@
11
import _ from 'lodash';
2-
import { middleware as tcMiddleware } from 'tc-core-library-js';
32

43
import models from '../../models';
54
import { ADMIN_ROLES } from '../../constants';
65
import util from '../../util';
76

8-
const permissions = tcMiddleware.permissions;
9-
107
module.exports = [
11-
permissions('copilotApplications.view'),
128
(req, res, next) => {
13-
const canAccessAllApplications = util.hasRoles(req, ADMIN_ROLES) || util.hasProjectManagerRole(req);
14-
const userId = req.authUser.userId;
9+
const isAdminOrPM = util.hasRoles(req, ADMIN_ROLES) || util.hasProjectManagerRole(req);
1510
const opportunityId = _.parseInt(req.params.id);
1611

1712
let sort = req.query.sort ? decodeURIComponent(req.query.sort) : 'createdAt desc';
@@ -24,17 +19,15 @@ module.exports = [
2419
}
2520
const sortParams = sort.split(' ');
2621

27-
// Admin can see all requests and the PM can only see requests created by them
2822
const whereCondition = _.assign({
2923
opportunityId,
3024
},
31-
canAccessAllApplications ? {} : { createdBy: userId },
3225
);
3326

3427
return models.CopilotOpportunity.findOne({
3528
where: {
3629
id: opportunityId,
37-
}
30+
},
3831
}).then((opportunity) => {
3932
if (!opportunity) {
4033
const err = new Error('No opportunity found');
@@ -51,13 +44,13 @@ module.exports = [
5144
],
5245
order: [[sortParams[0], sortParams[1]]],
5346
})
54-
.then(copilotApplications => {
47+
.then((copilotApplications) => {
5548
req.log.debug(`CopilotApplications ${JSON.stringify(copilotApplications)}`);
5649
return models.ProjectMember.getActiveProjectMembers(opportunity.projectId).then((members) => {
5750
req.log.debug(`Fetched existing active members ${JSON.stringify(members)}`);
5851
req.log.debug(`Applications ${JSON.stringify(copilotApplications)}`);
59-
const enrichedApplications = copilotApplications.map(application => {
60-
const m = members.find(m => m.userId === application.userId);
52+
const enrichedApplications = copilotApplications.map((application) => {
53+
const member = members.find(memberItem => memberItem.userId === application.userId);
6154

6255
// Using spread operator fails in lint check
6356
// While Object.assign fails silently during run time
@@ -77,22 +70,30 @@ module.exports = [
7770
copilotOpportunity: application.copilotOpportunity,
7871
};
7972

80-
if (m) {
81-
enriched.existingMembership = m;
73+
if (member) {
74+
enriched.existingMembership = member;
8275
}
8376

8477
req.log.debug(`Existing member to application ${JSON.stringify(enriched)}`);
8578

8679
return enriched;
8780
});
8881

82+
const response = isAdminOrPM
83+
? enrichedApplications
84+
: enrichedApplications.map(app => ({
85+
userId: app.userId,
86+
status: app.status,
87+
createdAt: app.createdAt,
88+
}));
89+
8990
req.log.debug(`Enriched Applications ${JSON.stringify(enrichedApplications)}`);
90-
res.status(200).send(enrichedApplications);
91+
res.status(200).send(response);
9192
});
92-
})
93+
});
9394
})
94-
.catch((err) => {
95-
util.handleError('Error fetching copilot applications', err, req, next);
96-
});
95+
.catch((err) => {
96+
util.handleError('Error fetching copilot applications', err, req, next);
97+
});
9798
},
9899
];

0 commit comments

Comments
 (0)