Add file triggers and file meta data #6344

stevestencil · 2020-01-15T03:31:22Z

Closes #6340
Closes #6341

updated master

…into query-hint

davimacedo · 2020-01-16T02:09:17Z

@stevestencil great job here! congrats! Can you please take a look at the tests?

stevestencil · 2020-01-16T02:13:40Z

@davimacedo I plan on putting some more work into this tomorrow. I will try to cleanup the tests as best I can. I've been having a hell of a time getting them to pass locally.

codecov · 2020-01-16T03:32:03Z

Codecov Report

Merging #6344 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6344   +/-   ##
=======================================
  Coverage   93.94%   93.94%           
=======================================
  Files         169      169           
  Lines       11939    11939           
=======================================
  Hits        11216    11216           
  Misses        723      723

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfeda1c...cfeda1c. Read the comment docs.

stevestencil · 2020-01-16T03:51:51Z

@davimacedo @dplewis I finally got the tests to pass. I'm still not 100% on how I have the fileObject setup. I am going to work on it some more tomorrow and add some tests for my changes. I also plan to add beforeDeleteFile and afterDeleteFile hooks too. Ideally we should have beforeGetFile and afterGetFile hooks too. However it does not appear there is any way to get the user making the request because the session token is not passed in? Thanks for your feedback!

stevestencil · 2020-01-16T19:23:53Z

@davimacedo @dplewis How can I run just the tests that I want to add? Or even just the tests in 1 specific file? I'm much more familiar with Jest

dplewis · 2020-01-16T19:27:07Z

@stevestencil

To run specific file:

npm test spec/MyFile.spec.js

