fix: Saving a new Parse.Object with an unsaved Parse.File fails

[swittk]

Fixes #1660 (Caused by passing a Parse.File into the LocalDataStore, which is unexpected)

New Pull Request Checklist

• I am not disclosing a vulnerability.
• I am creating this PR in reference to an issue.

Issue Description

Saving a new Parse.Object with an unsaved Parse.File fails.

Closes: #1660

Approach

Checks that the object being pinned to the localDataStore is a ParseObject.

[parse-github-assistant]

Thanks for opening this pull request!

• ❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.

[mtrezza]

Could you add test ?

[codecov]

Codecov Report

Base: 99.89% // Head: 99.89% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (d50b896) compared to base (1c96205).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##            alpha    #1662   +/-   ##
=======================================
  Coverage   99.89%   99.89%           
=======================================
  Files          61       61           
  Lines        5991     5992    +1     
  Branches     1373     1374    +1     
=======================================
+ Hits         5985     5986    +1     
  Misses          6        6           

Impacted Files	Coverage Δ
src/ParseObject.js	99.89% <100.00%> (+<0.01%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[swittk]

I've added the test case

[dplewis]

@swittk I wrote the entire LocalDatastore code myself. Can you add an integration test to replicate the issue? I might have overlooked this

[swittk]

It's not a bug related to the LocalDatastore itself I think; but rather in the save() method of ParseObject which allows both ParseObject | ParseFile

Parse-SDK-JS/src/ParseObject.js

Line 2371 in f27d043

save(target: ParseObject | Array<ParseObject | ParseFile>, options: RequestOptions) {

But then in the then clause after saving all; it pins them all into the LocalDatastore, which, as far as I understand, expects only Parse.Objects, so when a Parse.File goes in it crashes.

Parse-SDK-JS/src/ParseObject.js

Line 2494 in f27d043

for (const object of target) {

I've spent the past few hours trying to get it to fail in the integration test, but it won't. The situation does occur in-browser with Parse SDK 3.5.1 and 4.0.0 though (Parse.File reaching LocalDatastore causing the crash).

The test cases in ParseFile-test.js (not the integration ones) do fail due to the same _getId() calls, though, with Parse.Files getting passed into the save() method of ParseObject. So right now I'm not sure what the exact difference in code paths are between npm run test and npm run integration, and can't really demonstrate it; but the error really is there in-browser.

(Here's an example of what my current hotfix to the problem looks like in node_modules)

[dplewis]

@swittk I can verify that this is an issue. Can you add this failing test to your PR and rebase?

it('can save file to localDatastore', async () => {
    Parse.enableLocalDatastore();
    const file = new Parse.File('parse-js-sdk', [61, 170, 236, 120]);
    const object = new Parse.Object('TestObject');
    await object.pin();

    object.set('file', file);
    await object.save();

    const query = new Parse.Query(TestObject);
    query.fromLocalDatastore();
    const results = await query.find();

    const url = results[0].get('file').url();
    assert.equal(results.length, 1);
    assert.notEqual(url, undefined);
    assert.equal(url, file.url());
  });

[dplewis]

Everything looks good so far

[dplewis]

LGTM! Just waiting for the CI to pass

[swittk]

It seems like the CI hasn't run yet? ._.

[mtrezza]

Now it is running

[swittk]

I've tried running npm run integration on NodeJS 19.3.0 (same as the runner) and encountered the same issue.
It was solved by simply running npm install again after switching to node 19, and integration tests finished fine afterwards.

[dplewis]

LGTM! The flaky test will be fixed shortly.

[parseplatformorg]

🎉 This change has been released in version 4.0.0-alpha.9

[parseplatformorg]

🎉 This change has been released in version 4.1.0-beta.1

[parseplatformorg]

🎉 This change has been released in version 4.1.0-alpha.1

[parseplatformorg]

🎉 This change has been released in version 4.1.0