Skip to content

Incrementing embedded object property stopped working in 4.10.1+ #7653

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

Closed
4 tasks done
MobileVet opened this issue Oct 25, 2021 · 9 comments
Closed
4 tasks done

Incrementing embedded object property stopped working in 4.10.1+ #7653

MobileVet opened this issue Oct 25, 2021 · 9 comments
Labels
type:question Support or code-level question

Comments

@MobileVet
Copy link

MobileVet commented Oct 25, 2021

New Issue Checklist

Issue Description

Prior to version 4.10.1, one could increment an embedded object

Steps to Reproduce

const obj = new Parse.Object('TestObject', { doc: { a: 0 } })
obj.increment('doc.a')
obj.save()

Actual Outcome

However, after installing 4.10.1+, the save throws the following error:

Error: key doc.a must not contain '.'
....parse/lib/node/ParseObject.js:3137:35

It also isn't entirely clear where this error is being thrown from, the Server or the JS SDK.

Expected Outcome

Embedded document is incremented

Environment

Server

  • Parse Server version: 4.10.0 and 4.10.1-4
  • Operating system: Mac OS 11.6, Ubuntu 20
  • Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): Local & Heroku

Database

  • System (MongoDB or Postgres): MongoDB
  • Database version: 4.4.10
  • Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): MongoDB Atlas

Client

  • SDK (iOS, Android, JavaScript, PHP, Unity, etc): JavaScript
  • SDK version: JS 2.19.0 & 3.3.0

Logs

This PR removed some tests around embedded object incrementing as it indicated this wasn't supported by the server which is why we posted this bug report here and not in the JS SDK repo.

It actually has been supported... so I am a little confused by what this was trying to achieve. The included JS SDK 2.19.0 allowed this and all versions of the JS SDK include unit tests that appear to support embedded document increment.

I understood that @dplewis added embedded document incrementing a while back and we were really excited by that functionality since it is supported by Mongo and was working just fine.

@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 25, 2021

Thanks for opening this issue!

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

@mtrezza mtrezza added severity:medium type:bug Impaired feature or lacking behavior that is likely assumed labels Oct 25, 2021
@mtrezza
Copy link
Member

mtrezza commented Oct 25, 2021

@MobileVet Could you open a PR with a failing test for the master branch, to see whether this issue still occurs there?

@MobileVet
Copy link
Author

@MobileVet Could you open a PR with a failing test for the master branch, to see whether this issue still occurs there?

Yep, working on it. Trying to sort out exactly where the issue is as it appears tightly coupled between the server and the JS SDK (not surprisingly).

All unit and integration tests on the JS SDK pass, including new ones that I added to test the scenario. However, when I force Parse server 4.10.4 to use JS 2.19.0 instead of 3.3.0 the issue is resolved. This is what led me to believe it was in the JS SDK initially.

@mtrezza Do you have any insight on this interaction?

Focusing on the Server now... will have something later today.

@MobileVet
Copy link
Author

@mtrezza VERY interesting day here at the office. What we found was that the root cause of our issue was (for business logic reasons) we were incrementing a nested document value on a brand new object. Prior to Parse JS SDK 3.0.0, the increment was silently ignored by the Mongo driver. Because we were hydrating the object initially, we didn't notice that the increment didn't happen because the object was created successfully and that is what we were testing.

However, with Parse JS 3.0.0+, that combined create / increment operation throws an error and the object isn't saved at all. This is why our test failed when we updated. In reality, if we had also tested the incremented value itself, we would have seen that the older version wasn't doing the increment and should have failed as well.

We can definitely make a test that shows the failure of the increment on a new object, but I don't know that we should as that seems reasonable. You really shouldn't try to increment a value on a new object.

What is still a bit unknown to us is, what is the change in Parse JS SDK that led to this error bubbling up where as previously it didn't. We thought it was 1303 BUT that was included in 3.1.0 and 3.0.0 fails. We then thought it was the updated MongoDB dependency from 3.6.3 to 3.6.11... but pinning that and using it with Parse 3+ still fails.

I don't know if there needs to be further insight, but if anyone knows the core change we would appreciate learning that last piece of the puzzle.

@mtrezza
Copy link
Member

mtrezza commented Oct 26, 2021

That's quite an investigation!

We can definitely make a test that shows the failure of the increment on a new object, but I don't know that we should as that seems reasonable. You really shouldn't try to increment a value on a new object.

I would agree that a combined create/increment op seem odd. Good to know we can close this for now, but it surely would be interesting to solve this puzzle. Thanks for the update on this issue. I'm reclassifying this is a support question, as we could not proof that it's a bug.

@mtrezza mtrezza added type:question Support or code-level question and removed type:bug Impaired feature or lacking behavior that is likely assumed severity:medium labels Oct 26, 2021
@MobileVet
Copy link
Author

I'm reclassifying this is a support question, as we could not proof that it's a bug.

Agreed. Honestly it feels like the silent ignoring of the increment command was a bug and that is now resolved. This becomes more of an interesting observation at this point.

@mtrezza
Copy link
Member

mtrezza commented Oct 27, 2021

So this may be a bug that still exists on the Parse Server 4.x branch? If so, unless someone wants to open a PR for the 4.x branch, I would call it a "no-fix", because our internal resources are focused on the 5.x release for now.

@MobileVet
Copy link
Author

So this may be a bug that still exists on the Parse Server 4.x branch? If so, unless someone wants to open a PR for the 4.x branch, I would call it a "no-fix", because our internal resources are focused on the 5.x release for now.

Apologies, I wasn't clear. No it does not still exist... we are all good!

The bug (or suboptimal behavior) existed prior to 4.10.1. It has been resolved in 4.10.1+.

@mtrezza
Copy link
Member

mtrezza commented Oct 27, 2021

I see, thanks for clarifying, at least good to know we are good on all branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:question Support or code-level question
Projects
None yet
Development

No branches or pull requests

2 participants