search

rrbrambley / MessageBeast-Android

A library for building Message-based apps on App.net
MIT License
5 stars 0 forks source link

Message and ActionMessageSpec primary key should be changed #10

Closed rrbrambley closed 10 years ago

rrbrambley commented 10 years ago

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.

rrbrambley commented 10 years ago

As of f2ff01fb6b2ad850d7bd9ac5baff7369bbd68a79, the max id across all channels is used to determine what the new unsent message id should be. This is better solution, but there is still a potential problem, e.g.

  1. Create unsent message in channel 1.
  2. Create unsent message in channel 2.
  3. Attempt to retrieve messages in channel 1, cannot do so.
  4. Send unsent message in channel 1.
  5. Retrieve newest messages in channel 1.

In step 5, the messages pulled from channel 1 could have ids that conflict with the message ids in channel 2. Since we don't force all messages to be sent in all channels before pulling on an individual channel, we still have a problem. Probably pretty unlikely that this will happen in practice, but perhaps the solution is to just force ALL channels to have their unsent messages sent before ANY channels can retrieve.

rrbrambley commented 10 years ago

This happened. Uh oh.

D/MessageBeast_MessageManager(26676): Created and stored unsent message with id 2888944 for channel 45455 D/MessageBeast_MessageManager(26676): Created and stored unsent message with id 2888945 for channel 45456 D/AppDotNetClientTask(26676): Connecting to https://alpha-api.app.net/stream/0/channels/45455/messages?include_deleted=0&include_annotations=1&include_machine=1 D/MessageBeast_MessageManager(26676): Channel 45455; Successfully sent unsent message with id 2888944; replaced with message 2888945 D/MessageBeast_ActionMessageManager(26676): Updating action message spec; target id change: 2888944 --> 2888945 D/MessageBeast_ActionMessageManager(26676): replaced message's target message id annotation: null --> null D/MessageBeast_ActionMessageManager(26676): onUnsentMessagesSentPrivate() - sending all unsent messages for action channel 45456 D/AppDotNetClientTask(26676): Connecting to https://alpha-api.app.net/stream/0/channels/45456/messages?include_deleted=0&include_message_annotations=1&include_machine=1 <redacted - received unsent messages sent, refreshing adapter> D/MessageBeast_MessageManager(26676): Channel 45456; Successfully sent unsent message with id 2888945; replaced with message 2888946 D/MessageBeast_ActionMessageManager(26676): replaced action message spec; action message id 2888945 ---> 2888946

rrbrambley commented 10 years ago

There is really no reason to be using numerical ids for unsent messages any more. That was necessary when messages were pulled from the db by message id, but now that it's done by time, I shouldn't be doing this any more.

EDIT: The message table is using an external content table with docid as key, so it requires an int as the primary key. FML.