Unverified Commit fde47049 authored by Dave Reid's avatar Dave Reid Committed by Dave Reid

Issue #3134540 by Dave Reid: Fixed sitemap generation should fail if...

Issue #3134540 by Dave Reid: Fixed sitemap generation should fail if xmlsitemap_check_directory() returns FALSE. Fixed xmlsitemap_get_directory() should always return a valid directory even if config is empty. Removed exception catching in xmlsitemap_sitemap_get_max_filesize() which is no longer necessary.
parent 55ac401b
...@@ -8,6 +8,7 @@ use Drupal\Core\Config\ConfigFactoryInterface; ...@@ -8,6 +8,7 @@ use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Database\Connection; use Drupal\Core\Database\Connection;
use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\File\Exception\DirectoryNotReadyException;
use Drupal\Core\File\FileSystemInterface; use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Language\LanguageInterface;
use Drupal\Core\Language\LanguageManagerInterface; use Drupal\Core\Language\LanguageManagerInterface;
...@@ -402,7 +403,9 @@ class XmlSitemapGenerator implements XmlSitemapGeneratorInterface { ...@@ -402,7 +403,9 @@ class XmlSitemapGenerator implements XmlSitemapGeneratorInterface {
$context['sandbox']['max'] = XMLSITEMAP_MAX_SITEMAP_LINKS; $context['sandbox']['max'] = XMLSITEMAP_MAX_SITEMAP_LINKS;
// Clear the cache directory for this sitemap before generating any files. // Clear the cache directory for this sitemap before generating any files.
xmlsitemap_check_directory($context['sandbox']['sitemap']); if (!xmlsitemap_check_directory($context['sandbox']['sitemap'])) {
throw new DirectoryNotReadyException("The sitemap directory could not be created or is not writable.");
}
xmlsitemap_clear_directory($context['sandbox']['sitemap']); xmlsitemap_clear_directory($context['sandbox']['sitemap']);
} }
......
...@@ -29,23 +29,8 @@ class DirectoryTest extends KernelTestBase { ...@@ -29,23 +29,8 @@ class DirectoryTest extends KernelTestBase {
$fileSystem->saveData('File unrelated to XML sitemap', 'public://file.txt'); $fileSystem->saveData('File unrelated to XML sitemap', 'public://file.txt');
$fileSystem->saveData('Test contents', 'public://xmlsitemap/test/index.xml'); $fileSystem->saveData('Test contents', 'public://xmlsitemap/test/index.xml');
// Set the directory to an empty value.
\Drupal::configFactory()->getEditable('xmlsitemap.settings')->clear('path')->save();
drupal_static_reset('xmlsitemap_get_directory');
$result = xmlsitemap_clear_directory(NULL, TRUE);
// Test that nothing was deleted.
$this->assertFileExists('public://xmlsitemap/test/index.xml');
$this->assertDirectoryExists('public://not-xmlsitemap');
$this->assertFileExists('public://file.txt');
$this->assertFalse($result);
// Reset the value back to the default.
\Drupal::configFactory()->getEditable('xmlsitemap.settings')->set('path', 'xmlsitemap')->save();
drupal_static_reset('xmlsitemap_get_directory');
$result = xmlsitemap_clear_directory(NULL, TRUE);
// Test that only the xmlsitemap directory was deleted. // Test that only the xmlsitemap directory was deleted.
$result = xmlsitemap_clear_directory(NULL, TRUE);
$this->assertDirectoryNotExists('public://xmlsitemap/test'); $this->assertDirectoryNotExists('public://xmlsitemap/test');
$this->assertDirectoryExists('public://not-xmlsitemap'); $this->assertDirectoryExists('public://not-xmlsitemap');
$this->assertFileExists('public://file.txt'); $this->assertFileExists('public://file.txt');
......
...@@ -22,7 +22,6 @@ use Drupal\Core\Entity\ContentEntityTypeInterface; ...@@ -22,7 +22,6 @@ use Drupal\Core\Entity\ContentEntityTypeInterface;
use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Entity\Query\QueryInterface; use Drupal\Core\Entity\Query\QueryInterface;
use Drupal\Core\File\Exception\FileException;
use Drupal\Core\File\FileSystemInterface; use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Language\LanguageInterface;
...@@ -428,13 +427,7 @@ function xmlsitemap_sitemap_get_max_filesize(XmlSitemapInterface $sitemap) { ...@@ -428,13 +427,7 @@ function xmlsitemap_sitemap_get_max_filesize(XmlSitemapInterface $sitemap) {
$sitemap->setMaxFileSize(0); $sitemap->setMaxFileSize(0);
/** @var \Drupal\Core\File\FileSystemInterface $file_system */ /** @var \Drupal\Core\File\FileSystemInterface $file_system */
$file_system = \Drupal::service('file_system'); $file_system = \Drupal::service('file_system');
try {
$files = $file_system->scanDirectory($dir, '/\.xml$/'); $files = $file_system->scanDirectory($dir, '/\.xml$/');
}
catch (FileException $e) {
// Ignore and return empty array for BC.
$files = [];
}
foreach ($files as $file) { foreach ($files as $file) {
$sitemap->setMaxFileSize(max($sitemap->getMaxFileSize(), filesize($file->uri))); $sitemap->setMaxFileSize(max($sitemap->getMaxFileSize(), filesize($file->uri)));
} }
...@@ -483,13 +476,10 @@ function xmlsitemap_sitemap_uri(XmlSitemapInterface $sitemap) { ...@@ -483,13 +476,10 @@ function xmlsitemap_sitemap_uri(XmlSitemapInterface $sitemap) {
function xmlsitemap_get_directory(XmlSitemapInterface $sitemap = NULL) { function xmlsitemap_get_directory(XmlSitemapInterface $sitemap = NULL) {
$directory = &drupal_static(__FUNCTION__); $directory = &drupal_static(__FUNCTION__);
if (!isset($directory)) { if (!isset($directory)) {
$directory = \Drupal::config('xmlsitemap.settings')->get('path'); $directory = \Drupal::config('xmlsitemap.settings')->get('path') ?: 'xmlsitemap';
} }
if (empty($directory)) { if ($sitemap != NULL && !empty($sitemap->id)) {
return FALSE;
}
elseif ($sitemap != NULL && !empty($sitemap->id)) {
return file_build_uri($directory . '/' . $sitemap->id); return file_build_uri($directory . '/' . $sitemap->id);
} }
else { else {
...@@ -504,7 +494,7 @@ function xmlsitemap_check_directory(XmlSitemapInterface $sitemap = NULL) { ...@@ -504,7 +494,7 @@ function xmlsitemap_check_directory(XmlSitemapInterface $sitemap = NULL) {
$directory = xmlsitemap_get_directory($sitemap); $directory = xmlsitemap_get_directory($sitemap);
/** @var \Drupal\Core\File\FileSystemInterface $filesystem */ /** @var \Drupal\Core\File\FileSystemInterface $filesystem */
$filesystem = \Drupal::service('file_system'); $filesystem = \Drupal::service('file_system');
$result = $filesystem->prepareDirectory($directory, $filesystem::CREATE_DIRECTORY | $filesystem::MODIFY_PERMISSIONS); $result = $filesystem->prepareDirectory($directory, FileSystemInterface::CREATE_DIRECTORY | FileSystemInterface::MODIFY_PERMISSIONS);
if (!$result) { if (!$result) {
\Drupal::logger('file system')->error('The directory %directory does not exist or is not writable.', ['%directory' => $directory]); \Drupal::logger('file system')->error('The directory %directory does not exist or is not writable.', ['%directory' => $directory]);
} }
...@@ -551,12 +541,8 @@ function xmlsitemap_check_all_directories() { ...@@ -551,12 +541,8 @@ function xmlsitemap_check_all_directories() {
* Returns TRUE is operation was successful, FALSE otherwise. * Returns TRUE is operation was successful, FALSE otherwise.
*/ */
function xmlsitemap_clear_directory(XmlSitemapInterface $sitemap = NULL, $delete = FALSE) { function xmlsitemap_clear_directory(XmlSitemapInterface $sitemap = NULL, $delete = FALSE) {
if ($directory = xmlsitemap_get_directory($sitemap)) { $directory = xmlsitemap_get_directory($sitemap);
return _xmlsitemap_delete_recursive($directory, $delete); return _xmlsitemap_delete_recursive($directory, $delete);
}
else {
return FALSE;
}
} }
/** /**
...@@ -620,7 +606,7 @@ function _xmlsitemap_delete_recursive($path, $delete_root = FALSE) { ...@@ -620,7 +606,7 @@ function _xmlsitemap_delete_recursive($path, $delete_root = FALSE) {
if (is_dir($path)) { if (is_dir($path)) {
$dir = dir($path); $dir = dir($path);
while (($entry = $dir->read()) !== FALSE) { while (($entry = $dir->read()) !== FALSE) {
if ($entry == '.' || $entry == '..') { if ($entry === '.' || $entry === '..') {
continue; continue;
} }
$entry_path = $path . '/' . $entry; $entry_path = $path . '/' . $entry;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment