IISC this doesn't handle inline keyboards attached to inline result messages yet. If we revisit this PR, that's one issue to think about

We just discussed this and will be going forward with it. A few notes:

• Switch to UUID instead of id()
• built the data storage as LRU cache limiting memory usage
  • store the timestamp along with the timestamp so if the user passes maxsize=None they can take care of deleting based on time stamps on their own.
  • Still provide CallbackQuery/Context/Bot.drop_callback_data for fine grained control

sketch for a LRU data class:

class LRUDict:
    data = {}
    maxsize = 1000

def put(self, key, object):
    if len(self.data) == maxsize:
        self.data.pop(next(iter(self.data)))  # dicts are only guaranteed to be ordered since py3.7, so maybe use OrderedDict instead …
    self.data[key] = object

Edit: Couple with linked list to make efficient

@Bibo-Joshi Ordered dict is not LRU.
Ordered dict (in python semantics) refers to the order of insertion.
So it's either to remove and insert again for every object we use or we use a proper LRU.
A proper LRU will be implemented so that every access to a key will cause it to be marked as recently used. This is done internally by keeping a doubly linked list which has the most used entries in the head and least used entries in the tail.

Though, writing this, I think that if the intention is to immediately remove items when they're accessed then an Ordered Dict would be enough. (Though it might be limiting if we change the design)

Merging master was surprisingly easy :D
Adapted the LRU idea. Namings can ofc be changed.
Tests run fine locally, let's see how they perform in CI.

After internal discussion, using the uuid should be enough, so the latest commit removes the signing procedure.
However I became aware of two flaws in the current setup:

• the data in the keyboard attached to the incoming CallbackQuery is currently not replaced
• pressing a button does not automatically mean that the keyboard will be deleted. The bot might just call answer_callback_query(text='hint') to display some info, without changing the message or the keyboard. So we musn't delete the data on incoming CallbackQueries. This means that
  1. CallbackDataCache needs to be properly converted to a LRU-cache by updating the order of the items in the doubly linked queue
  2. We should provide convenience methods to delete the objects corresponding to the incoming query or the complete attached keyboard. Probably samething like CallbackQuery.{clear_data, clear_keyboard_data}, InlineKeyboardMarkup.clear_data, InlineKeyoardButton.clear_data() and Message.clear_keyboard_data

Edit I: We could think of making Message.edit_text/reply_markup/… and CallbackQuery.edit_message_text/reply_markup/… (i.e. everything that drops the keyboard) drop the data automatically 🤔

Edit II: just skipping CQs with invalid data is a bad idea. Users might wanna reply with a prompt telling them that the button no longer works. Rather set the data attribute to the InvalidCallbackData exception or something like that.

Edit III: Inline messages. We don't get the keyboard back when buttons from those are pressed. Idea: instead of making the LRU cache per button, make it per keyboard. we have space for 2 uuids in 64chars anyway. So once we get a CQ, we know all the other buttons/objects that we sent along with it and can make use of Edit I. Ofc when answering an inline query with multiple results that have an inline keyboard, we don't know which one actually gets sent, but that's what the LRU cache is good for.

Thanks for the thorough review @harshil21, you found a lot of things to improve 😃 So far I've gone through your comments and marked all as resolved that where either clear or where you reacted with 👍 to my comments. I will get to the TODO section soonish. Regarding the other comments:

There's also TTLCache from cachetools which would mean that we wouldn't need the access_time logic, as we can just use their implementation. Maybe that can help simplifying the code?

Unfortunately this doesn't allow for the entries not to expire :/

Regarding slots, should we make a new branch based on this and then merge here? The implementation detail should be kept separate from this imo. This PR is big enough already

Upsie, read that too late :D already merged that

Finally I think an example for this would help users

stay tuned 🎶

I added an example that can be reviewed.
Also I just cherry-picked the stuff from #2262 (comment) + made according adjustments here, so all the open todo items in the description are "clean-up" after merge.
Only thing left from my side is to try & up the coverage a bit when the latest tests are finished

Okay example looks good. However, I noticed one thing while running the example - When a user spam clicks a button (say '1'), it becomes an instance of InvalidCallbackData, and it fails. Of course when you remove the drop_callback_data line, this doesn't happen anymore. But maybe such behaviour should be at least documented somewhere?

Ah, yes, that's right. In fact it could be considered a feature, see #2539 :D Anyway, I've updated the wiki above as follows:

However, if you want to keep memory usage low, you have
additional options to drop data:

• on receiving a CallbackQuery, you can call context.drop_callback_data(callback_query). This will delete the data associated with the keyboard attached to the message that originated the CallbackQuery. Calling context.drop_callback_data is safe in any case where you change the keyboard, i.e. callback_query.edit_message_text/reply_markup/…
  Note: If the user clicks a button more than one time fast enough, but you call context.drop_callback_data for the first resulting CallbackQuery, the second one will have InvalidCallbackData as callback_data. However, this is usually not a problem, because one only wants one button click anyway.

Note that I also scraped the sentence about drop_callback_data not working for inline messages, b/c that's BS (commit incoming) :D

Also can we remove the cast() in the example and ask mypy to ignore them? makes it a little complicated for beginners.

I'm not too sure about this, tbh. If we drop cast, we'll have to add a type: ignore, which probably also confusing for beginners. I'd like to keep at least the cast in

# Get the data from the callback_data.
    number, number_list = cast(Tuple[int, List[int]], query.data)

because if you use type hinting, you'll have to do that cast in any case when using arbitrary callback data, so I think the example should showcase it. I could live with dropping the other two casts, though …. (edit: I did)