Added a cron job to clean expired API sessions

[kiatng]

Description (*)

The api session expiry is managed in the table api_session. However, the old sessions are only deleted during login for that user only, line 268:

magento-lts/app/code/core/Mage/Api/Model/User.php

Lines 263 to 270 in 2a2a2fb

	public function login($username, $apiKey)
	{
	$sessId = $this->getSessid();
	if ($this->authenticate($username, $apiKey)) {
	$this->setSessid($sessId);
	$this->getResource()->cleanOldSessions($this)
	->recordLogin($this)
	->recordSession($this);

If the client does not call endSession() and if the client doesn't login again, old sessions will accumulate in the table. In production, I have entries as far back as 2013.

This PR added a daily cron to remove all old sessions.

Related Pull Requests

PR #3477

Fixed Issues (if relevant)

Together with #3477, this PR fixed issue #3449

[kiatng]

Failed CS-Fixer due to misaligned comment. Does it fix automatically or do I need to make another commit to fix it?

[fballiano]

@kiatng it doesn't fix it automatically

[kiatng]

This PR is actually not very useful. The old sessions in the table do not cause any problem. I am considering closing it. What do you think?

[fballiano]

well I think we shouldn't have leftover records forever so I think it's kinda useful :-)

I'll retest it later

[kiatng]

I don't like the idea of another cron running everyday just to clean entries that may or may not be there. Especially with insta-login, the table will be empty. Off-topic, I cannot get the insta-login working in SOAP.

[fballiano]

well, at the end of the day if there are no records to clean it will run for 0.0 seconds ;-)

[elidrissidev]

Maybe running it everyday is an overkill. How about increasing it to 1 week or month?

[elidrissidev]

Off-topic, I cannot get the insta-login working in SOAP.

Did you try using the SOAP V1 way with the call method?

[fballiano]

I prefer it to stay daily because it will have to process fewer records, almost zero, but if some store has many sessions then it won't happen that the cron will have to process thousands of records

[kiatng]

Off-topic, I cannot get the insta-login working in SOAP.

Did you try using the SOAP V1 way with the call method?

Yes, I tried SOAP v1. insta-login uses HTTP basic auth, the error I got was related to getting the WSDL. But it's not important for me as JSON-RPC is now in production and is so much faster.

[empiricompany]

From my experience with different clients that make extensive use of APIs, the number of records involved is always low, but the primary goal is to ensure a clean database, not just to save memory.

It's just an idea, but we should consider merging it into a single cron job along with other future tasks for cleaning and maintaining the database, in order to avoid occupying a dedicated cron time slot?
I'm thinking of other actions to clean up obsolete data like this: #3270

[fballiano]

I still prefer to have small cron jobs because they run quickly and they're easier to debug in case something gets stuck. also, internal cron jobs run only one at a time, so if you have a long running one, you're blocking all of the other from running and that's something I surely prefer to avoid as much as possible

[fballiano]

tested, working (the code is actually the same at before so ;-)

[kiatng]

Great discussion. On @empiricompany idea of merging crons into one, problem is that modules can be disabled, so we can't merge different jobs in different modules.