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

FileSystemAdapter for saving files to the server's file system #716

Closed
wants to merge 12 commits into from

Conversation

dtsolis
Copy link
Contributor

@dtsolis dtsolis commented Feb 28, 2016

Can be used while starting the parse-server

var api = new ParseServer({
  databaseURI: 'mongodb://localhost:52853/dev',
  appId: 'appId',
  masterKey: "masterKey",
  serverURL: "http://localhost:1337/parse",
  filesAdapter: new FileSystemAdapter({
    filesSubDirectory: 'path/under/files/directory'
  })
});

@@ -40,3 +40,7 @@ lib/

# cache folder
.cache

# Folder created by FileSystemAdapter
/files
Copy link
Contributor

Choose a reason for hiding this comment

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

remove '/'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without '/' git ignores 'src/Adapters/Files/' folder too. I added this so git ignores only the first-level 'files' folder

@flovilmart
Copy link
Contributor

Thanks for the PR!

Please consider adding the according tests.

@facebook-github-bot
Copy link

@dtsolis updated the pull request.

@dtsolis
Copy link
Contributor Author

dtsolis commented Feb 29, 2016

Thanks for your comments! I updated the adapter. Also, regarding tests, since I am not that experienced writing tests for node it could take a while.

@kilabyte
Copy link

kilabyte commented Mar 4, 2016

Is there any special setup needed to the server for this? Or does the adapter just save the file to the specified directory?

@dtsolis
Copy link
Contributor Author

dtsolis commented Mar 4, 2016

It just saves the file to the specified directory. This directory will be created under a 'files' folder. So, even if you want to have more than one app under the same directory, the file structure could be the following:

| - files/
| | - subDirForApp1/
| | - subDirForApp2/
| - indexForApp1.js
| - indexForApp2.js

@codecov-io
Copy link

Current coverage is 90.73%

Merging #716 into master will decrease coverage by -0.89% as of cab1474

@@            master    #716   diff @@
======================================
  Files           73      74     +1
  Stmts         4432    4489    +57
  Branches       888     899    +11
  Methods          0       0       
======================================
+ Hit           4061    4073    +12
  Partial         10      10       
- Missed         361     406    +45

Review entire Coverage Diff as of cab1474

Powered by Codecov. Updated on successful CI builds.

@dtsolis
Copy link
Contributor Author

dtsolis commented Mar 12, 2016

@flovilmart Ok, so.. Not sure if i should but i did a rebase to update my branch with the latest changes on master, in order to fix some conflicts. I also i added the test on the FilesController.spec.js.

@flovilmart
Copy link
Contributor

All those commits don't seem right... You did not rebase your branch properly.

you need to cleanup you rebase

  • run git reflog // this will show you the history of all the git commands
  • identify when the rebase started: it should look like that: 2c991a4 HEAD@{25}: rebase: checkout master
  • do git checkout HEAD@{25} // replace 25 by the value you see
  • create a new branch: git checkout -b dstolis.fsadapter-proper
  • go on master: git checkout master
  • update: git pull origin master
  • go back to your new branch: git checkout -b dstolis.fsadapter-proper
  • run: git rebase master

if a conflict is detected, rebase will stop, and let you amend the commit/fix the conflict

  • for each conflicted file, git add file/myfile.js
  • once all the conflicts are resolved for the commit, run git rebase --continue

Hope that helps

@facebook-github-bot
Copy link

@dtsolis updated the pull request.

@flovilmart
Copy link
Contributor

@dtsolis still not good at all

@dtsolis dtsolis force-pushed the dtsolis.fsadapter branch from b0ac55e to a9b2f46 Compare March 12, 2016 19:42
@dtsolis
Copy link
Contributor Author

dtsolis commented Mar 12, 2016

@flovilmart i think now it's better


_mkdir(path, root) {
// snippet found on -> http://stackoverflow.com/a/10600228
var dirs = path.split('/'), dir = dirs.shift(), root = (root || '') + dir + '/';
Copy link
Contributor

Choose a reason for hiding this comment

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

please support windows too with separator

@facebook-github-bot
Copy link

@dtsolis updated the pull request.

@gfosco
Copy link
Contributor

gfosco commented Mar 17, 2016

Still have 12 commits here. Try updating the latest master, making a new branch, and bringing the changes in again. Close this one, then use https://github.com/ParsePlatform/parse-server/pull/716/files as a reference, and open a new PR with just 1 commit.

@@ -53,7 +53,7 @@ if (!options.serverURL) {
options.serverURL = `http://localhost:${options.port}${options.mountPath}`;
}

if (!options.appId || !options.masterKey || !options.serverURL) {
if (!program.appId || !program.masterKey || !program.serverURL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert those changes

@dtsolis dtsolis closed this Mar 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants