Skip to content

Commit c6f647f

Browse files
lawmichalawmicha
committed
address PR comments
1 parent 6d60fea commit c6f647f

File tree

8 files changed

+134
-63
lines changed

8 files changed

+134
-63
lines changed

‎Amplify/Categories/DataStore/DataStoreCategoryBehavior.swift‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ public protocol DataStoreSubscribeBehavior {
7373
/// - predicate: The predicate to match for filtered results
7474
/// - sortInput: The field and order of data to be returned
7575
@available(iOS 13.0, *)
76-
7776
func observeQuery<M: Model>(for modelType: M.Type,
7877
where predicate: QueryPredicate?,
7978
sort sortInput: QuerySortInput?)

‎Amplify/Categories/DataStore/Subscribe/DataStoreQuerySnapshot.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
import Foundation
99

10+
/// A snapshot of the items from DataStore, the changes since last snapshot, and whether this model has
11+
/// finished syncing and subscriptions are active
1012
public struct DataStoreQuerySnapshot<M: Model> {
1113

1214
/// All model instances from the local store

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/AWSDataStorePlugin+DataStoreBaseBehavior.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ extension AWSDataStorePlugin: DataStoreBaseBehavior {
181181

182182
public func stop(completion: @escaping DataStoreCallback<Void>) {
183183
operationQueue.operations.forEach { operation in
184-
if let operation = operation as? DataStoreObseverQueryOperation {
184+
if let operation = operation as? DataStoreObserveQueryOperation {
185185
operation.resetState()
186186
}
187187
}
@@ -200,7 +200,7 @@ extension AWSDataStorePlugin: DataStoreBaseBehavior {
200200

201201
public func clear(completion: @escaping DataStoreCallback<Void>) {
202202
operationQueue.operations.forEach { operation in
203-
if let operation = operation as? DataStoreObseverQueryOperation {
203+
if let operation = operation as? DataStoreObserveQueryOperation {
204204
operation.resetState()
205205
}
206206
}

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/AWSDataStorePlugin+DataStoreSubscribeBehavior.swift‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,19 @@ extension AWSDataStorePlugin: DataStoreSubscribeBehavior {
4949
-> AnyPublisher<DataStoreQuerySnapshot<M>, DataStoreError> {
5050
reinitStorageEngineIfNeeded()
5151

52+
guard let dataStorePublisher = dataStorePublisher else {
53+
return Fail(error: DataStoreError.unknown(
54+
"`dataStorePublisher` is expected to exist for deployment targets >=iOS13.0",
55+
"", nil)).eraseToAnyPublisher()
56+
}
5257
// Force-unwrapping: The optional 'dataStorePublisher' is expected
5358
// to exist for deployment targets >=iOS13.0
54-
let operation = AWSDataStoreObseverQueryOperation(modelType: modelType,
59+
let operation = AWSDataStoreObserveQueryOperation(modelType: modelType,
5560
modelSchema: modelSchema,
5661
predicate: predicate,
5762
sortInput: sortInput,
5863
storageEngine: storageEngine,
59-
dataStorePublisher: dataStorePublisher!)
64+
dataStorePublisher: dataStorePublisher)
6065
operationQueue.addOperation(operation)
6166
return operation.publisher
6267
}

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/AWSDataStorePlugin.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ final public class AWSDataStorePlugin: DataStoreCategoryPlugin {
121121
storageEngine.startSync { result in
122122

123123
self.operationQueue.operations.forEach { operation in
124-
if let operation = operation as? DataStoreObseverQueryOperation {
124+
if let operation = operation as? DataStoreObserveQueryOperation {
125125
operation.startObserveQuery()
126126
}
127127
}

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Subscribe/DataStoreObserveQueryOperation.swift‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import Amplify
99
import AWSPluginsCore
1010
import Combine
1111

12-
protocol DataStoreObseverQueryOperation {
12+
protocol DataStoreObserveQueryOperation {
1313
func resetState()
1414
func startObserveQuery()
1515
}
@@ -19,9 +19,9 @@ public class ObserveQueryPublisher<M: Model>: Publisher {
1919
public typealias Output = DataStoreQuerySnapshot<M>
2020
public typealias Failure = DataStoreError
2121

22-
weak var operation: AWSDataStoreObseverQueryOperation<M>?
22+
weak var operation: AWSDataStoreObserveQueryOperation<M>?
2323

24-
func configure(operation: AWSDataStoreObseverQueryOperation<M>) {
24+
func configure(operation: AWSDataStoreObserveQueryOperation<M>) {
2525
self.operation = operation
2626
}
2727

@@ -41,9 +41,9 @@ where Target.Input == DataStoreQuerySnapshot<M>, Target.Failure == DataStoreErro
4141
target: DispatchQueue.global())
4242
var target: Target?
4343
var sink: AnyCancellable?
44-
weak var operation: AWSDataStoreObseverQueryOperation<M>?
44+
weak var operation: AWSDataStoreObserveQueryOperation<M>?
4545

46-
init(operation: AWSDataStoreObseverQueryOperation<M>?) {
46+
init(operation: AWSDataStoreObserveQueryOperation<M>?) {
4747
self.operation = operation
4848
self.sink = operation?
4949
.passthroughPublisher
@@ -68,7 +68,12 @@ where Target.Input == DataStoreQuerySnapshot<M>, Target.Failure == DataStoreErro
6868
/// simply emit events according to its underlying operation
6969
/// but we still have to implement this method
7070
/// in order to conform to the Subscription protocol:
71-
func request(_ demand: Subscribers.Demand) {}
71+
func request(_ demand: Subscribers.Demand) {
72+
if demand != .unlimited {
73+
log.verbose("Setting the Demand is not supported. The subscriber will receive all values for this API.")
74+
}
75+
76+
}
7277

7378
/// When the subscription is cancelled, cancel the underlying operation
7479
func cancel() {
@@ -87,6 +92,9 @@ where Target.Input == DataStoreQuerySnapshot<M>, Target.Failure == DataStoreErro
8792
}
8893
}
8994

95+
@available(iOS 13.0, *)
96+
extension ObserveQuerySubscription: DefaultLogger { }
97+
9098
/// Publishes a stream of `DataStoreQuerySnapshot` events.
9199
///
92100
/// Flow: When the operation starts executing
@@ -99,7 +107,7 @@ where Target.Input == DataStoreQuerySnapshot<M>, Target.Failure == DataStoreErro
99107
/// - Update internal state of items based on the changed items
100108
/// - Generate new snapshot based on lates state of the items, with the items changed dedupped.
101109
@available(iOS 13.0, *)
102-
public class AWSDataStoreObseverQueryOperation<M: Model>: AsynchronousOperation, DataStoreObseverQueryOperation {
110+
public class AWSDataStoreObserveQueryOperation<M: Model>: AsynchronousOperation, DataStoreObserveQueryOperation {
103111

104112
private let serialQueue = DispatchQueue(label: "com.amazonaws.AWSDataStoreObseverQueryOperation.serialQueue",
105113
target: DispatchQueue.global())
@@ -116,7 +124,7 @@ public class AWSDataStoreObseverQueryOperation<M: Model>: AsynchronousOperation,
116124

117125
let observeQueryStarted: AtomicValue<Bool>
118126
let isSynced: AtomicValue<Bool>
119-
var currentItemsMap: [Model.Identifier: M] = [:]
127+
var currentItemsMap: [Model.Identifier: M]
120128
var itemsChangedSink: AnyCancellable?
121129
var dataStoreEventSink: AnyCancellable?
122130

@@ -144,6 +152,7 @@ public class AWSDataStoreObseverQueryOperation<M: Model>: AsynchronousOperation,
144152

145153
self.observeQueryStarted = AtomicValue(initialValue: false)
146154
self.isSynced = AtomicValue(initialValue: false)
155+
self.currentItemsMap = [:]
147156
self.passthroughPublisher = PassthroughSubject<DataStoreQuerySnapshot<M>, DataStoreError>()
148157
self.observeQueryPublisher = ObserveQueryPublisher()
149158
super.init()
@@ -165,12 +174,14 @@ public class AWSDataStoreObseverQueryOperation<M: Model>: AsynchronousOperation,
165174

166175
func resetState() {
167176
serialQueue.async {
177+
if !self.observeQueryStarted.getAndSet(false) {
178+
return
179+
}
168180
self.log.verbose("Resetting state")
169181
self.isSynced.set(false)
170182
self.currentItemsMap.removeAll()
171183
self.itemsChangedSink = nil
172184
self.dataStoreEventSink = nil
173-
self.observeQueryStarted.set(false)
174185
}
175186
}
176187

@@ -293,7 +304,7 @@ public class AWSDataStoreObseverQueryOperation<M: Model>: AsynchronousOperation,
293304
self.finish()
294305
return
295306
}
296-
guard !mutationEvents.isEmpty else {
307+
guard self.observeQueryStarted.get(), !mutationEvents.isEmpty else {
297308
return
298309
}
299310

@@ -307,7 +318,7 @@ public class AWSDataStoreObseverQueryOperation<M: Model>: AsynchronousOperation,
307318
if let sort = sortInput {
308319
sort.forEach { currentItems.sortModels(by: $0, modelSchema: modelSchema) }
309320
}
310-
publish(items: currentItems, isSynced: isSynced.get(), itemsChanged: itemsChanged)
321+
publishSnapshot(ofItems: currentItems, isSynced: isSynced.get(), itemsChanged: itemsChanged)
311322
}
312323

313324
func updateCurrentItems(with itemsChanged: [MutationEvent]) {
@@ -342,7 +353,7 @@ public class AWSDataStoreObseverQueryOperation<M: Model>: AsynchronousOperation,
342353

343354
// MARK: - Helpers
344355

345-
func publish(items: [M], isSynced: Bool, itemsChanged: [MutationEvent]) {
356+
func publishSnapshot(ofItems items: [M], isSynced: Bool, itemsChanged: [MutationEvent]) {
346357
let querySnapshot = DataStoreQuerySnapshot(items: items,
347358
isSynced: isSynced,
348359
itemsChanged: dedup(mutationEvents: itemsChanged))
@@ -361,4 +372,4 @@ public class AWSDataStoreObseverQueryOperation<M: Model>: AsynchronousOperation,
361372
}
362373

363374
@available(iOS 13.0, *)
364-
extension AWSDataStoreObseverQueryOperation: DefaultLogger { }
375+
extension AWSDataStoreObserveQueryOperation: DefaultLogger { }

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Subscribe/Support/Model+Sort.swift‎

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,17 @@ enum ModelValueCompare<T: Comparable> {
1919
case leftNil(value2: T)
2020
case rightNil(value1: T)
2121
case values(value1: T, value2: T)
22-
case unknown
2322

2423
init(value1Optional: T?, value2Optional: T?) {
25-
if value1Optional == nil && value2Optional == nil {
24+
switch (value1Optional, value2Optional) {
25+
case (nil, nil):
2626
self = .bothNil
27-
} else if value1Optional == nil, let value2 = value2Optional {
28-
self = .leftNil(value2: value2)
29-
} else if value2Optional == nil, let value1 = value1Optional {
30-
self = .rightNil(value1: value1)
31-
} else if let value1 = value1Optional, let value2 = value2Optional {
32-
self = .values(value1: value1, value2: value2)
33-
} else {
34-
self = .unknown
27+
case (nil, .some(let val2)):
28+
self = .leftNil(value2: val2)
29+
case (.some(let val1), nil):
30+
self = .rightNil(value1: val1)
31+
case (.some(let val1), .some(let val2)):
32+
self = .values(value1: val1, value2: val2)
3533
}
3634
}
3735

@@ -45,13 +43,34 @@ enum ModelValueCompare<T: Comparable> {
4543
return sortOrder == .descending
4644
case .values(let value1, let value2):
4745
return sortOrder == .ascending ? value1 < value2 : value1 > value2
48-
case .unknown:
49-
return false
5046
}
5147
}
5248
}
5349

5450
extension ModelSchema {
51+
52+
/// Compares two model's specified by the field and returns the sort direction based on the `sortBy` sort direction.
53+
///
54+
/// Models are compared with basic operators on their field values when the field type corresponds to a `Comparable`
55+
/// type such as String, Int, Double, Date. When the field cannot be compared using the basic operators, it will be
56+
/// turned to its String or Int representation for the comparison. For example, EnumPersistable's String
57+
/// value and bool's Int value are used instead. For Bool, this means `nil` is less than `false`, and `false`
58+
/// is less than `true`.
59+
///
60+
/// `nil` or non-existent values (values which do not exist on the model instance) are treated as less than the
61+
/// values which do exist/non-nil. This returns true when sort is ascending (false if descending) when the
62+
/// `model1`'s field value is `nil` and `model2`'s field value is some value.
63+
///
64+
/// Sorting on field types such as `.embedded`, `.embeddedCollection`, `.model`, `.collection`
65+
/// is undetermined behavior and is currently not supported.
66+
///
67+
/// - Note: Maintainers need to keep this utility updated when new field types are added.
68+
///
69+
/// - Parameters:
70+
/// - model1: model instance to be compared
71+
/// - model2: model instance to be compared
72+
/// - sortBy: The field and direction used to compare the two models
73+
/// - Returns: The resutting comparison between the two models based on `sortBy`
5574
// swiftlint:disable:next cyclomatic_complexity
5675
func comparator(model1: Model,
5776
model2: Model,
@@ -114,16 +133,9 @@ extension ModelSchema {
114133
guard let value1Optional = value1 as? Bool?, let value2Optional = value2 as? Bool? else {
115134
return false
116135
}
117-
if value1Optional == nil && value2Optional == nil {
118-
return sortOrder == .ascending
119-
} else if value1Optional == nil, value2Optional != nil {
120-
return sortOrder == .ascending
121-
} else if value1Optional != nil, value2Optional == nil {
122-
return sortOrder == .descending
123-
} else if let value1 = value1Optional, let value2 = value2Optional {
124-
return sortOrder == .ascending ?
125-
value1.intValue < value2.intValue : value1.intValue > value2.intValue
126-
}
136+
return ModelValueCompare(value1Optional: value1Optional?.intValue,
137+
value2Optional: value2Optional?.intValue)
138+
.sortComparator(sortOrder: sortOrder)
127139
case .enum:
128140
guard case .some(Optional<Any>.some(let value1Optional)) = value1,
129141
case .some(Optional<Any>.some(let value2Optional)) = value2 else {
@@ -136,21 +148,15 @@ extension ModelSchema {
136148
}
137149
let enumValue1Optional = (value1Optional as? EnumPersistable)?.rawValue
138150
let enumValue2Optional = (value2Optional as? EnumPersistable)?.rawValue
139-
if enumValue1Optional == nil && enumValue2Optional == nil {
140-
return sortOrder == .ascending
141-
} else if enumValue1Optional == nil, enumValue2Optional != nil {
142-
return sortOrder == .ascending
143-
} else if enumValue1Optional != nil, enumValue2Optional == nil {
144-
return sortOrder == .descending
145-
} else if let enumValue1 = enumValue1Optional, let enumValue2 = enumValue2Optional {
146-
return sortOrder == .ascending ?
147-
enumValue1 < enumValue2 : enumValue1 > enumValue2
148-
}
151+
return ModelValueCompare(value1Optional: enumValue1Optional,
152+
value2Optional: enumValue2Optional)
153+
.sortComparator(sortOrder: sortOrder)
149154
case .embedded, .embeddedCollection, .model, .collection:
150155
// Behavior is undetermined
156+
log.warn("Sorting on field type \(modelField.type) is unsupported")
151157
return false
152158
}
153-
return false
154159
}
155-
156160
}
161+
162+
extension ModelSchema: DefaultLogger { }

0 commit comments

Comments
 (0)