Allow running getUpdates without database connection

[jacklul]

This needs discussion whenever my approach works correctly in all cases.

Would love if you could test it out guys!

[noplanman]

@jacklul This only works for getUpdates in a loop, right? Not for a single getUpdates call.

Could you also add a changelog entry please!

[noplanman]

Would make sense to just save this value to $results outside the if, so that the foreach loop below can use it too.

Then you could use if ($last_result = end($results)) and ... = $last_result->getUpdateId();
(Array will then probably need a reset($results); to work with the foreach loop)

Question: Could this also just be moved into the loop and updated with each iteration, ending with the last one? Not sure about speed, was just thinking to cut the code a bit.

[jacklul]

As per API documentation:

An update is considered confirmed as soon as getUpdates is called with an offset higher than its update_id.

Calling it with value lower or equal 0 will fetch all unconfirmed updates, they will be marked as confirmed with next loop iteration so yeah, this won't work when getUpdates is called once - it will keep processing same updates over and over.

To prevent users from having issues with this I made it so that setting third optional parameter to true is required, otherwise old behaviour (returning error about no DB connection) is retained.

Will probably update getUpdatesCLI.php and add getUpdatesLoop.php to the example-bot repo later!

[noplanman]

Instead of adding a boolean parameter, it's better practice to implement this differently.
(Have a look here: https://softwareengineering.stackexchange.com/a/212100)

We could either do this with a handleGetUpdatesWithoutDatabase($limit = null, $timeout = null) method, or an extra property that can be set using a useGetUpdatesWithoutDatabase(bool $handle_get_updates_without_database = true) method.

The user obviously needs to be aware of the fact that it only works in a loop then!

---

Other option would be to just get the initial updates (1st request in the loop) to get the update ID to handle it, then remember the ID from there (similar to this). This would allow handleGetUpdates to work even outside a loop, but it will send off an extra request to confirm that the updates have been handled each time.
But that is then up to the user.
If we go this route, then we don't need the user to explicitly say if the database should be used, but we'd use it if it's available.

What do you think?

[jacklul]

Actually that extra request to mark them as read will only run once when in loop because after first loop we will already know the last update id so we can process new and mark previous updates at the same time!

I also noticed a problem here, if it's running in a loop and script is killed previous updates aren't marked as handled, so the next time script runs it reprocesses same updates again! So definitely running without DB can't be a default here. (the whole problem can be handled with signal handler)

[noplanman]

What is this for exactly? Why not just use the member variable directly?

[jacklul]

Good point, no idea why I put it there

[jacklul]

Oh wait! Probably to let user access last update id inside hook script, the purpose of that would be setting a signal handler (to prevent not marking updates as handled)
Couldn't find other way to access it...

        function signal_handler(Longman\TelegramBot\Telegram $telegram, $signo = null)
        {
            $last_update_id = $telegram->getLastUpdateId();
            if ($last_update_id > 0) {
                Longman\TelegramBot\Request::getUpdates(
                    [
                        'offset'  => $last_update_id + 1,
                        'limit'   => 1,
                    ]
                );
            }

            exit(PHP_EOL);
        }

[noplanman]

I didn't fully understand what you meant there. Where would this signal_handler method be called?
Also, the $signo parameter isn't being used, what is that?

[jacklul]

Okay, lets say the bot is heavily used and has to handle 40 updates at once, when bot crashes in the middle of processing they aren't marked as handled, next time script is run they are re-processed.

About $signo, for some reason that function refused to work without that parameter there... I tested it on linux subsystem since pcntl doesn't work on Windows.

Also that function is just an example, it can also be used to be called in catch block can also work after exceptions.

The point of that method is to returnid of the update that was processed (I should probably move the assignement in processUpdate somewhere after, will do later)

[noplanman]

Aaah, right! You're using the pcntl signal handling, thought it was just a custom function you call somewhere.

Ok. I'm just wondering if there is a cleaner way to do this. Also, we'll have to document this somehow with the extra code required to register the signal handler.

[jacklul]

Probably a wiki article mentioning and example code will suffice.

[noplanman]

@jacklul Check jacklul#12, then I'll merge this 👍

(sorry it took so long...)