status-im / status-protocol-go

Status Protocol implementation in Go
Mozilla Public License 2.0
0 stars 1 forks source link

Move messages in status-protocol-go #5

Closed cammellos closed 5 years ago

cammellos commented 5 years ago

As a developer I want to have messages stored in status-go-protocol So I don't have to re-implement it myself

Description

This task involves replacing the persistence layer from status-react to this library.

It needs to fullfil the current API that realm offers https://github.com/status-im/status-react/blob/develop/src/status_im/data_store/messages.cljs , or equivalent.

It does not involve porting the sorting logic as that's not strictly required for v1 and best done in a separate task.

I would also not have messages processed in this layer in the scope of this task, as that's potentially a much larger task and not strictly necessary for v1.

Depends on #2

Acceptance criteria

StatusWrike commented 5 years ago

➤ Wrike Bot commented:

Adam BabikAndrea Piana All the necessary task dependencies have been completed, so you can start working on it right away.

adambabik commented 5 years ago

@cammellos is it enough to store transit-encoded messages?

I identified the following interface:

type RawMessagesPersistance interface {
    ByID(id []byte) (Message, error)
    FromUser(from *ecdsa.PublicKey) ([]Message, error)
    UnviewedIDs() ([][]byte, error)
    Save(Message) error
    DeleteByID(id []byte) error
    DeleteByChatID(chatID string) error
    Exists(id []byte) (bool, error)
    MarkSeen(ids ...[]byte) error
    UpdateOutgoingStatus(id []byte, status OutgoingStatus) error
}

Message is the stored record. Am I missing any other fields?

type Message struct {
    ID []byte
    Content []byte
    From *ecdsa.PublicKey
    ChatID string
    OutgoingStatus OutgoingStatus
    Seen bool
}

Regarding the sorting, I believe we need to have it implemented at least in some basic form. For example, we don't want to return all user's messages but the latest and limit the number of items. In theory, we can rely on the order of inserts but that might result in some unexpected events.

cammellos commented 5 years ago

The fields we currently use are:

:message-id       :string
:old-message-id :string
:raw-payload-hash :string
:whisper-timestamp :int
:from             :string
 :to               {:type     :string  :optional true}
 :content          :string 
 :content-type     :string
 :username         {:type     :string  :optional true}
 :timestamp        :int
 :chat-id        :string
 :retry-count      {:type    :int  :default 0}
 :message-type     {:type     :string  :optional true}
 :message-status   {:type     :string :optional true}
 :clock-value      {:type    :int :default 0}
 :show?            {:type    :bool :default true}
:seen  {:type :bool :default false}
:outgoing-status {:type :string :optional true}

Some of these are likely not needed anymore, but I would start with like for like and err on the side of caution for now. Then we can remove them once we validate everything else works.

In terms of methods looks good, the only ones I don't see is:

ByChatID(chat-id string, cursor string) ([]Messages, cursor string/[]byte, hasMore bool) ByOldID(id string) // This one we can probably ditch, we can leave it out in the first implementation and then we can check if it's easy to remove

In terms of sorting, we should return messages ordered by a field:

${fixed-length-clock-value}-${message-id}

which we will store in the db (it's a cursor, so I would just call it cursor).

ByChatID should take as an input a chatid, and a cursor (or an empty string in case there's no cursor) and do:

SELECT * FROM messages WHERE cursor < ? LIMIT X + 1 (X can be passed it potentially, the +1 is only if you'd like to pass back a has-more flag, the 21st message is discarded if present and has-more is set to true ), and we need to returns the messages, a cursor to be used in the next request (i.e the cursor of the last message fetched), and ideally a has-more flag (if there's no has-more flag clients might have to make an extra call that returns an empty page, not a big deal, but it makes a nicer api, similarly for cursor, the client can look at the field cursor on each message, but I think it's nicer if we provide it, then we can strip it from the json)

Let me know if it's sounds resonable/question/concerns

adambabik commented 5 years ago

@cammellos do we wait for anything more to happen here?

cammellos commented 5 years ago

@adambabik I will keep it open for now, as it will possibly requires a few changes in order to integrate messages, and I'd rather track it here then in status-react, but don't worries I will take care of it