Endpoints for importing and exporting data #3046

davimacedo · 2016-11-13T16:50:10Z

Endpoints for:
a) Import Data into a class - it receives a file in the same format of parse.com and backwards the data that is exported;
b) Import Relation Data into a class relation - it receives a file in the same format of parse.com and backwards the data that is exported;
c) Export Data from a class - It returns a zipped .json file just like parse.com

flovilmart

Overall, I'm not sure that importation should live as part of the web server. For me that seems to be a command line tool that you run on a powerful enough machine.

The export and import methods won't handle anything any large collections, as we already see problems with push notifications sent to 100k users. Given that the data is stringified, buffered and zipped, you need at least 4x the memory required to to make that work, just for a single collection.

flovilmart · 2016-11-14T16:54:13Z

package.json

  "dependencies": {
+    "adm-zip": "0.4.7",
    "bcryptjs": "2.3.0",
+    "bluebird": "3.4.6",


Not sure we want bluebird as a dependency

We've removed it, but I think we should think about including it in the project. Take a look in the code the additional lines we had to include in order to manage the promises concurrency (probably not as effective as Bluebird does).

flovilmart · 2016-11-14T17:22:05Z

src/ParseServer.js

      maxUploadSize: maxUploadSize
    }));
+
+    api.use(ImportRouter.getRouter());


those should be protected by masterKey

flovilmart · 2016-11-14T17:22:20Z

src/Routers/ExportRouter.js

+export class ExportRouter extends PromiseRouter {
+
+  handleExportProgress(req) {
+    return Promise.resolve({ response: 100 });   


not implemented?

We let to implement it later, but it is implemented now.

flovilmart · 2016-11-14T17:23:04Z

src/Routers/ExportRouter.js

+
+
+    rest.find(req.config, req.auth, data.name,  data.where)
+    .then((results) => {


loading a full collection in memory is likely to crash the server.

We streamed everything and it is not crashing anymore. We tested with a 10M entries class.

flovilmart · 2016-11-14T17:29:49Z

src/Routers/ImportRelationRouter.js

+        res.status(200);
+        res.json({response: 'We are importing your data. You will be notified by e-mail once it is completed.'});
+      }
+      bluebird.map(restObjects, importRestObject, {concurrency: 100})


this code should not be duplicated, only the outcome of the import changes.

Refactored.

flovilmart · 2016-11-14T17:30:14Z

src/Routers/ImportRouter.js

+      res.status(200);
+      res.json({response: 'We are importing your data. You will be notified by e-mail once it is completed.'});
+    }
+    bluebird.map(restObjects, importRestObject, { concurrency: 100 })


duplicated code.

In the same file now and not duplicated anymore.

flovilmart · 2016-11-14T17:30:47Z

src/Routers/ImportRouter.js

+      delete restObject.createdAt;
+    }
+    if (restObject.updatedAt) {
+      delete restObject.updatedAt;


why deleting those keys? In case of import we don't want to update them right?

This is the same behavior parse.com had and in my opinion that's the correct one. If you are importing new rows today, I expect those rows with createdAt and updatedAt set to today. Parse.com (and also Parse Server) always ignore insert/update of those columns. We've only tried to reproduce the same behavior here.

flovilmart · 2016-11-14T17:31:29Z

src/Routers/ImportRouter.js

+    }
+    if (restObject.objectId) {
+      return rest
+        .update(req.config, req.auth, req.params.className, restObject.objectId, restObject)


we have a internal upset that just does that, probably use it instead of handling 2 paths.

hey @flovilmart I've checked the rest and RestWrite classes and they don't implement the upsert, we only have access to it directly from the DatabaseController, but on the import we need things that are only implemented on the RestWrite class such as the call to the expandFilesForExistingObjects() method.

Using the rest will also make all triggers, hooks, etc, be reached. That's the idea.

facebook-github-bot · 2016-11-22T23:47:43Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-11-23T10:24:37Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-11-23T14:47:20Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-11-28T21:57:17Z

@davimacedo updated the pull request - view changes

flovilmart · 2016-11-28T22:09:14Z

@davimacedo you probably need a rebase as well as running npm run lint before pushing :)

davimacedo · 2016-11-28T23:27:13Z

Sure thing... we will do it as soon as we have fixed all reviews you asked.

facebook-github-bot · 2016-11-30T13:15:19Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-12-02T12:59:24Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-12-02T13:01:36Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-12-02T23:56:00Z

@davimacedo updated the pull request - view changes

flovilmart · 2016-12-03T00:14:05Z

We've enforced harder rules for the coding style, please rebase on master.

facebook-github-bot · 2016-12-04T23:28:33Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-12-13T22:52:08Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-12-14T12:47:34Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-12-14T13:33:08Z

@davimacedo updated the pull request - view changes

facebook-github-bot · 2016-12-14T13:46:35Z

@davimacedo updated the pull request - view changes

davimacedo · 2016-12-14T14:02:56Z

Hi, @flovilmart
I understand your concern about memory, but people want it easily and friendly in the dashboard. If we run it outside parse server we will also loose the capability of reaching hooks, triggers, etc. For this kind of feature, maybe we could recommend users to run the dashboard connecting to a parse server running on a powered enough machine (maybe in the user localhost). But this is only a comment. Like I commented before, we think memory will not crash here.
Can you please check again?

davimacedo · 2016-12-14T14:03:42Z

Everything was also rebased and the lint is passing :)

