? ? Success
Referenced as Pull Request for: # 10286

User tests: Successful: Unsuccessful:

avatar Kubik-Rubik
Kubik-Rubik
7 Jul 2015

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!

avatar Kubik-Rubik Kubik-Rubik - open - 7 Jul 2015
avatar Kubik-Rubik Kubik-Rubik - change - 7 Jul 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 7 Jul 2015
Labels Added: ?
avatar Fedik
Fedik - comment - 7 Jul 2015

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?

avatar Kubik-Rubik
Kubik-Rubik - comment - 7 Jul 2015

@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?

avatar Fedik
Fedik - comment - 7 Jul 2015

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) :smile:

avatar wilsonge
wilsonge - comment - 7 Jul 2015

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

avatar Fedik
Fedik - comment - 7 Jul 2015

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

avatar Fedik
Fedik - comment - 7 Jul 2015

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

avatar Kubik-Rubik
Kubik-Rubik - comment - 7 Jul 2015

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

avatar wilsonge
wilsonge - comment - 7 Jul 2015

Yup

avatar Fedik
Fedik - comment - 7 Jul 2015

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

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

avatar Kubik-Rubik
Kubik-Rubik - comment - 7 Jul 2015

@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.

avatar Fedik
Fedik - comment - 7 Jul 2015

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? :wink:

avatar zero-24 zero-24 - change - 7 Jul 2015
Category Administration
avatar Kubik-Rubik
Kubik-Rubik - comment - 7 Jul 2015

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!

avatar Kubik-Rubik
Kubik-Rubik - comment - 7 Jul 2015

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

avatar mbabker
mbabker - comment - 7 Jul 2015

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(
avatar Kubik-Rubik
Kubik-Rubik - comment - 7 Jul 2015

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

avatar Fedik Fedik - test_item - 7 Jul 2015 - Tested successfully
avatar Fedik
Fedik - comment - 7 Jul 2015

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

avatar Kubik-Rubik Kubik-Rubik - change - 7 Jul 2015
Milestone Added:
avatar jwaisner
jwaisner - comment - 8 Jul 2015

@test

PR works great. tested with all handlers.


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

avatar jwaisner jwaisner - test_item - 8 Jul 2015 - Tested successfully
avatar zero-24 zero-24 - change - 9 Jul 2015
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 9 Jul 2015

RTC :smile:


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

avatar zero-24 zero-24 - change - 9 Jul 2015
Labels Added: ?
avatar zero-24 zero-24 - close - 9 Jul 2015
avatar roland-d roland-d - change - 9 Jul 2015
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2015-07-09 18:47:59
Closed_By roland-d
avatar roland-d roland-d - close - 9 Jul 2015
avatar zero-24 zero-24 - change - 14 Oct 2015
Labels Removed: ?
avatar joomla-cms-bot joomla-cms-bot - change - 14 Oct 2015
Labels Added: ?

Add a Comment

Login with GitHub to post a comment