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

How to call fun removeHandler(handler: Handler) correctly #262

Open
lifestreamy opened this issue Oct 10, 2022 · 7 comments
Open

How to call fun removeHandler(handler: Handler) correctly #262

lifestreamy opened this issue Oct 10, 2022 · 7 comments

Comments

@lifestreamy
Copy link

Say, for example, I have many nested callback queries, and they are added to bot handlers as they are created

command("enterdata") {
                        val data = mutableListOf<String>()
                        bot.sendMessage(
                            ChatId.fromId(update.message!!.chat.id),
                            text = "asdsad",
                            replyMarkup = InlineKeyboardMarkup.create(
                                listOf(InlineKeyboardButton.CallbackData(text = "data1", callbackData = "data1"))
                            )
                        )
                        var callbackText1 = "data1"
                        var callbackText2 = "data2"
                        var callbackText3 = "data3"
                        callbackQuery(callbackText1) callback1@{
                            val chatId1 = callbackQuery.message?.chat?.id ?: return@callback1
                            if (callbackQuery.data == callbackText1) {
                                bot.sendMessage(
                                    chatId = ChatId.fromId(chatId1),
                                    text = "received cb data1",
                                    replyMarkup = InlineKeyboardMarkup.create(
                                        listOf(
                                            InlineKeyboardButton.CallbackData(
                                                text = "data2",
                                                callbackData = callbackText2
                                            )
                                        )
                                    )
                                )
                            }
//                            callbackText1 = null.toString()
                            callbackQuery(callbackText2) callback2@{
                                val chatId2 = callbackQuery.message?.chat?.id ?: return@callback2
                                if (callbackQuery.data == callbackText2) {
                                    bot.sendMessage(
                                        chatId = ChatId.fromId(chatId2),
                                        text = "received cb data2",
                                        replyMarkup = InlineKeyboardMarkup.create(
                                            listOf(
                                                InlineKeyboardButton.CallbackData(
                                                    text = "data3",
                                                    callbackData = callbackText3
                                                )
                                            )
                                        )
                                    )
                                }
//                                callbackText2 = null.toString()
                                callbackQuery(callbackText3) callback3@{
                                    val chatId3 = callbackQuery.message?.chat?.id ?: return@callback3
                                    if (callbackQuery.data == callbackText3) {
                                        bot.sendMessage(
                                            chatId = ChatId.fromId(chatId3),
                                            text = "received cb data3"
                                        )
                                    }
//                                    callbackText3 = null.toString()
                                }
                            }
                        }
                    }

In this example, as you can see, the callbackQuery handlers are sequentially created and thus added to Dispatcher. You can't get a response from callback3@ until you had a response from callback2@ and the handler for callback3@ is created in it.

So, I figured, this way I could add a way to communicate back and forth with the bot.

For example, it asks you 3 questions in order, waiting for your answer after each one.

But after the handlers have been added, you callback queries can be executed from anywhere not bothering with the requirement of using an initial command or their order of creation

As you can see, I have commented out 3 lines here, that I used to fix this problem, so that you could only have your callbackQueries bodies execute in a strict order, first the command should be executed for the callback1@ to become available, and then the callback1@ should be executed for the callback2@ to become available

So if I add flags like
var callbackText1 = "data1"
var callbackText2 = "data2"
var callbackText3 = "data3"
and un-comment those 3 lines I have mentioned earlier, the queries can be called only in order, and no excess ones are created, as far as I understood

But for me this looks like a crutch, so I wondered if I could use removeHandler() in the callbackQuery after its execution.
So e.g. when callback1@ is executed, the callback2@ is created and callback1@ is immediately removed from handlers.

It would make so much more sense, but I have no idea what to pass as an argument to removeHandler(), and there is no documentation regarding this matter

So is what I am doing the only way to achieve a back and forth communication or I could use removeHandler(), but again, how, then?

@lifestreamy
Copy link
Author

I have just tried, and if I create a button that sends a null.toString() callbackQuery, then A LOT of callbackQueries are executed
Just like in this person's case #257

