Skip to content

Commit

Permalink
RCOCOA-2105 Fix crash when deleting a row from @ObservedSectionedResu…
Browse files Browse the repository at this point in the history
…lts (#8295)
  • Loading branch information
leemaguire authored Jun 17, 2024
1 parent 2a28fec commit c3492ed
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 13 deletions.
7 changes: 4 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
x.y.z Release notes (yyyy-MM-dd)
=============================================================
### Enhancements
* None.
* Add `@ObservedSectionedResults.remove(atOffsets:section:)` which adds the ability to
remove a Realm Object when using `onDelete` on `ForEach` in a SwiftUI `List`.

### Fixed
* <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-swift/issues/????), since v?.?.?)
* None.
* Deleting a Realm Object used in a `@ObservedSectionedResults` collection in `SwiftUI`
would cause a crash during the diff on the `View`. ([#8294](https://github.com/realm/realm-swift/issues/8294), since v10.29.0)

<!-- ### Breaking Changes - ONLY INCLUDE FOR NEW MAJOR version -->

Expand Down
14 changes: 8 additions & 6 deletions Realm/Tests/SwiftUITestHost/SwiftUITestHostApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,8 @@ struct ObservedSectionedResultsKeyPathTestView: View {
Section(header: Text(section.key)) {
ForEach(section) { object in
ObservedResultsKeyPathTestRow(list: object)
}.onDelete {
$reminders.remove(atOffsets: $0, section: section)
}
}
}
Expand Down Expand Up @@ -443,10 +445,10 @@ struct ObservedSectionedResultsSearchableTestView: View {
.navigationTitle("Reminders")
.navigationBarItems(trailing:
Button("add") {
let realm = $reminders.wrappedValue.realm
try! realm?.write {
realm?.add(ReminderList())
}
let realm = $reminders.wrappedValue.realm?.thaw()
try! realm?.write {
realm?.add(ReminderList())
}
}.accessibility(identifier: "addList"))
}
}
Expand Down Expand Up @@ -488,15 +490,15 @@ struct ObservedSectionedResultsConfiguration: View {
.navigationTitle("Reminders")
.navigationBarItems(leading:
Button("add A") {
let realm = $remindersA.wrappedValue.realm
let realm = $remindersA.wrappedValue.realm?.thaw()
try! realm?.write {
realm?.add(ReminderList())
}
}.accessibility(identifier: "addListA")
)
.navigationBarItems(trailing:
Button("add B") {
let realm = $remindersB.wrappedValue.realm
let realm = $remindersB.wrappedValue.realm?.thaw()
try! realm?.write {
realm?.add(ReminderList())
}
Expand Down
16 changes: 16 additions & 0 deletions Realm/Tests/SwiftUITestHostUITests/SwiftUITestHostUITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,22 @@ class SwiftUITests: XCTestCase {
XCTAssertEqual(app.tables.firstMatch.cells.count, 3)
}
XCTAssertEqual(realm.objects(ReminderList.self).count, 3)

let collectionViewsQuery = XCUIApplication().collectionViews
collectionViewsQuery.children(matching: .other).element(boundBy: 1).tap()
collectionViewsQuery.children(matching: .cell).element(boundBy: 1).children(matching: .other).element(boundBy: 1).children(matching: .other).element.swipeLeft()
collectionViewsQuery.buttons["Delete"].tap()
XCTAssertEqual(realm.objects(ReminderList.self).count, 2)

collectionViewsQuery.children(matching: .other).element(boundBy: 2).tap()
collectionViewsQuery.children(matching: .cell).element(boundBy: 2).children(matching: .other).element(boundBy: 1).children(matching: .other).element.swipeLeft()
collectionViewsQuery.buttons["Delete"].tap()
XCTAssertEqual(realm.objects(ReminderList.self).count, 1)

collectionViewsQuery.children(matching: .other).element(boundBy: 1).tap()
collectionViewsQuery.children(matching: .cell).element(boundBy: 1).children(matching: .other).element(boundBy: 1).children(matching: .other).element.swipeLeft()
collectionViewsQuery.buttons["Delete"].tap()
XCTAssertEqual(realm.objects(ReminderList.self).count, 0)
}

@MainActor
Expand Down
40 changes: 36 additions & 4 deletions RealmSwift/SwiftUI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,10 @@ extension Projection: _ObservedResultsValue { }
@propertyWrapper public struct ObservedSectionedResults<Key: _Persistable & Hashable, ResultType>: DynamicProperty, BoundCollection where ResultType: _ObservedResultsValue & RealmFetchable & KeypathSortable & Identifiable {
public typealias Element = ResultType

private class Storage: ObservableResultsStorage<SectionedResults<Key, ResultType>> {
private class Storage: ObservableResultsStorage<Results<ResultType>> {
var sectionedResults: SectionedResults<Key, ResultType>!
var token: AnyCancellable?

override func updateValue() {
let realm = try! Realm(configuration: configuration ?? Realm.Configuration.defaultConfiguration)
var results = realm.objects(ResultType.self)
Expand All @@ -659,7 +662,24 @@ extension Projection: _ObservedResultsValue { }
sortDescriptors.append(.init(keyPath: keyPathString, ascending: true))
}

value = results.sectioned(sortDescriptors: sortDescriptors, sectionBlock)
value = results

/*
Observing the sectioned results directly doesn't allow the SwiftUI diff to work
correctly as the previous state of the sectioned results will have the new values.
An example of when this is an issue is when an item is deleted in a List containing sectioned results,
the diff needs a stable state of the previous transaction but due to
the observation callback calling calculate_sections the collection will be brought up to date.
The solution around this is to store a frozen copy of the sectioned results and observe the parent `Results` instead.
Each time the results observation callback is invoked and the SwiftUI View is redrawn the sectioned results will be updated.
*/
sectionedResults = value.sectioned(sortDescriptors: sortDescriptors, sectionBlock).freeze()
token = self.objectWillChange.sink { [weak self] _ in
guard let self = self else { return }
self.sectionedResults = self.value.sectioned(sortDescriptors: self.sortDescriptors, self.sectionBlock).freeze()
}
}

var sortDescriptors: [SortDescriptor] = [] {
Expand All @@ -684,7 +704,7 @@ extension Projection: _ObservedResultsValue { }
if self.sortDescriptors.isEmpty {
throwRealmException("sortDescriptors must not be empty when sectioning ObservedSectionedResults with `sectionBlock`")
}
super.init(value.sectioned(sortDescriptors: self.sortDescriptors, self.sectionBlock), keyPaths)
super.init(value, keyPaths)
}
}

Expand Down Expand Up @@ -717,13 +737,25 @@ extension Projection: _ObservedResultsValue { }
/// :nodoc:
public var wrappedValue: SectionedResults<Key, ResultType> {
storage.setupValue()
return storage.value
return storage.sectionedResults
}
/// :nodoc:
public var projectedValue: Self {
return self
}

/// Removes items from an `@ObservedSectionedResults` collection
/// with a given `IndexSet` and `ResultsSection`.
/// - Parameters:
/// - offsets: Index offsets in the section.
/// - section: The section containing the items to remove.
public func remove(atOffsets offsets: IndexSet,
section: ResultsSection<Key, ResultType>) where ResultType: ObjectBase & ThreadConfined {
write(wrappedValue) { collection in
collection.realm?.delete(offsets.compactMap { section[$0].thaw() ?? nil })
}
}

private init(type: ResultType.Type,
sectionBlock: @escaping ((ResultType) -> Key),
sortDescriptors: [SortDescriptor] = [],
Expand Down
2 changes: 2 additions & 0 deletions RealmSwift/Tests/SwiftUITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,7 @@ class SwiftUITests: TestCase {
// add another default inited object for filter comparison
realm.add(object)
}
realm.refresh()
XCTAssertEqual(fullResults.wrappedValue.count, 1)
XCTAssertEqual(fullResults.wrappedValue[0].key, "abc")

Expand Down Expand Up @@ -780,6 +781,7 @@ class SwiftUITests: TestCase {
// add another default inited object for filter comparison
realm.add(object)
}
realm.refresh()
XCTAssertEqual(fullResults.wrappedValue.count, 1)
XCTAssertEqual(fullResults.wrappedValue[0].key, "abc")

Expand Down

0 comments on commit c3492ed

Please sign in to comment.