diff --git a/application/Espo/Core/Job/JobManager.php b/application/Espo/Core/Job/JobManager.php index ce6e38b13c..a4436f0d43 100644 --- a/application/Espo/Core/Job/JobManager.php +++ b/application/Espo/Core/Job/JobManager.php @@ -29,6 +29,8 @@ namespace Espo\Core\Job; +use Espo\Core\Exceptions\Error; + use Espo\Core\{ Utils\Config, Utils\File\Manager as FileManager, @@ -183,20 +185,30 @@ class JobManager $this->jobRunner->runThrowingException($job); } + /** + * @todo Move to a separate class. + */ private function getLastRunTime(): int { - $lastRunData = $this->fileManager->getPhpContents($this->lastRunTimeFile); + if ($this->fileManager->isFile($this->lastRunTimeFile)) { + try { + $data = $this->fileManager->getPhpContents($this->lastRunTimeFile); + } + catch (Error $e) { + $data = null; + } - if (is_array($lastRunData) && !empty($lastRunData['time'])) { - $lastRunTime = $lastRunData['time']; - } - else { - $lastRunTime = time() - intval($this->config->get('cronMinInterval', 0)) - 1; + if (is_array($data) && isset($data['time'])) { + return (int) $data['time']; + } } - return (int) $lastRunTime; + return time() - intval($this->config->get('cronMinInterval', 0)) - 1; } + /** + * @todo Move to a separate class. + */ private function updateLastRunTime(): void { $data = [ diff --git a/application/Espo/Core/Utils/Config/ConfigFileManager.php b/application/Espo/Core/Utils/Config/ConfigFileManager.php index 2186270803..3c965c58be 100644 --- a/application/Espo/Core/Utils/Config/ConfigFileManager.php +++ b/application/Espo/Core/Utils/Config/ConfigFileManager.php @@ -29,6 +29,8 @@ namespace Espo\Core\Utils\Config; +use Espo\Core\Exceptions\Error; + use Espo\Core\{ Utils\File\Manager as FileManager, Utils\Config, @@ -87,11 +89,18 @@ class ConfigFileManager } /** - * @return array|false + * @return array + * @throws Error */ - public function getPhpContents(string $path) + public function getPhpContents(string $path): array { - return $this->fileManager->getPhpContents($path); + $data = $this->fileManager->getPhpContents($path); + + if (!is_array($data)) { + throw new Error("Bad data stored in '{$path}."); + } + + /** @var array */ + return $data; } } - diff --git a/application/Espo/Core/Utils/Config/ConfigWriter.php b/application/Espo/Core/Utils/Config/ConfigWriter.php index 989dbe001a..9327128e95 100644 --- a/application/Espo/Core/Utils/Config/ConfigWriter.php +++ b/application/Espo/Core/Utils/Config/ConfigWriter.php @@ -117,6 +117,8 @@ class ConfigWriter /** * Save config changes to the file. + * + * @throws Error */ public function save(): void { @@ -130,7 +132,7 @@ class ConfigWriter $internalConfigPath = $this->config->getInternalConfigPath(); if (!$this->fileManager->isFile($configPath)) { - throw new Error("Config file '{$configPath}' was not found."); + throw new Error("Config file '{$configPath}' not found."); } $data = $this->fileManager->getPhpContents($configPath); @@ -138,10 +140,6 @@ class ConfigWriter $dataInternal = $this->fileManager->isFile($internalConfigPath) ? $this->fileManager->getPhpContents($internalConfigPath) : []; - if (!is_array($data)) { - $data = $this->fileManager->getPhpContents($configPath); - } - if (!is_array($data)) { throw new Error("Could not read config."); } diff --git a/application/Espo/Core/Utils/Config/ConfigWriterFileManager.php b/application/Espo/Core/Utils/Config/ConfigWriterFileManager.php index a390d563a5..2437a01461 100644 --- a/application/Espo/Core/Utils/Config/ConfigWriterFileManager.php +++ b/application/Espo/Core/Utils/Config/ConfigWriterFileManager.php @@ -29,13 +29,13 @@ namespace Espo\Core\Utils\Config; +use Espo\Core\Exceptions\Error; + use Espo\Core\{ Utils\File\Manager as FileManager, Utils\Config, }; -use RuntimeException; - class ConfigWriterFileManager { private FileManager $fileManager; @@ -76,14 +76,14 @@ class ConfigWriterFileManager /** * @param array $data - * @throws RuntimeException + * @throws Error */ protected function putPhpContentsInternal(string $path, array $data, bool $useRenaming = false): void { $result = $this->fileManager->putPhpContents($path, $data, true, $useRenaming); if ($result === false) { - throw new RuntimeException(); + throw new Error(); } } @@ -103,12 +103,25 @@ class ConfigWriterFileManager $this->putPhpContentsInternal($path, $data, false); } + /** - * @return mixed + * Supposed to return array. False means the file is being written or corrupted. + * @return array|false */ public function getPhpContents(string $path) { - return $this->fileManager->getPhpContents($path); + try { + $data = $this->fileManager->getPhpContents($path); + } + catch (Error $e) { + return false; + } + + if (!is_array($data)) { + return false; + } + + /** @var array */ + return $data; } } - diff --git a/application/Espo/Core/Utils/Database/Schema/Converter.php b/application/Espo/Core/Utils/Database/Schema/Converter.php index cc8176f9e4..2df5de300c 100644 --- a/application/Espo/Core/Utils/Database/Schema/Converter.php +++ b/application/Espo/Core/Utils/Database/Schema/Converter.php @@ -630,11 +630,19 @@ class Converter $fileList = $this->fileManager->getFileList($path, false, '\.php$', true); foreach ($fileList as $fileName) { - $fileData = $this->fileManager->getPhpContents($path . '/' . $fileName); + $itemPath = $path . '/' . $fileName; - if (is_array($fileData)) { - $tables = Util::merge($tables, $fileData); + if (!$this->fileManager->isFile($itemPath)) { + continue; } + + $fileData = $this->fileManager->getPhpContents($itemPath); + + if (!is_array($fileData)) { + continue; + } + + $tables = Util::merge($tables, $fileData); } return $tables; diff --git a/application/Espo/Core/Utils/File/Manager.php b/application/Espo/Core/Utils/File/Manager.php index 63a56e6880..657698cac4 100644 --- a/application/Espo/Core/Utils/File/Manager.php +++ b/application/Espo/Core/Utils/File/Manager.php @@ -217,15 +217,15 @@ class Manager * Get file contents. * * @param string $path - * @return string|false + * @throws Error If the file could not be read. */ - public function getContents($path) + public function getContents($path): string { /** @var mixed $path */ if (is_array($path)) { // For backward compatibility. - // @todo Remove support of arrays in v6.4. + // @todo Remove support of arrays in v7.3. trigger_error( 'Array parameter is deprecated for FileMaanger::getContents.', E_USER_DEPRECATED @@ -237,27 +237,35 @@ class Manager throw new InvalidArgumentException(); } - if (file_exists($path)) { - return file_get_contents($path); + if (!file_exists($path)) { + throw new Error("File '{$path}' does not exist."); } - return false; + $contents = file_get_contents($path); + + if ($contents === false) { + throw new Error("Could not open file '{$path}'."); + } + + return $contents; } /** * Get data from a PHP file. * - * @return mixed|false + * @return mixed */ public function getPhpContents(string $path) { - if (file_exists($path) && strtolower(substr($path, -4)) == '.php') { - $phpContents = include($path); - - return $phpContents; + if (!file_exists($path)) { + throw new Error("File '$path' does not exist."); } - return false; + if (strtolower(substr($path, -4)) !== '.php') { + throw new Error("File '$path' is not PHP."); + } + + return include($path); } /** @@ -429,18 +437,16 @@ class Manager */ public function mergeJsonContents(string $path, array $data): bool { - $currentContents = $this->getContents($path); + $currentData = []; - if ($this->isFile($path) && $currentContents === false) { - throw new Error("FileManager: Failed to read file '{$path}'."); + if ($this->isFile($path)) { + $currentContents = $this->getContents($path); + + $currentData = Json::decode($currentContents, true); } - $currentData = $this->isFile($path) ? - Json::decode($currentContents, true) : - []; - if (!is_array($currentData)) { - throw new Error("Neither an array nor object in '{$path}'."); + throw new Error("Neither array nor object in '{$path}'."); } $mergedData = Util::merge($currentData, $data); @@ -470,16 +476,12 @@ class Manager */ public function unsetJsonContents(string $path, array $unsets): bool { - if (!file_exists($path)) { + if (!$this->isFile($path)) { return true; } $currentContents = $this->getContents($path); - if (!$currentContents) { - return true; - } - $currentData = Json::decode($currentContents, true); $unsettedData = Util::unsetInArray($currentData, $unsets, true); diff --git a/application/Espo/Core/Utils/Metadata.php b/application/Espo/Core/Utils/Metadata.php index 9578ca7fa2..df1c73ec33 100644 --- a/application/Espo/Core/Utils/Metadata.php +++ b/application/Espo/Core/Utils/Metadata.php @@ -377,13 +377,13 @@ class Metadata { $filePath = $this->customPath . "/{$key1}/{$key2}.json"; - $fileContent = $this->fileManager->getContents($filePath); - - if ($fileContent) { - return Json::decode($fileContent); + if (!$this->fileManager->isFile($filePath)) { + return $default; } - return $default; + $fileContent = $this->fileManager->getContents($filePath); + + return Json::decode($fileContent); } /** diff --git a/application/Espo/Modules/Crm/Classes/FormulaFunctions/ExtGroup/AccountGroup/FindByEmailAddressType.php b/application/Espo/Modules/Crm/Classes/FormulaFunctions/ExtGroup/AccountGroup/FindByEmailAddressType.php index d2181f40f0..2511b5db2e 100644 --- a/application/Espo/Modules/Crm/Classes/FormulaFunctions/ExtGroup/AccountGroup/FindByEmailAddressType.php +++ b/application/Espo/Modules/Crm/Classes/FormulaFunctions/ExtGroup/AccountGroup/FindByEmailAddressType.php @@ -86,7 +86,7 @@ class FindByEmailAddressType extends BaseFunction implements $ignoreList = json_decode( $this->fileManager->getContents( 'application/Espo/Modules/Crm/Resources/data/freeEmailProviderDomains.json' - ) ?: '[]' + ) ) ?? []; $contact = $em->getRDBRepository('Contact') diff --git a/application/Espo/Tools/EntityManager/EntityManager.php b/application/Espo/Tools/EntityManager/EntityManager.php index dff3f22140..96865cf58a 100644 --- a/application/Espo/Tools/EntityManager/EntityManager.php +++ b/application/Espo/Tools/EntityManager/EntityManager.php @@ -421,10 +421,6 @@ class EntityManager $scopesDataContents = $this->fileManager->getContents($filePath); - if ($scopesDataContents === false) { - throw new Error("Could not open file `{$filePath}`."); - } - $scopesDataContents = str_replace('{entityType}', $name, $scopesDataContents); foreach ($replaceData as $key => $value) { @@ -454,10 +450,6 @@ class EntityManager $entityDefsDataContents = $this->fileManager->getContents($filePath); - if ($entityDefsDataContents === false) { - throw new Error("Could not open file `{$filePath}`."); - } - $entityDefsDataContents = str_replace('{entityType}', $name, $entityDefsDataContents); $entityDefsDataContents = str_replace('{entityTypeLowerFirst}', lcfirst($name), $entityDefsDataContents); @@ -473,10 +465,6 @@ class EntityManager $clientDefsContents = $this->fileManager->getContents($filePath); - if ($clientDefsContents === false) { - throw new Error("Could not open file `{$filePath}`."); - } - $clientDefsContents = str_replace('{entityType}', $name, $clientDefsContents); foreach ($replaceData as $key => $value) { diff --git a/tests/unit/Espo/Core/Utils/Config/ConfigWriterTest.php b/tests/unit/Espo/Core/Utils/Config/ConfigWriterTest.php index 3a3c8095cc..abb269a191 100644 --- a/tests/unit/Espo/Core/Utils/Config/ConfigWriterTest.php +++ b/tests/unit/Espo/Core/Utils/Config/ConfigWriterTest.php @@ -133,7 +133,7 @@ class ConfigWriterTest extends \PHPUnit\Framework\TestCase [$this->configPath], [$this->configPath], ) - ->willReturnOnConsecutiveCalls($previousData); + ->willReturnOnConsecutiveCalls($previousData, $previousData); $this->configWriter->save(); }