fix: Nested objects are encoded incorrectly for MongoDB

[dblythy]

New Pull Request Checklist

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

Issue Description

The mongoDB adapter currently only encodes objects one level down, and the rest is stored as JSON. This results in a lot of nested objects being saved incorrectly, for example, nested dates saved as JSON with iso and type rather than a Date.

Related issue: #7575
Closes: #7575

Approach

Loops through nested keys and encodes as expected.

TODOs before merging

• Add tests
• A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

[parse-github-assistant]

Thanks for opening this pull request!

• 🎉 We are excited about your hands-on contribution!

[codecov]

Codecov Report

Base: 94.27% // Head: 94.25% // Decreases project coverage by -0.02% ⚠️

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

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha    #8209      +/-   ##
==========================================
- Coverage   94.27%   94.25%   -0.03%     
==========================================
  Files         180      180              
  Lines       13987    13989       +2     
==========================================
- Hits        13186    13185       -1     
- Misses        801      804       +3     

Impacted Files	Coverage Δ
src/Adapters/Storage/Mongo/MongoTransform.js	88.45% <100.00%> (+0.03%)	⬆️
src/batch.js	92.98% <0.00%> (-1.76%)	⬇️
src/ParseServerRESTController.js	96.96% <0.00%> (-1.52%)	⬇️
src/Adapters/Files/GridFSBucketAdapter.js	93.43% <0.00%> (-0.73%)	⬇️

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.

[mtrezza]

Looks good!

[dblythy]

I wonder if this is a breaking change, as this changes how nested keys are saved, which can impact nested indexes, or nested queries

[mtrezza]

It is a breaking change. I think we can prepare the PR and merge it for Parse Server 6-alpha in November.

[hariprasadiit]

@dblythy thanks for the PR. Should we consider applying similar fix for pointers as well? Nested pointers are also saved as plain JSON object instead of Class$ObjectId in MongoDB

[dblythy]

This PR probably does change how pointers are changed - I'll investigate

[parse-github-assistant]

I will reformat the title to use the proper commit message syntax.

[dblythy]

This doesn't actually change that as lines 558 - 562 are programmed to return the JSON:

parse-server/src/Adapters/Storage/Mongo/MongoTransform.js

Lines 555 to 574 in 16b1b2a

	const transformInteriorAtom = atom => {
	// TODO: check validity harder for the __type-defined types
	if (typeof atom === 'object' && atom && !(atom instanceof Date) && atom.__type === 'Pointer') {
	return {
	__type: 'Pointer',
	className: atom.className,
	objectId: atom.objectId,
	};
	} else if (typeof atom === 'function' || typeof atom === 'symbol') {
	throw new Parse.Error(Parse.Error.INVALID_JSON, `cannot transform value: ${atom}`);
	} else if (DateCoder.isValidJSON(atom)) {
	return DateCoder.JSONToDatabase(atom);
	} else if (BytesCoder.isValidJSON(atom)) {
	return BytesCoder.JSONToDatabase(atom);
	} else if (typeof atom === 'object' && atom && atom.$regex !== undefined) {
	return new RegExp(atom.$regex);
	} else {
	return atom;
	}
	};

Do we see any benefit of changing this?

[hariprasadiit]

This doesn't actually change that as lines 558 - 562 are programmed to return the JSON:

parse-server/src/Adapters/Storage/Mongo/MongoTransform.js

Lines 555 to 574 in 16b1b2a

	const transformInteriorAtom = atom => {
	// TODO: check validity harder for the __type-defined types
	if (typeof atom === 'object' && atom && !(atom instanceof Date) && atom.__type === 'Pointer') {
	return {
	__type: 'Pointer',
	className: atom.className,
	objectId: atom.objectId,
	};
	} else if (typeof atom === 'function' || typeof atom === 'symbol') {
	throw new Parse.Error(Parse.Error.INVALID_JSON, `cannot transform value: ${atom}`);
	} else if (DateCoder.isValidJSON(atom)) {
	return DateCoder.JSONToDatabase(atom);
	} else if (BytesCoder.isValidJSON(atom)) {
	return BytesCoder.JSONToDatabase(atom);
	} else if (typeof atom === 'object' && atom && atom.$regex !== undefined) {
	return new RegExp(atom.$regex);
	} else {
	return atom;
	}
	};

Do we see any benefit of changing this?

It'll remove possible edge cases with querying nested pointers. Right now, query on nested pointers works but it depends on the order of the props in the generated JSON and I think it's not future proof. Moreover how Postgres handles query on nested pointers is something I haven't tested

We'll also have consistent data format in the database

[dblythy]

@parse-community/server what are your thoughts regarding #8209 (comment)

[dblythy]

I think this is ready for merge

[mtrezza]

https://github.com/orgs/parse-community/teams/server what are your thoughts regarding #8209 (comment)

Seems there are some open questions about implications, so I would see this out of scope for this PR. Can someone open a separate issue so we can take our time to look into this?

I'll go ahead and merge this PR once the CI passes.

[mtrezza]

Suggestion for changelog entry:

BREAKING CHANGE

• Nested objects are now properly stored in the database using JSON serialization; previously, due to a bug only top-level objects were serialized, but nested objects were saved as raw JSON; for example, a nested Date object was saved as a JSON object like { "__type": "Date", "iso": "2020-01-01T00:00:00.000Z" } instead of its serialized representation 2020-01-01T00:00:00.000Z.

Does that sound right?

[dblythy]

Yep!!

[parseplatformorg]

🎉 This change has been released in version 6.0.0-alpha.15

[bdevore17]

@mtrezza I know this is marked as a breaking change, but is this safe in general? Unless there is a database migration, many existing queries are going to break. I think a lot of users have found workarounds for this bug in the past, and are now stuck in a bit of a quagmire with this change.

For example,

This is an existing query in our app:

const negotiationReminderQuery = new Parse.Query(Influencer)
	.lessThan("activeNegotiation.offer.expirationTimestamp.iso", new Date().toISOString())
	.findAll({ useMasterKey: true });

We can't rewrite this query unless we know that every document serialized before this PR has been updated.

If this change is to be made, it really seems like it needs to be tied to some kind of migration process and feature flagging.

[mtrezza]

@bdevore17 Could you please open a new issue with the info you posted here? Then let's discuss there.

[parseplatformorg]

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

[parseplatformorg]

🎉 This change has been released in version 6.0.0

[magnacartatron]

Hi Team,
This change is a little confusing.
On Parse V6 release I've created a new Class called "Test". This contains an object field.
When I try to save an object with a date in that field, when I query said object the date is still {}

I can see in that in MongoDB the value is stored correctly. When I hook up a V5 Parse Server I can retrieve the value but trying to retrieve the object that was created on V6 from V6 gives me an empty object.

Is there some kind of data migration that I need to perform?