Caching of list view unable to use paigination

Twincarb commented

2018-03-01 16:25:40 +00:00

(Migrated from github.com)

Steps to reproduce the issue

Component with one site list view which has pagination, global caching is set to On

Expected result

Pagination links should work with caching set to on

Actual result

First page is loaded into cache and unable to open the next pagination page in the pagination links.

System information (as much as possible)

OS Name & Version: Centos 7
MySql Version:
Apache Version: 2.4
PHP Version: 7.0
Joomla Version: 3.8.5
JCB Version: staging
Browser: chrome

Additional comments

I can see that the default is set below in JControllerLegacySITE.php

changing $cachable = true; to $cachable = false; or commenting out the line solves the issue, is this something that needs to be overridden in the component being created?

Having a quick look at Joomla com_content $cachable isn't set for the view,

class ###Component###Controller extends JControllerLegacy
{
	/**
	 * display task
	 *
	 * @return void
	 */
	function display($cachable = false, $urlparams = false)
	{
		// set default view if not set
		$view		= $this->input->getCmd('view', '###SITE_DEFAULT_VIEW###');
		$isEdit		= $this->checkEditView($view);
		$layout		= $this->input->get('layout', null, 'WORD');
		$id			= $this->input->getInt('id');
		$cachable	= true;

### Steps to reproduce the issue Component with one site list view which has pagination, global caching is set to On ### Expected result Pagination links should work with caching set to on ### Actual result First page is loaded into cache and unable to open the next pagination page in the pagination links. ### System information (as much as possible) - OS Name & Version: Centos 7 - MySql Version: - Apache Version: 2.4 - PHP Version: 7.0 - Joomla Version: 3.8.5 - JCB Version: staging - Browser: chrome ### Additional comments I can see that the default is set below in JControllerLegacySITE.php changing $cachable = true; to $cachable = false; or commenting out the line solves the issue, is this something that needs to be overridden in the component being created? Having a quick look at Joomla com_content $cachable isn't set for the view, ``` class ###Component###Controller extends JControllerLegacy { /** * display task * * @return void */ function display($cachable = false, $urlparams = false) { // set default view if not set $view = $this->input->getCmd('view', '###SITE_DEFAULT_VIEW###'); $isEdit = $this->checkEditView($view); $layout = $this->input->get('layout', null, 'WORD'); $id = $this->input->getInt('id'); $cachable = true; ```

Llewellyn commented

2018-03-01 17:23:45 +00:00

I must say the whole cache issue has been a headache to me from a long time. The pagination issue has been around for some time, I always thought it is a joomla issue. but hey maybe I am doing something wrong.

So here is the question, removing the $cachable = true; line from the display does it resolve the issue?

I know it use to be relevant, but Joomla is giving huge strides forward, so it may have changed.

I must say the whole cache issue has been a headache to me from a long time. The pagination issue has been around for some time, I always thought it is a joomla issue. but hey maybe I am doing something wrong. So here is the question, removing the `$cachable = true;` line from the display does it resolve the issue? I know it use to be relevant, but [Joomla is giving huge strides forward](https://github.com/vdm-io/Joomla-Component-Builder/issues/126), so it may have changed.

Llewellyn commented

2018-03-01 17:27:54 +00:00

Look here is the first reference to this issue.

Look here is the [first reference to this issue](https://github.com/SermonDistributor/Joomla-3-Component/issues/15).

Twincarb commented

2018-03-01 20:18:20 +00:00

(Migrated from github.com)

I have just had another look, removing that line does as is expected but disables the cache completely which isn't the desired effect for a site where the view is to the public against a view that's for logged in users only.

I have had a look at com_content and the way that it manages it and have come up with this which I think is heading in the right direction but has issues when you change the number of items to display.

    public function display($cachable = false, $urlparams = false)
    {
        // set default view if not set
        $view		= $this->input->getCmd('view', '');
        $isEdit		= $this->checkEditView($view);
        $layout		= $this->input->get('layout', null, 'WORD');
        $id			= $this->input->getInt('id');
        $cachable	= true;
        
        $safeurlparams = array('catid' => 'INT', 'id' => 'INT', 'cid' => 'ARRAY', 'year' => 'INT', 'month' => 'INT', 'limitstart' => 'INT', 'start' => 'INT', 'limit' => 'UINT', 'limitstart' => 'UINT', 'showall' => 'INT', 'return' => 'BASE64', 'filter' => 'STRING', 'filter_order' => 'CMD', 'filter_order_Dir' => 'CMD', 'filter-search' => 'STRING', 'print' => 'BOOLEAN', 'lang' => 'CMD', 'Itemid'=>'INT');
        
        return parent::display($cachable, $safeurlparams);
        
        // Check for edit form.

The $safeurlparams needs sanitising to the options that are actually available (I copied from com_content)

When I was looking at the code used in com_content I noticed that when a user is logged in they aren't shown a cached copy and it is disabled using the following code.

    $user = JFactory::getUser();
    if ($user->get('id') ||
    ($this->input->getMethod() == 'POST' &&
    (($vName == 'category' && $this->input->get('layout') != 'blog') || $vName == 'archive' ))) {
        $cachable = false;
    }

With the component I am currently working on I would ~~want~~ need the logged in users to be able to see non cached content, which is why I am adding it here as well.

I have just had another look, removing that line does as is expected but disables the cache completely which isn't the desired effect for a site where the view is to the public against a view that's for logged in users only. I have had a look at com_content and the way that it manages it and have come up with this which I think is heading in the right direction but has issues when you change the number of items to display. ``` public function display($cachable = false, $urlparams = false) { // set default view if not set $view = $this->input->getCmd('view', ''); $isEdit = $this->checkEditView($view); $layout = $this->input->get('layout', null, 'WORD'); $id = $this->input->getInt('id'); $cachable = true; $safeurlparams = array('catid' => 'INT', 'id' => 'INT', 'cid' => 'ARRAY', 'year' => 'INT', 'month' => 'INT', 'limitstart' => 'INT', 'start' => 'INT', 'limit' => 'UINT', 'limitstart' => 'UINT', 'showall' => 'INT', 'return' => 'BASE64', 'filter' => 'STRING', 'filter_order' => 'CMD', 'filter_order_Dir' => 'CMD', 'filter-search' => 'STRING', 'print' => 'BOOLEAN', 'lang' => 'CMD', 'Itemid'=>'INT'); return parent::display($cachable, $safeurlparams); // Check for edit form. ``` The `$safeurlparams` needs sanitising to the options that are actually available (I copied from com_content) When I was looking at the code used in com_content I noticed that when a user is logged in they aren't shown a cached copy and it is disabled using the following code. ``` $user = JFactory::getUser(); if ($user->get('id') || ($this->input->getMethod() == 'POST' && (($vName == 'category' && $this->input->get('layout') != 'blog') || $vName == 'archive' ))) { $cachable = false; } ``` With the component I am currently working on I would ~~want~~ need the logged in users to be able to see non cached content, which is why I am adding it here as well.

Twincarb commented

2018-03-01 20:27:48 +00:00

(Migrated from github.com)

If the non sef url has the following structure it solves the issue of selecting how many items are to be display because it's in the url
/index.php?option=com_component&view=listview&Itemid=1687&limitstart=15&limit=5

If the non sef url has the following structure it solves the issue of selecting how many items are to be display because it's in the url `/index.php?option=com_component&view=listview&Itemid=1687&limitstart=15&limit=5`

Llewellyn commented

2018-03-01 21:13:19 +00:00

Must tell man this is painful.... this still does not resolve the issue. I mean the $safeurlparams will be anything you can imagine. By that I mean in the content case they have the set values, but with dynamicGet those could be any value. So it is very hard to set those correctly in a dynamic way.

Hmmm okay lets see the pagination url on the other hand is build by joomla since we are using the Joomla pagination object.

I do not have a system setup to validate all these things, so you are my eyes in a way. So it seems like we need to add more details to the populateState method in the models

	/**
	 * Method to auto-populate the model state.
	 *
	 * Note. Calling getState in this method will result in recursion.
	 *
	 * @since   1.6
	 *
	 * @return void
	 */
	protected function populateState()
	{
		$app = JFactory::getApplication('site');

		// Load state from the request.
		$pk = $app->input->getInt('id');
		$this->setState('article.id', $pk);

		$offset = $app->input->getUInt('limitstart');
		$this->setState('list.offset', $offset);

		// Load the parameters.
		$params = $app->getParams();
		$this->setState('params', $params);

		// TODO: Tune these values based on other permissions.
		$user = JFactory::getUser();

		if ((!$user->authorise('core.edit.state', 'com_content')) && (!$user->authorise('core.edit', 'com_content')))
		{
			$this->setState('filter.published', 1);
			$this->setState('filter.archived', 2);
		}

		$this->setState('filter.language', JLanguageMultilang::isEnabled());
	}

This is what the ContentModelArticle has.

The pagination url does it have the full &limitstart=15&limit=5 as part of the url?

Must tell man this is painful.... this still does not resolve the issue. I mean the `$safeurlparams` will be anything you can imagine. By that I mean in the content case they have the set values, but with dynamicGet those could be any value. So it is very hard to set those correctly in a dynamic way. Hmmm okay lets see the pagination url on the other hand is build by joomla since we are using the Joomla pagination object. I do not have a system setup to validate all these things, so you are my eyes in a way. So it seems like we need to add more details to the populateState method in the models ``` /** * Method to auto-populate the model state. * * Note. Calling getState in this method will result in recursion. * * @since 1.6 * * @return void */ protected function populateState() { $app = JFactory::getApplication('site'); // Load state from the request. $pk = $app->input->getInt('id'); $this->setState('article.id', $pk); $offset = $app->input->getUInt('limitstart'); $this->setState('list.offset', $offset); // Load the parameters. $params = $app->getParams(); $this->setState('params', $params); // TODO: Tune these values based on other permissions. $user = JFactory::getUser(); if ((!$user->authorise('core.edit.state', 'com_content')) && (!$user->authorise('core.edit', 'com_content'))) { $this->setState('filter.published', 1); $this->setState('filter.archived', 2); } $this->setState('filter.language', JLanguageMultilang::isEnabled()); } ``` This is what the ContentModelArticle has. The pagination url does it have the full `&limitstart=15&limit=5` as part of the url?

Llewellyn commented

2018-03-01 21:27:05 +00:00

I quickly pulled up sermon distributor with some demo content. and it seems like the pagination does not add those values it only adds start=10 this means the default Joomla implementation does not even use the &limitstart=15&limit=5 in the url.

The limit is handled by javascript with that little dropdown box. So lets assume that the limit is okay. and we need only act on the start value in the model. then I can add the following to the populateState()

// set the start
$start = $app->input->getInt('start');
$this->setState('list.start', $start);

This should resolve the issue, can you test that and let me know.

I quickly pulled up sermon distributor with some demo content. and it seems like the pagination does not add those values it only adds `start=10` this means the default Joomla implementation does not even use the `&limitstart=15&limit=5` in the url. The limit is handled by javascript with that little dropdown box. So lets assume that the limit is okay. and we need only act on the `start` value in the model. then I can add the following to the populateState() ``` // set the start $start = $app->input->getInt('start'); $this->setState('list.start', $start); ``` This should resolve the issue, can you test that and let me know.

Llewellyn commented

2018-03-01 21:29:48 +00:00

As for the tweak for $cachable in the view, let me look at it little closer. I am sure we can also fix that up. Will be great if pagination can start working with caching enabled 👍

As for the tweak for `$cachable` in the view, let me look at it little closer. I am sure we can also fix that up. Will be great if pagination can start working with caching enabled :+1:

Twincarb commented

2018-03-01 21:58:39 +00:00

(Migrated from github.com)

As a quick fix remove $cachable = true which just means at components don't support caching but it doesn't force an admin to disable caching for the whole site, then when the issue is resolved it can be added back in with the fix.

I manually added the &limit=5 into the url it doesn't matter what you use 5,10,15,20,25 the dropdown mirrors it. the &limitstart=0 is added automatically I believe by Joomla's pagination.

I have SH404sef installed on the site so not sure I am getting totally valid results, I need to set up another test domain to test without SH404sef.

with &safeurlparams I have just reduced it to as below and it still works fine, unless someone adds something funky to the url structure that would probably work.

$safeurlparams = array('limitstart' => 'INT', 'limit' => 'INT');

As a quick fix remove `$cachable = true` which just means at components don't support caching but it doesn't force an admin to disable caching for the whole site, then when the issue is resolved it can be added back in with the fix. I manually added the `&limit=5` into the url it doesn't matter what you use 5,10,15,20,25 the dropdown mirrors it. the `&limitstart=0` is added automatically I believe by Joomla's pagination. I have SH404sef installed on the site so not sure I am getting totally valid results, I need to set up another test domain to test without SH404sef. with `&safeurlparams` I have just reduced it to as below and it still works fine, unless someone adds something funky to the url structure that would probably work. ``` $safeurlparams = array('limitstart' => 'INT', 'limit' => 'INT'); ```

Llewellyn commented

2018-03-01 22:00:58 +00:00

Well I have played around with the model a bit and I must tell you it seems very weired to say the least.

Add that little tweak makes matter worse, with no cache all works well, moment I turn that on it seems to freeze on the first page no matter what I do.

If you are able to get it to work, I mean really.. then please post the code here and where it should be then I can try an automate this. Reality is that I have spend to much time on it. I will get back to this in a day or so... have some other things I need to look at first.

Please do try and resolve this, if you have time.

Well I have played around with the model a bit and I must tell you it seems very weired to say the least. Add that little tweak makes matter worse, with no cache all works well, moment I turn that on it seems to freeze on the first page no matter what I do. If you are able to get it to work, I mean really.. then please post the code here and where it should be then I can try an automate this. Reality is that I have spend to much time on it. I will get back to this in a day or so... have some other things I need to look at first. Please do try and resolve this, if you have time.

Twincarb commented

2018-03-01 22:02:58 +00:00

(Migrated from github.com)

No worries, I am not in work tomorrow so will be able to have a proper look...

Llewellyn commented

2018-03-01 22:06:51 +00:00

From what I understand we are working with this class /libraries/src/MVC/View/HtmlView.php as the view and these models /libraries/src/MVC/Model

Llewellyn commented

2018-03-01 22:08:51 +00:00

According to this line in the classmap.php file:
JLoader::registerAlias('JViewLegacy', '\\Joomla\\CMS\\MVC\\View\\HtmlView', '5.0');

According to this line in the classmap.php file: `JLoader::registerAlias('JViewLegacy', '\\Joomla\\CMS\\MVC\\View\\HtmlView', '5.0');`

Llewellyn commented

2018-03-03 03:57:52 +00:00

Okay I made a few changes, added the quick fix to help get this issue out of the way, but also started working on a long term fix.

I am pushing the quick fix out, please test and give feedback. Also any pointers to that will help resolve this long term will be great.

Okay I made a few changes, added the quick fix to help get this issue out of the way, but also started working on a long term fix. I am pushing the quick fix out, please test and give feedback. Also any pointers to that will help resolve this long term will be great.

Llewellyn commented

2018-03-03 04:01:16 +00:00

I have tested it with sermon distributor, our demo component. But more testing would be ideal.

Twincarb commented

2018-03-05 06:23:41 +00:00

(Migrated from github.com)

I have tested over the weekend and can't find any unexpected issues with it.

Caching of list view unable to use paigination #238

Steps to reproduce the issue

Expected result

Actual result

System information (as much as possible)

Additional comments