Skip to content

Commit 941a6f2

Browse files
rmenezesrmenezes
authored
Merge branch 'badoo:master' into wrong_text_calculation_attributed_string
2 parents 2858ca7 + 28e0eaa commit 941a6f2

File tree

11 files changed

+332
-38
lines changed

11 files changed

+332
-38
lines changed

‎Chatto/Chatto.xcodeproj/project.pbxproj

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
065DDCAA28726F3F00FAF3B9 /* CollectionUpdateProviderProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 065DDCA928726F3F00FAF3B9 /* CollectionUpdateProviderProtocol.swift */; };
1313
065DDCAC28726F4D00FAF3B9 /* NewChatMessageCollectionAdapter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 065DDCAB28726F4D00FAF3B9 /* NewChatMessageCollectionAdapter.swift */; };
1414
065DDCAE2872882600FAF3B9 /* ChatCollectionViewLayoutModelFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 065DDCAD2872882600FAF3B9 /* ChatCollectionViewLayoutModelFactory.swift */; };
15+
06C6737A2AF89729000E7EA5 /* ObservableAsyncStreamTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 06C673792AF89729000E7EA5 /* ObservableAsyncStreamTests.swift */; };
1516
26D653F2251043BD007BC13C /* ReplyFeedbackGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 26D653F1251043BD007BC13C /* ReplyFeedbackGenerator.swift */; };
1617
3565429D203DB99300B29DA1 /* ChatLayoutConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3565429C203DB99300B29DA1 /* ChatLayoutConfiguration.swift */; };
1718
A40DC13A26F205DB00166C55 /* KeyboardTracker.swift in Sources */ = {isa = PBXBuildFile; fileRef = A40DC13926F205DB00166C55 /* KeyboardTracker.swift */; };
@@ -70,6 +71,7 @@
7071
065DDCA928726F3F00FAF3B9 /* CollectionUpdateProviderProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CollectionUpdateProviderProtocol.swift; sourceTree = "<group>"; };
7172
065DDCAB28726F4D00FAF3B9 /* NewChatMessageCollectionAdapter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewChatMessageCollectionAdapter.swift; sourceTree = "<group>"; };
7273
065DDCAD2872882600FAF3B9 /* ChatCollectionViewLayoutModelFactory.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ChatCollectionViewLayoutModelFactory.swift; sourceTree = "<group>"; };
74+
06C673792AF89729000E7EA5 /* ObservableAsyncStreamTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = ObservableAsyncStreamTests.swift; path = Tests/ObservableAsyncStreamTests.swift; sourceTree = SOURCE_ROOT; };
7375
26D653F1251043BD007BC13C /* ReplyFeedbackGenerator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReplyFeedbackGenerator.swift; sourceTree = "<group>"; };
7476
3565429C203DB99300B29DA1 /* ChatLayoutConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ChatLayoutConfiguration.swift; sourceTree = "<group>"; };
7577
55E85D821BE390BE001885AD /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
@@ -258,6 +260,7 @@
258260
isa = PBXGroup;
259261
children = (
260262
C342D0C31C638FDF008A4605 /* SerialTaskQueueTests.swift */,
263+
06C673792AF89729000E7EA5 /* ObservableAsyncStreamTests.swift */,
261264
E0878A2D191D859A01D1DEB7 /* ChatController */,
262265
E9E5D0A68A700507CA829D11 /* Chat Items */,
263266
EA3D58FE22DCC34C00191EDF /* ChatItemCompanionCollectionTests.swift */,
@@ -489,6 +492,7 @@
489492
C3383E281BFFA49F00244F5C /* BaseChatViewControllerTestHelpers.swift in Sources */,
490493
A410D4BB26C56FF100A48342 /* ChatMessagesViewControllerTests.swift in Sources */,
491494
C36281F21BF12A4B004D6BCE /* ChatCollectionViewLayoutTests.swift in Sources */,
495+
06C6737A2AF89729000E7EA5 /* ObservableAsyncStreamTests.swift in Sources */,
492496
C321C3961BE78835009803D1 /* CollectionChangesTests.swift in Sources */,
493497
A410D4BD26C570B100A48342 /* ChatMessagesViewControllerHelpers.swift in Sources */,
494498
C321DDBE1BE964DC00DE88CC /* BaseChatItemPresenterTests.swift in Sources */,

