Skip to content

Hot fix for #1237 and #1202 #1256

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

Merged

Conversation

ananfang
Copy link

@ananfang ananfang commented Feb 6, 2018

1. Fix typo PFPreconditionFailOnError and PFPreconditionFailAndSetError.

  1. If there is a new object in OfflineStore, should not send the exception when the saved object update its OfflineStore cache:

change the assert condition in function - (void)updateObjectIdForObject:oldObjectId:newObjectId: of PFOfflineStore.m

// If this object is just saved, update local store without exception.
BOOL isNewlySaved = (oldObjectId == nil) && (object.createdAt != nil);
PFConsistencyAssert(isNewlySaved || existing == nil || existing == object,
                    @"Attempted to change an objectId to one that's already known to the OfflineStore. className: %@ old: %@, new: %@",
                    className, oldObjectId, newObjectId);

…or. 2. If there is a new object in OfflineStore, should not send the exception when the saved object update its OfflineStore cache
@flovilmart
Copy link
Contributor

@ananfang thanks for the PR, PFPreconditionFailOnError doesn’t exist I’m not even sure how it compiles, can you revert to PFPreconditionFailOnError? This seem unrelated to that PR.

@ananfang
Copy link
Author

ananfang commented Feb 6, 2018

@flovilmart
I just reverted PFPreconditionBailOnError and PFPreconditionBailAndSetError.

Copy link
Contributor

@flovilmart flovilmart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try it, let's hope it doesn't yield any corruption issue.

@flovilmart flovilmart merged commit 6b066c9 into parse-community:release-1.17.0 Feb 6, 2018
@ananfang ananfang deleted the hotfix-issue-1237 branch February 7, 2018 02:56
@ananfang
Copy link
Author

ananfang commented Feb 7, 2018

@flovilmart
Thanks for your help 👍

This hotfix is just a workaround, the actual problem is SDK assigns twice (or more) objectId to newly saved object. The first time objectId is assigned, existing == nil, pass the assert. But the second time, existing != nil && existing != object, raises the exception.

@flovilmart
Copy link
Contributor

DO you have some code that reproduces the issue?

@ananfang
Copy link
Author

ananfang commented Feb 8, 2018

Followings are my scenario:

  1. A chat view with two users.
  2. Use ParseLiveQuery to listen messages created.
  3. User sends message as a PFObject, save it by saveInBackground() method.

It will not always crash, about 50/50.

@flovilmart
Copy link
Contributor

flovilmart commented Feb 8, 2018 via email

@ananfang
Copy link
Author

ananfang commented Feb 8, 2018

No, I don't mutate messages from LiveQuery

@flovilmart
Copy link
Contributor

Would you be able to share a demo project that reproduces the issue? I believe it’s related to liveQuery

@brianyyz
Copy link

brianyyz commented Feb 8, 2018

+1

@ananfang
Copy link
Author

ananfang commented Feb 9, 2018

@flovilmart
Demo Project

Run and tap "Add Message" button, it will crash all the time by "Attempted to change an objectId to one that's already known to the Offline Store"

@ananfang
Copy link
Author

ananfang commented Feb 9, 2018

I found a PFObject Equality bug comes from my "isNewlySaved" workaround.

Following is scenario:

  1. Enable Local Datastore
  2. Use Live Query to listen new Message object (PFObject) created.
  3. Save a new Message object.
  4. There will be two Message objects assigned the same new objectId.
  5. Without "isNewlySaved" condition, raise assert Attempted to change an objectId to one that's already known to the Offline Store. With "isNewlySaved" condition, it will only store the last of two Message objects in the local datastore.
  6. If we check these two newly saved objects with same objectId, they are not equal. (But they are supposedly equal when enable locale datastore)

Thus, DO NOT use my "isNewlySaved" workaround.

Instead, we should find out why Parse / LiveQuery assign the new objectId into two (or more) different objects.

@flovilmart
Copy link
Contributor

Ok so basically, creating and receiving an object through liveQuery will yield the issue. I need to either update LiveQuery or make sure the liveQUery received objects are treated the same as the ones fetched.

@ananfang
Copy link
Author

ananfang commented Feb 9, 2018

Yes, you can try this Chat demo project

@ananfang
Copy link
Author

ananfang commented Mar 20, 2018

@flovilmart
Should we open a new issue for this?
Or it was already fixed in release 1.17.0?

I found you already fixed it in release 1.17.0, I'll check if there is any new issue. If there is, I'll open a new one, thanks for your effort.

@flovilmart
Copy link
Contributor

Thanks! Please use the release-1.17.1 branch in the meantime as 1.17.0 is broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants