Flexible logging added, multiple bug fixes

[tifroz]

• Added flexible logging (see SMLogger, README)
• Bugs fixes
  1. Refresh NSManagedObjectContexts after saving the context
  2. Wrapped all core data call in context.perform {} to avoid issues with concurrent access (crashes)
  3. Fixed multiple issues with previousServerChangeToken, subscriptions that caused very inefficient sync, and unwanted behavior (delete notifications not called) - see SMStoreSyncOperation for commented out code

[tifroz]

Good catch - yes I will make both changes.

It's probably best to wait another few days before merging - I have an app that hit the app store yesterday with the new code, and found out there is at least one issue that I wasn't able to reproduce in my test environment (affects ~1% of users). So there will be at least one more change coming.

I will ping this thread to give you the green light after I am confident that the code is fully vetted by production usage.

On Wed, Nov 28, 2018 at 6:17 PM Paul Wilkinson notifications@github.com wrote:

@paulw11 requested changes on this pull request.

In README.md https://github.com/paulw11/Seam3/pull/97#discussion_r237331996:

@@ -133,6 +133,31 @@ lazy var persistentContainer: NSPersistentContainer = {

 }()

+By default, logs will be written to `os_log`, but you can route log messages to your own class by extending `SMLogger`:
+```
+class WTVAppDelegate: SMLogDelegate {

Can you change this to AppDelegate - I think this will be clearer for
people
------------------------------

In Sources/Classes/CKRecord+NSManagedObject.swift
<https://github.com/paulw11/Seam3/pull/97#discussion_r237332407>:

> @@ -75,10 +75,10 @@ extension CKRecord {
             let referencesValuesDictionary = self.dictionaryWithValues(forKeys: self.allReferencesKeys(usingRelationshipsByNameFromEntity: entity.relationshipsByName))
             var managedObjectsDictionary: Dictionary<String,AnyObject> = Dictionary<String,AnyObject>()
             for (key,value) in referencesValuesDictionary {
-               /* if (value as? String) != nil && (value as! String) == SMStore.SMCloudRecordNilValue {
-                    managedObjectsDictionary[key] = SMStore.SMCloudRecordNilValue as AnyObject?
-                    continue
-                }*/
+                /* if (value as? String) != nil && (value as! String) == SMStore.SMCloudRecordNilValue {

Let's just delete this obsolete code instead of leaving it around as a
comment; I can do it after the PR merge if you like

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<https://github.com/paulw11/Seam3/pull/97#pullrequestreview-179603220>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJLX9P2YAOrNRPNBWbxE_mU4HhanTvzks5uz0OqgaJpZM4Yt8gK>
.

[tifroz]

I made one bug fix to SMStoreSyncOperation.swift in addition to the changes your requested. This latest version of code (with the bug fix) has been in production for the last few days with no quality issues detected.