Skip to content

Add file triggers and file meta data #6344

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

Merged
merged 44 commits into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
3e6de75
Merge pull request #1 from parse-community/master
stevestencil Jan 7, 2020
7ce5209
added hint to aggregate
stevestencil Jan 7, 2020
a8d404e
added support for hint in query
stevestencil Jan 7, 2020
00a6b27
added else clause to aggregate
stevestencil Jan 8, 2020
d41d035
fixed tests
stevestencil Jan 8, 2020
288bb2a
updated tests
stevestencil Jan 8, 2020
c3470bb
Merge branch 'master' of https://github.com/stevestencil/parse-server…
stevestencil Jan 8, 2020
0863e84
Add tests and clean up
dplewis Jan 10, 2020
7d0c4e2
added beforeSaveFile and afterSaveFile triggers
stevestencil Jan 13, 2020
eecc3ff
Add support for explain
dplewis Jan 14, 2020
93c9548
Merge branch 'master' into file-triggers
stevestencil Jan 14, 2020
c22ef49
added some validation
stevestencil Jan 15, 2020
8d1576b
added support for metadata and tags
stevestencil Jan 15, 2020
1c389ce
tests?
stevestencil Jan 16, 2020
22a2699
Merge branch 'query-hint' into file-triggers
stevestencil Jan 16, 2020
70a4c81
trying tests
stevestencil Jan 16, 2020
370d091
added tests
stevestencil Jan 16, 2020
c4f6bd4
fixed failing tests
stevestencil Jan 16, 2020
663e355
added some docs for fileObject
stevestencil Jan 17, 2020
5b1c0f4
updated hooks to use Parse.File
stevestencil Jan 17, 2020
14b41b3
added test for already saved file being returned in hook
stevestencil Jan 17, 2020
a0087a0
added beforeDeleteFile and afterDeleteFile hooks
stevestencil Jan 19, 2020
d89b358
removed contentLength because it's already in the header
stevestencil Jan 19, 2020
2982df9
added fileSize param to FileTriggerRequest
stevestencil Jan 19, 2020
ac5ba86
added support for client side metadata and tags
stevestencil Jan 22, 2020
2cfe086
removed fit test
stevestencil Jan 22, 2020
36222dc
removed unused import
stevestencil Jan 22, 2020
0c4e191
added loging to file triggers
stevestencil Feb 1, 2020
51db0a6
updated error message
stevestencil Feb 1, 2020
221eafb
Merge tag '4.1.0' into file-triggers
stevestencil Mar 4, 2020
711c9fd
updated error message
stevestencil Mar 4, 2020
946014d
Merge branch 'file-triggers' of https://github.com/stevestencil/parse…
stevestencil Mar 4, 2020
299135d
fixed tests
stevestencil Mar 4, 2020
476052d
fixed typos
stevestencil Mar 22, 2020
70f9b30
Update package.json
stevestencil Mar 31, 2020
f6ddc0b
Merge branch 'master' into file-triggers
stevestencil Mar 31, 2020
f43c26c
fixed failing test
stevestencil Mar 31, 2020
ffad97c
fixed error message
stevestencil Mar 31, 2020
f34a8db
fixed failing tests (hopefully)
stevestencil Mar 31, 2020
deea148
TESTS!!!
stevestencil Mar 31, 2020
b7edf4b
Update FilesAdapter.js
stevestencil Mar 31, 2020
bed35c9
added test for changing file name
stevestencil Apr 1, 2020
859a603
updated comments
stevestencil Apr 1, 2020
cfeda1c
Merge branch 'file-triggers' of https://github.com/stevestencil/parse…
stevestencil Apr 1, 2020
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
253 changes: 253 additions & 0 deletions spec/CloudCode.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@ const request = require('../lib/request');
const InMemoryCacheAdapter = require('../lib/Adapters/Cache/InMemoryCacheAdapter')
.InMemoryCacheAdapter;

const mockAdapter = {
createFile: async (filename) => ({
name: filename,
location: `http://www.somewhere.com/${filename}`,
}),
deleteFile: () => {},
getFileData: () => {},
getFileLocation: (config, filename) => `http://www.somewhere.com/${filename}`,
validateFilename: () => {
return null;
},
};

