From fc665203c494da56c287b68309c4fc100021f1ff Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 19 Jan 2023 16:21:58 +0100 Subject: [PATCH 01/12] Improve PollAggregator API --- MatrixSDK/Room/Polls/PollAggregator.swift | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/MatrixSDK/Room/Polls/PollAggregator.swift b/MatrixSDK/Room/Polls/PollAggregator.swift index be77d13985..244daa4b2e 100644 --- a/MatrixSDK/Room/Polls/PollAggregator.swift +++ b/MatrixSDK/Room/Polls/PollAggregator.swift @@ -71,7 +71,7 @@ public class PollAggregator { } } - public convenience init(session: MXSession, room: MXRoom, pollEvent: MXEvent) throws { + public convenience init(session: MXSession, room: MXRoom, pollEvent: MXEvent, delegate: PollAggregatorDelegate? = nil) throws { var pollStartEventId: String? switch pollEvent.eventType { @@ -87,14 +87,15 @@ public class PollAggregator { throw PollAggregatorError.invalidPollStartEvent } - try self.init(session: session, room: room, pollStartEventId: pollStartEventId) + try self.init(session: session, room: room, pollStartEventId: pollStartEventId, delegate: delegate) } - public init(session: MXSession, room: MXRoom, pollStartEventId: String) throws { + public init(session: MXSession, room: MXRoom, pollStartEventId: String, delegate: PollAggregatorDelegate? = nil) throws { self.session = session self.room = room self.pollStartEventId = pollStartEventId self.pollBuilder = PollBuilder() + self.delegate = delegate NotificationCenter.default.addObserver(self, selector: #selector(handleRoomDataFlush), name: .mxRoomDidFlushData, object: self.room) setupEditListener() From 9a080073131c19e6b506806b77a5da90ccb01d72 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Thu, 19 Jan 2023 18:12:56 +0100 Subject: [PATCH 02/12] Add startDate in the Poll protocol --- MatrixSDK/Room/Polls/PollAggregator.swift | 13 ++++++++++--- MatrixSDK/Room/Polls/PollBuilder.swift | 13 ++++++++++--- MatrixSDK/Room/Polls/PollModels.swift | 6 +++--- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/MatrixSDK/Room/Polls/PollAggregator.swift b/MatrixSDK/Room/Polls/PollAggregator.swift index 244daa4b2e..ebd6b1b3d7 100644 --- a/MatrixSDK/Room/Polls/PollAggregator.swift +++ b/MatrixSDK/Room/Polls/PollAggregator.swift @@ -45,6 +45,7 @@ public class PollAggregator { private let pollStartEventId: String private let pollBuilder: PollBuilder + private var pollStartedEvent: MXEvent! private var pollStartEventContent: MXEventContentPollStart! private var referenceEventsListener: Any? @@ -126,11 +127,13 @@ public class PollAggregator { throw PollAggregatorError.invalidPollStartEvent } + pollStartedEvent = event pollStartEventContent = eventContent hasBeenEdited = (event.unsignedData.relations?.replace != nil) poll = pollBuilder.build(pollStartEventContent: eventContent, + pollStartEvent: pollStartedEvent, events: events, currentUserIdentifier: session.myUserId, hasBeenEdited: hasBeenEdited) @@ -160,21 +163,25 @@ public class PollAggregator { let eventTypes = [kMXEventTypeStringPollResponse, kMXEventTypeStringPollResponseMSC3381, kMXEventTypeStringPollEnd, kMXEventTypeStringPollEndMSC3381] self.referenceEventsListener = self.room.listen(toEventsOfTypes: eventTypes) { [weak self] event, direction, state in - guard let self = self, - let relatedEventId = event.relatesTo?.eventId, - relatedEventId == self.pollStartEventId else { + guard + let self = self, + let relatedEventId = event.relatesTo?.eventId, + relatedEventId == self.pollStartEventId + else { return } self.events.append(event) self.poll = self.pollBuilder.build(pollStartEventContent: self.pollStartEventContent, + pollStartEvent: self.pollStartedEvent, events: self.events, currentUserIdentifier: self.session.myUserId, hasBeenEdited: self.hasBeenEdited) } as Any self.poll = self.pollBuilder.build(pollStartEventContent: self.pollStartEventContent, + pollStartEvent: self.pollStartedEvent, events: self.events, currentUserIdentifier: self.session.myUserId, hasBeenEdited: self.hasBeenEdited) diff --git a/MatrixSDK/Room/Polls/PollBuilder.swift b/MatrixSDK/Room/Polls/PollBuilder.swift index 44d17127f1..a0aebe7218 100644 --- a/MatrixSDK/Room/Polls/PollBuilder.swift +++ b/MatrixSDK/Room/Polls/PollBuilder.swift @@ -22,9 +22,14 @@ struct PollBuilder { static let maxAnswerOptionCount = 20 } - func build(pollStartEventContent: MXEventContentPollStart, events: [MXEvent], currentUserIdentifier: String, hasBeenEdited: Bool = false) -> PollProtocol { + func build(pollStartEventContent: MXEventContentPollStart, + pollStartEvent: MXEvent, + events: [MXEvent], + currentUserIdentifier: String, + hasBeenEdited: Bool = false) -> PollProtocol { let poll = Poll() + poll.startDate = Date(timeIntervalSince1970: Double(pollStartEvent.originServerTs) / 1000) poll.hasBeenEdited = hasBeenEdited poll.hasDecryptionError = events.contains(where: { $0.isEncrypted && $0.clear == nil }) @@ -55,9 +60,11 @@ struct PollBuilder { var filteredEvents = events.filter { event in guard - let eventContent = event.content, event.eventType == __MXEventType.pollResponse, + let eventContent = event.content, + event.eventType == .pollResponse, let response = pollResponseFromEventContent(eventContent), - let _ = response[kMXMessageContentKeyExtensiblePollAnswers] else { + let _ = response[kMXMessageContentKeyExtensiblePollAnswers] + else { return false } diff --git a/MatrixSDK/Room/Polls/PollModels.swift b/MatrixSDK/Room/Polls/PollModels.swift index f8f565cca7..dbf57d3d40 100644 --- a/MatrixSDK/Room/Polls/PollModels.swift +++ b/MatrixSDK/Room/Polls/PollModels.swift @@ -33,6 +33,7 @@ public protocol PollProtocol { var text: String { get } var answerOptions: [PollAnswerOptionProtocol] { get } var kind: PollKind { get } + var startDate: Date { get } var maxAllowedSelections: UInt { get } var isClosed: Bool { get } var totalAnswerCount: UInt { get } @@ -43,7 +44,6 @@ public protocol PollProtocol { class PollAnswerOption: PollAnswerOptionProtocol { var id: String = "" var text: String = "" - var count: UInt = 0 var isWinner: Bool = false var isCurrentUserSelection: Bool = false @@ -52,14 +52,14 @@ class PollAnswerOption: PollAnswerOptionProtocol { class Poll: PollProtocol { var text: String = "" var answerOptions: [PollAnswerOptionProtocol] = [] - var kind: PollKind = .disclosed + var startDate: Date = .distantPast var maxAllowedSelections: UInt = 1 var isClosed: Bool = false var hasBeenEdited: Bool = false var hasDecryptionError: Bool = false var totalAnswerCount: UInt { - return self.answerOptions.reduce(0) { $0 + $1.count} + answerOptions.reduce(0) { $0 + $1.count } } } From 8d6813c8122abdab879fe78bb0df99314adba9cd Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Fri, 20 Jan 2023 11:23:22 +0100 Subject: [PATCH 03/12] Add id in PollProtocol --- MatrixSDK/Room/Polls/PollBuilder.swift | 1 + MatrixSDK/Room/Polls/PollModels.swift | 2 ++ 2 files changed, 3 insertions(+) diff --git a/MatrixSDK/Room/Polls/PollBuilder.swift b/MatrixSDK/Room/Polls/PollBuilder.swift index a0aebe7218..af5fc5e613 100644 --- a/MatrixSDK/Room/Polls/PollBuilder.swift +++ b/MatrixSDK/Room/Polls/PollBuilder.swift @@ -29,6 +29,7 @@ struct PollBuilder { hasBeenEdited: Bool = false) -> PollProtocol { let poll = Poll() + poll.id = pollStartEvent.eventId poll.startDate = Date(timeIntervalSince1970: Double(pollStartEvent.originServerTs) / 1000) poll.hasBeenEdited = hasBeenEdited poll.hasDecryptionError = events.contains(where: { $0.isEncrypted && $0.clear == nil }) diff --git a/MatrixSDK/Room/Polls/PollModels.swift b/MatrixSDK/Room/Polls/PollModels.swift index dbf57d3d40..8c007b0fe7 100644 --- a/MatrixSDK/Room/Polls/PollModels.swift +++ b/MatrixSDK/Room/Polls/PollModels.swift @@ -30,6 +30,7 @@ public protocol PollAnswerOptionProtocol { } public protocol PollProtocol { + var id: String { get } var text: String { get } var answerOptions: [PollAnswerOptionProtocol] { get } var kind: PollKind { get } @@ -50,6 +51,7 @@ class PollAnswerOption: PollAnswerOptionProtocol { } class Poll: PollProtocol { + var id: String = "" var text: String = "" var answerOptions: [PollAnswerOptionProtocol] = [] var kind: PollKind = .disclosed From 4d75ec41fc583c59b3988eda876bfacd5bd50693 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Mon, 23 Jan 2023 12:05:37 +0100 Subject: [PATCH 04/12] Fix memory leak --- MatrixSDK/Room/Polls/PollAggregator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MatrixSDK/Room/Polls/PollAggregator.swift b/MatrixSDK/Room/Polls/PollAggregator.swift index ebd6b1b3d7..5d6324e4e8 100644 --- a/MatrixSDK/Room/Polls/PollAggregator.swift +++ b/MatrixSDK/Room/Polls/PollAggregator.swift @@ -60,7 +60,7 @@ public class PollAggregator { } } - public var delegate: PollAggregatorDelegate? + public weak var delegate: PollAggregatorDelegate? deinit { if let referenceEventsListener = referenceEventsListener { From dd9de84c4a6b69f5b74bc720e74e79ce4ba86c76 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Mon, 23 Jan 2023 15:45:55 +0100 Subject: [PATCH 05/12] Cleanup PollAggregator --- MatrixSDK/Room/Polls/PollAggregator.swift | 44 +++++++++++------------ 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/MatrixSDK/Room/Polls/PollAggregator.swift b/MatrixSDK/Room/Polls/PollAggregator.swift index 5d6324e4e8..c7d137191d 100644 --- a/MatrixSDK/Room/Polls/PollAggregator.swift +++ b/MatrixSDK/Room/Polls/PollAggregator.swift @@ -42,11 +42,10 @@ public class PollAggregator { private let session: MXSession private let room: MXRoom - private let pollStartEventId: String private let pollBuilder: PollBuilder - private var pollStartedEvent: MXEvent! - private var pollStartEventContent: MXEventContentPollStart! + private let pollStartedEvent: MXEvent + private let pollStartEventContent: MXEventContentPollStart private var referenceEventsListener: Any? private var editEventsListener: Any? @@ -94,13 +93,22 @@ public class PollAggregator { public init(session: MXSession, room: MXRoom, pollStartEventId: String, delegate: PollAggregatorDelegate? = nil) throws { self.session = session self.room = room - self.pollStartEventId = pollStartEventId self.pollBuilder = PollBuilder() self.delegate = delegate + guard + let event = session.store.event(withEventId: pollStartEventId, inRoom: room.roomId), + let eventContent = MXEventContentPollStart(fromJSON: event.content), + eventContent.answerOptions.count >= Constants.minAnswerOptionCount + else { + throw PollAggregatorError.invalidPollStartEvent + } + + pollStartedEvent = event + pollStartEventContent = eventContent NotificationCenter.default.addObserver(self, selector: #selector(handleRoomDataFlush), name: .mxRoomDidFlushData, object: self.room) setupEditListener() - try buildPollStartContent() + buildPoll() } private func setupEditListener() { @@ -111,28 +119,18 @@ public class PollAggregator { return } - do { - try self.buildPollStartContent() - } catch { - self.delegate?.pollAggregator(self, didFailWithError: PollAggregatorError.invalidPollStartEvent) - } + self.buildPoll() } } - 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 - else { - throw PollAggregatorError.invalidPollStartEvent - } - - pollStartedEvent = event - pollStartEventContent = eventContent - - hasBeenEdited = (event.unsignedData.relations?.replace != nil) + private var pollStartEventId: String { + pollStartedEvent.eventId + } + + private func buildPoll() { + hasBeenEdited = (pollStartedEvent.unsignedData.relations?.replace != nil) - poll = pollBuilder.build(pollStartEventContent: eventContent, + poll = pollBuilder.build(pollStartEventContent: pollStartEventContent, pollStartEvent: pollStartedEvent, events: events, currentUserIdentifier: session.myUserId, From ed57de52c2558092a83543e6375e754421ef06e8 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Mon, 23 Jan 2023 15:53:55 +0100 Subject: [PATCH 06/12] Add changelog.d file --- changelog.d/pr-1691.change | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/pr-1691.change diff --git a/changelog.d/pr-1691.change b/changelog.d/pr-1691.change new file mode 100644 index 0000000000..053da27a47 --- /dev/null +++ b/changelog.d/pr-1691.change @@ -0,0 +1 @@ +Polls: add more information in PollProtocol for poll history. From f5f8fab3b9e203778fdeee275d6f388a51e97484 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Mon, 23 Jan 2023 16:40:36 +0100 Subject: [PATCH 07/12] Fix UTs --- MatrixSDKTests/MXPollAggregatorTests.swift | 14 +++++++---- MatrixSDKTests/MXPollBuilderTests.swift | 27 ++++++++++++++-------- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/MatrixSDKTests/MXPollAggregatorTests.swift b/MatrixSDKTests/MXPollAggregatorTests.swift index da236d8b26..22fd8e6751 100644 --- a/MatrixSDKTests/MXPollAggregatorTests.swift +++ b/MatrixSDKTests/MXPollAggregatorTests.swift @@ -58,12 +58,14 @@ class MXPollAggregatorTest: XCTestCase { } dispatchGroup.notify(queue: .main) { - self.pollAggregator.delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { + let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0) expectation.fulfill() }) + + self.pollAggregator.delegate = delegate } } } @@ -77,11 +79,13 @@ class MXPollAggregatorTest: XCTestCase { bobSession.pause() - self.pollAggregator.delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { + let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0) }) + self.pollAggregator.delegate = delegate + bobRoom.sendPollResponse(for: pollStartEvent, withAnswerIdentifiers: ["1"], threadId:nil, localEcho: nil) { _ in bobSession.resume { expectation.fulfill() @@ -106,11 +110,12 @@ class MXPollAggregatorTest: XCTestCase { aliceRoom.sendPollResponse(for: pollStartEvent, withAnswerIdentifiers: ["1", "2"], threadId:nil, localEcho: nil) { _ in self.matrixSDKTestsData.for(aliceSession.matrixRestClient, andRoom: aliceRoom.roomId, sendMessages: 50, testCase: self) { - self.pollAggregator.delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { + let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) // One from Bob and one from Alice XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 1) // One from Alice expectation.fulfill() }) + self.pollAggregator.delegate = delegate bobSession.resume { @@ -137,7 +142,7 @@ class MXPollAggregatorTest: XCTestCase { maxSelections: oldContent.maxSelections, answerOptions: []) - self.pollAggregator.delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { + let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { XCTAssertEqual(self.pollAggregator.poll.text, "Some other question") XCTAssertEqual(self.pollAggregator.poll.answerOptions.count, 0) @@ -146,6 +151,7 @@ class MXPollAggregatorTest: XCTestCase { expectation.fulfill() self.pollAggregator.delegate = nil }) + self.pollAggregator.delegate = delegate bobRoom.sendPollUpdate(for: pollStartEvent, oldContent: oldContent, newContent: newContent, localEcho: nil) { result in diff --git a/MatrixSDKTests/MXPollBuilderTests.swift b/MatrixSDKTests/MXPollBuilderTests.swift index 14b63cd819..7bb5ccc1ba 100644 --- a/MatrixSDKTests/MXPollBuilderTests.swift +++ b/MatrixSDKTests/MXPollBuilderTests.swift @@ -25,7 +25,7 @@ class MXPollBuilderTest: XCTestCase { events.append(MXEvent(fromJSON: pollResponseEventWithSender("Alice", answerIdentifiers: ["1"]))!) events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", answerIdentifiers: ["1"]))!) - let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 7), events: events, currentUserIdentifier: "") + let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 7), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "") XCTAssertEqual(poll.maxAllowedSelections, 7) XCTAssertEqual(poll.text, "Question") XCTAssertEqual(poll.kind, .disclosed) @@ -37,6 +37,9 @@ class MXPollBuilderTest: XCTestCase { XCTAssertEqual(poll.answerOptions.last?.id, "2") XCTAssertEqual(poll.answerOptions.last?.text, "Second answer") XCTAssertEqual(poll.answerOptions.last?.count, 0) + + XCTAssertEqual(poll.id, "$eventId") + XCTAssertEqual(poll.startDate, Date(timeIntervalSince1970: 0)) } func testSpoiledResponseEmpty() { @@ -44,7 +47,7 @@ class MXPollBuilderTest: XCTestCase { events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 0, answerIdentifiers: ["1"]))!) events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 1, answerIdentifiers: []))!) - let poll = builder.build(pollStartEventContent: pollStartEventContent(), events: events, currentUserIdentifier: "") + let poll = builder.build(pollStartEventContent: pollStartEventContent(), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "") XCTAssertEqual(poll.answerOptions.first?.count, 0) XCTAssertEqual(poll.answerOptions.last?.count, 0) } @@ -54,7 +57,7 @@ class MXPollBuilderTest: XCTestCase { events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 0, answerIdentifiers: ["1"]))!) events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 1, answerIdentifiers: ["1", "2"]))!) - let poll = builder.build(pollStartEventContent: pollStartEventContent(), events: events, currentUserIdentifier: "") + let poll = builder.build(pollStartEventContent: pollStartEventContent(), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "") XCTAssertEqual(poll.answerOptions.first?.count, 0) XCTAssertEqual(poll.answerOptions.last?.count, 0) } @@ -64,7 +67,7 @@ class MXPollBuilderTest: XCTestCase { events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 0, answerIdentifiers: ["1"]))!) events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 1, answerIdentifiers: ["1", "2", "3"]))!) - let poll = builder.build(pollStartEventContent: pollStartEventContent(), events: events, currentUserIdentifier: "") + let poll = builder.build(pollStartEventContent: pollStartEventContent(), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "") XCTAssertEqual(poll.answerOptions.first?.count, 0) XCTAssertEqual(poll.answerOptions.last?.count, 0) } @@ -73,7 +76,7 @@ class MXPollBuilderTest: XCTestCase { var events = [MXEvent]() events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", answerIdentifiers: ["1", "1", "1"]))!) - let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 100), events: events, currentUserIdentifier: "") + let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 100), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "") XCTAssertEqual(poll.answerOptions.first?.count, 1) XCTAssertEqual(poll.answerOptions.last?.count, 0) } @@ -82,7 +85,7 @@ class MXPollBuilderTest: XCTestCase { var events = [MXEvent]() events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", answerIdentifiers: ["1", "1", "2", "1", "2", "2", "1", "2"]))!) - let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 100), events: events, currentUserIdentifier: "") + let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 100), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "") XCTAssertEqual(poll.answerOptions.first?.count, 1) XCTAssertEqual(poll.answerOptions.last?.count, 1) } @@ -95,7 +98,7 @@ class MXPollBuilderTest: XCTestCase { events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 1, answerIdentifiers: []))!) // Too few events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 3, answerIdentifiers: ["1", "2"]))!) // Too many - let poll = builder.build(pollStartEventContent: pollStartEventContent(), events: events, currentUserIdentifier: "") + let poll = builder.build(pollStartEventContent: pollStartEventContent(), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "") XCTAssertEqual(poll.answerOptions.first?.count, 0) XCTAssertEqual(poll.answerOptions.last?.count, 1) } @@ -111,7 +114,7 @@ class MXPollBuilderTest: XCTestCase { events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 10, answerIdentifiers: []))!) events.append(MXEvent(fromJSON: pollResponseEventWithSender("Alice", timestamp:10, answerIdentifiers: ["1", "2"]))!) - let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 10), events: events, currentUserIdentifier: "") + let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 10), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "") XCTAssert(poll.isClosed) @@ -126,7 +129,7 @@ class MXPollBuilderTest: XCTestCase { var events = [MXEvent]() events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", answerIdentifiers: ["1"]))!) - let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 7), events: events, currentUserIdentifier: "Bob") + let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 7), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "Bob") XCTAssertEqual(poll.answerOptions.first?.isCurrentUserSelection, true) XCTAssertEqual(poll.answerOptions.last?.isCurrentUserSelection, false) } @@ -136,7 +139,7 @@ class MXPollBuilderTest: XCTestCase { events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 0, answerIdentifiers: ["1"]))!) events.append(MXEvent(fromJSON: pollResponseEventWithSender("Bob", timestamp: 1, answerIdentifiers: ["2", "1"]))!) - let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 7), events: events, currentUserIdentifier: "Bob") + let poll = builder.build(pollStartEventContent: pollStartEventContent(maxSelections: 7), pollStartEvent: pollStartedEvent(), events: events, currentUserIdentifier: "Bob") XCTAssertEqual(poll.answerOptions.first?.isCurrentUserSelection, true) XCTAssertEqual(poll.answerOptions.last?.isCurrentUserSelection, true) } @@ -153,6 +156,10 @@ class MXPollBuilderTest: XCTestCase { answerOptions: answerOptions) } + private func pollStartedEvent() -> MXEvent { + .init(fromJSON: pollResponseEventWithSender("Bob", answerIdentifiers: ["1", "2"])) + } + private func pollResponseEventWithSender(_ sender: String, timestamp: Int = 0, answerIdentifiers:[String]) -> [String: Any] { return [ "event_id": "$eventId", From 8b7631b3a9dfddd71c1cb6a49e9ec8e844cbce08 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Tue, 24 Jan 2023 12:15:50 +0100 Subject: [PATCH 08/12] Fix UTs --- MatrixSDKTests/MXPollAggregatorTests.swift | 49 ++++++++++------------ 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/MatrixSDKTests/MXPollAggregatorTests.swift b/MatrixSDKTests/MXPollAggregatorTests.swift index 22fd8e6751..9c5b25555c 100644 --- a/MatrixSDKTests/MXPollAggregatorTests.swift +++ b/MatrixSDKTests/MXPollAggregatorTests.swift @@ -39,6 +39,12 @@ class MXPollAggregatorTest: XCTestCase { self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId) + let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { + XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) + XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0) + expectation.fulfill() + }) + let dispatchGroup = DispatchGroup() for _ in 1...5 { @@ -58,13 +64,6 @@ class MXPollAggregatorTest: XCTestCase { } dispatchGroup.notify(queue: .main) { - let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { - XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) - XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0) - - expectation.fulfill() - }) - self.pollAggregator.delegate = delegate } } @@ -72,6 +71,11 @@ class MXPollAggregatorTest: XCTestCase { func testSessionPausing() { self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in + let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { + XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) + XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0) + }) + self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId) XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 1) // One from Alice @@ -79,11 +83,6 @@ class MXPollAggregatorTest: XCTestCase { bobSession.pause() - let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { - XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) - XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0) - }) - self.pollAggregator.delegate = delegate bobRoom.sendPollResponse(for: pollStartEvent, withAnswerIdentifiers: ["1"], threadId:nil, localEcho: nil) { _ in @@ -100,6 +99,12 @@ class MXPollAggregatorTest: XCTestCase { self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId) + let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { + XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) // One from Bob and one from Alice + XCTAssertEqual(self.pollAggregator.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) @@ -109,12 +114,6 @@ class MXPollAggregatorTest: XCTestCase { bobRoom.sendPollResponse(for: pollStartEvent, withAnswerIdentifiers: ["1"], threadId:nil, localEcho: nil) { _ in aliceRoom.sendPollResponse(for: pollStartEvent, withAnswerIdentifiers: ["1", "2"], threadId:nil, localEcho: nil) { _ in self.matrixSDKTestsData.for(aliceSession.matrixRestClient, andRoom: aliceRoom.roomId, sendMessages: 50, testCase: self) { - - let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { - XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) // One from Bob and one from Alice - XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 1) // One from Alice - expectation.fulfill() - }) self.pollAggregator.delegate = delegate bobSession.resume { @@ -136,14 +135,7 @@ class MXPollAggregatorTest: XCTestCase { self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId) - let oldContent = MXEventContentPollStart(fromJSON: pollStartEvent.content)! - let newContent = MXEventContentPollStart(question: "Some other question", - kind: oldContent.kind, - maxSelections: oldContent.maxSelections, - answerOptions: []) - let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { - XCTAssertEqual(self.pollAggregator.poll.text, "Some other question") XCTAssertEqual(self.pollAggregator.poll.answerOptions.count, 0) XCTAssertTrue(self.pollAggregator.poll.hasBeenEdited) @@ -151,6 +143,12 @@ class MXPollAggregatorTest: XCTestCase { expectation.fulfill() self.pollAggregator.delegate = nil }) + + let oldContent = MXEventContentPollStart(fromJSON: pollStartEvent.content)! + let newContent = MXEventContentPollStart(question: "Some other question", + kind: oldContent.kind, + maxSelections: oldContent.maxSelections, + answerOptions: []) self.pollAggregator.delegate = delegate bobRoom.sendPollUpdate(for: pollStartEvent, oldContent: oldContent, newContent: newContent, localEcho: nil) { result in @@ -206,7 +204,6 @@ class MXPollAggregatorTest: XCTestCase { } private class PollAggregatorBlockWrapper: PollAggregatorDelegate { - let dataUpdateCallback: ()->(Void) internal init(dataUpdateCallback: @escaping () -> (Void)) { From c1412b505e1875a3cb934144cb1bff4286a66b1b Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Tue, 24 Jan 2023 15:11:43 +0100 Subject: [PATCH 09/12] Revert "Cleanup PollAggregator" This reverts commit dd9de84c4a6b69f5b74bc720e74e79ce4ba86c76. --- MatrixSDK/Room/Polls/PollAggregator.swift | 44 ++++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/MatrixSDK/Room/Polls/PollAggregator.swift b/MatrixSDK/Room/Polls/PollAggregator.swift index c7d137191d..5d6324e4e8 100644 --- a/MatrixSDK/Room/Polls/PollAggregator.swift +++ b/MatrixSDK/Room/Polls/PollAggregator.swift @@ -42,10 +42,11 @@ public class PollAggregator { private let session: MXSession private let room: MXRoom + private let pollStartEventId: String private let pollBuilder: PollBuilder - private let pollStartedEvent: MXEvent - private let pollStartEventContent: MXEventContentPollStart + private var pollStartedEvent: MXEvent! + private var pollStartEventContent: MXEventContentPollStart! private var referenceEventsListener: Any? private var editEventsListener: Any? @@ -93,22 +94,13 @@ public class PollAggregator { public init(session: MXSession, room: MXRoom, pollStartEventId: String, delegate: PollAggregatorDelegate? = nil) throws { self.session = session self.room = room + self.pollStartEventId = pollStartEventId self.pollBuilder = PollBuilder() self.delegate = delegate - guard - let event = session.store.event(withEventId: pollStartEventId, inRoom: room.roomId), - let eventContent = MXEventContentPollStart(fromJSON: event.content), - eventContent.answerOptions.count >= Constants.minAnswerOptionCount - else { - throw PollAggregatorError.invalidPollStartEvent - } - - pollStartedEvent = event - pollStartEventContent = eventContent NotificationCenter.default.addObserver(self, selector: #selector(handleRoomDataFlush), name: .mxRoomDidFlushData, object: self.room) setupEditListener() - buildPoll() + try buildPollStartContent() } private func setupEditListener() { @@ -119,18 +111,28 @@ public class PollAggregator { return } - self.buildPoll() + do { + try self.buildPollStartContent() + } catch { + self.delegate?.pollAggregator(self, didFailWithError: PollAggregatorError.invalidPollStartEvent) + } } } - private var pollStartEventId: String { - pollStartedEvent.eventId - } - - private func buildPoll() { - hasBeenEdited = (pollStartedEvent.unsignedData.relations?.replace != nil) + 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 + else { + throw PollAggregatorError.invalidPollStartEvent + } + + pollStartedEvent = event + pollStartEventContent = eventContent + + hasBeenEdited = (event.unsignedData.relations?.replace != nil) - poll = pollBuilder.build(pollStartEventContent: pollStartEventContent, + poll = pollBuilder.build(pollStartEventContent: eventContent, pollStartEvent: pollStartedEvent, events: events, currentUserIdentifier: session.myUserId, From dbdcbe221074c5e1579c6f78a109021ed0253b3d Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Tue, 24 Jan 2023 15:29:16 +0100 Subject: [PATCH 10/12] Fix MXPollAggregatorTests --- MatrixSDKTests/MXPollAggregatorTests.swift | 56 ++++++++++++---------- Podfile.lock | 2 +- fastlane/Fastfile | 2 +- 3 files changed, 33 insertions(+), 27 deletions(-) diff --git a/MatrixSDKTests/MXPollAggregatorTests.swift b/MatrixSDKTests/MXPollAggregatorTests.swift index 9c5b25555c..9862243eda 100644 --- a/MatrixSDKTests/MXPollAggregatorTests.swift +++ b/MatrixSDKTests/MXPollAggregatorTests.swift @@ -18,33 +18,35 @@ import Foundation class MXPollAggregatorTest: XCTestCase { private var matrixSDKTestsData: MatrixSDKTestsData! - private var matrixSDKTestsE2EData: MatrixSDKTestsE2EData! - private var pollAggregator: PollAggregator! + private var delegate: PollAggregatorBlockWrapper! + private var isFirstDelegateUpdate: Bool = true override func setUp() { super.setUp() matrixSDKTestsData = MatrixSDKTestsData() matrixSDKTestsE2EData = MatrixSDKTestsE2EData(matrixSDKTestsData: matrixSDKTestsData) + isFirstDelegateUpdate = true } override func tearDown() { matrixSDKTestsData = nil matrixSDKTestsE2EData = nil + delegate = nil super.tearDown() } func testAggregations() { self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in - self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId) - - let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { + self.delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { pollAggregator in 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) + let dispatchGroup = DispatchGroup() for _ in 1...5 { @@ -64,16 +66,16 @@ class MXPollAggregatorTest: XCTestCase { } dispatchGroup.notify(queue: .main) { - self.pollAggregator.delegate = delegate + self.pollAggregator.delegate = self.delegate } } } func testSessionPausing() { self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in - let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { - XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) - XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 0) + let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { aggregator in + 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) @@ -99,9 +101,9 @@ class MXPollAggregatorTest: XCTestCase { self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId) - let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { - XCTAssertEqual(self.pollAggregator.poll.answerOptions.first!.count, 2) // One from Bob and one from Alice - XCTAssertEqual(self.pollAggregator.poll.answerOptions.last!.count, 1) // One from Alice + 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 expectation.fulfill() }) @@ -114,7 +116,7 @@ class MXPollAggregatorTest: XCTestCase { bobRoom.sendPollResponse(for: pollStartEvent, withAnswerIdentifiers: ["1"], threadId:nil, localEcho: nil) { _ in aliceRoom.sendPollResponse(for: pollStartEvent, withAnswerIdentifiers: ["1", "2"], threadId:nil, localEcho: nil) { _ in self.matrixSDKTestsData.for(aliceSession.matrixRestClient, andRoom: aliceRoom.roomId, sendMessages: 50, testCase: self) { - self.pollAggregator.delegate = delegate + self.pollAggregator.delegate = self.delegate bobSession.resume { @@ -135,24 +137,28 @@ class MXPollAggregatorTest: XCTestCase { self.createScenarioForBobAndAlice { bobSession, aliceSession, bobRoom, aliceRoom, pollStartEvent, expectation in self.pollAggregator = try! PollAggregator(session: bobSession, room: bobRoom, pollStartEventId: pollStartEvent.eventId) - let delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { - XCTAssertEqual(self.pollAggregator.poll.text, "Some other question") - XCTAssertEqual(self.pollAggregator.poll.answerOptions.count, 0) - XCTAssertTrue(self.pollAggregator.poll.hasBeenEdited) - + self.delegate = PollAggregatorBlockWrapper(dataUpdateCallback: { aggregator in + defer { + self.isFirstDelegateUpdate = false + } + guard self.isFirstDelegateUpdate else { + return + } + XCTAssertEqual(aggregator.poll.text, "Some other question") + XCTAssertEqual(aggregator.poll.answerOptions.count, 2) + XCTAssertTrue(aggregator.poll.hasBeenEdited) expectation.fulfill() - self.pollAggregator.delegate = nil }) let oldContent = MXEventContentPollStart(fromJSON: pollStartEvent.content)! let newContent = MXEventContentPollStart(question: "Some other question", kind: oldContent.kind, maxSelections: oldContent.maxSelections, - answerOptions: []) - self.pollAggregator.delegate = delegate + answerOptions: oldContent.answerOptions) + bobRoom.sendPollUpdate(for: pollStartEvent, oldContent: oldContent, newContent: newContent, localEcho: nil) { result in - + self.pollAggregator.delegate = self.delegate } failure: { error in XCTFail("The operation should not fail - NSError: \(String(describing: error))") } @@ -204,9 +210,9 @@ class MXPollAggregatorTest: XCTestCase { } private class PollAggregatorBlockWrapper: PollAggregatorDelegate { - let dataUpdateCallback: ()->(Void) + let dataUpdateCallback: (PollAggregator) -> (Void) - internal init(dataUpdateCallback: @escaping () -> (Void)) { + internal init(dataUpdateCallback: @escaping (PollAggregator) -> (Void)) { self.dataUpdateCallback = dataUpdateCallback } @@ -223,6 +229,6 @@ private class PollAggregatorBlockWrapper: PollAggregatorDelegate { } func pollAggregatorDidUpdateData(_ aggregator: PollAggregator) { - dataUpdateCallback() + dataUpdateCallback(aggregator) } } diff --git a/Podfile.lock b/Podfile.lock index 1dd208427d..b92f7922ad 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -62,7 +62,7 @@ SPEC REPOS: - SwiftyBeaver SPEC CHECKSUMS: - AFNetworking: 7864c38297c79aaca1500c33288e429c3451fdce + AFNetworking: 3bd23d814e976cd148d7d44c3ab78017b744cd58 GZIP: 416858efbe66b41b206895ac6dfd5493200d95b3 libbase58: 7c040313537b8c44b6e2d15586af8e21f7354efd MatrixSDKCrypto: 862d9b4dbb6861da030943f5a18c39258ed7345b diff --git a/fastlane/Fastfile b/fastlane/Fastfile index a9fdad60b3..81187bdfef 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -18,7 +18,7 @@ platform :ios do before_all do # Ensure used Xcode version - xcversion(version: "~> 13.2") + xcversion(version: "~> 14.2") end #### Pod #### From bcb67678eaf3e648495acf91b34b6994f1009dc2 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Tue, 24 Jan 2023 15:31:31 +0100 Subject: [PATCH 11/12] Amend Fastfile --- fastlane/Fastfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fastlane/Fastfile b/fastlane/Fastfile index 81187bdfef..a9fdad60b3 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -18,7 +18,7 @@ platform :ios do before_all do # Ensure used Xcode version - xcversion(version: "~> 14.2") + xcversion(version: "~> 13.2") end #### Pod #### From 99df89958d1824b59490ad99064ab6ab7be65884 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Tue, 24 Jan 2023 15:32:34 +0100 Subject: [PATCH 12/12] Amdend Podfile.lock --- Podfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Podfile.lock b/Podfile.lock index b92f7922ad..1dd208427d 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -62,7 +62,7 @@ SPEC REPOS: - SwiftyBeaver SPEC CHECKSUMS: - AFNetworking: 3bd23d814e976cd148d7d44c3ab78017b744cd58 + AFNetworking: 7864c38297c79aaca1500c33288e429c3451fdce GZIP: 416858efbe66b41b206895ac6dfd5493200d95b3 libbase58: 7c040313537b8c44b6e2d15586af8e21f7354efd MatrixSDKCrypto: 862d9b4dbb6861da030943f5a18c39258ed7345b