‎Chatto/Tests/ObservableAsyncStreamTests.swift

+80
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
//
2+
// The MIT License (MIT)
3+
//
4+
// Copyright (c) 2015-present Badoo Trading Limited.
5+
//
6+
// Permission is hereby granted, free of charge, to any person obtaining a copy
7+
// of this software and associated documentation files (the "Software"), to deal
8+
// in the Software without restriction, including without limitation the rights
9+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
// copies of the Software, and to permit persons to whom the Software is
11+
// furnished to do so, subject to the following conditions:
12+
//
13+
// The above copyright notice and this permission notice shall be included in
14+
// all copies or substantial portions of the Software.
15+
//
16+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
// THE SOFTWARE.
23+
24+
import Foundation
25+
import XCTest
26+
import Chatto
27+
import Dispatch
28+
29+
@available(iOS 13, *)
30+
final class ObservableAsyncStreamTests: XCTestCase {
31+
32+
func test_GivenStreamFromObservable_WhenObservableUpdates_ThenValueDeliveredToStream() async throws {
33+
let observable = Observable(0)
34+
let stream = observable.values
35+
36+
async let firstReceivedValue: Int? = stream.first { _ in true }
37+
observable.value = 1
38+
let first = await firstReceivedValue
39+
XCTAssertEqual(first, 1)
40+
41+
async let secondReceivedValue: Int? = stream.first { _ in true }
42+
observable.value = 2
43+
let second = await secondReceivedValue
44+
XCTAssertEqual(second, 2)
45+
}
46+
47+
func test_WhenObservableDeallocated_ThenStreamCompletes() async throws {
48+
var observable: Observable<Void>? = .init(())
49+
let stream = observable!.values
50+
let iterateAllStreamValuesTask = Task {
51+
for await _ in stream {}
52+
}
53+
observable = nil
54+
55+
try await withTimeout {
56+
await withTaskCancellationHandler {
57+
await iterateAllStreamValuesTask.value
58+
} onCancel: {
59+
iterateAllStreamValuesTask.cancel()
60+
}
61+
}
62+
}
63+
64+
// MARK: - Timeout utils
65+
66+
private struct TimeoutError: Error {}
67+
68+
private func withTimeout<T>(_ operation: @escaping @Sendable () async throws -> T) async throws -> T {
69+
try await withThrowingTaskGroup(of: T.self) { group in
70+
group.addTask { try await operation() }
71+
group.addTask {
72+
try await Task.sleep(nanoseconds: NSEC_PER_SEC / 10)
73+
throw TimeoutError()
74+
}
75+
let result = try await group.next()!
76+
group.cancelAll()
77+
return result
78+
}
79+
}
80+
}

‎Chatto/sources/ChatController/BaseChatViewController.swift

+2-1
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,8 @@ public final class BaseChatViewController: UIViewController {
281281
let diff = lastUsedBounds.height - collectionView.bounds.height
282282
// When collectionView is scrolled to bottom and height increases,
283283
// collectionView adjusts its contentOffset automatically
284-
let isScrolledToBottom = contentSize.height <= collectionView.bounds.maxY - collectionView.contentInset.bottom
284+
let currentBottomPosition = collectionView.bounds.maxY - collectionView.contentInset.bottom
285+
let isScrolledToBottom = contentSize.height - currentBottomPosition <= CGFloat.bma_epsilon
285286
return isScrolledToBottom ? max(0, diff) : diff
286287
}()
287288
self.previousBoundsUsedForInsetsAdjustment = collectionView.bounds

‎Chatto/sources/ChatController/ChatMessages/ChatMessageCollectionAdapter.swift

