file get contents refactoring

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 = [

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<string,mixed>|false
+     * @return array<string,mixed>
+     * @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<string,mixed> */
+        return $data;
     }
 }
-

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.");
         }

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<string,mixed> $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<string,mixed>|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<string,mixed> */
+        return $data;
     }
 }
-

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;

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);

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);
     }
 
     /**

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')

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) {

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();
     }