Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Universal message storage #993

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pwetrifork
Copy link
Contributor

Introduction

This work is provided by Erlang Solutions. Our core product in the XMPP area is the MongooseIM platform and prior related framework contributions include the MUC Light implementation by Inaka.
While we are focused on the backend components at the moment, it is in our best interest to make the client-side development easier. This is particularly relevant to those of our customers whose systems do not have XMPP messaging as the core functionality and who may be therefore lacking technical expertise in that area.

Motivation

We have been developing a demo/tech showcase iOS chat app based on XMPPFramework. Among the observations we gathered during that time are the following:

  • Implementing non-trivial message processing where multiple XEPs are involved takes surprisingly high amount of effort.
  • In particular, the framework provides very limited assistance when it comes to local/offline message storage. The existing XEP-0136/0045/MUC Light storage implementations in their basic form are best suited to handling messages at the core level and making them aware of extensions feels like a quite involved task.

When researching other open-source clients based on the framework, we found out there is a common usage pattern where an app provides its own custom storage and a message processing module that attempts to implement some custom processing path specific to a particular set of XMPP extensions.
We believe this goes against the framework's goal, i.e. shifting the app developer's focus from providing actual XMPP processing logic towards simply configuring the framework to suit their usage scenario.

Proposed solution

General

The aim of this pull request is to provide client message storage functionality with the following features:

  • When deciding to employ a supported XMPP extension, app developer's involvement should be limited to enabling a framework module and dealing with app-facing storage API.
  • XMPP extension interdependencies should be handled on app configuration level instead of module implementation level. This would allow an app to employ any subset of supported extensions and also make extension handling logic cleaner.
  • Adding support for new XMPP extensions should not require modification of the core storage model.

Description

Top level implementation

The implementation (XMPPMessageCoreDataStorage) is based on XMPPCoreDataStorage. Its object model can be divided into two sections:

  • The base section represented by XMPPMessageCoreDataStorageObject entity
  • The extensible context section consisting of XMPPMessageContextCoreDataStorageObject and several XMPPMessageContextXXXItemCoreDataStorageObject entities

The base section models the core XMPP message properties while all other information, including extension data stored by modules and metadata like local timestamps, belongs to the extensible context.

The extensible context is a weakly typed structure where each message can be associated with several XMPPMessageContextCoreDataStorageObject instances, which in turn aggregate XMPPMessageContextXXXItemCoreDataStorageObject instances containing primitive tagged values.
The tags and aggregation structure are defined for individual XMPP extensions by the respective module implementations. From application's point of view this all becomes an implementation detail as the API is provided via categories on XMPPMessageContextCoreDataStorageObject.

The main trait of the above scheme is that it enables local database queries involving extensible context attributes, while still giving significant degree of flexibility.

Transactions

As message processing in the framework is highly asynchronous with individual modules running on different queues, there has to be a way to ensure that applications do not witness partially processed message storage objects.
To do so, when processing a module delegate callback, an application obtains an XMPPMessageCoreDataStorageTransaction object and uses it to perform storage updates, as opposed to scheduling them directly. The transaction object then batches all the updates related to a given message and executes them as a single action.
The transaction API is provided via categories on XMPPMessageCoreDataStorageTransaction implemented by module authors and operates on the object model internally, just like in case of the XMPPMessageContextCoreDataStorageObject API mentioned above.

XMPPStream element events

The section about transactional processing mentions the application obtaining a transaction from within a message processing module delegate callback and later the transaction batching storage actions. That would not be possible with the current XMPPStream implementation due to the following shortcomings:

  • It is impossible to reliably identify a given message across delegate callbacks in different modules. We were not satisfied with the approaches we have seen so far due to:
    • Reliance on the other messaging party behavior.
    • Requiring the use of willReceiveMessage: stream delegate callback, which is heavily dependent on the delegate order.
    • Involving testing XMPPMessage instances for pointer equality, which we do not consider appropriate for a class conforming to NSCopying.
  • There is no indication that delegate processing for a given XMPP element has been finished.

To address the issue, we introduced the concept of stream element events. It is now possible to obtain an XMPPElementEvent instance while in the context of a stream delegate element send/receive callback invocation. This event serves as handle that:

  • Uniquely identifies the related XMPP element across all delegates.
  • Is available not only in the immediate stream callbacks but also in the module callbacks that follow.
  • Provides processing completion notifications.

The important point here is that this change is transparent w.r.t. the stream-module interface, i.e. even the existing modules will see the events propagated in their delegate callbacks.

Currently supported XMPP extensions

This pull request introduces several message processing modules together with their respective storage implementation helpers:

  • XMPPOneToOneChat - a very basic one-to-one chat handling module, provided as an equivalent to the existing groupchat modules.
  • XMPPDelayedDelivery - in the existing storage implementations XEP-0203 handling used to be hardcoded, now it can be implemented on par with the remaining XEPs.
  • XMPPManagedMessaging - handles XEP-0198 message acknowledgement statuses.
  • XMPPLastMessageCorrection - implements XEP-0308 message corrections.
  • XMPPOutOfBandResourceMessaging - manages XEP-0066 auxiliary resource URIs embedded in messages.

Storage handling has also been added for certain existing modules:

  • XMPPRoomLight
  • XMPPMessageArchiveManagement
  • XMPPMessageDeliveryReceipts

Alternatives to this pull request

We understand that this is a rather large-scale pull request. If you believe that it extends beyond the framework's scope, please let us know if you are interested in merging some portions of it. The un-squashed branches are as follows:

This commit represents squashed 'wip/universal-message-storage-modules' branch
@chrisballinger
Copy link
Collaborator

Interesting! This is a beast of a pull request. It would be easier for me to review each un-squashed branch at a time. Could you submit the PRs one by one? Some questions off the top of my head to answer for each PR:

  • Does this break the public API of any modules?
  • Are backwards-compatible additions to the existing public API necessary, or in the proper location?
  • Does this break any existing functionality?
  • Does this cause issues for people using alternative storage backends to Core Data, like YapDatabase or Realm?
  • Does this use modern Obj-C for nullability, generics, etc?
  • Are the changes documented? Especially the public API.
  • Are there new unit tests? Do all of the old unit tests pass?

@pwetrifork
Copy link
Contributor Author

Noted, I will look into it later this week.

@pwetrifork
Copy link
Contributor Author

The first batch of PRs is ready: #996, #997, #998, #999, #1000, #1001, #1002, #1003, #1004. There will be more to follow that have the above as prerequisites.

Some of them will cause umbrella header/project file merge conflicts. I will try to check back and rebase, time permitting of course.

Unless stated in descriptions of the individual PRs, none of them break public APIs/existing functionality, all use modern Obj-C, add unit tests and include HeaderDoc comments in public headers.

Since there are no storage API-breaking changes, none of this should affect applications using alternative backends.

@chrisballinger
Copy link
Collaborator

Awesome, thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants