Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add file upload api #267

Merged
merged 75 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from 53 commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
703f4fc
Add minio to docker-compose for local dev
Jan 13, 2021
a41b773
Add handler for file upload endpoint
Jan 13, 2021
6881b4e
add POST /files endpoint to upload files to s3
Jan 13, 2021
d8cdb09
fmt
Jan 13, 2021
788d8ef
fmt
Jan 13, 2021
ed89a9d
Add model, migrations and seeders for File
Jan 14, 2021
237e764
Add db logic to handler.js for file upload
Jan 14, 2021
cb0e6bf
Move s3Uploader to lib/s3Uploader.js
Jan 15, 2021
8e30cac
update branch with latest from main
Jan 15, 2021
d45bbd4
Add first test
Jan 15, 2021
6a4050a
Add versioning tests
Jan 18, 2021
d0b267c
Add s3 Uploader test
Jan 18, 2021
58ff705
update .env.example
Jan 18, 2021
8378bc4
remove debugging statement
Jan 18, 2021
61d8165
Update files model
Jan 19, 2021
630680b
Merge remote-tracking branch 'origin/js-saving-activity-report-fronte…
Jan 19, 2021
d2cff7f
update create-files migration
Jan 19, 2021
9b71eaf
update files/handlers.js to update
Jan 19, 2021
386086f
merge in main
Jan 19, 2021
f8ca042
merge main
Jan 19, 2021
9a22bd3
merge main
Jan 19, 2021
e0ae000
fix merge
Jan 19, 2021
61b57f0
Merge branch 'main' into add-file-upload-api
Jan 19, 2021
59be6bb
merge main
Jan 19, 2021
345db1a
merge main
Jan 19, 2021
e27bb8b
merge main
Jan 19, 2021
8774025
merge main
Jan 19, 2021
168b6d8
update .env.example for non-docker development
Jan 19, 2021
fea7f25
update deps
Jan 19, 2021
9e9b0b4
fmt
Jan 19, 2021
39ab0fe
add openapi documenetation
Jan 19, 2021
cddd72c
update openapi docs for file upload endpoint
Jan 19, 2021
396437d
Add testfile for testing uploads
Jan 20, 2021
335f13e
Add model tests for File upload
Jan 20, 2021
f0e7e33
Add non-happy path tests to routes/files/handers
Jan 21, 2021
096b8cc
update handlers.test.js
Jan 21, 2021
e5265ab
merge main into add-file-upload-api`
Jan 21, 2021
b676dc2
update yarn.lock
Jan 21, 2021
de05f37
merge main into add-file-upload-api
Jan 21, 2021
76bef22
merge main into add-file-upload-api
Jan 21, 2021
9fc044e
fmt
Jan 21, 2021
2d782ea
add bypassauth for tests
Jan 21, 2021
6d43c5c
fmt
Jan 21, 2021
488abe3
Fix merge conflicts
Jan 21, 2021
25d3c96
add attachmentType to file upload api
Jan 21, 2021
41d4762
update openapi documentation
Jan 21, 2021
b603c49
update openapi docs
Jan 22, 2021
8d0ec3e
update .env.example and handlers.js
Jan 22, 2021
5bc0575
fix test
Jan 22, 2021
4c32383
update README and .env.example
Jan 22, 2021
ef551f5
Output logs in json format
rahearn Jan 25, 2021
a242b9c
Update production URLs
rahearn Jan 25, 2021
5d1b75e
Add actual topics
jasalisbury Jan 25, 2021
87d7cbc
Merge branch 'main' into add-file-upload-api
Jan 25, 2021
53826cf
Update README.md
dcmcand Jan 25, 2021
ae110b2
Merge pull request #119 from adhocteam/add-file-upload-api
dcmcand Jan 25, 2021
22d8506
Merge branch 'main' into js-235-add-topics
jasalisbury Jan 25, 2021
58a854f
Merge pull request #129 from adhocteam/js-235-add-topics
jasalisbury Jan 26, 2021
aa912bc
Merge branch 'main' into update-deployed-env-vars
rahearn Jan 26, 2021
eb70469
Add user management workflow diagrams
rahearn Jan 15, 2021
0551a81
Move rds & s3 inside of accreditation boundary
rahearn Jan 26, 2021
4eeb54a
The file attachment migration drops the file enum type
jasalisbury Jan 26, 2021
aab85eb
Set sandbox branch
jasalisbury Jan 26, 2021
2c03fe8
update file upload api
Jan 26, 2021
570207e
Merge pull request #131 from adhocteam/js-drop-attachment-enum
jasalisbury Jan 26, 2021
8b070ef
Merge branch 'main' into update-deployed-env-vars
rahearn Jan 27, 2021
89ec60d
Merge pull request #128 from HHS/update-deployed-env-vars
rahearn Jan 27, 2021
1232a3f
Cleanup s3 variables
Jan 27, 2021
1e009d2
update file upload api
Jan 26, 2021
3a38aa3
Cleanup s3 variables
Jan 27, 2021
99005ad
Merge branch 'cm-pr-267-add-file-upload-api' of github.com:adhocteam/…
Jan 27, 2021
04bbf74
Update src/lib/s3Uploader.js
dcmcand Jan 27, 2021
9618281
update s3uploader.js
Jan 27, 2021
17ea941
Merge branch 'cm-pr-267-add-file-upload-api' of github.com:adhocteam/…
Jan 27, 2021
3cff2f4
Merge pull request #132 from adhocteam/cm-pr-267-add-file-upload-api
dcmcand Jan 27, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,15 @@ HSES_DATA_PASSWORD=password
# In production, Sequelize instance is created with a postgres URI.
# This URI is automatically dropped into the cloud.gov environment as the env variable DATABASE_URL
DATABASE_URL=secret
# Local development variables to support s3 endpoint development and testing
LOCAL_DEV=true
# Comment this out if you are not using docker
S3_ENDPOINT=http://minio:9000
# Uncomment the following if you aren't using docker for development. Note: This requires minio to be running locally on port 9000
## and manually creating the bucket specified in $S3_BUCKET
# S3_ENDPOINT=http://localhost:9000
S3_BUCKET=ttadp-test
AWS_SECRET_ACCESS_KEY=EXAMPLESECRETKEY
AWS_ACCESS_KEY_ID=EXAMPLEID
MINIO_ROOT_PASSWORD=EXAMPLESECRETKEY
MINIO_ROOT_USER=EXAMPLEID
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ For the latest on our product mission, goals, initiatives, and KPIs, see the [Pr
7. Optionally, set `CURRENT_USER` to your current user's uid:gid. This will cause files created by docker compose to be owned by your user instead of root.
8. Run `yarn docker:db:migrate` to run DB migrations
9. Run `yarn docker:db:seed` to seed the database with test data.
10. Run `yarn docker:start` to start the application. The frontend will be available on `localhost:3000` and the backend will run on `localhost:8080`.
10. Run `yarn docker:start` to start the application. The frontend will be available on `localhost:3000` and the backend will run on `localhost:8080` and minio will run on `localhost:9000`.
11. Run `yarn docker:stop` to stop the servers and remove the docker containers.

The frontend [proxies requests](https://create-react-app.dev/docs/proxying-api-requests-in-development/) to paths it doesn't recognize to the backend.
Expand All @@ -30,9 +30,11 @@ Api documentation uses [Redoc](https://github.com/Redocly/redoc) to serve docume

You can also run build commands directly on your host (without docker). Make sure you install dependencies when changing execution method. You could see some odd errors if you install dependencies for docker and then run yarn commands directly on the host, especially if you are developing on windows. If you want to use the host yarn commands be sure to run `yarn deps:local` before any other yarn commands. Likewise if you want to use docker make sure you run `yarn docker:deps`.

You must also install and run minio locally to use the file upload functionality. Please comment out `S3_ENDPOINT=http://minio:9000` and uncomment `S3_ENDPOINT=http://localhost:9000`

