Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken caching system when array of allowed parameters used #6475

Merged
merged 2 commits into from
Sep 4, 2022
Merged

Fix broken caching system when array of allowed parameters used #6475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c48b59
Fix broken caching system when array of allowed parameters used
JavaDeveloperKiev Sep 2, 2022
2b5dae8
Create test for Cache Query String options
JavaDeveloperKiev Sep 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions system/CodeIgniter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,9 +730,12 @@ protected function generateCacheName(Cache $config): string
}

$uri = $this->request->getUri();

if ($config->cacheQueryString) {
$name = URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery());
if (is_array($config->cacheQueryString)) {
$name = URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery(['only' => $config->cacheQueryString]));
} else {
$name = URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath(), $uri->getQuery());
}
} else {
$name = URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath());
}
Expand Down
78 changes: 78 additions & 0 deletions tests/system/CodeIgniterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use CodeIgniter\Test\Filters\CITestStreamFilter;
use CodeIgniter\Test\Mock\MockCodeIgniter;
use Config\App;
use Config\Cache;
use Config\Filters;
use Config\Modules;
use Tests\Support\Filters\Customfilter;
Expand Down Expand Up @@ -622,4 +623,81 @@ public function testPageCacheSendSecureHeaders()
stream_filter_remove($outputStreamFilter);
stream_filter_remove($errorStreamFilter);
}

/**
* @param array|bool $cacheQueryStringValue
* @dataProvider cacheQueryStringProvider
*
* @see https://github.com/codeigniter4/CodeIgniter4/pull/6410
*/
public function testPageCacheWithCacheQueryString($cacheQueryStringValue, int $expectedPagesInCache, array $testingUrls)
{
// Suppress command() output
CITestStreamFilter::$buffer = '';
$outputStreamFilter = stream_filter_append(STDOUT, 'CITestStreamFilter');
$errorStreamFilter = stream_filter_append(STDERR, 'CITestStreamFilter');

// Create cache config with cacheQueryString value from the dataProvider
$cacheConfig = new Cache();
$cacheConfig->cacheQueryString = $cacheQueryStringValue;

// Clear cache before starting the test
command('cache:clear');

// Calculate amount of items in the cache before the test
$cache = \Config\Services::cache();
$cacheStartCounter = count($cache->getCacheInfo());

// Generate request to each URL from the testing array
foreach ($testingUrls as $testingUrl) {
$_SERVER['REQUEST_URI'] = '/' . $testingUrl;
$routes = Services::routes(true);
$routes->add($testingUrl, static function () {
CodeIgniter::cache(0); // Dont cache the page in the run() function because CodeIgniter class will create default $cacheConfig and overwrite settings from the dataProvider
$response = Services::response();
$string = 'This is a test page, to check cache configuration';

return $response->setBody($string);
});

// Inject router
$router = Services::router($routes, Services::request(null, false));
Services::injectMock('router', $router);

// Cache the page output using default caching function and $cacheConfig with value from the data provider
$this->codeigniter->useSafeOutput(true)->run();
$this->codeigniter->cachePage($cacheConfig); // Cache the page using our own $cacheConfig confugration
}

// Calculate how much cached items exist in the cache after the test requests
$cacheEndCounter = count($cache->getCacheInfo());
$newPagesCached = $cacheEndCounter - $cacheStartCounter;

// Clear cache after the test
command('cache:clear');

// Check that amount of new items created in the cache matching expected value from the data provider
$this->assertSame($expectedPagesInCache, $newPagesCached);

// Remove stream filters
stream_filter_remove($outputStreamFilter);
stream_filter_remove($errorStreamFilter);
}

public function cacheQueryStringProvider(): array
{
$testingUrls = [
'test', // URL #1
'test?important_parameter=1', // URL #2
'test?important_parameter=2', // URL #3
'test?important_parameter=1&not_important_parameter=2', // URL #4
'test?important_parameter=1&not_important_parameter=2&another_not_important_parameter=3', // URL #5
];

return [
'$cacheQueryString=false' => [false, 1, $testingUrls], // We expect only 1 page in the cache, because when cacheQueryString is set to false, all GET parameter should be ignored, and page URI will be absolutely same "/test" string for all 5 requests
'$cacheQueryString=true' => [true, 5, $testingUrls], // We expect all 5 pages in the cache, because when cacheQueryString is set to true, all GET parameter should be processed as unique requests
'$cacheQueryString=array' => [['important_parameter'], 3, $testingUrls], // We expect only 3 pages in the cache, because when cacheQueryString is set to array with important parameters, we should ignore all parameters thats not in the array. Only URL #1, URL #2 and URL #3 should be cached. URL #4 and URL #5 is duplication of URL #2 (with value ?important_parameter=1), so they should not be processed as new unique requests and application should return already cached page for URL #2
];
}
}