@lifestreamy
Copy link
Author

Also, possibly the concept of a mutable set in Dispatcher at
private val commandHandlers = linkedSetOf()
is broken and it keeps adding old handlers with the same values as a new ones.

Simple addition of an equals method would probably resolve this

@lifestreamy
Copy link
Author

@seik

@vjgarciag96
Copy link
Member

vjgarciag96 commented Oct 18, 2022

So, you could get around this by adding an identity to the underlying handler as you suggested in one of the issues. You can do this without the need of changing the library (of course, it would be nicer if the library provides a built-in solution for this), so if you want to get around this before we get an impl. for this in the library and publish it, you can use something like the next:

fun Dispatcher.callbackQuery(id: String, handleCallbackQuery: HandleCallbackQuery) {
    addHandler(IdentifiableHandler(id, CallbackQueryHandler(id, handleCallbackQuery)))
}

class IdentifiableHandler(
    private val id: String,
    private val delegate: Handler,
) : Handler by delegate {

    override fun equals(other: Any?): Boolean {
        if (this === other) {
            return true
        }
        if (javaClass != other?.javaClass) {
            return false
        }

        other as IdentifiableHandler

        if (id != other.id) {
            return false
        }

        return true
    }

    override fun hashCode(): Int {
        return id.hashCode()
    }

    companion object {
        fun cacheBuster(id: String): Handler {
            return IdentifiableHandler(id = id, delegate = NoOpHandler)
        }

        private object NoOpHandler: Handler {
            override fun checkUpdate(update: Update): Boolean = false
            override fun handleUpdate(bot: Bot, update: Update) = Unit
        }
    }
}

class CallbackQueryHandler(
    private val callbackData: String,
    private val handleCallbackQuery: HandleCallbackQuery,
) : Handler {

    override fun checkUpdate(update: Update): Boolean {
        return update.callbackQuery?.data?.contains(callbackData, ignoreCase = true) ?: false
    }

    override fun handleUpdate(bot: Bot, update: Update) {
        val callbackQuery = update.callbackQuery
        checkNotNull(callbackQuery)

        val callbackQueryHandlerEnv = CallbackQueryHandlerEnvironment(
            bot,
            update,
            callbackQuery
        )
        handleCallbackQuery(callbackQueryHandlerEnv)

        bot.answerCallbackQuery(callbackQueryId = callbackQuery.id)
    }
}

Then you could add and remove callback query handlers as you want by using the next:

callbackQuery(id = "callback1") {
    removeHandler(IdentifiableHandler.cacheBuster(id = "callback1"))
}

Does this make sense to you? @lifestreamy

@lifestreamy
Copy link
Author

lifestreamy commented Oct 19, 2022

No, it doesn't, it seems overly complicated. Why not just change the library to assign an id to every handler upon creation?

In my fork, I've changed commandHandlers to mutableMap with id's as keys, and added handlerName as a parameter to all handler upon creation.
And it is perfectly useful. No bugs encountered. Little to no coding for the actual project required.

In your example I should write hundreds of lines, whereas in mine I only pass an additional argument to handlers like message("message1"){}
and I can call removeHandler("message1") from anywhere.
I can even omit passing it and
call message{} as usual and the code will still give no errors.

The library should help get the job done, not cause a waste of time trying to understand how to workaround this and this, and how developers intended it to work.

Since there is little to no documentation it is hard to even get a sense of what is happening inside, so I have spent like 40h in total understanding how this all works, encountering obstacles in half of the steps that it took to create my project.

And additional like 10h trying to invent a crutch for removing handlers.

My point is, once again:
I want to spend 1-2h getting used to how library works and 100h on the actual project, not 40 and 60 respectively.

@lifestreamy
Copy link
Author

So I will resort to using my fork for now.

@lifestreamy
Copy link
Author

Also, thank you for your answer @vjgarciag96
I realize it might have taken you some time to write.

But it is just not the road I wish to go.

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

No branches or pull requests

2 participants