Modify TemporaryMessageStorage API, so that we can have two storages: one (created by default init) for messages and the other for rescinds.
ClosedPublic
Actions

Authored by marcin on Mar 1 2023, 4:09 AM.

Details

Reviewers

bartek
tomek
atul
kamil

Commits

rCOMM7d230d26f9cb: Modify TemporaryMessageStorage API, so that we can have two storages: one…

Summary

This differential modifies TemporaryMesssageStorage API. Changes are:

Public API does not allow to create TemporaryMessageStorage at an arbitrary disk location. There are just two locations: one for messages with messageInfos and the other for rescinds.
Location for rescinds will use different directory and will prefix semaphores with differenti strings.

REMINDER: On iOS we can't store messages in SQLite directly from Notification Service. We agreed on workaround where NotificationService stores messages in a temporary flat file. AppDelegate reads and then removes (clears) content of this file when app is launched and transfers messages to the database. The class being subject of this differential provides an API to do that. Initially it was used exclusively for visible notifications since rescinds were handled by AppDelegate, Now that rescinds are handled by NSE we have to store them in a flat file as well so that AppDelegate can read them and update unread statuses of relevant threads on App start.

Test Plan

Remove codeVersion > 1000 condition. Kill the app. Send some notifications. Attach debugger to moveMessagesToDatabase. Launch the app and observe that messages payloads were persisted. This tests that previous functionality was not broken. Future differentials will use
new api for temporarily storing rescinds.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Mar 1 2023, 4:09 AM

Herald added a subscriber: ashoat. · View Herald TranscriptMar 1 2023, 4:09 AM

marcin added a parent revision: D6919: Remove relevant notification from notifications center when receiving rescind in NSE.Mar 1 2023, 4:09 AM

marcin retitled this revision from Modify TemporaryMessageStorage API, so that we can have to storages: one (created by default init) for messages and the other for rescinds. to Modify TemporaryMessageStorage API, so that we can have two storages: one (created by default init) for messages and the other for rescinds..

marcin edited the summary of this revision. (Show Details)Mar 1 2023, 4:19 AM

Harbormaster completed remote builds in B17037: Diff 23332.Mar 1 2023, 4:22 AM

marcin requested review of this revision.Mar 1 2023, 4:22 AM

marcin added a child revision: D6921: Persist rescinds payloads in NSE so that AppDelegate can update thread statuses on app start..Mar 1 2023, 4:23 AM

marcin added 1 blocking reviewer(s): tomek.Mar 8 2023, 2:16 AM

atul accepted this revision.Mar 9 2023, 2:15 PM

tomek accepted this revision.Mar 13 2023, 6:37 AM

tomek added inline comments.

native/ios/Comm/TemporaryMessageStorage/TemporaryMessageStorage.mm
173–174 ↗	(On Diff #23332)	Why do we need different file prefixes? Shouldn't the content be distinguished by the directory name?

This revision is now accepted and ready to land.Mar 13 2023, 6:37 AM

marcin added inline comments.Mar 13 2023, 6:50 AM

native/ios/Comm/TemporaryMessageStorage/TemporaryMessageStorage.mm
173–174 ↗	(On Diff #23332)	The content is distinguished by the directory name, but the semaphore name isn't. Unfortunately POSIX semaphore impose restriction on how long semaphore name can be. I don't remember by heart what the exact limit was but I remember that using an absolute path to the file as the name would exceed it.