+34-8
Original file line numberDiff line numberDiff line change
@@ -200,11 +200,12 @@ extension ChatMessageCollectionAdapter: ChatDataSourceDelegateProtocol {
200200
let performInBackground = updateType != .firstLoad
201201

202202
self.isLoadingContents = true
203-
let performBatchUpdates: (CollectionChanges, @escaping () -> Void) -> Void = { [weak self] changes, updateModelClosure in
203+
let performBatchUpdates: (CollectionChanges, @escaping () -> Void, Bool) -> Void = { [weak self] changes, updateModelClosure, areCollectionChangesConsistent in
204204
self?.performBatchUpdates(
205205
updateModelClosure: updateModelClosure,
206206
changes: changes,
207-
updateType: updateType
207+
updateType: updateType,
208+
areCollectionChangesConsistent: areCollectionChangesConsistent
208209
) {
209210
self?.isLoadingContents = false
210211
completion()
@@ -224,13 +225,13 @@ extension ChatMessageCollectionAdapter: ChatDataSourceDelegateProtocol {
224225
guard let modelUpdate = createModelUpdate() else { return }
225226

226227
DispatchQueue.main.async {
227-
performBatchUpdates(modelUpdate.changes, modelUpdate.updateModelClosure)
228+
performBatchUpdates(modelUpdate.changes, modelUpdate.updateModelClosure, modelUpdate.areChangesConsistent)
228229
}
229230
}
230231
} else {
231232
guard let modelUpdate = createModelUpdate() else { return }
232233

233-
performBatchUpdates(modelUpdate.changes, modelUpdate.updateModelClosure)
234+
performBatchUpdates(modelUpdate.changes, modelUpdate.updateModelClosure, modelUpdate.areChangesConsistent)
234235
}
235236
}
236237

@@ -263,7 +264,7 @@ extension ChatMessageCollectionAdapter: ChatDataSourceDelegateProtocol {
263264
}
264265
}
265266

266-
private func createModelUpdates(newItems: [ChatItemProtocol], oldItems: ChatItemCompanionCollection, collectionViewWidth: CGFloat) -> (changes: CollectionChanges, updateModelClosure: () -> Void) {
267+
private func createModelUpdates(newItems: [ChatItemProtocol], oldItems: ChatItemCompanionCollection, collectionViewWidth: CGFloat) -> (changes: CollectionChanges, updateModelClosure: () -> Void, areChangesConsistent: Bool) {
267268
let newDecoratedItems = self.chatItemsDecorator.decorateItems(newItems)
268269
let changes = generateChanges(
269270
oldCollection: oldItems.map(HashableItem.init),
@@ -275,7 +276,26 @@ extension ChatMessageCollectionAdapter: ChatDataSourceDelegateProtocol {
275276
self?.layoutModel = layoutModel
276277
self?.chatItemCompanionCollection = itemCompanionCollection
277278
}
278-
return (changes, updateModelClosure)
279+
let areCollectionChangesConsistent = Self.validateCollectionChangeModel(
280+
changes,
281+
oldItems: oldItems,
282+
newItems: newDecoratedItems
283+
)
284+
return (changes, updateModelClosure, areCollectionChangesConsistent)
285+
}
286+
287+
private static func validateCollectionChangeModel(
288+
_ collection: CollectionChanges,
289+
oldItems: ChatItemCompanionCollection,
290+
newItems: [DecoratedChatItem]
291+
) -> Bool {
292+
let deletionChangesCount = collection.deletedIndexPaths.count
293+
let insertionChangesCount = collection.insertedIndexPaths.count
294+
295+
let oldItemsCount = oldItems.count
296+
let newItemsCount = newItems.count
297+
298+
return (newItemsCount - oldItemsCount) == (insertionChangesCount - deletionChangesCount)
279299
}
280300

281301
// Returns scrolling position in interval [0, 1], 0 top, 1 bottom
@@ -370,6 +390,7 @@ extension ChatMessageCollectionAdapter: ChatDataSourceDelegateProtocol {
370390
private func performBatchUpdates(updateModelClosure: @escaping () -> Void, // swiftlint:disable:this cyclomatic_complexity
371391
changes: CollectionChanges,
372392
updateType: UpdateType,
393+
areCollectionChangesConsistent: Bool,
373394
completion: @escaping () -> Void) {
374395
guard let collectionView = self.collectionView else {
375396
completion()
@@ -385,7 +406,7 @@ extension ChatMessageCollectionAdapter: ChatDataSourceDelegateProtocol {
385406
// a) It's unsafe to perform reloadData while there's a performBatchUpdates animating: https://github.com/diegosanchezr/UICollectionViewStressing/tree/master/GhostCells
386407
// Note: using reloadSections instead reloadData is safe and might not need a delay. However, using always reloadSections causes flickering on pagination and a crash on the first layout that needs a workaround. Let's stick to reloaData for now
387408
// b) If it's a performBatchUpdates but visible cells are invalid let's wait until all finish (otherwise we would give wrong cells to presenters in updateVisibleCells)
388-
let mustDelayUpdate = hasUnfinishedBatchUpdates && (wantsReloadData || !visibleCellsAreValid)
409+
let mustDelayUpdate = hasUnfinishedBatchUpdates && (!areCollectionChangesConsistent || wantsReloadData || !visibleCellsAreValid)
389410
guard !mustDelayUpdate else {
390411
// For reference, it is possible to force the current performBatchUpdates to finish in the next run loop, by cancelling animations:
391412
// self.collectionView.subviews.forEach { $0.layer.removeAllAnimations() }
@@ -395,14 +416,19 @@ extension ChatMessageCollectionAdapter: ChatDataSourceDelegateProtocol {
395416
updateModelClosure: updateModelClosure,
396417
changes: changes,
397418
updateType: updateType,
419+
areCollectionChangesConsistent: areCollectionChangesConsistent,
398420
completion: completion
399421
)
400422
}
401423
return
402424
}
403425
// ... if they are still invalid the only thing we can do is a reloadData
404426
let mustDoReloadData = !visibleCellsAreValid // Only way to recover from this inconsistent state
405-
usesBatchUpdates = !wantsReloadData && !mustDoReloadData
427+
usesBatchUpdates = !wantsReloadData && !mustDoReloadData && areCollectionChangesConsistent
428+
429+
if !wantsReloadData && !mustDoReloadData && !areCollectionChangesConsistent {
430+
assertionFailure("Collection changes are invalid.")
431+
}
406432
}
407433

408434
let scrollAction: ScrollAction

‎Chatto/sources/Utils/Observable.swift

+125-12
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
import Foundation
2626

27+
// MARK: - Observable
28+
2729
// Be aware this is not thread safe!
2830
// Why class? https://lists.swift.org/pipermail/swift-users/Week-of-Mon-20160711/002580.html
2931
public class Observable<T> {
@@ -34,34 +36,145 @@ public class Observable<T> {
3436

3537
public var value: T {
3638
didSet {
37-
self.cleanDeadObservers()
38-
for observer in self.observers {
39-
observer.closure(oldValue, self.value)
40-
}
39+
self.lock.lock()
40+
self.observers.cleanDead()
41+
let observers = self.observers
42+
self.lock.unlock()
43+
observers.forEach { $0.notify(oldValue, self.value) }
4144
}
4245
}
4346

4447
public func observe(_ observer: AnyObject, closure: @escaping (_ old: T, _ new: T) -> Void) {
45-
self.observers.append(Observer(owner: observer, closure: closure))
46-
self.cleanDeadObservers()
48+
self.addObserver(Observer(owner: observer, closure: closure))
4749
}
4850

4951
public func removeObserver(_ observer: AnyObject) {
50-
self.observers.removeAll { $0.owner === observer }
52+
self.lock.lock()
53+
defer { self.lock.unlock() }
54+
55+
self.observers.removeAll { $0.isOwner(observer) }
56+
}
57+
58+
// MARK: - Private
59+
60+
private func addObserver<Observer: ObserverProtocol>(_ observer: Observer) where Observer.T == T {
61+
self.lock.lock()
62+
defer { self.lock.unlock() }
63+
64+
self.observers.append(.init(observer))
65+
self.observers.cleanDead()
66+
}
67+
68+
private let lock = NSLock()
69+
private var observers: [AnyObserver<T>] = []
70+
}
71+
72+
private extension Array where Element: ObserverProtocol {
73+
mutating func cleanDead() {
74+
self.removeAll { $0.terminated }
75+
}
76+
}
77+
78+
// MARK: - Observer
79+
80+
// MARK: Protocol
81+
82+
private protocol ObserverProtocol<T> {
83+
associatedtype T
84+
var terminated: Bool { get }
85+
func notify(_ old: T, _ new: T)
86+
func isOwner(_ owner: AnyObject) -> Bool
87+
}
88+
89+
private struct AnyObserver<T>: ObserverProtocol {
90+
91+
private let _isOwner: (AnyObject) -> Bool
92+
private let _notify: (T, T) -> Void
93+
private let _terminated: () -> Bool
94+
95+
init<U: ObserverProtocol>(_ observer: U) where U.T == T {
96+
self._isOwner = observer.isOwner
97+
self._notify = observer.notify
98+
self._terminated = { observer.terminated }
5199
}
52100

53-
private func cleanDeadObservers() {
54-
self.observers = self.observers.filter { $0.owner != nil }
101+
var terminated: Bool { self._terminated() }
102+
103+
func notify(_ old: T, _ new: T) {
104+
self._notify(old, new)
55105
}
56106

57-
private lazy var observers = [Observer<T>]()
107+
func isOwner(_ owner: AnyObject) -> Bool {
108+
self._isOwner(owner)
109+
}
58110
}
59111

60-
private struct Observer<T> {
112+
// MARK: Implementation
113+
114+
private struct Observer<T>: ObserverProtocol {
61115
weak var owner: AnyObject?
62116
let closure: (_ old: T, _ new: T) -> Void
63-
init (owner: AnyObject, closure: @escaping (_ old: T, _ new: T) -> Void) {
117+
init(owner: AnyObject, closure: @escaping (_ old: T, _ new: T) -> Void) {
64118
self.owner = owner
65119
self.closure = closure
66120
}
121+
122+
var terminated: Bool { self.owner == nil }
123+
124+
func notify(_ old: T, _ new: T) {
125+
self.closure(old, new)
126+
}
127+
128+
func isOwner(_ owner: AnyObject) -> Bool {
129+
self.owner === owner
130+
}
131+
}
132+
133+
// MARK: - AsyncStream mapping
134+
135+
@available(iOS 13, *)
136+
extension Observable {
137+
public var values: AsyncStream<T> {
138+
AsyncStream(bufferingPolicy: .bufferingNewest(1)) { continuation in
139+
self.addObserver(AsyncStreamObserver(continuation: continuation))
140+
}
141+
}
142+
}
143+
144+
@available(iOS 13, *)
145+
private final class AsyncStreamObserver<T>: ObserverProtocol, @unchecked Sendable {
146+
147+
private let continuation: AsyncStream<T>.Continuation
148+
149+
private let lock = NSLock()
150+
private var _terminated: Bool = false
151+
private(set) var terminated: Bool {
152+
get {
153+
self.lock.lock()
154+
defer { self.lock.unlock() }
155+
return self._terminated
156+
}
157+
set {
158+
self.lock.lock()
159+
defer { self.lock.unlock() }
160+
self._terminated = newValue
161+
}
162+
}
163+
164+
deinit {
165+
self.continuation.finish()
166+
}
167+
168+
init(continuation: AsyncStream<T>.Continuation) {
169+
self.continuation = continuation
170+
self.continuation.onTermination = { [weak self] _ in
171+
self?.terminated = true
172+
}
173+
}
174+
175+
func notify(_: T, _ new: T) {
176+
self.continuation.yield(new)
177+
}
178+
179+
func isOwner(_: AnyObject) -> Bool { false }
67180
}

0 commit comments

Comments
 (0)