### Running Tests

Run `yarn docker:deps` to install dependencies. Run `yarn docker:db:migrate` and `yarn docker:test` to run all tests for the frontend and backend.
Run `yarn docker:deps` to install dependencies. Run `yarn docker:db:migrate`, `yarn docker:db:seed` and `yarn docker:test` to run all tests for the frontend and backend.

### Docker on Windows

Expand Down
15 changes: 15 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,20 @@ services:
- "6543:5432"
volumes:
- dbdata:/var/lib/postgresql/data
minio:
image: minio/minio
env_file: .env
ports:
- "9000:9000"
volumes:
- miniodata:/data
command: server /data
aws-cli:
image: amazon/aws-cli
env_file: .env
command: ["--endpoint-url", "$S3_ENDPOINT", "s3api", "create-bucket", "--bucket", "$S3_BUCKET"]


volumes:
dbdata: {}
miniodata: {}
23 changes: 23 additions & 0 deletions docs/openapi/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,29 @@ components:
type: number
name:
type: string
fileUpload:
type: object
properties:
attachmentType:
type: string
description: "Type of attachment. Acceptable values are ATTACHMENT or RESOURCE"
reportId:
type: number
description: "id of the Activity report the file is associated with"
File:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for consistency, this should be named file

type: string
format: binary
description: "File to be uploaded"
required:
- attachmentType
- reportId
- File
fileResponse:
type: object
description: ID of file created
properties:
id:
type: number
permission:
type: object
description: A combination of region, scope and user allowing that user to perform an action in a region
Expand Down
20 changes: 20 additions & 0 deletions docs/openapi/paths/files.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
post:
tags:
- files
operationId: postFile
description: Upload a file to s3
requestBody:
description: upload a file for an Activity Report
required: true
content:
multipart/form-data:
schema:
$ref: '../index.yaml#/components/schemas/fileUpload'
responses:
200:
description: One user by an id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this description means

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is cause it is nonsense. I think it got confused in a merge.

