gotgproto icon indicating copy to clipboard operation
gotgproto copied to clipboard

Resolve Incorrect Update.userId Assignment

Open saiaapiz opened this issue 1 year ago • 3 comments

Referencing issue #85.

The problem arises because Go maps are unordered, and getDifferent is called before fillUserIdFromMessage. This sequence can lead to an incorrect userId being assigned to Update.userId due to inconsistent entity processing. Ensuring proper order of operations will resolve this.

saiaapiz avatar Oct 07 '24 10:10 saiaapiz

Hello @saiaapiz14! First of all, thank you for your contribution! Now let's talk about the changes you made, the issue is get difference is being called before assigning userid because telegram doesn't send entities for private message updates and hence it's a necessity, we should rather find out a more reliable solution to this problem that solves both the needs (which is correct user id and entities being available for mapping).

celestix avatar Oct 07 '24 10:10 celestix

You’re welcome, and thank you for building such a great library!

To address the issue at hand: the root of the problem is that getDifference is called before assigning userId, particularly because Telegram doesn't send entities for private message updates, making it crucial to handle this sequence properly. Here's a streamlined solution that fulfills both needs—assigning the correct userId and ensuring entities are properly mapped:

Solution:

  1. Assign userId Early:

    • First, use fillUserIdFromMessage(selfUserId) to attempt to set userId immediately after constructing EffectiveMessage. This ensures that any available data is used to capture the user information directly.
  2. Fallback Mechanism:

    • If userId cannot be assigned initially (e.g., for private messages where FromID may be nil), we iterate through value.NewMessages from client.UpdatesGetDifference.
    • Specifically, check whether FromID is nil. If so, use PeerID to identify the user.

Updated Code:

case *tg.UpdateNewMessage:
    m := update.GetMessage()
    u.EffectiveMessage = types.ConstructMessage(m)
    u.fillUserIdFromMessage(selfUserId)  // Try to assign userId first

    diff, err := client.UpdatesGetDifference(ctx, &tg.UpdatesGetDifferenceRequest{
        Pts:  update.Pts - 1,
        Date: int(time.Now().Unix()),
    })

    if err == nil {
        if value, ok := diff.(*tg.UpdatesDifference); ok {
            // Fallback to assign userId if necessary
            for _, nmsg := range value.NewMessages {
                if msg, ok := nmsg.(*tg.Message); ok {
                    var effectiveUser tg.PeerClass
                    if msg.FromID == nil {
                        effectiveUser = msg.PeerID
                    } else {
                        effectiveUser = msg.FromID
                    }

                    if peerUser, ok := effectiveUser.(*tg.PeerUser); ok {
                        u.userId = peerUser.UserID
                        break
                    }
                }
            }

            // Continue mapping entities
            for _, vu := range value.Chats {
                switch chat := vu.(type) {
                case *tg.Chat:
                    p.AddPeer(chat.ID, storage.DefaultAccessHash, storage.TypeChat, storage.DefaultUsername)
                    e.Chats[chat.ID] = chat
                case *tg.Channel:
                    p.AddPeer(chat.ID, chat.AccessHash, storage.TypeChannel, chat.Username)
                    e.Channels[chat.ID] = chat
                }
            }
            for _, vu := range value.Users {
                if user, ok := vu.AsNotEmpty(); ok {
                    p.AddPeer(user.ID, user.AccessHash, storage.TypeUser, user.Username)
                    e.Users[user.ID] = user
                }
            }
        }
    }

Benefits:

  • Reliable userId Assignment: By setting userId as early as possible and falling back if necessary, this approach ensures that the correct userId is consistently assigned.

Thank you again for such a great library and for the opportunity to collaborate on this solution.

saiaapiz avatar Oct 07 '24 14:10 saiaapiz

Hi @saiaapiz14, sorry for the late reply :/

Can you test this new approach for PMs? Please consider testing for a bot account a user account both.

celestix avatar Oct 09 '24 05:10 celestix