acegreen · 2017-01-08T04:28:34Z

This should be prioritized with the recent hacks on MongoDB databases

flovilmart · 2017-01-08T14:53:17Z

@acegreen people should have DB backups, not CSV dumps.

jordanhaven · 2017-01-09T22:58:05Z

I would really appreciate this, as well. We have customer service people that need to export records for credit card disputes, and the Parse.com behavior is way better/safer than trying to get them into mongo scripting.

flovilmart · 2017-01-09T23:32:19Z

Not sure if that export would fit your bill as it the entirety of the database that will be exported

jordanhaven · 2017-01-09T23:47:43Z

I haven't actually looked at the contents of the PR, I was assuming it was similar to the export feature in Parse.com, that would let you filter a class and then export those results. Maybe that's in the related PR in parse-dashboard (I thought, being by the same author, the two were separate but necessary implementations of the same feature). If that's not the case, my bad!

davimacedo · 2017-01-11T18:42:00Z

It will not export the entire database, but an entire class or an entire relation. I have it running in my servers and it is actually running pretty good so far.

facebook-github-bot · 2017-01-12T02:35:55Z

@davimacedo updated the pull request - view changes

acinader

so this looks really good to me. sorry a lot of my comments are silly formatting nits, but that's just how i read :).

i have one or two actually substantive questions....

acinader · 2017-01-12T02:42:28Z

src/Routers/ExportRouter.js

+        jsonFileStream.write(',\n');
+      }
+
+      jsonFileStream.write(JSON.stringify(data.results, null, 2).substr(1).slice(0,-1));


nit: space after , slice(0, -1)

acinader · 2017-01-12T02:44:25Z

src/Routers/ExportRouter.js

+
+    jsonFileStream.write('{\n"results" : [\n');
+
+    const findPromise = data.name.indexOf('_Join') === 0 ?


more super nit, but could format this ternary more consistently/nicer??

Is it better now?

acinader · 2017-01-12T02:44:43Z

src/Routers/ExportRouter.js