content:
application/json:
schema:
type: object
$ref: '../index.yaml#/components/schemas/fileResponse'
2 changes: 2 additions & 0 deletions docs/openapi/paths/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@
$ref: './activity-reports/activity-reports-id.yaml'
'/activity-reports/{activityReportId}/submit':
$ref: './activity-reports/submit.yaml'
'/files':
$ref: './files.yaml'
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@
"@babel/runtime": "^7.12.1",
"@cucumber/cucumber": "^7.0.0-rc.0",
"adm-zip": "^0.5.1",
"aws-sdk": "^2.826.0",
"axios": "^0.21.1",
"chromedriver": "^87.0.0",
"client-oauth2": "^4.3.3",
Expand All @@ -147,9 +148,11 @@
"express": "^4.17.1",
"express-unless": "^0.5.0",
"express-winston": "^4.0.5",
"file-type": "^16.1.0",
"helmet": "^4.1.0",
"http-codes": "^1.0.0",
"lodash": "^4.17.20",
"multiparty": "^4.2.2",
"moment": "^2.29.1",
"mz": "^2.7.0",
"newrelic": "^7.0.1",
Expand All @@ -159,6 +162,7 @@
"sequelize": "^5.22.3",
"sequelize-cli": "^6.2.0",
"url-join": "^4.0.1",
"uuid": "^8.3.2",
"winston": "^3.3.3",
"xml2json": "^0.12.0",
"yargs": "^16.1.1"
Expand Down
45 changes: 45 additions & 0 deletions src/lib/s3Uploader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { S3 } from 'aws-sdk';

export const s3 = new S3({
accessKeyId: process.env.AWS_ACCESS_KEY_ID,
endpoint: process.env.S3_ENDPOINT,
secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have another requested change for this as well. In deployed environments, we should pull these variables (as well as the bucket name) out of the process.env.VCAP_SERVICES environment variable.

signatureVersion: 'v4',
s3ForcePathStyle: true,
});

