Skip to content
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

Issue when fetching objects with reference/relationship #140

Open
davidrothera opened this issue Jun 7, 2019 · 21 comments
Open

Issue when fetching objects with reference/relationship #140

davidrothera opened this issue Jun 7, 2019 · 21 comments
Assignees

Comments

@davidrothera
Copy link
Contributor

Similar to #95 there is an issue when doing a "fresh" fetch from a device (for example after installing on a new device) where sometimes objects have their reference fields set to nil.

This issue only happens sometimes and so its clear that its a race condition in that the child objects are being created locally before the parent ones.

Expected behavior

All objects fetched from iCloud correctly

Actual behavior(optional)

Some objects fetched with bad linking relationships which causes issues and even ends up syncing bad data back to iCloud

Steps to reproduce the problem(optional)

  • Build/run project at https://github.com/davidrothera/IceCreamBug
  • Press button to create test data
  • Check that data is all created in CloudKit dashboard
  • Delete application from device/simulator
  • Run application again
  • Check Realm file and observe nil relationships
@davidrothera
Copy link
Contributor Author

The issue comes from the section of code where we are inserting objects to the Realm, dynamicObject(ofType:forPrimaryKey) can return a nil (which it would if said item didn't yet exist.

} else if let owner = record.value(forKey: prop.name) as? CKRecord.Reference, let ownerType = prop.objectClassName {
recordValue = realm.dynamicObject(ofType: ownerType, forPrimaryKey: primaryKeyForRecordID(recordID: owner.recordID))
// Because we use the primaryKey as recordName when object converting to CKRecord
}

One way I can think of solving this is that all objects which are being inserted are added to a queue, if you try and resolve an object like this and fail then you put that item to the back of the queue.

You would still need to solve for the issues of if the reference is never valid in which case the queue could hold some kind of struct holding both the CK item as well as some kind of counter of how many times the action has been retried and at a certain stage give up.

@davidrothera
Copy link
Contributor Author

After looking into it further I think adding CKRecord objects to a queue would work however it would completely change the way that things work as at the moment every time a record is downloaded it is updated once fetched.

Another possibility here is to change the pull() method to be able to take an optional completion which is executed once the fetch is complete, this way you resolve the issue by:

  • Running a pull()
  • Supplying a completion that upon finishing executes code as described in Reference syncing problem #95 to find and resolve broken references

@luispadron
Copy link

I'm also experiencing this issue.
Seems to be random at times? I have models that look like:

class Class {
    var id: UUID
    var semester: Semester?
    //...
}

class Semester {
    var id: UUID
    var year: Int
    var term: String
    //...
}

What ends up happening is that the Class objects (the parents) are always synced and ready across devices, that is, adding a class on one device will populate the database of another device with that new class object. However, sometimes the semester field of the new class object will be nil in Realm on the other device.

I've added a completion handler to pull on my fork of this repo and this doesn't seem to have fixed the issue. I also tried calling pushAll after adding any class objects to the database and again, does not seem to fix the issue.

My project is open-source here in case you would like to debug (it's rather big) so maybe using the example @davidrothera would be better.

I'm new to CloudKit but not Realm so I may be doing something wrong on the CloudKit side of things but I don't believe so since it works correctly at times (syncs everything).

Any ideas?

@d-sandman
Copy link

d-sandman commented Aug 1, 2019

Same situation with the to many relationship:

class Person: Object {
  // ...
  let dogs = List<Dog>()
  @objc dynamic var cat: Cat?
  // ...
}
class Dog: Object {
  // ...
  let owners = LinkingObjects(fromType: Person.self, property: "dogs")
  // ...
}
class Cat: Object {
  // ...
  let owner = LinkingObjects(fromType: Person.self, property: "cat")
  // ...
}

Neither of references gets restored on initial fetch, not sometimes but always.

@d-sandman
Copy link

As soon as I understand the problem occurs when we try to create a reference on an object which wasn't yet imported?
The simplest solution I could think of could be importing references after all records are imported. As an example, Realm does exactly this when converting local realm to synced https://realm.io/docs/cookbook/js/local-realm-to-synced/

PS: Honestly I still think that I'm doing something wrong, I refuse to believe that so many people use it without references being synced :) @caiyue1993 if only you could shed some light! And thanks for the amazing work!

@plivesey
Copy link

I'm not sure if this helps (I'm new to both this library and cloudkit), but I don't think to-many relationships are supported:

// To-many relationship is not supported yet.

I just found this now and am a bit confused since this seems to be a massive limitation of the library. I'm going to look more into this tomorrow...

@davidrothera
Copy link
Contributor Author

I might be wrong but I believe it’s meaning many-to-many which require a look through table to implement usually where as most of the above reported issues have been one-to-many

