Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve Incorrect Update.userId Assignment #94

Closed
wants to merge 1 commit into from

Conversation

saiaapiz14
Copy link

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.

Referencing issue [celestix#85](celestix#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.
@celestix
Copy link
Owner

celestix commented Oct 7, 2024

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).

@saiaapiz14
Copy link
Author

saiaapiz14 commented Oct 7, 2024

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.

@celestix
Copy link
Owner

celestix commented Oct 9, 2024

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.

@saiaapiz14 saiaapiz14 closed this Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants