Slyce-Inc / SlyceMessaging

A messaging library for Android
MIT License
968 stars 173 forks source link

Double outgoing messages #30

Open atroutt opened 7 years ago

atroutt commented 7 years ago

I hooked up my SlyceMessaging UI to a Firebase backend and I'm seeing the issue when I send a message there are two showing up on the screen. It's because the library adds the first copy when the message is sent and then Firebase notifies me of the new message coming back from the server and I display that as well.

What is the recommended pattern for avoiding this?

I don't want to simply ignore messages from the local user coming from the server because then I won't be able to display the history correctly (messages I sent in previous sessions).

MatthewTPage commented 7 years ago

So the way I handled this was actually having different logic for populating the message history (where you accept all messages) versus listening for new messages from the server (where you accept all messages that are not from the local user).

The only other workaround I can think of is not having the library display the messages as the user sends them. The reason I chose not to do this when I created the library was so that if the user had a slow connection to the network, they wouldn't have a long delay between sending a message and viewing it. This seems to be how most other messaging applications handle it. Though I'm open to dissenting opinions.

Shudy commented 7 years ago

HI! I think I'm getting same problem as @atroutt .

I have a Realm DB in the app. What I do is, recieve the message in a listener add it to db. Then when I open the chat. So the first I do is call a method (loadHistory) to show the last N messages. Then I do something like private void loadHistory(String contactJid, RealmResults historyMessages) {

    historyMessages.sort("date");
    List<Message> messageHistory = new ArrayList<>();
    for (int i = 0; i < historyMessages.size() && i < 21; i++) {

        es.in2.in2tant.realm.Model.Message mess = historyMessages.get(i);
        if(!mess.isDelivered()) {
            realm = Realm.getDefaultInstance();
            realm.beginTransaction();
            mess.setDelivered(true);
            realm.commitTransaction();
            realm.close();
        }

        if (mess.getMimeType().equals(ChatMessage.MediaType.IMAGE.toString())) {
            MediaMessage mediaMessage = new MediaMessage();
            mediaMessage.setUrl(mess.getBodyMessage());

            if (mess.getChatID().equals(contactJid))
                mediaMessage.setSource(MessageSource.LOCAL_USER);
            else
                mediaMessage.setSource(MessageSource.EXTERNAL_USER);

            mediaMessage.setDisplayName(mConnection.getUser().getLocalpart().toString());
            mediaMessage.setUserId(mConnection.getUser().toString());
            mediaMessage.setInitials(mConnection.getUser().getLocalpart().toString().substring(0, 1).toUpperCase());
            slyceMessagingFragment.addNewMessage(mediaMessage);

        } else {

            TextMessage textMessage =  new TextMessage();
            if(mess.getFrom().equals(mConnection.getUser().toString())){ //LOCAL USER
                textMessage.setSource(MessageSource.LOCAL_USER);
            }else{ //EXTERNAL USER
                textMessage.setSource(MessageSource.EXTERNAL_USER);
            }
            textMessage.setUserId(mess.getFrom());
            textMessage.setInitials(mess.getFrom().substring(0,1).toUpperCase());
            textMessage.setText(mess.getBodyMessage());
            textMessage.setDate(mess.getDate().getTime());
            slyceMessagingFragment.addNewMessage(textMessage);
        }

}

Since here, it works OK. BUT, the last message is duplicated. Then, when I write somthing , aumatically refresh the layout and the last message turns to the right one.

P.S: Great job with the library ,)

ghost commented 7 years ago

I worked around this the way you suggested @MatthewPageCS --I set up a one-time sync with the servers on creation of the chat screen and only after that completes I set up a listener that still receives all updates from the server, but does not pass along outgoing messages to the UI (since they are already displayed by the library).

Example for Android with Firebase:

        // create listener for ongoing messages, to be used after first sync is done
        final ChildEventListener messageListener = new ChildEventListener() {
            @Override
            public void onChildAdded(DataSnapshot dataSnapshot, String s) {
                TextMessage textMessage = getTextMessage(dataSnapshot);
                /// ignore outgoing messages because they will already be displayed by the library
                if (textMessage.getSource() == MessageSource.EXTERNAL_USER) {
                    slyceMessagingFragment.addNewMessage(textMessage);
                }
            }

            // ...
        };

        // First time in, load everything
        mFirebaseDatabaseReference.child(MESSAGES_CHILD).addListenerForSingleValueEvent(new ValueEventListener() {
            @Override
            public void onDataChange(DataSnapshot dataSnapshot) {
                final Iterable<DataSnapshot> children = dataSnapshot.getChildren();
                if (children != null) {
                    for (DataSnapshot message: children) {
                        TextMessage textMessage = getTextMessage(message);
                        if (textMessage != null) {
                            // display all the messages, regardless of source
                            slyceMessagingFragment.addNewMessage(textMessage);
                        }
                    }
                }
                // then add the ongoing message listener
mFirebaseDatabaseReference.child(MESSAGES_CHILD).addChildEventListener(messageListener);
            }

            // ...
        });
ghost commented 7 years ago

@hasaudrey But this causes double incoming messages. The ValueEventListener loads all messages at once for the first time and then the ChildEventListener also adds the same incoming messages again. Is there any workaround to this? One way is to somehow disableSlyce from adding the message to the SlyceMessagingFragment, but as @MatthewPageCS mentioned, that is definitely a UX flaw. Any Suggestions?

FrescoFlacko commented 7 years ago

I managed to find a temporary workaround to this.

I created an ArrayList of messages and for each message I get from the server, I add it onto the ArrayList and I did slyceMessagingFragment.replaceMessages(messages);. That way, it will always replace the messages currently displaying and will potentially replace the duplicate message.

That's the only workaround in case anybody else runs into this situation. Any other suggestions are welcome!