Fix behavior keepalive - Revert

[Kubik-Rubik]

This is an improvement (some kind of revert) of the PR that I've merged yesterday. After a discussion with @wilsonge and @mbabker we decided to restore some parts of the code. There is no need to refresh the session independent of the set session time. In the previous PR the refresh period was set to a fixed value (45 seconds), so if the session time was set ,for example, to 30 minutes there would be a lot of unnecessary requests.

Now the refresh time is always one minute less than the session time except if the session time is set to one minute, then the fresh time is 45 seconds.

Another important change is to use root instead of base for the request URL because the request created a 500 Internal Error on the login page of the administrator area. I've just found this bug while testing my own PR...

How to test

Since we restore the functionality to the default behavior, you won't see a "before / after" change. But you can test the keepalive behavior in general (here backend, also applies to the frontend):

1. Set the session time to 1 minute in the global configuration.
2. Open the console of the browser, then the tab "Network"
3. Open an existing article in the edit mode or click on the button "New"
4. Wait for 45 seconds, then you should see an Ajax request to the server which refreshes the session
5. Do the same with another time value (e.g. 5 minutes), then the refresh request will be triggered one minute before the ssessions ends.

@wilsonge or @mbabker Please check and commit the PR. Thanks!

[wilsonge]

Can the refresh time ever be less than 0 now we have the check on the life time being less than 60000 above?

[Kubik-Rubik]

It could be 0 but not less in the current constellation (if you set the value to 0 in the configuration, then the session time is set to 15 min internally). I just took this check from code that we had before I've merged the PR. So, this should be fine.

[Fedik]

then what about that issue #6730 ?

... use root instead of base ...

but this means that session will be refreshed only for the Site, and not for the Administrator if I login to Administrator, or I wrong?

[Kubik-Rubik]

@Fedik Valid point, you are right. The request has to be executed to the backend to refresh the correct session. To avoid the 500 Internal Error we could just call the index.php instead of the Ajax component. What do you think?

[Fedik]

hm, idea was to reduce the load ...
When you do request to index.php then Joomla render whole page, with component and all modules on that page ... that is not efficient, especially if that page is "heavy"
And when you do request to Ajax component, then Joomla actually do nothing ...

upd:
I just checked and "500 Internal Error" only when you on Administrator login page (less important, but yes, would be good to fix), and after login it works well (more important) 😄

[wilsonge]

If the session time is set to something large (e.g. 1 hour) and it's now every 45 seconds you haven't really reduced load :P

[Fedik]

why not? when you do request to Ajax component, then Joomla render nothing, and do not do request to DB for load "Latest Article", "Popular Article" and so on

[Fedik]

but about that #6730 issue,
if you want to do request depend from session length in the configuration, then maybe compromise will be: do this only when Session handler is Database, and in other cases do request in fixed time, eg 1-2 minutes

[Kubik-Rubik]

So, we need 2 changes:

1. Get around the 500 error. For the frontend the Ajax request is great (to reduce the load), for the backend we need a workaround (maybe just call /index.php?).
2. Fixed time slot for non-database handlers like the option "None" (PHP session lifetime) because this value can not be influenced by Joomla! but is a server setting.

Right? @Fedik @wilsonge

[wilsonge]

Yup

[Fedik]

yes, from my point of view 😉
but I do not like request to /index.php, because in case when handler not "Database", then it really will make unnecessary load 😄

about error 500 (View not found [name, type, prefix]: login, json, loginView), what can reason that Joomla tries search for "view" when User on login form, and do not do this when user logged in? not very understand

[Kubik-Rubik]

@Fedik The "problem" is caused in administrator/includes/helper.php - function findOption. If the user is not logged in, we set the option variable to com_login which leads to this error. We could add an exception for the Ajax component but we would have to examine whether such a change would not produce side-effects.

[Fedik]

then maybe in this case more simple and safe is to do workaround for keepalive, than change administrator/includes/helper.php behavior
maybe:

if($app->isAdmin() && !JFactory::getUser()->get('id'))
{
  $url = JUri::base(true) . '/index.php';
}
else
{
  $url = JUri::base(true) . '/index.php?option=com_ajax&format=json';
}

or something like that...
or too tricky? 😉

[Kubik-Rubik]

Okay, I think we can live with the request to the index.php in the backend for now and later optimize it further. I will update the PR soon!

[Kubik-Rubik]

I think that I've found the perfect solution! :-)

If the handler is not "Database", then the refresh time is set to 5 minutes. Usually the PHP session timeout is set to 24 minutes, so 5 minutes should be safe enough. For the database handler we have the normal behavior depending on the session lifetime.

We also always use the Ajax component to reduce the load except on the login page of the backend. Only on this login page we call the index.php to avoid the 500 Error.

Please test, thanks! @Fedik @wilsonge

[mbabker]

This diff will fix the test failure:

diff --git a/tests/unit/suites/libraries/cms/html/JHtmlBehaviorTest.php b/tests/unit/suites/libraries/cms/html/JHtmlBehaviorTest.php
index 987b926..31e2342 100644
--- a/tests/unit/suites/libraries/cms/html/JHtmlBehaviorTest.php
+++ b/tests/unit/suites/libraries/cms/html/JHtmlBehaviorTest.php
@@ -630,17 +630,10 @@ class JHtmlBehaviorTest extends TestCase
        // We generate a random template name so that we don't collide or hit anything//
        $template = 'mytemplate' . rand(1, 10000);

-       // We create a stub (not a mock because we don't enforce whether it is called or not)
-       // to return a value from getTemplate
-       $mock = $this->getMock('myMockObject', array('getTemplate'));
-       $mock->expects($this->any())
+       // We create a stub (not a mock because we don't enforce whether it is called or not) to return a value from getTemplate
+       JFactory::$application->expects($this->any())
            ->method('getTemplate')
-           ->will($this->returnValue($template));
-
-       // @todo We need to mock this.
-       $mock->input = new JInput;
-
-       JFactory::$application = $mock;
+           ->willReturn($template);

        JHtmlBehaviorInspector::keepalive();
        $this->assertEquals(

[Kubik-Rubik]

@mbabker Thank you, Michael. I will update the test method.

[Fedik]

@Kubik-Rubik thanks! works good here
tried with handler: Database, APC (that is APCu in php 5.5), and None (that is File)

[jwaisner]

@test

PR works great. tested with all handlers.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7363.}

[zero-24]

RTC 😄

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7363.}