From 493e6f17093792cade5fb8c9d881c603e2badbed Mon Sep 17 00:00:00 2001 From: Markus Opolka Date: Tue, 27 Jan 2026 11:19:02 +0100 Subject: [PATCH] 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. --- application/forms/FeedForm.php | 4 +- doc/03-Configuration.md | 2 +- library/Feeds/Storage/FeedDefinition.php | 6 +- library/Feeds/Storage/FilesystemStorage.php | 146 ++++++++++++-------- library/Feeds/Storage/StorageFactory.php | 2 +- 5 files changed, 94 insertions(+), 66 deletions(-) diff --git a/application/forms/FeedForm.php b/application/forms/FeedForm.php index 78eb273..b6de061 100644 --- a/application/forms/FeedForm.php +++ b/application/forms/FeedForm.php @@ -188,7 +188,7 @@ protected function onSuccess(): void $cache = FeedCache::instance('feeds'); if ($this->shouldBeDeleted()) { - $this->storage->removeFeed($this->feed); + $this->storage->removeFeed($this->feed->name); // Clear the cache on a delete $cache->clear('feed-' . $this->feed->name); } elseif ($this->getSubmitButton()->hasBeenPressed() ?? false) { @@ -227,7 +227,7 @@ protected function onSuccess(): void Notification::error($this->translate(sprintf("A feed with the name %s already exists", $name))); return; } - $this->storage->removeFeed($this->feed); + $this->storage->removeFeed($this->feed->name); } $this->feed->name = $name; diff --git a/doc/03-Configuration.md b/doc/03-Configuration.md index 77f0ad4..e7507b4 100644 --- a/doc/03-Configuration.md +++ b/doc/03-Configuration.md @@ -20,7 +20,7 @@ duration = 43200 Feeds can be added, configured and removed in Icinga Web. -The list of configured feeds is stored here: `/etc/icingaweb2/modules/feeds/feeds.json` +The the configured feeds are stored here: `/etc/icingaweb2/modules/feeds/` To configure feed in the dashboard you can adjust the URLs. Example: diff --git a/library/Feeds/Storage/FeedDefinition.php b/library/Feeds/Storage/FeedDefinition.php index 8497471..e93c823 100644 --- a/library/Feeds/Storage/FeedDefinition.php +++ b/library/Feeds/Storage/FeedDefinition.php @@ -40,11 +40,11 @@ public function toArray(): array public static function fromArray(array $data): FeedDefinition { return new self( - trim($data['name']), - trim($data['url']), + trim($data['name'] ?? ''), + trim($data['url'] ?? ''), trim($data['description'] ?? ''), $data['is_visible'] ?? true, - FeedType::fromDisplay(trim($data['type']) ?? 'auto'), + FeedType::fromDisplay(trim($data['type'] ?? 'auto')), $data['polling_interval'] ?? null, ); } diff --git a/library/Feeds/Storage/FilesystemStorage.php b/library/Feeds/Storage/FilesystemStorage.php index 308414d..e400b03 100644 --- a/library/Feeds/Storage/FilesystemStorage.php +++ b/library/Feeds/Storage/FilesystemStorage.php @@ -3,32 +3,40 @@ namespace Icinga\Module\Feeds\Storage; use Icinga\Application\Icinga; +use Icinga\Exception\NotReadableError; +use Icinga\Exception\NotWritableError; use Icinga\Exception\SystemPermissionException; +use Icinga\Util\DirectoryIterator; +use Icinga\Util\Json; /** * 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 + /** + * getConfigDir returns this module's configuration directory + */ + protected function getConfigDir(): string { return Icinga::app() ->getModuleManager() ->getModule('feeds') - ->getConfigDir() . DIRECTORY_SEPARATOR . self::FILE_NAME; + ->getConfigDir(); } + /** + * ensureConfigDir ensures the module's configuration directory exists + */ protected function ensureConfigDir(): void { - $file = $this->getConfigFile(); - $dir = dirname($file); + $dir = $this->getConfigDir(); if (!is_dir($dir)) { if (!is_dir(dirname($dir))) { @@ -42,20 +50,67 @@ protected function ensureConfigDir(): void } } - protected function ensureConfigFile(): void + /** + * loadFeedFile loads a feed's file by its name + */ + 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); - if (!is_file($file)) { - $data = ['version' => self::VERSION]; + return $feed; + } + + /** + * storeFeedFile stores a feed as JSON in the configuration directory + */ + protected function storeFeedFile(FeedDefinition $feed): void + { + // Note: The frontend form validates the characters in a feed's name + $filePath = $this->getConfigDir() . DIRECTORY_SEPARATOR . $feed->name . self::FILE_SUFFIX; + + $exists = file_exists($filePath); + $content = Json::encode($feed->toArray(), static::JSON_FLAGS); + // Not atomic but that's fine for now + if (file_put_contents($filePath, $content, LOCK_EX) === false) { + throw new NotWritableError('Could not save to %s', $filePath); + } - if (file_put_contents($file, json_encode($data, static::JSON_FLAGS)) === false) { - throw new SystemPermissionException('Could not write config file "%s"', dirname($file)); + // 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); } } } + /** + * removeFeedFile removes a feed's file by its anme + */ + public function removeFeedFile(string $filename): bool + { + $filePath = $this->getConfigDir() . DIRECTORY_SEPARATOR . $filename . self::FILE_SUFFIX; + + if (file_exists($filePath)) { + return unlink($filePath); + } + + return false; + } + public function getFeeds(): array { $this->load(); @@ -68,21 +123,10 @@ public function getFeedByName(string $name): ?FeedDefinition return $this->feeds[$name] ?? null; } - public function removeFeed(string|FeedDefinition $feed): bool + public function removeFeed(string $feedname): bool { - if (!is_string($feed)) { - return $this->removeFeed($feed->name); - } - - if (!$this->getFeedByName($feed)) { - return false; - } - - unset($this->feeds[$feed]); - - $this->flush(); - - return true; + // TODO: This won't work when the feedname and filename don't match + return $this->removeFeedFile($feedname); } public function addFeed(FeedDefinition $feed): bool @@ -102,21 +146,10 @@ public function addFeed(FeedDefinition $feed): bool public function flush(): void { - $data = [ - 'version' => self::VERSION, - '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,24 @@ 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); + // Load the JSON files for the feeds from the config directory + $directory = new DirectoryIterator($this->getConfigDir(), self::FILE_SUFFIX); - if ($json === null) { - throw new SystemPermissionException('Could not read config file "%s"', $this->getConfigFile()); - } + foreach ($directory as $name => $path) { + if (is_dir($path)) { + // Do not descend and ignore directories + continue; + } - if (!array_key_exists('version', $json)) { - throw new SystemPermissionException("Config file doesn't contain a version number. File: %s", $this->getConfigFile()); - } + $feed = $this->loadFeedFile($name); - if (array_key_exists('feeds', $json)) { - foreach ($json['feeds'] as $feedData) { - $feed = FeedDefinition::fromArray($feedData); - $this->feeds[$feed->name] = $feed; + if ($feed->name === '') { + continue; } + + $this->feeds[$feed->name] = $feed; } $this->loaded = true; @@ -159,7 +187,7 @@ public function reload(): void { // NOTE: This completely removes the old data before loading the new data. // So if the new version is invalid there is no fallback data to rely on. - $this->loaded = true; + $this->loaded = false; $this->load(); } } diff --git a/library/Feeds/Storage/StorageFactory.php b/library/Feeds/Storage/StorageFactory.php index b4589aa..ba00732 100644 --- a/library/Feeds/Storage/StorageFactory.php +++ b/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(); }