rrbrambley / messagebeast-objc Goto Github PK

This was added to the Android version of this library, so it could be implemented here too.

rrbrambley/MessageBeast-Android#12

It'd be nice to have something that can update the configuration automatically, using these guidelines: https://developers.app.net/reference/resources/config/#resource-configuration

Without this functionality, you cannot have a sign out feature in an app. This would include:

1. Clearing all in-memory manager data
2. Removing all rows from all DB tables
3. Removing persisted channels and any other NSUserDefaults stuff

This corresponds to the java class here: https://github.com/rrbrambley/ADNLib-utils/blob/master/src/main/java/com/alwaysallthetime/adnlibutils/manager/ChannelSyncManager.java

Currently, upon fetching new messages from the server, the message manager (pretty much) always assumes it is safe to keep the messages in the channel's message map and incorporate their ids and dates into the MinMaxPair for that channel. However, because an app could create a new message with an old display date (e.g. ohai display date annotation), it is not safe to incorporate all new messages' dates/ids into the MinMaxPair – doing so means that subsequent calls to load persisted messages will use the min max pair and potentially leave gaps of messages out. This is bad.

I think the new naming convention should be:

load methods = load to memory, store in manager's memory map
get = get from persistence, do not store in map
retrieve = get from cloud, persist, store in map

See Android implementation.

See java version for implementation

Currently we rely on the Message Id for both of these tables. This is incorrect because:

• Offline message ids are created by examining the greatest message id in its target channel
• If an offline message is created in a channel, it is possible that the message id that is used already exists for a message in a different channel

The solution is most likely to change the primary key to be a messageId+channelId combo, but I will have to look into the performance implications for this.

Currently, when loading messages from the sqlite db, the min Id of the currently loaded messages are used to determine the "before date" – the date that must come after the display date of all messages to be loaded.

However, when the displayDate != createdAt on a message (e.g. an ohai date is used), the min id may correspond to a message with a display date that is after a message with a greater id. I think.

Perhaps the solution is to always keep the min display date in memory and query that way?

This would require that a "beforeDate" can be accepted on methods for getting messages by annotation, display location, hashtag, etc.

To support using custom places from Ohai, store them on sync and offer methods for getting custom places.

Dates are failing to evaluate as equal. It seems as though using NSNumber timeIntervalSince1970 could work, but that could look messy.

(lldb) p [(NSDate *)[channelMessages.allKeys objectAtIndex:0] timeIntervalSince1970]
(double) $2 = 1394469366.168542
(lldb) p [(NSDate *)[channelMessages.allKeys objectAtIndex:0] timeIntervalSince1970] == messagePlus.displayDate.timeIntervalSince1970
(bool) $3 = true
(lldb) p [(NSDate *)[channelMessages.allKeys objectAtIndex:0] isEqualToDate:messagePlus.displayDate]
(char) $4 = '\0'
(lldb) p [(NSDate *)[channelMessages.allKeys objectAtIndex:0] isEqual:messagePlus.displayDate]
(char) $6 = '\0'

This evaluates to YES:

[[NSNumber numberWithDouble:[(NSDate *)[channelMessages.allKeys objectAtIndex:0] timeIntervalSince1970]] isEqual:[NSNumber numberWithDouble:messagePlus.displayDate.timeIntervalSince1970]]

Is it possible to just use some other dictionary key? Sigh.

I realized this was pointless to have after implementing this in the Android repo.

Use case:

1. Create unsent message with file attachment, attempt send
2. Meanwhile, app shows the unsent message in the app.
3. The user chooses to delete the message as the upload is executing

This should presumably cancel the upload request, etc.

See the java bug: rrbrambley/MessageBeast-Android#45
Make sure this isn't a problem in the ObjC implementation

This involves handling replacement values for places. See the java implementation.

This mostly affects:

AATTADNDatabase
AATTMessageManager
AATTActionMessageManager

Currently, unsubscribe is available, but deactivate would be more appropriate: http://developers.app.net/docs/resources/channel/lifecycle/#deactivate-a-channel

Perhaps this should be optional, or an array of annotations should be optional.