@plivesey
Copy link

No, that sounds right...lmk if you find anything here. I'll let you know if I find anything which could help. (Also, I decided not to implement to many relationships and instead am just doing the inverted way)

@plivesey
Copy link

plivesey commented Sep 9, 2019

So yeah, you were right. And I've also ran into the same problem here and do think that it's a bug in IceCream. If it helps, I solved it by always resolving the sync objects in order. So, parents are always written first, followed by children.

Of course, this isn't a good generic solution because it's possible to have a child and parent of the same type. But for me, that's not a problem (I don't have that relationship), and it sounds like you may not either? So, maybe you can do the same thing.

Here's the commit with my work (it's not tested yet, but I think something like this should work). It's based off my own fork, so you can't copy the code directly, but it's pretty simple: plivesey@24c2b84

@ivanvorobei
Copy link

I found this also. And I use relations.
@plivesey , you found solution? You can describe more details about it?

@plivesey
Copy link

@ivanvorobei did you see the commit I referenced above? Basically, it processes the objects in order from parent to child. You could also take a look at my fork which has many bug fixes and changes (like shared dbs). It still needs a lot of work but eventually, I'm hoping to merge this back in.

@ivanvorobei
Copy link

ivanvorobei commented Oct 12, 2019

@plivesey for me strange situation. I have to class - Debt and Person.

class Debt: Object {
    @objc dynamic var owner: Person?
}

It to-one relationships. Also class Person:

class Person: Object {
    let debts = LinkingObjects(fromType: Debt.self, property: "owner")
}

I use this scheme because Person can have many debts, but debt have one person.

I think Debt is parent in this example. It true? But if start sync parent, I have trouble with sync (because Persons sync after).

I decided that I have the wrong model, but looked at the realm website. Example of Many-to-one here: https://realm.io/docs/swift/latest/#relationships

If I am right, your commit work incorrect sometimes (for my example).
I propose use order when we configure IceCream. We pass objects for sync when configure, maybe can use this order of objects. Write me what you think about it?

SyncEngine(objects: [FistSync, SecondSync...], databaseScope: .private)

Sorry for long-read.

@plivesey
Copy link

Oh yeah...parent/child is confusing here. Not sure what the right terminology is here. In this case, I think your model is correct. Your model is actually correct. It's unfortunate, but CloudKit does one to many relationships differently: https://developer.apple.com/library/archive/documentation/DataManagement/Conceptual/CloudKitQuickStart/AddingReferences/AddingReferences.html. So you can't use the canonical realm way of doing it.

Given my code above, you should sync Person first, and then Debt.

I think my commit works correctly for your example, you just need to pass in the person sync object first and the debt sync object second. This line is what should handle this: plivesey@24c2b84#diff-972caea3ef15b318792017da9ba0f50aR164

@ivanvorobei
Copy link

@plivesey in your commit check order fir objects? Or how sync start Person?

@plivesey
Copy link

When you create the sync engine:

self.engine = SyncEngine(objects: [SyncObject<Person>(...), SyncObject<Debt>(...)])

This ensures that all person objects will be added to the db before debt objects and therefore there won't be any missing references.

@plivesey
Copy link

So, the order is created here on initialization.

@ivanvorobei
Copy link

ivanvorobei commented Oct 15, 2019

@plivesey Oh, true, I mean it. I propose it in my long-read, maybe my English bad)
but I did not understand - is it only in your version or is it in the original version?
In original it not work, I am test.

UPD: sorry, understand. Maybe propose it to merge?

@plivesey
Copy link

This is only in my version. It's only in this commit: plivesey@24c2b84 which is only in my fork. My fork has a bunch of changes which I do eventually intend to merge back in, but I keep changing things as I find better ways of doing things/find bugs.

@adamsmaka
Copy link

adamsmaka commented Nov 11, 2019

So unsupported List<Dog>() is a bug which could be fixed soon or should I reconstruct my app to handle this case? My model has to-many relationship to the same ObjectType and it's lost on app restart when IceCream refetch data from iCloud.

@Fonceur
Copy link

Fonceur commented Jul 3, 2020

This is only in my version. It's only in this commit: plivesey@24c2b84 which is only in my fork.

Any chance you could make a small pull request with just that reordering code, as it seems to be a good work around for now?

@caiyue1993 caiyue1993 self-assigned this Jul 5, 2020
@plivesey
Copy link

plivesey commented Jul 6, 2020

@Fonceur sorry, but I ended up just writing my own solution because ice cream was missing too many features. So, I don't really remember much about this code/have the time to get it read to merge. Sorry about that, but good luck.

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

No branches or pull requests

8 participants