Skip to content

Commit

Permalink
Making the ChangeTime object thread safe (#46)
Browse files Browse the repository at this point in the history 
Currently, DataModelManager methods aren't thread safe because they use this object
  • Loading branch information
@plivesey
plivesey authored Oct 27, 2016
1 parent 61d560c commit ee9bee3
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 8 deletions.
8 changes: 4 additions & 4 deletions RocketData.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
611562DB1CC16B490001F5CE /* CollectionChangeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 611562DA1CC16B490001F5CE /* CollectionChangeTests.swift */; };
6117DDF81CD7A8EB002F57C1 /* BatchDataProviderListener.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6117DDE51CD7A8EB002F57C1 /* BatchDataProviderListener.swift */; };
6117DDFA1CD7A8EB002F57C1 /* CacheDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6117DDE71CD7A8EB002F57C1 /* CacheDelegate.swift */; };
6117DDFB1CD7A8EB002F57C1 /* ChangeClock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6117DDE81CD7A8EB002F57C1 /* ChangeClock.swift */; };
6117DDFB1CD7A8EB002F57C1 /* ChangeTime.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6117DDE81CD7A8EB002F57C1 /* ChangeTime.swift */; };
6117DDFC1CD7A8EB002F57C1 /* CollectionChange.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6117DDE91CD7A8EB002F57C1 /* CollectionChange.swift */; };
6117DDFD1CD7A8EB002F57C1 /* CollectionDataProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6117DDEA1CD7A8EB002F57C1 /* CollectionDataProvider.swift */; };
6117DDFE1CD7A8EB002F57C1 /* ConsistencyContextWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6117DDEB1CD7A8EB002F57C1 /* ConsistencyContextWrapper.swift */; };
Expand Down Expand Up @@ -66,7 +66,7 @@
611562DA1CC16B490001F5CE /* CollectionChangeTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CollectionChangeTests.swift; sourceTree = "<group>"; };
6117DDE51CD7A8EB002F57C1 /* BatchDataProviderListener.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BatchDataProviderListener.swift; sourceTree = "<group>"; };
6117DDE71CD7A8EB002F57C1 /* CacheDelegate.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CacheDelegate.swift; sourceTree = "<group>"; };
6117DDE81CD7A8EB002F57C1 /* ChangeClock.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ChangeClock.swift; sourceTree = "<group>"; };
6117DDE81CD7A8EB002F57C1 /* ChangeTime.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ChangeTime.swift; sourceTree = "<group>"; };
6117DDE91CD7A8EB002F57C1 /* CollectionChange.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CollectionChange.swift; sourceTree = "<group>"; };
6117DDEA1CD7A8EB002F57C1 /* CollectionDataProvider.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CollectionDataProvider.swift; sourceTree = "<group>"; };
6117DDEB1CD7A8EB002F57C1 /* ConsistencyContextWrapper.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ConsistencyContextWrapper.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -147,7 +147,7 @@
6117DDF41CD7A8EB002F57C1 /* ParsingHelpers.swift */,
6117DDF01CD7A8EB002F57C1 /* Errors.swift */,
6117DDF21CD7A8EB002F57C1 /* Logger.swift */,
6117DDE81CD7A8EB002F57C1 /* ChangeClock.swift */,
6117DDE81CD7A8EB002F57C1 /* ChangeTime.swift */,
6117DDEB1CD7A8EB002F57C1 /* ConsistencyContextWrapper.swift */,
6117DDED1CD7A8EB002F57C1 /* DataHolder.swift */,
6117DDF61CD7A8EB002F57C1 /* SharedCollectionManager.swift */,
Expand Down Expand Up @@ -458,7 +458,7 @@
6117DE041CD7A8EB002F57C1 /* Logger.swift in Sources */,
6117DDF81CD7A8EB002F57C1 /* BatchDataProviderListener.swift in Sources */,
6117DE051CD7A8EB002F57C1 /* Model.swift in Sources */,
6117DDFB1CD7A8EB002F57C1 /* ChangeClock.swift in Sources */,
6117DDFB1CD7A8EB002F57C1 /* ChangeTime.swift in Sources */,
6117DDFD1CD7A8EB002F57C1 /* CollectionDataProvider.swift in Sources */,
6117DDFA1CD7A8EB002F57C1 /* CacheDelegate.swift in Sources */,
6117DE091CD7A8EB002F57C1 /* WeakSharedCollectionArray.swift in Sources */,
Expand Down
14 changes: 10 additions & 4 deletions RocketData/ChangeClock.swift → RocketData/ChangeTime.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,27 @@ import Foundation
/**
This class keeps a global clock which is used to record when changes happen.
Whenever you create a new ChangeTime, it will be after any previous times and before any future times.
It is not thread-safe. You must call it on the main thread.
This is a thread safe class. You can call it from any thread.
*/
struct ChangeTime: Equatable {
/// Keeps track of the last time we updated
private static var lastTime = 1
/// We use this serial queue to sync on different threads
private static let queue = DispatchQueue(label: "com.rocketdata.changeTime")

fileprivate let time: Int

/**
Creates a new ChangeTime instance. This is guarenteed to be after any previous times created.
*/
init() {
Log.sharedInstance.assert(Thread.isMainThread, "The ChangeClock was accessed on a different thread than the main thread. This probably means you are accessing something in the library that is not thread-safe on a different thread. This can cause race conditions.")
self.time = ChangeTime.lastTime
ChangeTime.lastTime += 1
var time = 0
ChangeTime.queue.sync {
time = ChangeTime.lastTime
ChangeTime.lastTime += 1
}
Log.sharedInstance.assert(time != 0, "Dispatch sync is behaving incorrectly. This is a bug.")
self.time = time
}

/**
Expand Down
78 changes: 78 additions & 0 deletions RocketDataTests/DataModelManagerTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import XCTest
import RocketData

class DataModelManagerTests: RocketDataTestCase {

// MARK: Update Model

func testUpdateModelWithCache() {
let cacheExpectation = expectation(description: "Wait for cache")
Expand Down Expand Up @@ -67,6 +69,41 @@ class DataModelManagerTests: RocketDataTestCase {
XCTAssertEqual(dataProvider[0], newModel)
}

func testUpdateModelDifferentThread() {
let cacheExpectation = expectation(description: "Wait for cache")
let cache = ExpectCacheDelegate()
let dataModelManager = DataModelManager(cacheDelegate: cache)
let dataProvider = CollectionDataProvider<ParentModel>(dataModelManager: dataModelManager)

let initialModel = ParentModel(id: 1, name: "initial", requiredChild: ChildModel(), otherChildren: [])
let newModel = ParentModel(id: 1, name: "new", requiredChild: ChildModel(), otherChildren: [])

let delegate = ClosureCollectionDataProviderDelegate() { (collectionChanges, context) in
XCTAssertEqual(context as? String, "context")
}
dataProvider.delegate = delegate

cache.setModelCalled = { model, key, context in
XCTAssertEqual(model as? ParentModel, newModel)
XCTAssertEqual(context as? String, "context")
XCTAssertEqual(key, "ParentModel:1")
cacheExpectation.fulfill()
}

dataProvider.setData([initialModel], cacheKey: nil, context: "wrong")

DispatchQueue.global(qos: .default).async {
dataModelManager.updateModel(newModel, context: "context")
}

waitForExpectations(timeout: 10, handler: nil)
waitForConsistencyManagerToFlush(dataModelManager.consistencyManager)

XCTAssertEqual(dataProvider[0], newModel)
}

// MARK: Update Models

func testUpdateModelsWithCache() {
let cacheExpectation = expectation(description: "Wait for cache")
let cache = ExpectCacheDelegate()
Expand Down Expand Up @@ -133,6 +170,47 @@ class DataModelManagerTests: RocketDataTestCase {
XCTAssertEqual(dataProvider[1], otherNewModel)
}

func testUpdateModelsDifferentThread() {
let cacheExpectation = expectation(description: "Wait for cache")
let cache = ExpectCacheDelegate()
let dataModelManager = DataModelManager(cacheDelegate: cache)
let dataProvider = CollectionDataProvider<ParentModel>(dataModelManager: dataModelManager)

let initialModel = ParentModel(id: 1, name: "initial", requiredChild: ChildModel(), otherChildren: [])
let otherInitialModel = ParentModel(id: 2, name: "initial", requiredChild: ChildModel(), otherChildren: [])
let newModel = ParentModel(id: 1, name: "new", requiredChild: ChildModel(), otherChildren: [])
let otherNewModel = ParentModel(id: 2, name: "new", requiredChild: ChildModel(), otherChildren: [])

let delegate = ClosureCollectionDataProviderDelegate() { (collectionChanges, context) in
XCTAssertEqual(context as? String, "context")
}
dataProvider.delegate = delegate

var setModelCalled = 0
cache.setModelCalled = { model, key, context in
setModelCalled += 1
XCTAssertEqual(model as? ParentModel, setModelCalled == 1 ? newModel : otherNewModel)
XCTAssertEqual(context as? String, "context")
XCTAssertEqual(key, "ParentModel:\(setModelCalled)")
if setModelCalled == 2 {
cacheExpectation.fulfill()
}
}

dataProvider.setData([initialModel, otherInitialModel], cacheKey: nil, context: "wrong")
DispatchQueue.global(qos: .default).async {
dataModelManager.updateModels([newModel, otherNewModel], context: "context")
}

waitForExpectations(timeout: 10, handler: nil)
waitForConsistencyManagerToFlush(dataModelManager.consistencyManager)

XCTAssertEqual(dataProvider[0], newModel)
XCTAssertEqual(dataProvider[1], otherNewModel)
}

// MARK: Compilation Tests

/**
Previously, this test failed because of https://bugs.swift.org/browse/SR-3038
This test passes if it compiles.
Expand Down

0 comments on commit ee9bee3

Please sign in to comment.