+    : rest.find(req.config, req.auth, data.name,  data.where, { count: true, limit: 0 });
+
+    return findPromise
+    .then((result) => {


acinader · 2017-01-12T02:47:06Z

src/Routers/ExportRouter.js

+    .then((fileData) => {
+
+      return emailControllerAdapter.sendMail({
+        text: `We have successfully exported your data from the class ${req.body.name}.\n


who's we? :)

Improved! :)

acinader · 2017-01-12T02:48:13Z

src/Routers/ExportRouter.js

+        subject: 'Export failed'
+      });
+    })
+    .then(() => {


should this be an always? will this get called if an error condition happens and there's a catch?

Now it will always execute

acinader · 2017-01-12T02:50:22Z

src/Routers/ImportRouter.js

+          subject: 'Import failed'
+        });
+      } else {
+        throw new Error('Internal server error: ' + error);


you use string templates everywhere else...better than concatenation... :)

acinader · 2017-01-12T02:50:47Z

src/Routers/ImportRouter.js

+
+  expressRouter() {
+    const router = express.Router();
+    const upload = multer();


acinader · 2017-01-12T02:51:29Z

src/middlewares.js

  res.header('Access-Control-Allow-Origin', '*');
  res.header('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE,OPTIONS');
-  res.header('Access-Control-Allow-Headers', 'X-Parse-Master-Key, X-Parse-REST-API-Key, X-Parse-Javascript-Key, X-Parse-Application-Id, X-Parse-Client-Version, X-Parse-Session-Token, X-Requested-With, X-Parse-Revocable-Session, Content-Type');
+  res.header('Access-Control-Allow-Headers', 'X-Parse-Master-Key, X-Parse-REST-API-Key, X-Parse-Javascript-Key, X-Parse-Application-Id, X-Parse-Client-Version, X-Parse-Session-Token, X-Requested-With, X-Parse-Revocable-Session, Content-Type, X-CSRF-Token');


i wish i knew enough to know if this is as conservative as possible??...

I don't think it will bring additional vulnerability. Maybe some has a different opinion?

cause i hadn't fully read all the comments

acinader · 2017-01-12T03:00:47Z

the busted test may/probably is because i change archiver from "archiver": "^1.2.0" to "archiver": "1.3.0" so it'll play well with greenkeeper.io

jordanhaven · 2017-01-24T15:55:23Z

@davimacedo could you clarify if this exports an entire class, or a class WITH any filters you've applied in the dashboard (ie: the Parse.com behavior)?

facebook-github-bot · 2017-02-11T18:01:51Z

@davimacedo I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

jordanhaven · 2017-02-11T18:09:32Z

^ Meant to write back, but to answer my previous question, this (and the corresponding parse-dashboard) PRs only export a full class. I made some updates to support passing through the filters to the exports (parse-dashboard: LearnWithHomer/parse-dashboard@8d68daa, parse-server: LearnWithHomer@6c3ccb1), and it's working good for us at replicating the Parse.com behavior.

AmrKafina · 2017-05-06T16:04:44Z

Any reason this is being held up? I recently put together a tool that automates importing data, but having the ability to manually set objectIds (as I believe this PR adds) would make it a lot better.

codecov · 2017-05-24T18:49:31Z

Codecov Report

Merging #3046 into master will increase coverage by <.01%.
The diff coverage is 86.18%.

@@            Coverage Diff             @@
##           master    #3046      +/-   ##
==========================================
+ Coverage   90.14%   90.15%   +<.01%     
==========================================
  Files         114      116       +2     
  Lines        7550     7726     +176     
==========================================
+ Hits         6806     6965     +159     
- Misses        744      761      +17

Impacted Files	Coverage Δ
src/Routers/FeaturesRouter.js	`100% <ø> (ø)`	⬆️
src/ParseServer.js	`88.75% <100%> (+0.21%)`	⬆️
src/RestWrite.js	`93.28% <100%> (+0.4%)`	⬆️
src/middlewares.js	`97.26% <100%> (ø)`	⬆️
src/Controllers/SchemaController.js	`97.04% <100%> (ø)`	⬆️
src/Config.js	`94.92% <100%> (+0.03%)`	⬆️
src/rest.js	`98.5% <100%> (+1.49%)`	⬆️
src/Routers/ImportRouter.js	`78.82% <78.82%> (ø)`
src/Routers/ExportRouter.js	`91.86% <91.86%> (ø)`
... and 5 more

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 1f11ad5...330a195. Read the comment docs.

davimacedo · 2017-05-24T19:04:16Z

Guys... Sorry about the delay with this PR.

@jordanhaven Thanks for the contribution. I've just incorporated it in the PR.

@flovilmart and @acinader I've just done the requested changes and rebased to the master. Can you guys check again please?

jordanhaven · 2017-05-24T19:23:04Z

@davimacedo thanks for incorporating! Can you do the same for the parse-dashboard changes?

ETA: By that I mean passing the filters through as in this commit:LearnWithHomer/parse-dashboard@8d68daa

Thanks!

davimacedo · 2017-05-24T23:03:41Z

@jordanhaven Just done. Thanks!

nitrag · 2017-05-25T03:26:10Z

+1 thanks for this!

tutchmedia · 2017-09-05T13:12:56Z

Hey Folks, Does anyone know what is happening with this please?

jpgupta · 2018-02-14T10:58:02Z

bump :) would be a really cool feature

mnlbox · 2018-07-25T16:23:52Z

@davimacedo Any news about this?
Any plan to merge this PR?

flovilmart · 2018-07-25T16:53:37Z

It’s unlikely at this point as the main memory usage concerns are not addressed. This belongs to a standalone tool.

I’m not against exposing a simpler API to ParseServer so one can import objects based on a stream representation (with the objectId preserved) and export them with a streaming API.
As for exposing such endpoints, now that everyone has access to the mongodb / Postgres, I don’t see how CSV is a valid backup / export solution. As you can already backup/export right from your DB.

mnlbox · 2018-07-30T06:09:14Z

@flovilmart CSV is a general purpose format and we can import it in many applications like BI tools, Data analysing, ...
I think this PR implemented a very useful feature and suggest you to reopen it.

Backup/Export from parse rather than mongo dump is a very brilliant feature and can useful for many people like me specially if we can call it from parse-dashboard (related parse-dashboard PR that blocked and waiting this PR: parse-community/parse-dashboard#585).

/CC: @davimacedo

georgesjamous · 2018-07-30T06:31:01Z

I agree with @flovilmart
I think that Importing and Exporting the data should be performed on the RAW data store in the database. Since no additional restrictions or checking needs to be done server side, it makes sense not to include parse in this process. You can take snapshots of your database as you go on as backup for a production case.

If the format file is what you worry about, to my knowledge you can export Mongo into CSV and vice-versa. (haven't tried it)
mongoexport is a utility that produces a JSON or CSV export of data stored in a MongoDB instance
mongoexport docs
relevant post on stackoverflow

flovilmart · 2018-07-30T11:25:51Z

@mnlbox it is likely that this feature, as it is implemented now, blocks completely the server for an undetermined period of time, at import or export. It is also possible that this feature doesn’t work if the process takes more than 30s (the usual request timeout).

For those reasons and the one explained above, we’ll keep this one closed.

davimacedo mentioned this pull request Nov 13, 2016

Endpoint for importing json data in class #2350

Closed

facebook-github-bot added the GH Review: review-needed label Nov 13, 2016

davimacedo mentioned this pull request Nov 13, 2016

UI for importing and exporting data parse-community/parse-dashboard#585

Closed

flovilmart suggested changes Nov 14, 2016

View reviewed changes

Jcarlosjunior force-pushed the importAndExport branch from 9dc61c8 to 18680e1 Compare November 23, 2016 14:47

Jcarlosjunior force-pushed the importAndExport branch from 87ec12f to 7f30de6 Compare December 2, 2016 12:59

Jcarlosjunior force-pushed the importAndExport branch from 7f30de6 to 86fffad Compare December 2, 2016 13:01

Jcarlosjunior force-pushed the importAndExport branch from e010a13 to ae6f46f Compare December 13, 2016 22:46

rsouzas force-pushed the importAndExport branch from ae6f46f to 9350a18 Compare December 14, 2016 12:47

rsouzas force-pushed the importAndExport branch from 9350a18 to efd6930 Compare December 14, 2016 13:33

rsouzas force-pushed the importAndExport branch from efd6930 to c6b784c Compare December 14, 2016 13:45

acinader previously approved these changes Jan 12, 2017

View reviewed changes

natanrolnik mentioned this pull request Jan 26, 2017

Not receiving email link from Parse export #3435

Closed

davimacedo force-pushed the importAndExport branch from 7efba55 to d7087c1 Compare May 24, 2017 18:49

Endpoints for importing and exporting data

330a195

davimacedo force-pushed the importAndExport branch from d7087c1 to 330a195 Compare May 24, 2017 18:53

flovilmart closed this Jul 25, 2018



		rest.find(req.config, req.auth, data.name, data.where)
		.then((results) => {


		jsonFileStream.write('{\n"results" : [\n');

		const findPromise = data.name.indexOf('_Join') === 0 ?

Uh oh!

Endpoints for importing and exporting data #3046

Endpoints for importing and exporting data #3046

Uh oh!

Conversation

davimacedo commented Nov 13, 2016

Uh oh!

flovilmart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Nov 22, 2016

Uh oh!

facebook-github-bot commented Nov 23, 2016

Uh oh!

facebook-github-bot commented Nov 23, 2016

Uh oh!

facebook-github-bot commented Nov 28, 2016

Uh oh!

flovilmart commented Nov 28, 2016

Uh oh!

davimacedo commented Nov 28, 2016

Uh oh!

facebook-github-bot commented Nov 30, 2016

Uh oh!

facebook-github-bot commented Dec 2, 2016

Uh oh!

facebook-github-bot commented Dec 2, 2016

Uh oh!

facebook-github-bot commented Dec 2, 2016

Uh oh!

flovilmart commented Dec 3, 2016

Uh oh!

facebook-github-bot commented Dec 4, 2016

Uh oh!

facebook-github-bot commented Dec 13, 2016

Uh oh!

facebook-github-bot commented Dec 14, 2016

Uh oh!

facebook-github-bot commented Dec 14, 2016

Uh oh!

facebook-github-bot commented Dec 14, 2016

Uh oh!

davimacedo commented Dec 14, 2016

Uh oh!

davimacedo commented Dec 14, 2016

Uh oh!