Closed themoonlitknight closed 4 years ago
Hello @themoonlitknight
I had the same problem and after seeing that this issue is still open I just built a new adapter based on this one (also supports optional "other files directory" argument):
https://github.com/samuelantonioli/parse-server-fs-store-adapter
Hopefully it's possible to merge it in the end.
The {"error": "unauthorized"}
seems to be from lib/middlewares.js
, maybe someone can elaborate
Hi @samuelantonioli
I've just tested your module and it seems definitely working. Great job! Thank you for your contribution.
@themoonlitknight this file adapter works with https domains? I have a big issue with mixed content
Unfortunately I haven't tested it yet with https
/ping @flovilmart parse-server-fs-adapter package still gets a lot of downloads (see npm) although it contains bugs.
I've wrote a new one - parse-server-fs-store-adapter - which is essentially the same with all bugs fixed. Please consider to point to it or otherwise let people know that they are downloading a non-working solution.
@samuelantonioli, pull requests on the official adapter are welcome.
I would like to remove the mkdir functionality because it looks way too hacky and uses *Sync
functions (in this function and here). I would prefer that people just create the directory that they want to use themselves.
Would you accept such a pull request? (which essentially drops the mkdir functionality)
I’m not sure how removing such functionality is fixing the issue. I’d rather fix it, add proper tests etc...
You were mentioning that this project had ‘many’ issues, which are the others?
I think it's better if the module only checks the existence of the directory in the constructor (and not on each request - see _getLocalFilePath
). It wouldn't be too bad to use the Sync
-functions then because it will only be executed on start-up.
I still think that it's easy to let some subtle bugs in the mkdir functionality slip and I think it's ok to avoid error-prone code and just give people clear instructions on the usage. But I wouldn't say no to mkdir functionality if it only executes it in the constructor and gets some tests.
I didn't say many (the code isn't that long) - I'm not trying to be negative. There are subtle bugs
fs.unlink(filepath, (unlinkErr) => {
if(err !== null) { // <- should be unlinkErr
return reject(unlinkErr);
}
and the indentation is not consistent. There are also things that I don't like e.g. it calls _getApplicationDir
on each request which builds the same path over and over (instead of calling it once in the constructor and reuse it).
You know what’s great, it’s opensource, and you can open a / many pull requests.
You know what’s great, it’s opensource, and you can open a / many pull requests.
No problem. It just seemed that this is a stalled project (because of no reactions in the issues and no code updates). And honestly, this ("You know what’s great") sounds snarky.
Thousands of people download the package every week and get disappointed. This is not a good first impression for the whole parse-server project. I just try to help here.
I can add mkdir functionality in the code of parse-server-fs-store-adapter and make a PR. I now know that the project isn't stalled.
Look, we have 30+ repositories to maintain, and local file system storage for files isn’t a recommended method as it causes issues as soon as you use more than 2 instances. The issue was opened a while ago, and yes, we didn’t had a big look on it as it’s minor and all intégration tests pass. If you’re eager to help out, again feel free to open a PR.
and I’m not in favor removing the mkdir functionality as it’s a breaking change.
I understand you. As I've said, will do (with mkdir).
I think you as one of the core developers of parse-server are doing a great job and I appreciate what you and the other team members built. I'll see if I can help with other issues in the Parse project family.
I'm testing the fs-adapter and it works on creating a file on disk. I got an error when I try to retrieve the file though; this is the stack trace
error: Uncaught internal server error. [TypeError: Cannot read property 'getFileData' of undefined] TypeError: Cannot read property 'getFileData' of undefined at getHandler (/root/parse/node_modules/parse-server/lib/Routers/FilesRouter.js:69:22) at Layer.handle [as handle_request] (/root/parse/node_modules/parse-server/node_modules/express/lib/router/layer.js:95:5) at next (/root/parse/node_modules/parse-server/node_modules/express/lib/router/route.js:131:13) at Route.dispatch (/root/parse/node_modules/parse-server/node_modules/express/lib/router/route.js:112:3) at Layer.handle [as handle_request] (/root/parse/node_modules/parse-server/node_modules/express/lib/router/layer.js:95:5) at /root/parse/node_modules/parse-server/node_modules/express/lib/router/index.js:277:22 at param (/root/parse/node_modules/parse-server/node_modules/express/lib/router/index.js:349:14) at param (/root/parse/node_modules/parse-server/node_modules/express/lib/router/index.js:365:14) at param (/root/parse/node_modules/parse-server/node_modules/express/lib/router/index.js:365:14) at Function.process_params (/root/parse/node_modules/parse-server/node_modules/express/lib/router/index.js:410:3)
I got that when I specify the 'filesSubDirectory' option while creating the adapter. Instead when I leave options blank I receive an {"error": "unauthorized"}
Am I missing something?