Skip to content

Commit 0e58c8d

Browse files
flovilmartflovilmart
committed
Merge pull request #738 from ParsePlatform/flovilmart.s3Improvements
S3 Improvements
2 parents 0b990b6 + 7257ee8 commit 0e58c8d

File tree

7 files changed

+168
-55
lines changed

7 files changed

+168
-55
lines changed

‎spec/FilesController.spec.js‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,33 @@
11
var FilesController = require('../src/Controllers/FilesController').FilesController;
22
var GridStoreAdapter = require("../src/Adapters/Files/GridStoreAdapter").GridStoreAdapter;
3+
var S3Adapter = require("../src/Adapters/Files/S3Adapter").S3Adapter;
34
var Config = require("../src/Config");
45

6+
var FCTestFactory = require("./FilesControllerTestFactory");
7+
8+
59
// Small additional tests to improve overall coverage
610
describe("FilesController",()=>{
711

8-
it("should properly expand objects", (done) => {
9-
var config = new Config(Parse.applicationId);
10-
var adapter = new GridStoreAdapter();
11-
var filesController = new FilesController(adapter);
12-
var result = filesController.expandFilesInObject(config, function(){});
12+
// Test the grid store adapter
13+
var gridStoreAdapter = new GridStoreAdapter('mongodb://localhost:27017/parse');
14+
FCTestFactory.testAdapter("GridStoreAdapter", gridStoreAdapter);
15+
16+
if (process.env.S3_ACCESS_KEY && process.env.S3_SECRET_KEY) {
17+
18+
// Test the S3 Adapter
19+
var s3Adapter = new S3Adapter(process.env.S3_ACCESS_KEY, process.env.S3_SECRET_KEY, 'parse.server.tests');
1320

14-
expect(result).toBeUndefined();
21+
FCTestFactory.testAdapter("S3Adapter",s3Adapter);
1522

16-
var fullFile = {
17-
type: '__type',
18-
url: "http://an.url"
19-
}
23+
// Test S3 with direct access
24+
var s3DirectAccessAdapter = new S3Adapter(process.env.S3_ACCESS_KEY, process.env.S3_SECRET_KEY, 'parse.server.tests', {
25+
directAccess: true
26+
});
2027

21-
var anObject = {
22-
aFile: fullFile
23-
}
24-
filesController.expandFilesInObject(config, anObject);
25-
expect(anObject.aFile.url).toEqual("http://an.url");
28+
FCTestFactory.testAdapter("S3AdapterDirect", s3DirectAccessAdapter);
2629

27-
done();
28-
})
29-
})
30+
} else if (!process.env.TRAVIS) {
31+
console.log("set S3_ACCESS_KEY and S3_SECRET_KEY to test S3Adapter")
32+
}
33+
});

‎spec/FilesControllerTestFactory.js‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
2+
var FilesController = require('../src/Controllers/FilesController').FilesController;
3+
var Config = require("../src/Config");
4+
5+
var testAdapter = function(name, adapter) {
6+
// Small additional tests to improve overall coverage
7+
8+
var config = new Config(Parse.applicationId);
9+
var filesController = new FilesController(adapter);
10+
11+
describe("FilesController with "+name,()=>{
12+
13+
it("should properly expand objects", (done) => {
14+
15+
var result = filesController.expandFilesInObject(config, function(){});
16+
17+
expect(result).toBeUndefined();
18+
19+
var fullFile = {
20+
type: '__type',
21+
url: "http://an.url"
22+
}
23+
24+
var anObject = {
25+
aFile: fullFile
26+
}
27+
filesController.expandFilesInObject(config, anObject);
28+
expect(anObject.aFile.url).toEqual("http://an.url");
29+
30+
done();
31+
})
32+
33+
it("should properly create, read, delete files", (done) => {
34+
var filename;
35+
filesController.createFile(config, "file.txt", "hello world").then( (result) => {
36+
ok(result.url);
37+
ok(result.name);
38+
filename = result.name;
39+
expect(result.name.match(/file.txt/)).not.toBe(null);
40+
return filesController.getFileData(config, filename);
41+
}, (err) => {
42+
fail("The adapter should create the file");
43+
console.error(err);
44+
done();
45+
}).then((result) => {
46+
expect(result instanceof Buffer).toBe(true);
47+
expect(result.toString('utf-8')).toEqual("hello world");
48+
return filesController.deleteFile(config, filename);
49+
}, (err) => {
50+
fail("The adapter should get the file");
51+
console.error(err);
52+
done();
53+
}).then((result) => {
54+
55+
filesController.getFileData(config, filename).then((res) => {
56+
fail("the file should be deleted");
57+
done();
58+
}, (err) => {
59+
done();
60+
});
61+
62+
}, (err) => {
63+
fail("The adapter should delete the file");
64+
console.error(err);
65+
done();
66+
});
67+
}, 5000); // longer tests
68+
});
69+
}
70+
71+
module.exports = {
72+
testAdapter: testAdapter
73+
}

‎src/Adapters/Files/FilesAdapter.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// database adapter.
1313

1414
export class FilesAdapter {
15-
createFile(config, filename, data) { }
15+
createFile(config, filename: string, data, contentType: string) { }
1616

1717
deleteFile(config, filename) { }
1818

‎src/Adapters/Files/GridStoreAdapter.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export class GridStoreAdapter extends FilesAdapter {
2828

2929
// For a given config object, filename, and data, store a file
3030
// Returns a promise
31-
createFile(config, filename: string, data) {
31+
createFile(config, filename: string, data, contentType) {
3232
return this._connect().then(database => {
3333
let gridStore = new GridStore(database, filename, 'w');
3434
return gridStore.open();

‎src/Adapters/Files/S3Adapter.js‎

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -48,38 +48,61 @@ export class S3Adapter extends FilesAdapter {
4848
};
4949
AWS.config._region = this._region;
5050
this._s3Client = new AWS.S3(s3Options);
51+
this._hasBucket = false;
52+
}
53+
54+
createBucket() {
55+
var promise;
56+
if (this._hasBucket) {
57+
promise = Promise.resolve();
58+
} else {
59+
promise = new Promise((resolve, reject) => {
60+
this._s3Client.createBucket(() => {
61+
this._hasBucket = true;
62+
resolve();
63+
});
64+
});
65+
}
66+
return promise;
5167
}
5268

5369
// For a given config object, filename, and data, store a file in S3
5470
// Returns a promise containing the S3 object creation response
55-
createFile(config, filename, data) {
71+
createFile(config, filename, data, contentType) {
5672
let params = {
5773
Key: this._bucketPrefix + filename,
5874
Body: data
5975
};
6076
if (this._directAccess) {
6177
params.ACL = "public-read"
6278
}
63-
return new Promise((resolve, reject) => {
64-
this._s3Client.upload(params, (err, data) => {
65-
if (err !== null) {
66-
return reject(err);
67-
}
68-
resolve(data);
79+
if (contentType) {
80+
params.ContentType = contentType;
81+
}
82+
return this.createBucket().then(() => {
83+
return new Promise((resolve, reject) => {
84+
this._s3Client.upload(params, (err, data) => {
85+
if (err !== null) {
86+
return reject(err);
87+
}
88+
resolve(data);
89+
});
6990
});
7091
});
7192
}
7293

7394
deleteFile(config, filename) {
74-
return new Promise((resolve, reject) => {
75-
let params = {
76-
Key: this._bucketPrefix + filename
77-
};
78-
this._s3Client.deleteObject(params, (err, data) =>{
79-
if(err !== null) {
80-
return reject(err);
81-
}
82-
resolve(data);
95+
return this.createBucket().then(() => {
96+
return new Promise((resolve, reject) => {
97+
let params = {
98+
Key: this._bucketPrefix + filename
99+
};
100+
this._s3Client.deleteObject(params, (err, data) =>{
101+
if(err !== null) {
102+
return reject(err);
103+
}
104+
resolve(data);
105+
});
83106
});
84107
});
85108
}
@@ -88,12 +111,18 @@ export class S3Adapter extends FilesAdapter {
88111
// Returns a promise that succeeds with the buffer result from S3
89112
getFileData(config, filename) {
90113
let params = {Key: this._bucketPrefix + filename};
91-
return new Promise((resolve, reject) => {
92-
this._s3Client.getObject(params, (err, data) => {
93-
if (err !== null) {
94-
return reject(err);
95-
}
96-
resolve(data.Body);
114+
return this.createBucket().then(() => {
115+
return new Promise((resolve, reject) => {
116+
this._s3Client.getObject(params, (err, data) => {
117+
if (err !== null) {
118+
return reject(err);
119+
}
120+
// Something happend here...
121+
if (data && !data.Body) {
122+
return reject(data);
123+
}
124+
resolve(data.Body);
125+
});
97126
});
98127
});
99128
}

‎src/Controllers/FilesController.js‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,31 @@ import { Parse } from 'parse/node';
33
import { randomHexString } from '../cryptoUtils';
44
import AdaptableController from './AdaptableController';
55
import { FilesAdapter } from '../Adapters/Files/FilesAdapter';
6+
import path from 'path';
7+
import mime from 'mime';
68

79
export class FilesController extends AdaptableController {
810

911
getFileData(config, filename) {
1012
return this.adapter.getFileData(config, filename);
1113
}
1214

13-
createFile(config, filename, data) {
15+
createFile(config, filename, data, contentType) {
16+
17+
let extname = path.extname(filename);
18+
19+
const hasExtension = extname.length > 0;
20+
21+
if (!hasExtension && contentType && mime.extension(contentType)) {
22+
filename = filename + '.' + mime.extension(contentType);
23+
} else if (hasExtension && !contentType) {
24+
contentType = mime.lookup(filename);
25+
}
26+
1427
filename = randomHexString(32) + '_' + filename;
28+
1529
var location = this.adapter.getFileLocation(config, filename);
16-
return this.adapter.createFile(config, filename, data).then(() => {
30+
return this.adapter.createFile(config, filename, data, contentType).then(() => {
1731
return Promise.resolve({
1832
url: location,
1933
name: filename

‎src/Routers/FilesRouter.js‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import express from 'express';
22
import BodyParser from 'body-parser';
33
import * as Middlewares from '../middlewares';
44
import { randomHexString } from '../cryptoUtils';
5-
import mime from 'mime';
65
import Config from '../Config';
6+
import mime from 'mime';
77

88
export class FilesRouter {
99

@@ -41,7 +41,7 @@ export class FilesRouter {
4141
var contentType = mime.lookup(filename);
4242
res.set('Content-Type', contentType);
4343
res.end(data);
44-
}).catch(() => {
44+
}).catch((err) => {
4545
res.status(404);
4646
res.set('Content-Type', 'text/plain');
4747
res.end('File not found.');
@@ -66,20 +66,13 @@ export class FilesRouter {
6666
'Filename contains invalid characters.'));
6767
return;
6868
}
69-
let extension = '';
7069

71-
// Not very safe there.
72-
const hasExtension = req.params.filename.indexOf('.') > 0;
70+
const filename = req.params.filename;
7371
const contentType = req.get('Content-type');
74-
if (!hasExtension && contentType && mime.extension(contentType)) {
75-
extension = '.' + mime.extension(contentType);
76-
}
77-
78-
const filename = req.params.filename + extension;
7972
const config = req.config;
8073
const filesController = config.filesController;
8174

82-
filesController.createFile(config, filename, req.body).then((result) => {
75+
filesController.createFile(config, filename, req.body, contentType).then((result) => {
8376
res.status(201);
8477
res.set('Location', result.url);
8578
res.json(result);

0 commit comments

Comments
 (0)