export const verifyVersioning = async (bucket = process.env.bucket, s3Client = s3) => {
const versioningConfiguration = {
MFADelete: 'Disabled',
Status: 'Enabled',
};
let params = {
Bucket: bucket,
};
const data = await s3Client.getBucketVersioning(params);
if (!(data) || data.Status !== 'Enabled') {
params = {
Bucket: bucket,
VersioningConfiguration: versioningConfiguration,
};
return s3Client.putBucketVersioning(params);
}
return new Promise((resolve) => resolve(data));
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should enable versioning on the bucket if it isn't enabled closing #130

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 🎉

const s3Uploader = async (buffer, name, type, s3Client = s3) => {
const params = {
Body: buffer,
Bucket: process.env.S3_BUCKET,
ContentType: type.mime,
Key: name,
};
// Only check for versioning if not using Minio
if (process.env.LOCAL_DEV !== 'true') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we drop LOCAL_DEV in favor of process.env.NODE_ENV as it's being used here:

if (process.env.NODE_ENV === 'production') {
app.use('*', (req, res) => {
res.sendFile(path.join(__dirname, 'client', 'index.html'));
});
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good approach.

await verifyVersioning();
}

const upload = await s3Client.upload(params);
return upload.promise();
};
export default s3Uploader;
72 changes: 72 additions & 0 deletions src/lib/s3Uploader.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { v4 as uuidv4 } from 'uuid';
import s3Uploader, { verifyVersioning, s3 } from './s3Uploader';

const mockData = {
MFADelete: 'Disabled',
Status: 'Enabled',
};

// make sure we save to original value so we can restore it
const oldEndpoint = process.env.LOCAL_DEV;

describe('s3Uploader.verifyVersioning', () => {
let mockGet = jest.spyOn(s3, 'getBucketVersioning').mockImplementation(async () => mockData);
const mockPut = jest.spyOn(s3, 'putBucketVersioning').mockImplementation(async (params) => new Promise((res) => res(params)));
beforeEach(() => {
mockGet.mockClear();
mockPut.mockClear();
});
it('Doesn\'t change things if versioning is enabled', async () => {
const got = await verifyVersioning();
expect(mockGet.mock.calls.length).toBe(1);
expect(mockPut.mock.calls.length).toBe(0);
expect(got).toBe(mockData);
});
it('Enables versioning if it is disabled', async () => {
mockGet = jest.spyOn(s3, 'getBucketVersioning').mockImplementationOnce(async () => {});
const got = await verifyVersioning();
expect(mockGet.mock.calls.length).toBe(1);
expect(mockPut.mock.calls.length).toBe(1);
expect(got.Bucket).toBe(process.env.S3_BUCKET);
expect(got.VersioningConfiguration.MFADelete).toBe(mockData.MFADelete);
expect(got.VersioningConfiguration.Status).toBe(mockData.Status);
});
});

describe('s3Uploader', () => {
const goodType = { ext: 'pdf', mime: 'application/pdf' };
const buf = Buffer.from('Testing, Testing', 'UTF-8');
const name = `${uuidv4()}.${goodType.ext}`;
const response = {
ETag: '"8b03d1d48774bfafdb26691256fc7b2b"',
Location: `${process.env.S3_ENDPOINT}/${process.env.S3_BUCKET}/${name}`,
key: `${name}`,
Key: `${name}`,
Bucket: `${process.env.S3_BUCKET}`,
};
const promise = {
promise: () => new Promise((resolve) => resolve(response)),
};
const mockUpload = jest.spyOn(s3, 'upload').mockImplementation(async () => promise);
const mockGet = jest.spyOn(s3, 'getBucketVersioning').mockImplementation(async () => mockData);
beforeEach(() => {
mockUpload.mockClear();
mockGet.mockClear();
});
afterAll(() => {
process.env.LOCAL_DEV = oldEndpoint;
});

it('Correctly Uploads the file', async () => {
process.env.LOCAL_DEV = 'true';
const got = await s3Uploader(buf, name, goodType);
expect(mockGet.mock.calls.length).toBe(0);
await expect(got).toBe(response);
});
it('Correctly Uploads the file and checks versioning', async () => {
process.env.LOCAL_DEV = 'false';
const got = await s3Uploader(buf, name, goodType);
expect(mockGet.mock.calls.length).toBe(1);
await expect(got).toBe(response);
});
});
44 changes: 44 additions & 0 deletions src/migrations/20210114193123-create-file.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
module.exports = {
up: async (queryInterface, Sequelize) => {
await queryInterface.createTable('Files', {
id: {
allowNull: false,
autoIncrement: true,
primaryKey: true,
type: Sequelize.INTEGER,
},
activityReportId: {
type: Sequelize.INTEGER,
allowNull: false,
references: {
model: {
tableName: 'ActivityReports',
},
},
},
originalFileName: {
type: Sequelize.STRING,
allowNull: false,
},
key: {
type: Sequelize.STRING,
allowNull: false,
},
status: {
type: Sequelize.ENUM('UPLOADING', 'UPLOADED', 'UPLOAD_FAILED', 'SCANNING', 'APPROVED', 'REJECTED'),
allowNull: false,
},
createdAt: {
allowNull: false,
type: Sequelize.DATE,
},
updatedAt: {
allowNull: false,
type: Sequelize.DATE,
},
});
},
down: async (queryInterface) => {
await queryInterface.dropTable('Files');
},
};
14 changes: 14 additions & 0 deletions src/migrations/20210121192644-add-attachment-type-to-files.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module.exports = {
up: async (queryInterface, Sequelize) => {
await queryInterface.addColumn('Files',
'attachmentType',
{
type: Sequelize.ENUM('ATTACHMENT', 'RESOURCE'),
allowNull: false,
});
},

down: async (queryInterface) => {
await queryInterface.removeColumn('Files', 'attachmentType');
},
};
1 change: 1 addition & 0 deletions src/models/activityReport.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default (sequelize, DataTypes) => {
ActivityReport.belongsTo(models.User, { foreignKey: 'lastUpdatedById', as: 'lastUpdatedBy' });
ActivityReport.belongsTo(models.User, { foreignKey: 'approvingManagerId', as: 'approvingManager' });
ActivityReport.hasMany(models.ActivityRecipient, { foreignKey: 'activityReportId', as: 'activityRecipients' });
ActivityReport.hasMany(models.File, { foreignKey: 'activityReportId', as: 'activityFiles' });
}
}
ActivityReport.init({
Expand Down
40 changes: 40 additions & 0 deletions src/models/file.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
const { Model } = require('sequelize');

module.exports = (sequelize, DataTypes) => {
class File extends Model {
/**
* Helper method for defining associations.
* This method is not a part of Sequelize lifecycle.
* The `models/index` file will call this method automatically.
*/
static associate(models) {
File.belongsTo(models.ActivityReport, { foreignKey: 'activityReportId' });
}
}
File.init({
activityReportId: {
allowNull: false,
type: DataTypes.INTEGER,
},
originalFileName: {
allowNull: false,
type: DataTypes.STRING,
},
key: {
type: DataTypes.STRING,
allowNull: false,
},
status: {
type: DataTypes.ENUM('UPLOADING', 'UPLOADED', 'UPLOAD_FAILED', 'SCANNING', 'APPROVED', 'REJECTED'),
allowNull: false,
},
attachmentType: {
type: DataTypes.ENUM('ATTACHMENT', 'RESOURCE'),
allowNull: false,
},
}, {
sequelize,
modelName: 'File',
});
return File;
};
3 changes: 2 additions & 1 deletion src/routes/apiDirectory.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import join from 'url-join';
import authMiddleware, { login } from '../middleware/authMiddleware';
import handleErrors from '../lib/apiErrorHandler';
import adminRouter from './user';
import filesRouter from './files';
import activityReportsRouter from './activityReports';
import { userById } from '../services/users';
import { auditLogger } from '../logger';
Expand All @@ -19,7 +20,7 @@ router.use(authMiddleware.unless({ path: [join('/api', loginPath)] }));

router.use('/admin/users', adminRouter);
router.use('/activity-reports', activityReportsRouter);

router.use('/files', filesRouter);
router.use('/hello', (req, res) => {
res.send('Hello from ttadp');
});
Expand Down
Loading