Skip to content

Support public database #124

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
merged 16 commits into from
Apr 26, 2019
Merged

Support public database #124

merged 16 commits into from
Apr 26, 2019

Conversation

caiyue1993
Copy link
Owner

Re #58

@caiyue1993 caiyue1993 changed the title Support public database [WIP] Support public database Apr 7, 2019
@caiyue1993 caiyue1993 changed the title [WIP] Support public database Support public database Apr 20, 2019
@ianbradbury-bizzibody
Copy link

I'm getting an error when building the pull request example project.

Ambiguous use of 'firesOnRecordCreation' on the following line of code.

let subscription = CKQuerySubscription(recordType: syncObject.recordType, predicate: predict, subscriptionID: IceCreamSubscription.cloudKitPublicDatabaseSubscriptionID.id, options: [.firesOnRecordCreation, .firesOnRecordUpdate, .firesOnRecordDeletion])

From this function.

private func createSubscriptionInPublicDatabase(on syncObject: Syncable) {
    let predict = NSPredicate(value: true)
    let subscription = CKQuerySubscription(recordType: syncObject.recordType, predicate: predict, subscriptionID: IceCreamSubscription.cloudKitPublicDatabaseSubscriptionID.id, options: [.firesOnRecordCreation, .firesOnRecordUpdate, .firesOnRecordDeletion])
    
    let notificationInfo = CKSubscription.NotificationInfo()
    notificationInfo.shouldSendContentAvailable = true // Silent Push
    
    subscription.notificationInfo = notificationInfo
    
    let createOp = CKModifySubscriptionsOperation(subscriptionsToSave: [subscription], subscriptionIDsToDelete: [])
    createOp.modifySubscriptionsCompletionBlock = { _, _, _ in
        
    }
    createOp.qualityOfService = .utility
    database.add(createOp)
}

@caiyue1993 caiyue1993 merged commit c8cebae into master Apr 26, 2019
@rodericj
Copy link

If I'm not mistaken this gets us like 40% of the way to a viable solution for #4. Great stuff. I'll start from the latest then.

@caiyue1993
Copy link
Owner Author

@rodericj Glad to hear that. Indeed I've refactored the code structure and make it much more reasonable I think.

@caiyue1993 caiyue1993 deleted the feature/public-database-support branch April 27, 2019 16:26

@objc func add() {
let user = Person()
user.name = "Yue Cai"

Choose a reason for hiding this comment

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

What's interesting is that the first time I ran this I got "Jim" added to my Realm. I see that it's defaulted to Jim, but I don't really see how that wasn't overridden given that this is how we add the new users.

@rodericj
Copy link

rodericj commented May 6, 2019

I'm getting pretty deep into the implementation here but I'm wondering what the expected use is for this conditional:

if let token = self.notificationToken {
    try! self.realm.commitWrite(withoutNotifying: [token])
} else {
    try! self.realm.commitWrite()
}

I'm looking to use something similar, or repurpose this to feed in other notification tokens in the shared database implementation.

What I've done so far is to create 3 separate database managers:

  1. private (this is the standard case)
  2. public (this is the case introduced here)
  3. shared (this is my new case).

It would seem as though this PR assumes you'll either use public OR private as it forces you to only use one database. What I've done is created 3 different databaseManagers that all live in parallel. The issue becomes when the realm receives a new SyncObject via user action (adding a dog or cat for instance), the notification token stored in each database manager would fire and the new object would be added to all 3 remote databases which is undesirable.

So the question is, what is the expected logic where the notificationToken would not be set? How do we get to that point? AFAIK this is always set when we registerLocalDatabase().

@caiyue1993
Copy link
Owner Author

Hi @rodericj , actually the situation where notificationToken would not be set will not be happen. So the code could be optimized to the following:

self.notificationToken.flatMap { try! self.realm.commitWrite(withoutNotifying: [$0]) }

Though I haven't got deep into shared database, I thought shared database is a special private database. So

The issue becomes when the realm receives a new SyncObject via user action (adding a dog or cat for instance), the notification token stored in each database manager would fire and the new object would be added to all 3 remote databases

this issue could be a little tough to handle. As in the current version of IceCream, we suggest using syncing data in the same scope of database per SyncEngine. So we may need to tweak the existing code structure to make it happen.

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