Skip to content

Commit

Permalink
Fix NSInternalInconsistencyException crash (#6)
Browse files Browse the repository at this point in the history
* Added unit test to verify updates sent to UIKit views

* Added safeguard against incremental updates likely to crash UITableView

* Pinned SwiftyJSON pod to version 4.2

* Added naive safeguard against `NSInternalInconsistencyException` during updates of UICollectionView

* Removed SwiftyJSON pod version pinning
  • Loading branch information
thorfroelich authored and heiberg committed Dec 3, 2019
1 parent fd9c700 commit 479b2b3
Show file tree
Hide file tree
Showing 5 changed files with 267 additions and 32 deletions.
1 change: 0 additions & 1 deletion Sources/BasicData/BasicDataSource.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import Foundation
import Dwifft

public class BasicDataSource<Section>: DataSourceType where Section: SectionType
{
Expand Down
34 changes: 18 additions & 16 deletions Sources/UICollectionView/UICollectionView+Updates.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,26 @@ extension UICollectionView {

public var defaultViewUpdate: IndexedUpdateHandler.Observer {
return { [weak self] update in
guard let _ = self?.window else {
self?.reloadData()
guard let self = self else { return }

if self.window == nil || update.isLikelyToCrashUIKitViews {
self.reloadData()
return
}
self?.performBatchUpdates({
switch update {
case let .delta(insertedSections, updatedSections, deletedSections, insertedRows, updatedRows, deletedRows):
guard let strongSelf = self else { return }
strongSelf.insertSections(insertedSections)
strongSelf.deleteSections(deletedSections)
strongSelf.reloadSections(updatedSections)
strongSelf.insertItems(at: insertedRows)
strongSelf.deleteItems(at: deletedRows)
strongSelf.reloadItems(at: updatedRows)
case .full:
self?.reloadData()
}
}, completion: { _ in })

switch update {
case let .delta(insertedSections, updatedSections, deletedSections, insertedRows, updatedRows, deletedRows):
self.performBatchUpdates({
self.insertSections(insertedSections)
self.deleteSections(deletedSections)
self.reloadSections(updatedSections)
self.insertItems(at: insertedRows)
self.deleteItems(at: deletedRows)
self.reloadItems(at: updatedRows)
}, completion: { _ in })
case .full:
self.reloadData()
}
}
}
}
35 changes: 21 additions & 14 deletions Sources/UITableView/UITableView+Updates.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,33 @@ extension UITableView {

public func defaultViewUpdate(with animation: UITableView.RowAnimation = .fade) -> IndexedUpdateHandler.Observer {
return { [weak self] update in
guard let _ = self?.window else {
self?.reloadData()
guard let self = self else { return }

guard self.window != nil else {
self.reloadData()
return
}

switch update {
case .delta(let insertedSections, let updatedSections, let deletedSections, let insertedRows, let updatedRows, let deletedRows):
self?.beginUpdates()
self?.insertSections(insertedSections, with: animation)
self?.deleteSections(deletedSections, with: animation)
self?.reloadSections(updatedSections, with: animation)
self?.insertRows(at: insertedRows, with: animation)
self?.deleteRows(at: deletedRows, with: animation)
self?.reloadRows(at: updatedRows, with: animation)
self?.endUpdates()
if update.isLikelyToCrashUIKitViews { break }
self.beginUpdates()
self.insertSections(insertedSections, with: animation)
self.deleteSections(deletedSections, with: animation)
self.reloadSections(updatedSections, with: animation)
self.insertRows(at: insertedRows, with: animation)
self.deleteRows(at: deletedRows, with: animation)
self.reloadRows(at: updatedRows, with: animation)
self.endUpdates()
return
case .full:
guard let `self` = self else { return }
UIView.transition(with: self, duration: 0.4, options: .transitionCrossDissolve, animations: {
self.reloadData()
}, completion: nil)
break
}

// Fall-back animated full reload
UIView.transition(with: self, duration: 0.4, options: .transitionCrossDissolve, animations: {
self.reloadData()
}, completion: nil)
}
}
}
13 changes: 12 additions & 1 deletion Sources/Updates.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import UIKit

public enum IndexedUpdate {
public enum IndexedUpdate: Equatable {
case delta(
insertedSections: IndexSet,
updatedSections: IndexSet,
Expand Down Expand Up @@ -28,6 +28,17 @@ public enum IndexedUpdate {
return false
}
}

var isLikelyToCrashUIKitViews: Bool {
switch self {
case let .delta(insertedSections, updatedSections, deletedSections, insertedRows, updatedRows, deletedRows):
let hasItemUpdates = !(insertedRows.isEmpty && updatedRows.isEmpty && deletedRows.isEmpty)
let hasSectionUpdates = !(insertedSections.isEmpty && updatedSections.isEmpty && deletedSections.isEmpty)
return hasItemUpdates && hasSectionUpdates
case .full:
return false
}
}
}

public class IndexedUpdateHandler {
Expand Down
216 changes: 216 additions & 0 deletions Tests/UIKitViewUpdateTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
import UIKit
import Quick
import Nimble
@testable import SimpleSource

protocol DataSourceUpdateable: UIViewController {
typealias Section = BasicIdentifiableSection<UIKitViewUpdateTests.Item>
var dataSource: BasicDataSource<Section> { get }
var onViewUpdate: ((IndexedUpdate) ->Void)? { get set }
var viewNumberOfSections: Int { get }
init(sections: [Section])
func viewNumberOfRows(inSection: Int) -> Int
}

class UIKitViewUpdateTests: QuickSpec {
typealias Section = BasicIdentifiableSection<Item>
typealias DataSource = BasicDataSource<Section>

static let cellIdentifier = "Cell"

struct Item: Equatable, IdentifiableItem {
let itemIdentifier: String
var value: Bool
}

override func spec() {
let vcTypes: [DataSourceUpdateable.Type] = [TableViewController.self, CollectionViewController.self]
for type in vcTypes {

describe("A \(type) with a data source") {

var initialSections: [Section]!
var vc: DataSourceUpdateable!
var window: UIWindow!

beforeEach {
initialSections = stride(from: 0, to: 3, by: 1).map { index -> Section in
let item = Item(itemIdentifier: "\(index)", value: true)
return Section(sectionIdentifier: "\(index)", items: [item])
}

vc = type.init(sections: initialSections)
window = UIWindow()
window.rootViewController = vc
vc.loadViewIfNeeded()
window.makeKeyAndVisible()
}


it("updates its view") {
expect(vc.viewNumberOfSections).toEventually(equal(initialSections.count))
expect(vc.viewNumberOfRows(inSection: 0)).toEventually(equal(initialSections[0].items.count))
}

it("performs a simple diff'ed update") {
var sections = initialSections!
sections[0].items[0].value.toggle()
sections[2].items[0].value.toggle()

var didPerformDiffedUpdate = false
var updateIsLikelyToCrashUITableView = true
vc.onViewUpdate = { update in
updateIsLikelyToCrashUITableView = update.isLikelyToCrashUIKitViews
switch update {
case .delta:
didPerformDiffedUpdate = true
case .full:
break
}
}

DispatchQueue.main.async {
vc.dataSource.sections = sections
}

expect(updateIsLikelyToCrashUITableView).toEventually(equal(false))
expect(didPerformDiffedUpdate).toEventually(equal(true))
}

it("survives a complex update") {
// Move all items from section 1 to section 0 and delete section 1
var sections = initialSections!
sections[0].items.append(contentsOf: sections[1].items)
sections.remove(at: 1)

// Change the value of the first item in the (previous section 2 but now) section 1
sections[1].items[0].value.toggle()

var updateIsLikelyToCrashUITableView = false
vc.onViewUpdate = { update in
updateIsLikelyToCrashUITableView = update.isLikelyToCrashUIKitViews
}

DispatchQueue.main.async {
vc.dataSource.sections = sections
}

expect(updateIsLikelyToCrashUITableView).toEventually(equal(true))
expect(vc.viewNumberOfSections).toEventually(equal(sections.count))
}
}
}
}
}

extension UIKitViewUpdateTests {
fileprivate final class TableViewController: UITableViewController, DataSourceUpdateable {
typealias ViewDataSource = TableViewDataSource<DataSource, TableViewFactory<Item>>
typealias Cell = UITableViewCell

let sections: [Section]

var onViewUpdate: ((IndexedUpdate) ->Void)?

lazy var viewDataSource: ViewDataSource = {
let dataSource = DataSource(sections: self.sections)
let viewFactory = TableViewFactory<Item> { _,_ in UIKitViewUpdateTests.cellIdentifier }
let configuration: (Cell, Item, IndexPath) -> Void = { (_, _, _) in }
viewFactory.registerCell(
method: .classBased(Cell.self),
reuseIdentifier: UIKitViewUpdateTests.cellIdentifier,
in: tableView,
configuration: configuration
)
let defaultViewUpdate = tableView.defaultViewUpdate()
let viewUpdate: IndexedUpdateHandler.Observer = { [weak self] update in
self?.onViewUpdate?(update)
defaultViewUpdate(update)
}
return ViewDataSource(
dataSource: dataSource,
viewFactory: viewFactory,
viewUpdate: viewUpdate
)
}()

var dataSource: BasicDataSource<BasicIdentifiableSection<Item>> {
return viewDataSource.dataSource
}

var viewNumberOfSections: Int { return tableView!.numberOfSections }

func viewNumberOfRows(inSection: Int) -> Int {
return tableView.numberOfRows(inSection: inSection)
}

init(sections: [Section]) {
self.sections = sections
super.init(nibName: nil, bundle: nil)
}

required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

override func viewDidLoad() {
super.viewDidLoad()
tableView.dataSource = viewDataSource
}
}

fileprivate final class CollectionViewController: UICollectionViewController, DataSourceUpdateable {
typealias ViewDataSource = CollectionViewDataSource<DataSource, CollectionViewFactory<Item>>
typealias Cell = UICollectionViewCell

let sections: [Section]

var onViewUpdate: ((IndexedUpdate) ->Void)?

lazy var viewDataSource: ViewDataSource = {
let dataSource = DataSource(sections: self.sections)
let viewFactory = CollectionViewFactory<Item> { _,_ in UIKitViewUpdateTests.cellIdentifier }
let configuration: (Cell, Item, IndexPath) -> Void = { (_, _, _) in }
viewFactory.registerCell(
method: .classBased(Cell.self),
reuseIdentifier: UIKitViewUpdateTests.cellIdentifier,
in: collectionView,
configuration: configuration
)
let defaultViewUpdate = collectionView.defaultViewUpdate
let viewUpdate: IndexedUpdateHandler.Observer = { [weak self] update in
self?.onViewUpdate?(update)
defaultViewUpdate(update)
}
return ViewDataSource(
dataSource: dataSource,
viewFactory: viewFactory,
viewUpdate: viewUpdate
)
}()

var dataSource: BasicDataSource<BasicIdentifiableSection<Item>> {
return viewDataSource.dataSource
}

var viewNumberOfSections: Int { return collectionView!.numberOfSections }

func viewNumberOfRows(inSection: Int) -> Int {
return collectionView.numberOfItems(inSection: inSection)
}

init(sections: [Section]) {
self.sections = sections
super.init(collectionViewLayout: UICollectionViewFlowLayout())
}

required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

override func viewDidLoad() {
super.viewDidLoad()
collectionView.dataSource = viewDataSource
}
}
}

0 comments on commit 479b2b3

Please sign in to comment.