Merge pull request matrix-org#1776 from matrix-org/nimau/7497_timelin…

…e_closed_polls

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.