Because the message id changes after a message is sent, action messages that are created for unsent messages no longer apply after the unsent message gets sent. After a message is sent, we should somehow, automatically determine which actions have been applied to it and create new action messages for that message.

as described here: https://github.com/appdotnet/object-metadata/blob/master/channel-types/net.app.ohai.journal.md

AATTActionMessageManager should provide methods for getting actioned messages by date/limit (in pages), so we need the sqlite db to store target message display dates in the action message specs

We shouldn't have to wait for the deletion failure to occur.

The AATTOrderedMessageBatch contains a property of type NSOrderedDictionary, whose keys are message display dates. So, technically, if two messages are created at the same millisecond, then one of them will not be seen. Is this a problem? I don't think so. Just never create two messages with the same date. Multiple apps could post messages simultaneously though. Will they have the same exact date? Probably not. sigh

I'm not sure what exactly would happen if you give a bogus factual id, but since we provide caching of Places via sqlite, we should handle the case where you try to use a factual id that no longer exists.

When a pending file is no longer available (the user deleted it?), we need a way to notify the client app and delete the unsent message.

I haven't even attempted to test the AATTMessageManager and other manager classes for threading issues. Is this something to be concerned about, or should it be the burden of the client to not go willy-nilly with multiple threads?

The configuration object currently defines the maximum message length as 2048 characters. It'd be great if we could add way off automatically gluing together multiple messages (via annotations or something) so that they could appear as one message – for those messages that need to exceed the max.

Although action messages could lay around and have no effect when a their target messages are deleted, it'd be nice to have the action messages deleted automatically. A couple ideas:

1. ActionMessageManager listens for a broadcast that is sent whenever a delete happens, looks for action messages associated with the deleted message, deletes them.
2. ChannelSyncManager has a method for deleting a message and does the same work as in 1) above.

We should add a flag to the AATTMessageManagerConfiguration so that a user can specify whether they want this feature on or not.

AATTMessageManager has a sendAllUnsent... method that should only be used when

1. the specified channel is not an action channel, or
2. the action messages in the specified action channel do not correspond to unsent target messages.

So, there is actually a method in AATTActionMessageManager that can be used to send unsent messages in action channels, and it does the right thing – only sends the messages if their target messages have been sent already. However, there is nothing preventing client apps from using the method in AATTMessageManager instead of this one.

In the Android version of this library, an exception is thrown if the MessageManager method is used for action channels, and then there is a package-visible method that the ActionMessageManager can access on the MessageManager when it needs to force sending in other scenarios. However, Objective-C doesn't really support this same thing.

Do something.

https://github.com/appdotnet/object-metadata/blob/master/annotations/net.app.core.attachments.md

This should be handled very similarly to how pending OEmbeds work. Perhaps just a modification of that.
Also, when deleting messages and sending pending message deletions, file attachments should be deleted.

When the AATTDisplayLocations are assigned upon loading messages from the database, a value of false is passed for the persist flag in lookupLocation(). This is because we assume that the display location was already assigned once and persisted when the retrieveMessages() method was called, so we optimize by avoiding a unnecessary insert. This could technically fail for all kinds of display locations, but geolocations are more susceptible to this failure because reverse geocoding can fail easily (e.g. when performing a ton of them upon full sync).

One solution might be to just assign display locations at the DB level - i.e. to rely on the display location instances table to populate the display locations when they are being pulled from sqlite. Then, only if a message does not have a display location we will attempt location lookup and always persist one if we find it.

Many apps could benefit from being able to easily access messages that have specific annotations. There is already support for retrieving OEmbed instances from the sqlite database, but maybe the configuration should enable the extraction of any annotation types.

... for cases where you don't want to create those manually, outside of the CSM

Currently, reverse geolocation will be performed rapidly when processing messages that need it (so, especially when a full sync is being performed).

However, I don't think Apple wants this (too many rapid fire requests) and I think it causes failures sometimes. Perhaps a queue should be maintained and they should be fired off gradually? I'm actually not sure what the best solution for this is.

Entities are missing for offline messages. This should be fixed by writing a utility to generate the entities as best as possible so that an application can display message entities when messages have yet to be sent to the server.

The Android version has this, let's do the same in ObjC