describe('Cloud Code', () => {
it('can load absolute cloud code file', done => {
reconfigureServer({
Expand Down Expand Up @@ -2595,6 +2608,246 @@ describe('beforeLogin hook', () => {
expect(beforeFinds).toEqual(1);
expect(afterFinds).toEqual(1);
});

it('beforeSaveFile should not change file if nothing is returned', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
Parse.Cloud.beforeSaveFile(() => {
return;
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
const result = await file.save({ useMasterKey: true });
expect(result).toBe(file);
});

it('beforeSaveFile should return file that is already saved and not save anything to files adapter', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough();
Parse.Cloud.beforeSaveFile(() => {
const newFile = new Parse.File('some-file.txt');
newFile._url = 'http://www.somewhere.com/parse/files/some-app-id/some-file.txt';
return newFile;
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
const result = await file.save({ useMasterKey: true });
expect(result).toBe(file);
expect(result._name).toBe('some-file.txt');
expect(result._url).toBe('http://www.somewhere.com/parse/files/some-app-id/some-file.txt');
expect(createFileSpy).not.toHaveBeenCalled();
});

it('beforeSaveFile should throw error', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
Parse.Cloud.beforeSaveFile(() => {
throw new Parse.Error(400, 'some-error-message');
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
try {
await file.save({ useMasterKey: true });
} catch (error) {
expect(error.message).toBe('some-error-message');
}
});

it('beforeSaveFile should change values of uploaded file by editing fileObject directly', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough();
Parse.Cloud.beforeSaveFile(async (req) => {
expect(req.triggerName).toEqual('beforeSaveFile');
expect(req.master).toBe(true);
req.file.addMetadata('foo', 'bar');
req.file.addTag('tagA', 'some-tag');
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
const result = await file.save({ useMasterKey: true });
expect(result).toBe(file);
const newData = new Buffer([1, 2, 3]);
const newOptions = {
tags: {
tagA: 'some-tag',
},
metadata: {
foo: 'bar',
},
};
expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), newData, 'text/plain', newOptions);
});

it('beforeSaveFile should change values by returning new fileObject', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough();
Parse.Cloud.beforeSaveFile(async (req) => {
expect(req.triggerName).toEqual('beforeSaveFile');
expect(req.fileSize).toBe(3);
const newFile = new Parse.File('donald_duck.pdf', [4, 5, 6], 'application/pdf');
newFile.setMetadata({ foo: 'bar' });
newFile.setTags({ tagA: 'some-tag' });
return newFile;
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
const result = await file.save({ useMasterKey: true });
expect(result).toBeInstanceOf(Parse.File);
const newData = new Buffer([4, 5, 6]);
const newContentType = 'application/pdf';
const newOptions = {
tags: {
tagA: 'some-tag',
},
metadata: {
foo: 'bar',
},
};
expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), newData, newContentType, newOptions);
const expectedFileName = 'donald_duck.pdf';
expect(file._name.indexOf(expectedFileName)).toBe(file._name.length - expectedFileName.length);
});

it('beforeSaveFile should contain metadata and tags saved from client', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough();
Parse.Cloud.beforeSaveFile(async (req) => {
expect(req.triggerName).toEqual('beforeSaveFile');
expect(req.fileSize).toBe(3);
expect(req.file).toBeInstanceOf(Parse.File);
expect(req.file.name()).toBe('popeye.txt');
expect(req.file.metadata()).toEqual({ foo: 'bar' });
expect(req.file.tags()).toEqual({ bar: 'foo' });
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
file.setMetadata({ foo: 'bar' });
file.setTags({ bar: 'foo' });
const result = await file.save({ useMasterKey: true });
expect(result).toBeInstanceOf(Parse.File);
const options = {
metadata: { foo: 'bar' },
tags: { bar: 'foo' },
};
expect(createFileSpy).toHaveBeenCalledWith(jasmine.any(String), jasmine.any(Buffer), 'text/plain', options);
});

it('beforeSaveFile should return same file data with new file name', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
const config = Config.get('test');
config.filesController.options.preserveFileName = true;
Parse.Cloud.beforeSaveFile(async ({ file }) => {
expect(file.name()).toBe('popeye.txt');
const fileData = await file.getData();
const newFile = new Parse.File('2020-04-01.txt', { base64: fileData });
return newFile;
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
const result = await file.save({ useMasterKey: true });
expect(result.name()).toBe('2020-04-01.txt');
});

it('afterSaveFile should set fileSize to null if beforeSave returns an already saved file', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
const createFileSpy = spyOn(mockAdapter, 'createFile').and.callThrough();
Parse.Cloud.beforeSaveFile((req) => {
expect(req.fileSize).toBe(3);
const newFile = new Parse.File('some-file.txt');
newFile._url = 'http://www.somewhere.com/parse/files/some-app-id/some-file.txt';
return newFile;
});
Parse.Cloud.afterSaveFile((req) => {
expect(req.fileSize).toBe(null);
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
const result = await file.save({ useMasterKey: true });
expect(result).toBe(result);
expect(result._name).toBe('some-file.txt');
expect(result._url).toBe('http://www.somewhere.com/parse/files/some-app-id/some-file.txt');
expect(createFileSpy).not.toHaveBeenCalled();
});

it('afterSaveFile should throw error', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
Parse.Cloud.afterSaveFile(async () => {
throw new Parse.Error(400, 'some-error-message');
});
const filename = 'donald_duck.pdf';
const file = new Parse.File(filename, [1, 2, 3], 'text/plain');
try {
await file.save({ useMasterKey: true });
} catch (error) {
expect(error.message).toBe('some-error-message');
}
});

it('afterSaveFile should call with fileObject', async (done) => {
await reconfigureServer({ filesAdapter: mockAdapter });
Parse.Cloud.beforeSaveFile(async (req) => {
req.file.setTags({ tagA: 'some-tag' });
req.file.setMetadata({ foo: 'bar' });
});
Parse.Cloud.afterSaveFile(async (req) => {
expect(req.master).toBe(true);
expect(req.file._tags).toEqual({ tagA: 'some-tag' });
expect(req.file._metadata).toEqual({ foo: 'bar' });
done();
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
await file.save({ useMasterKey: true });
});

it('afterSaveFile should change fileSize when file data changes', async (done) => {
await reconfigureServer({ filesAdapter: mockAdapter });
Parse.Cloud.beforeSaveFile(async (req) => {
expect(req.fileSize).toBe(3);
expect(req.master).toBe(true);
const newFile = new Parse.File('donald_duck.pdf', [4, 5, 6, 7, 8, 9], 'application/pdf');
return newFile;
});
Parse.Cloud.afterSaveFile(async (req) => {
expect(req.fileSize).toBe(6);
expect(req.master).toBe(true);
done();
});
const file = new Parse.File('popeye.txt', [1, 2, 3], 'text/plain');
await file.save({ useMasterKey: true });
});

it('beforeDeleteFile should call with fileObject', async () => {
await reconfigureServer({ filesAdapter: mockAdapter });
Parse.Cloud.beforeDeleteFile((req) => {
expect(req.file).toBeInstanceOf(Parse.File);
expect(req.file._name).toEqual('popeye.txt');
expect(req.file._url).toEqual('http://www.somewhere.com/popeye.txt');
expect(req.fileSize).toBe(null);
});
const file = new Parse.File('popeye.txt');
await file.destroy({ useMasterKey: true });
});

it('beforeDeleteFile should throw error', async (done) => {
await reconfigureServer({ filesAdapter: mockAdapter });
Parse.Cloud.beforeDeleteFile(() => {
throw new Error('some error message');
});
const file = new Parse.File('popeye.txt');
try {
await file.destroy({ useMasterKey: true });
} catch (error) {
expect(error.message).toBe('some error message');
done();
}
})

it('afterDeleteFile should call with fileObject', async (done) => {
await reconfigureServer({ filesAdapter: mockAdapter });
Parse.Cloud.beforeDeleteFile((req) => {
expect(req.file).toBeInstanceOf(Parse.File);
expect(req.file._name).toEqual('popeye.txt');
expect(req.file._url).toEqual('http://www.somewhere.com/popeye.txt');
});
Parse.Cloud.afterDeleteFile((req) => {
expect(req.file).toBeInstanceOf(Parse.File);
expect(req.file._name).toEqual('popeye.txt');
expect(req.file._url).toEqual('http://www.somewhere.com/popeye.txt');
done();
});
const file = new Parse.File('popeye.txt');
await file.destroy({ useMasterKey: true });
});
});

describe('afterLogin hook', () => {
Expand Down
2 changes: 1 addition & 1 deletion spec/FilesController.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('FilesController', () => {
expect(log1.level).toBe('error');

const log2 = logs.find(
x => x.message === 'Could not store file: yolo.txt.'
x => x.message === 'it failed with xyz'
);
expect(log2.level).toBe('error');
expect(log2.code).toBe(130);
Expand Down
6 changes: 5 additions & 1 deletion spec/ParseFile.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,11 @@ describe('Parse.File testing', () => {
}).then(fail, response => {
expect(response.status).toBe(400);
const body = response.text;
expect(body).toEqual('{"code":153,"error":"Could not delete file."}');
expect(typeof body).toBe('string');
const { code, error } = JSON.parse(body);
expect(code).toBe(153);
expect(typeof error).toBe('string');
expect(error.length).toBeGreaterThan(0);
done();
});
});
Expand Down
11 changes: 10 additions & 1 deletion src/Adapters/Files/FilesAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,19 @@ export class FilesAdapter {
* @param {*} data - the buffer of data from the file
* @param {string} contentType - the supposed contentType
* @discussion the contentType can be undefined if the controller was not able to determine it
* @param {object} options - (Optional) options to be passed to file adapter (S3 File Adapter Only)
* - tags: object containing key value pairs that will be stored with file
* - metadata: object containing key value pairs that will be sotred with file (https://docs.aws.amazon.com/AmazonS3/latest/user-guide/add-object-metadata.html)
* @discussion options are not supported by all file adapters. Check the your adapter's documentation for compatibility
*
* @return {Promise} a promise that should fail if the storage didn't succeed
*/
createFile(filename: string, data, contentType: string): Promise {}
createFile(
filename: string,
data,
contentType: string,
options: Object
): Promise {}

/** Responsible for deleting the specified file
*
Expand Down
14 changes: 8 additions & 6 deletions src/Controllers/FilesController.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class FilesController extends AdaptableController {
return this.adapter.getFileData(filename);
}

createFile(config, filename, data, contentType) {
createFile(config, filename, data, contentType, options) {
const extname = path.extname(filename);

const hasExtension = extname.length > 0;
Expand All @@ -31,12 +31,14 @@ export class FilesController extends AdaptableController {
}

const location = this.adapter.getFileLocation(config, filename);

Choose a reason for hiding this comment

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

Don't we have a problem here deciding the location prior to your hooks being called?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is the same problem as described here: #6518 the location is computed before the triggers and adapters get a chance to potentially modify the file name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is currently no way to change the name of the Parse.File because there is no setter. I believe with these hooks you could create a new Parse.File (with a new name) using the same data as the original file and return it in the hook. Your new file (with the new file name) will be saved instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

@stevestencil Would it be possible to add a test case for this. When a client SDK asks to save file A.txt and in beforeSave you create new File with name B.txt and the same contents, verify that it is actually saved into B.txt? That would be awesome to consider this practice as a blessed way to change filename during the upload. To explain some background. Client SDK wants to save A.txt but when pushing to S3 you want to make the filename YYYY-MM-DD-A.txt so that you can later clean up files on S3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote a test case for your example and it passes. You'll just have to make sure preserveFileName is set to true in your parse-server config so that the random text does not get prepended to your file names.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to slow this down, I can't wait to get it in and use it in production, but what happens when preserveFileName: false which is the default. The beforeSave handler should receive a filename passed from the client sdk prepended with a random string, and will have a change to work from that, correct? It should work both ways.

Copy link
Contributor

Choose a reason for hiding this comment

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

In other words, prepending a filename passed in from client SDK is fine IMHO, this is the default server behavior to make sure that all saved files are unique. The beforeSave trigged should get called on such file, and there if you change anything, it should still work, regardless whether the filename in beforeSave was kept intact from client SDL (preserveFilename: true) or whether it was prepended with random chars (preserveFilename: false - the default behavior). Correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no matter if preserveFileName is true or false, the beforeSaveFile hook will get the file name as it came from the client (lets say foo.txt)... The file name gets changed by adding the random text to the beginning of the file name inside of parse-server. if you set preserveFileName to false this will not happen.

So in a nutshell. The random text gets added after the beforeSaveFile hook and before the file actually saves to whatever storage solution you're using.

return this.adapter.createFile(filename, data, contentType).then(() => {
return Promise.resolve({
url: location,
name: filename,
return this.adapter
.createFile(filename, data, contentType, options)
.then(() => {
return Promise.resolve({
url: location,
name: filename,
});
});
});
}

deleteFile(config, filename) {
Expand Down
Loading