Fix: Refreshing the poll when receiving pollEnd can break the chronological order in the store.

MatrixSDK/Aggregations/MXAggregations.m

@@ -315,8 +315,6 @@ - (void)registerListener
                     [self.beaconAggregations handleBeaconWithEvent:event];
                 }
                 break;
-            case MXEventTypePollEnd:
-                [self.aggregatedPollsUpdater refreshPollAfter:event];
             default:
                 break;
         }

MatrixSDK/Room/Polls/PollAggregator.swift

@@ -54,7 +54,7 @@ public class PollAggregator {
     private var events: [MXEvent] = []
     private var hasBeenEdited = false
 
-    public private(set) var poll: PollProtocol! {
+    public private(set) var poll: PollProtocol? {
         didSet {
             delegate?.pollAggregatorDidUpdateData(self)
         }
@@ -88,10 +88,10 @@ public class PollAggregator {
             throw PollAggregatorError.invalidPollStartEvent
         }
 
-        try self.init(session: session, room: room, pollStartEventId: pollStartEventId, delegate: delegate)
+        self.init(session: session, room: room, pollStartEventId: pollStartEventId, delegate: delegate)
     }
 
-    public init(session: MXSession, room: MXRoom, pollStartEventId: String, delegate: PollAggregatorDelegate? = nil) throws {
+    public init(session: MXSession, room: MXRoom, pollStartEventId: String, delegate: PollAggregatorDelegate? = nil) {
         self.session = session
         self.room = room
         self.pollStartEventId = pollStartEventId
@@ -100,7 +100,9 @@ public class PollAggregator {
 
         NotificationCenter.default.addObserver(self, selector: #selector(handleRoomDataFlush), name: .mxRoomDidFlushData, object: self.room)
         setupEditListener()
-        try buildPollStartContent()
+        buildPollStartContent()
+
+        reloadPollData()
     }
 
     private func setupEditListener() {
@@ -111,34 +113,35 @@ public class PollAggregator {
                 return
             }
 
-            do {
-                try self.buildPollStartContent()
-            } catch {
-                self.delegate?.pollAggregator(self, didFailWithError: PollAggregatorError.invalidPollStartEvent)
-            }
+            self.buildPollStartContent()
         }
     }
 
-    private func buildPollStartContent() throws {
-        guard let event = session.store.event(withEventId: pollStartEventId, inRoom: room.roomId),
-              let eventContent = MXEventContentPollStart(fromJSON: event.content),
-              eventContent.answerOptions.count >= Constants.minAnswerOptionCount
+    private func buildPollStartContent() {
+        let event = session.store.event(withEventId: pollStartEventId, inRoom: room.roomId)
+        tryUpdatePollStartedEvent(with: event)
+        if let pollStartedEvent = pollStartedEvent {
+            poll = pollBuilder.build(pollStartEventContent: pollStartEventContent,
+                                     pollStartEvent: pollStartedEvent,
+                                     events: events,
+                                     currentUserIdentifier: session.myUserId,
+                                     hasBeenEdited: hasBeenEdited)
+        }
+    }
+
+    private func tryUpdatePollStartedEvent(with event: MXEvent?) {
+        guard
+            let event = event,
+            let eventContent = MXEventContentPollStart(fromJSON: event.content),
+            eventContent.answerOptions.count >= Constants.minAnswerOptionCount
         else {
-            throw PollAggregatorError.invalidPollStartEvent
+            delegate?.pollAggregator(self, didFailWithError: PollAggregatorError.invalidPollStartEvent)
+            return
         }
 
         pollStartedEvent = event
         pollStartEventContent = eventContent
-
         hasBeenEdited = (event.unsignedData.relations?.replace != nil)
-
-        poll = pollBuilder.build(pollStartEventContent: eventContent,
-                                 pollStartEvent: pollStartedEvent,
-                                 events: events,
-                                 currentUserIdentifier: session.myUserId,
-                                 hasBeenEdited: hasBeenEdited)
-
-        reloadPollData()
     }
 
     @objc private func handleRoomDataFlush(sender: Notification) {
@@ -157,8 +160,12 @@ public class PollAggregator {
                 return
             }
 
-            self.events.removeAll()
+            self.tryUpdatePollStartedEvent(with: response.originalEvent)
+            if self.pollStartedEvent == nil {
+                return
+            }
 
+            self.events.removeAll()
             self.events.append(contentsOf: response.chunk)
 
             let eventTypes = [kMXEventTypeStringPollResponse, kMXEventTypeStringPollResponseMSC3381, kMXEventTypeStringPollEnd, kMXEventTypeStringPollEndMSC3381]
@@ -179,7 +186,7 @@ public class PollAggregator {
                                                    currentUserIdentifier: self.session.myUserId,
                                                    hasBeenEdited: self.hasBeenEdited)
             } as Any
-            
+
             self.poll = self.pollBuilder.build(pollStartEventContent: self.pollStartEventContent,
                                                pollStartEvent: self.pollStartedEvent,
                                                events: self.events,

MatrixSDKTests/MXCryptoSecretShareTests.m

@@ -114,7 +114,7 @@ - (void)testSecretShare
                 // -> She gets the secret
                 XCTAssertEqualObjects(sharedSecret, secret);
                 [expectation fulfill];
-
+                return YES;
             } failure:^(NSError * _Nonnull error) {
                 XCTFail(@"The operation should not fail - NSError: %@", error);
                 [expectation fulfill];

MatrixSDKTests/MXPollAggregatorTests.swift

@@ -40,12 +40,12 @@ class MXPollAggregatorTest: XCTestCase {
     func testAggregations() {
         self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in
             self.delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { pollAggregator in
-                XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2)
-                XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0)
+                XCTAssertEqual(self.pollAggregator.poll?.answerOptions.first!.count, 2)
+                XCTAssertEqual(self.pollAggregator.poll?.answerOptions.last!.count, 0)
                 expectation.fulfill()
             })
 
-            self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId)
+            self.pollAggregator = PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId)
 
             let dispatchGroup = DispatchGroup()
 
@@ -74,14 +74,14 @@ class MXPollAggregatorTest: XCTestCase {
     func testSessionPausing() {
         self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in
             let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { aggregator in
-                XCTAssertEqual(aggregator.poll.answerOptions.first!.count, 2)
-                XCTAssertEqual(aggregator.poll.answerOptions.last!.count, 0)
+                XCTAssertEqual(aggregator.poll?.answerOptions.first!.count, 2)
+                XCTAssertEqual(aggregator.poll?.answerOptions.last!.count, 0)
             })
 
-            self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId)
+            self.pollAggregator = PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId)
 
-            XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 1) // One from Alice
-            XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0)
+            XCTAssertEqual(self.pollAggregator.poll?.answerOptions.first!.count, 1) // One from Alice
+            XCTAssertEqual(self.pollAggregator.poll?.answerOptions.last!.count, 0)
 
             bobSession.pause()
 