To run specific test: use fit instead of it (don't commit fit, remember to change it back to it)

fit('myTest', done => done());

We should document fit in the Contributing guide.

stevestencil · 2020-01-16T19:29:00Z

Ahh... much better!... Now I will feel productive again!

stevestencil · 2020-01-16T23:50:13Z

@davimacedo @dplewis I added some tests... can you point me in the right direction with why the 1 test will not pass with postgres?

dplewis · 2020-01-16T23:52:37Z

@stevestencil That is a flaky test. It fails randomly. I restarted the tests.

stevestencil · 2020-01-16T23:53:15Z

Awesome... How does this look?

stevestencil · 2020-01-17T00:02:00Z

I did just think of one other thing. If the user changes the data in the beforeSaveFile hook, the fileObject.contentLength does not get updated with the new data size when it gets passed to the afterSaveHook

stevestencil · 2020-01-17T00:05:50Z

Also, I think adding a way the user can return an already saved Parse.File (bypassing the save to storage all together) should be added too. Any objections on making this happen?

mman · 2020-03-29T08:49:46Z

Can’t wait to get this in, it will also address #6518 if I read the code correctly. Any help needed?

stevestencil · 2020-03-29T19:38:29Z

@dplewis can you restart this build. 🤞🏼

dplewis · 2020-03-29T19:40:07Z

Of course, doing an SDK release first to merge into the server

mpatnode · 2020-03-30T18:57:23Z

src/Controllers/FilesController.js

@@ -31,12 +31,14 @@ export class FilesController extends AdaptableController {
    }

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


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

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.

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.

@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.

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.

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.

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?

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.

dplewis · 2020-03-31T15:08:22Z

@stevestencil The latest version of the JS SDK has been merged.

Can you update your branch from master to see if the test run?

stevestencil · 2020-03-31T15:34:28Z

@dplewis I did and it looks like 1 test failed... can you look and see if that's one of those flakey tests that needs to be reran?

dplewis · 2020-03-31T15:37:40Z

It doesn't look like a flaky test. It is related to the FileController. Can you have a look?

stevestencil · 2020-03-31T15:38:26Z

I'm on it!

stevestencil · 2020-03-31T19:16:59Z

@dplewis I finally got them to pass 🤦🏻‍♂️

fixed comment

acinader

looks good to me. some super minor comment nits...

spec/FilesController.spec.js

src/Adapters/Files/FilesAdapter.js

src/cloud-code/Parse.Cloud.js

…-server into file-triggers

acinader

lgtm

TomWFox · 2020-04-04T16:41:16Z

@stevestencil If you have a chance, it would be great to add this to the cloud code guide, even just a simple mention & explanation would be fine.

I may be able to get round to this at some point if you can't but I'm busy setting up the forum right now & it would be great if you could do it because you will be able to explain it in more detail! 🙂

stevestencil and others added 13 commits January 7, 2020 09:01

Merge pull request #1 from parse-community/master

3e6de75

updated master

added hint to aggregate

7ce5209

added support for hint in query

a8d404e

added else clause to aggregate

00a6b27

fixed tests

d41d035

updated tests

288bb2a

Merge branch 'master' of https://github.com/stevestencil/parse-server …

c3470bb

…into query-hint

Add tests and clean up

0863e84

added beforeSaveFile and afterSaveFile triggers

7d0c4e2

Add support for explain

eecc3ff

Merge branch 'master' into file-triggers

93c9548

added some validation

c22ef49

added support for metadata and tags

8d1576b

stevestencil added 2 commits January 15, 2020 22:21

tests?

1c389ce

Merge branch 'query-hint' into file-triggers

22a2699

trying tests

70a4c81

stevestencil added 2 commits January 16, 2020 18:12

added tests

370d091

fixed failing tests

c4f6bd4

mman mentioned this pull request Mar 30, 2020

Upload files in folder with S3 adapter #6534

Open

mpatnode reviewed Mar 30, 2020

View reviewed changes

stevestencil added 2 commits March 31, 2020 01:27

Update package.json

70f9b30

Merge branch 'master' into file-triggers

f6ddc0b

stevestencil added 4 commits March 31, 2020 13:31

fixed failing test

f43c26c

fixed error message

ffad97c

fixed failing tests (hopefully)

f34a8db

TESTS!!!

deea148

stevestencil added 2 commits March 31, 2020 15:20

Update FilesAdapter.js

b7edf4b

fixed comment

added test for changing file name

bed35c9

dplewis requested review from davimacedo and acinader April 1, 2020 15:27

acinader suggested changes Apr 1, 2020

View reviewed changes

spec/FilesController.spec.js Outdated Show resolved Hide resolved

src/Adapters/Files/FilesAdapter.js Show resolved Hide resolved

src/cloud-code/Parse.Cloud.js Outdated Show resolved Hide resolved

src/cloud-code/Parse.Cloud.js Outdated Show resolved Hide resolved

stevestencil added 2 commits April 1, 2020 14:10

updated comments

859a603

Merge branch 'file-triggers' of https://github.com/stevestencil/parse…

cfeda1c

…-server into file-triggers

acinader approved these changes Apr 1, 2020

View reviewed changes

dplewis merged commit a9dba44 into parse-community:master Apr 2, 2020

dplewis mentioned this pull request Apr 2, 2020

Prep release 4.2 #6560

Merged

This was referenced Apr 3, 2020

Add support for server side generated filenames (custom filename prefix for AWS S3 upload) #6518

Closed

S3 filename generated by generateKey option is ignored by Parse Server parse-community/parse-server-s3-adapter#87

Open

TomWFox mentioned this pull request Apr 4, 2020

📕Cloud code file triggers parse-community/docs#725

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add file triggers and file meta data #6344

Add file triggers and file meta data #6344

stevestencil commented Jan 15, 2020 •

edited by TomWFox

Loading

davimacedo commented Jan 16, 2020

stevestencil commented Jan 16, 2020

codecov bot commented Jan 16, 2020 •

edited

Loading

stevestencil commented Jan 16, 2020

stevestencil commented Jan 16, 2020 •

edited

Loading

dplewis commented Jan 16, 2020 •

edited

Loading

stevestencil commented Jan 16, 2020

stevestencil commented Jan 16, 2020

dplewis commented Jan 16, 2020

stevestencil commented Jan 16, 2020

stevestencil commented Jan 17, 2020

stevestencil commented Jan 17, 2020

mman commented Mar 29, 2020

stevestencil commented Mar 29, 2020

dplewis commented Mar 29, 2020

mpatnode Mar 30, 2020

mman Mar 30, 2020

stevestencil Mar 30, 2020

mman Apr 1, 2020

stevestencil Apr 1, 2020

mman Apr 1, 2020

mman Apr 1, 2020

stevestencil Apr 1, 2020

dplewis commented Mar 31, 2020

stevestencil commented Mar 31, 2020

dplewis commented Mar 31, 2020

stevestencil commented Mar 31, 2020

stevestencil commented Mar 31, 2020

acinader left a comment

acinader left a comment

TomWFox commented Apr 4, 2020 •

edited

Loading

		@@ -31,12 +31,14 @@ export class FilesController extends AdaptableController {
		}

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

Add file triggers and file meta data #6344

Add file triggers and file meta data #6344

Conversation

stevestencil commented Jan 15, 2020 • edited by TomWFox Loading

davimacedo commented Jan 16, 2020

stevestencil commented Jan 16, 2020

codecov bot commented Jan 16, 2020 • edited Loading

Codecov Report

stevestencil commented Jan 16, 2020

stevestencil commented Jan 16, 2020 • edited Loading

dplewis commented Jan 16, 2020 • edited Loading

stevestencil commented Jan 16, 2020

stevestencil commented Jan 16, 2020

dplewis commented Jan 16, 2020

stevestencil commented Jan 16, 2020

stevestencil commented Jan 17, 2020

stevestencil commented Jan 17, 2020

mman commented Mar 29, 2020

stevestencil commented Mar 29, 2020

dplewis commented Mar 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dplewis commented Mar 31, 2020

stevestencil commented Mar 31, 2020

dplewis commented Mar 31, 2020

stevestencil commented Mar 31, 2020

stevestencil commented Mar 31, 2020

acinader left a comment

Choose a reason for hiding this comment

acinader left a comment

Choose a reason for hiding this comment

TomWFox commented Apr 4, 2020 • edited Loading

stevestencil commented Jan 15, 2020 •

edited by TomWFox

Loading

codecov bot commented Jan 16, 2020 •

edited

Loading

stevestencil commented Jan 16, 2020 •

edited

Loading

dplewis commented Jan 16, 2020 •

edited

Loading

TomWFox commented Apr 4, 2020 •

edited

Loading