Refactor FilesystemStorage to use single files per feed

The rationale behind this is to make shipping configuration
with a installation package simpler. So that users can package
the module with custom feeds and not override configuration upon
updates.

Author: martialblog

library/Feeds/Storage/FilesystemStorage.php

@@ -3,32 +3,34 @@
 namespace Icinga\Module\Feeds\Storage;
 
 use Icinga\Application\Icinga;
+use Icinga\Util\DirectoryIterator;
 use Icinga\Exception\SystemPermissionException;
+use Icinga\Exception\NotWritableError;
+use Icinga\Exception\NotReadableError;
 
 /**
  * FilesystemStorage is used to store the feeds configuration locally
  */
 class FilesystemStorage implements StorageInterface
 {
-    const FILE_NAME = "feeds.json";
+    const FILE_SUFFIX = ".json";
     const JSON_FLAGS = JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES;
     const VERSION = 1;
 
     protected array $feeds = [];
     protected bool $loaded = false;
 
-    protected function getConfigFile(): string
+    protected function getConfigDir(): string
     {
         return Icinga::app()
             ->getModuleManager()
             ->getModule('feeds')
-            ->getConfigDir() . DIRECTORY_SEPARATOR . self::FILE_NAME;
+            ->getConfigDir();
     }
 
     protected function ensureConfigDir(): void
     {
-        $file = $this->getConfigFile();
-        $dir = dirname($file);
+        $dir = $this->getConfigDir();
 
         if (!is_dir($dir)) {
             if (!is_dir(dirname($dir))) {
@@ -42,20 +44,57 @@ protected function ensureConfigDir(): void
         }
     }
 
-    protected function ensureConfigFile(): void
+    protected function loadFeedFile(string $filename): FeedDefinition
     {
-        $file = $this->getConfigFile();
-        $this->ensureConfigDir();
+        $filePath = $this->getConfigDir() . DIRECTORY_SEPARATOR . $filename;
+
+        if (!is_readable($filePath)) {
+            throw new NotReadableError('Could not read file %s', $filePath);
+        }
+
+        $data = file_get_contents($filePath);
+
+        if ($data === false) {
+            throw new NotReadableError('Could not read file %s', $filePath);
+        }
+
+        $json = json_decode($data, true);
+        $feed = FeedDefinition::fromArray($json);
+
+        return $feed;
+    }
 
-        if (!is_file($file)) {
-            $data = ['version' => self::VERSION];
+    protected function storeFeedFile(FeedDefinition $feed): void
+    {
+        $filePath = $this->getConfigDir() . DIRECTORY_SEPARATOR . $feed->name . self::FILE_SUFFIX;
+
+        $exists = file_exists($filePath);
+        $content = json_encode($feed->toArray(), static::JSON_FLAGS);
 
-            if (file_put_contents($file, json_encode($data, static::JSON_FLAGS)) === false) {
-                throw new SystemPermissionException('Could not write config file "%s"', dirname($file));
+        if (file_put_contents($filePath, $content) === false) {
+            throw new NotWritableError('Could not save to %s', $filePath);
+        }
+
+        // If this is a new file, we make sure to set the mode
+        if ($exists === false) {
+            $fileMode = intval('0660', 8);
+            if (false === @chmod($filePath, $fileMode)) {
+                throw new NotWritableError('Failed to set file mode "0660" on file "%s"', $filePath);
             }
         }
     }
 
+    public function removeFeedFile(string $filename): bool
+    {
+        $filePath = $this->getConfigDir() . DIRECTORY_SEPARATOR . $filename . self::FILE_SUFFIX;
+
+        if (file_exists($filePath)) {
+            return unlink($filePath);
+        }
+
+        return true;
+    }
+
     public function getFeeds(): array
     {
         $this->load();
@@ -70,16 +109,15 @@ public function getFeedByName(string $name): ?FeedDefinition
 
     public function removeFeed(string|FeedDefinition $feed): bool
     {
+        // TODO: This won't work when the feedname and filename don't match
         if (!is_string($feed)) {
-            return $this->removeFeed($feed->name);
+            return $this->removeFeedFile($feed->name);
         }
 
         if (!$this->getFeedByName($feed)) {
             return false;
         }
 
-        unset($this->feeds[$feed]);
-
         $this->flush();
 
         return true;
@@ -107,16 +145,11 @@ public function flush(): void
             'feeds' => []
         ];
 
+        $this->ensureConfigDir();
+
         foreach ($this->getFeeds() as $feed) {
             $data['feeds'][] = $feed->toArray();
-        }
-
-        $this->ensureConfigFile();
-
-        $file = $this->getConfigFile();
-
-        if (file_put_contents($file, json_encode($data, static::JSON_FLAGS)) === false) {
-            throw new SystemPermissionException('Could not write config file "%s"', dirname($file));
+            $this->storeFeedFile($feed);
         }
     }
 
@@ -127,29 +160,19 @@ protected function load(): void
         }
 
         $this->feeds = [];
+        $this->ensureConfigDir();
 
-        $this->ensureConfigFile();
-        $rawData = file_get_contents($this->getConfigFile());
-
-        if ($rawData === false) {
-            throw new SystemPermissionException('Could not read config file "%s"', $this->getConfigFile());
-        }
-
-        $json = json_decode($rawData, true);
-
-        if ($json === null) {
-            throw new SystemPermissionException('Could not read config file "%s"', $this->getConfigFile());
-        }
-
-        if (!array_key_exists('version', $json)) {
-            throw new SystemPermissionException("Config file doesn't contain a version number. File: %s", $this->getConfigFile());
-        }
+        // Load the JSON files for the feeds from the config directory
+        $directory = new DirectoryIterator($this->getConfigDir(), self::FILE_SUFFIX);
 
-        if (array_key_exists('feeds', $json)) {
-            foreach ($json['feeds'] as $feedData) {
-                $feed = FeedDefinition::fromArray($feedData);
-                $this->feeds[$feed->name] = $feed;
+        foreach ($directory as $name => $path) {
+            if (is_dir($path)) {
+                // Do not descend and ignore directories
+                continue;
             }
+
+            $feed = $this->loadFeedFile($name);
+            $this->feeds[$feed->name] = $feed;
         }
 
         $this->loaded = true;

library/Feeds/Storage/StorageFactory.php

@@ -8,7 +8,7 @@ class StorageFactory
 
     protected static function getStorageImpl(): StorageInterface
     {
-        // TODO: Once there are multiple storage options they can be determined
+        // NOTE: Once there are multiple storage options they can be determined
         // from the config file
         return new FilesystemStorage();
     }