@@ -99,16 +99,16 @@ class MXPollAggregatorTest: XCTestCase {
 
     func testGappySync() {
         self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in
-            self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId)
+            self.pollAggregator = PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId)
 
             self.delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { aggregator in
-                XCTAssertEqual(aggregator.poll.answerOptions.first!.count, 2) // One from Bob and one from Alice
-                XCTAssertEqual(aggregator.poll.answerOptions.last!.count, 1) // One from Alice
+                XCTAssertEqual(aggregator.poll?.answerOptions.first!.count, 2) // One from Bob and one from Alice
+                XCTAssertEqual(aggregator.poll?.answerOptions.last!.count, 1) // One from Alice
                 expectation.fulfill()
             })
 
-            XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 1) // One from Alice
-            XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0)
+            XCTAssertEqual(self.pollAggregator.poll?.answerOptions.first!.count, 1) // One from Alice
+            XCTAssertEqual(self.pollAggregator.poll?.answerOptions.last!.count, 0)
 
             bobSession.pause()
 
@@ -135,7 +135,7 @@ class MXPollAggregatorTest: XCTestCase {
 
     func testEditing() {
         self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in
-            self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId)
+            self.pollAggregator = PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId)
 
             self.delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { aggregator in
                 defer {
@@ -144,9 +144,9 @@ class MXPollAggregatorTest: XCTestCase {
                 guard self.isFirstDelegateUpdate else {
                     return
                 }
-                XCTAssertEqual(aggregator.poll.text, "Some other question")
-                XCTAssertEqual(aggregator.poll.answerOptions.count, 2)
-                XCTAssertTrue(aggregator.poll.hasBeenEdited)
+                XCTAssertEqual(aggregator.poll?.text, "Some other question")
+                XCTAssertEqual(aggregator.poll?.answerOptions.count, 2)
+                XCTAssertEqual(aggregator.poll?.hasBeenEdited, true)
                 expectation.fulfill()
             })
 

changelog.d/pr-1776.bugfix

@@ -0,0 +1 @@
+Poll: Refreshing the poll when receiving pollEnd can break the chronological order in the store.