Skip to content

_op: delete returning on after save object result #8386

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

Open
matheusfrozzi opened this issue Jan 14, 2023 · 13 comments · May be fixed by #8481
Open

_op: delete returning on after save object result #8386

matheusfrozzi opened this issue Jan 14, 2023 · 13 comments · May be fixed by #8481
Labels
type:bug Impaired feature or lacking behavior that is likely assumed

Comments

@matheusfrozzi
Copy link

New Issue Checklist

Issue Description

When we save an object on parse and delete a property, it is returning in the API { __op: 'Delete' }
We manipulate the object on AfterSave and print it before returning, and it doesn't appear these values.
It looks like it is added after the afterSave.

Steps to reproduce

Delete a value on a property field:

    if (!helper.isNullOrUndefined(skippedAt) && dirtyKeys.includes("skippedAt")) {
        eventOccurrence.set("skippedAt", new Date(skippedAt));
    } else {
        eventOccurrence.set('skippedAt', { __op: 'Delete' });
    }

Actual Outcome

"skippedAt":{"__op":"Delete"},
"rescheduledTo":{"__op":"Delete"},
"deletedAt":{"__op":"Delete"}

Expected Outcome

Don't return these information, because it broke the Model serialization in the apps front end

Environment

Parse server Version 5.4.0 with MongoDB 5

Server

  • Parse Server version: 5.40
  • Operating system: Linux Ubuntu
  • Local or remote host: Google Cloud

Database

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

Logs

@parse-github-assistant
Copy link

Thanks for opening this issue!

  • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

@mtrezza mtrezza added the type:bug Impaired feature or lacking behavior that is likely assumed label Jan 16, 2023
@dblythy
Copy link
Member

dblythy commented Mar 13, 2023

Isn't this expected? The purpose of afterSave is to return different data to the client. In your example you've set the fields to an object, so that object is going to be returned to the client.

I think if a key is unset in a beforeSave trigger though, the op is required so the JS and other SDKs know how to handle this response. In this case, should null be returned instead?

@matheusfrozzi
Copy link
Author

the op is returning to the client, this is not expected. This never happened in previous versions.
The thing is, I'm unsetting the fields in beforeSave. This should return null on the cliente response.

In iOS for example, I use codable, and to the serialization works, it have to return String or null, if it return an different type it will not work.

@matheusfrozzi
Copy link
Author

from @dblythy in other issue I created, I think this is the solution (I don't know if this problem is on the version 6)
"We updated the save response so it would only be keys that are updated. But in the case of pendingOps, perhaps it could make sense to return the latest database value instead of the op."

@dblythy
Copy link
Member

dblythy commented Mar 13, 2023

So does returning null work with the codeable and clear the value?

@matheusfrozzi
Copy link
Author

Yes, because if I'm unsetting the field, it is optional, so it will work with null.
But I like the idea of returning the latest database value instead of the op. haha

@dblythy
Copy link
Member

dblythy commented Mar 13, 2023

Yeah, I think that's how it should work for all other ops. I guess for an increment op, it could help keep the values in sync across multiple editors. What are your thoughts @mtrezza?

@dblythy
Copy link
Member

dblythy commented Mar 13, 2023

So from my understanding, this is enough to return an op?

Parse.Cloud.beforeSave("Test", obj => {
  obj.unset('field');
});
const test = new Parse.Object("Test");
test.set("field")
test.save() // field will be an op

@dblythy dblythy linked a pull request Mar 14, 2023 that will close this issue
1 task
@mtrezza
Copy link
Member

mtrezza commented Mar 14, 2023

I think a complete example with actual vs expected outcome would help to understand the issue and possible implications of the change.

@matheusfrozzi
Copy link
Author

Yes this is enough to understand.
Just to make it clear of the problem, look at the other issue I created.
When I was creating a user, it was returning the op: delete for password lol: #8387

@dblythy
Copy link
Member

dblythy commented Mar 15, 2023

At the moment, if unset is used in cloud code, {__op: Delete} is returned to the client. The JS SDK knows how to interpret this, but other SDKs that use JSON serialization (such as the Swift SDK) have implicit type declarations and can't decode this response. The expected outcome for an {__op: Delete} op on the backend should be null returned, which would need changes in the JS SDK.

It's also possible to return ops in afterSave (where the unsaved object is returned). This means that effectively you can tell the client to increment on next save for example. I don't think this behaviour is expected - i think if increment is used in afterSave, it should return value + 1 to the client

@mtrezza
Copy link
Member

mtrezza commented Mar 16, 2023

As I read it, op means a pending operation. There is no guarantee that that operation will execute successfully. So any assumption about the result of a pending operation is speculative.

Is there a currently existing feature that has the purpose of transmitting pending potential future states to clients?

If there is one, but a client SDK doesn't support it, then it either lacks a feature or has a bug.

If there is no such feature, then we probably shouldn't consider it one, just because it appears to exist because it triggers something on the client side for certain SDKs. It may not actually work as is but require further analysis and proper implementation. In other words, if a developer decides to manipulate the response object in afterSave in undocumented ways, they cannot expect a consistent outcome.

We probably also cannot make any assumptions about speculative operations, therefore, the known only value is the value before the operation. So in lieu of op we probably shouldn't assume that the operation will succeed and return and resulting value of that operation, but instead return the last known actual value.

@matheusfrozzi
Copy link
Author

The question is, that never was sent to the after save, because in the after save the op is not pending at all. It was successful done.
What changed that make it happen?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Impaired feature or lacking behavior that is likely assumed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@dblythy @mtrezza @matheusfrozzi and others