diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ca9aecb..867072c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,6 +50,15 @@ jobs: working-directory: FreshRSS/extensions/xExtension-RssCloud run: find . \( -name '*.php' -o -name '*.phtml' \) -print0 | xargs -0 -n1 php -l > /dev/null + - name: phpunit + working-directory: FreshRSS/extensions/xExtension-RssCloud + # Core's PHPUnit and core's autoloader, so there is no second set of dev dependencies to + # keep in step. Runs against both cores in the matrix, which is the point: the tests + # exercise the extension against whichever FreshRSS is checked out beside it. + run: >- + ../../vendor/bin/phpunit --bootstrap tests/bootstrap.php + --display-notices --display-deprecations tests + - name: phpcs working-directory: FreshRSS run: | diff --git a/README.md b/README.md index e8dc72a..5409735 100644 --- a/README.md +++ b/README.md @@ -195,6 +195,10 @@ class it touches lives there — so clone it into a FreshRSS checkout at ```sh composer install +# phpunit, using core's autoloader as the bootstrap +( cd extensions/xExtension-RssCloud && \ + ../../vendor/bin/phpunit --bootstrap tests/bootstrap.php tests ) + # phpstan, using core's ruleset scoped to this extension ( cd extensions/xExtension-RssCloud && ../../vendor/bin/phpstan analyse -c phpstan.neon ) @@ -203,7 +207,9 @@ sed '/(?-i:extensions)/d' phpcs.xml > phpcs-extensions.xml vendor/bin/phpcs --standard=phpcs-extensions.xml extensions/xExtension-RssCloud -s ``` -CI runs exactly these against FreshRSS `edge` on every push and pull request. +CI runs exactly these on every push and pull request, against **both** ends of the supported range: +FreshRSS `edge` and `1.29.0`. Analysing only the development tip once let a call to an edge-only +core method ship as a fatal error on every released version, so the floor is checked too. ### Commits diff --git a/RssCloud/Registry.php b/RssCloud/Registry.php index fae1028..1074f05 100644 --- a/RssCloud/Registry.php +++ b/RssCloud/Registry.php @@ -40,13 +40,32 @@ public function directory(string $resourceUrl): string { * @return RssCloudState|null */ public function load(string $resourceUrl): ?array { - $json = @file_get_contents($this->directory($resourceUrl) . '/!cloud.json'); + return self::readState($this->directory($resourceUrl), $resourceUrl); + } + + /** + * Read and normalise the state file held in one subscription directory. + * + * Shared by {@see self::load()} and {@see self::all()}, which differ only in how they arrive at + * a directory: `load()` derives it from a resource URL, while `all()` walks them and recovers + * the URL from the file, the directory name being a one-way hash. Parsing this in two places + * left the copies free to diverge, and they had. + * + * An absent file is unremarkable — {@see self::save()} creates the directory before writing it, + * and {@see self::addSubscriber()} can create one on its own — so only content that exists and + * cannot be used is worth logging. + * + * @param string $context names the subscription in the log + * @return RssCloudState|null + */ + private static function readState(string $directory, string $context): ?array { + $json = @file_get_contents($directory . '/!cloud.json'); if (!is_string($json) || $json === '') { return null; } $state = json_decode($json, true); - if (!is_array($state) || !is_string($state['url'] ?? null)) { - Minz_Log::warning('rssCloud: invalid state JSON for ' . $resourceUrl, RSSCLOUD_LOG); + if (!is_array($state) || !is_string($state['url'] ?? null) || $state['url'] === '') { + Minz_Log::warning('rssCloud: invalid state JSON for ' . $context, RSSCLOUD_LOG); return null; } return self::normalise($state); @@ -59,7 +78,8 @@ public function load(string $resourceUrl): ?array { private static function normalise(array $state): array { return [ 'url' => is_string($state['url'] ?? null) ? $state['url'] : '', - 'kind' => is_string($state['kind'] ?? null) ? $state['kind'] : self::KIND_FEED, + // Constrained to the two known kinds, so that a caller may key off it without checking. + 'kind' => ($state['kind'] ?? null) === self::KIND_OPML ? self::KIND_OPML : self::KIND_FEED, 'endpoint' => is_string($state['endpoint'] ?? null) ? $state['endpoint'] : '', 'registerProcedure' => is_string($state['registerProcedure'] ?? null) ? $state['registerProcedure'] : '', 'lease_start' => is_numeric($state['lease_start'] ?? null) ? (int)$state['lease_start'] : 0, @@ -145,18 +165,18 @@ public function forget(string $resourceUrl): void { /** * Iterate over every known subscription. * + * Yields in directory order, which is `sha1()` order and so arbitrary: anything displaying + * these has to sort them itself. Being a generator, the result is single-pass and cannot be + * counted without first collecting it. + * * @return iterable */ public function all(): iterable { $directories = @glob($this->basePath . '/resources/*', GLOB_ONLYDIR | GLOB_NOSORT); foreach ($directories ?: [] as $directory) { - $json = @file_get_contents($directory . '/!cloud.json'); - if (!is_string($json) || $json === '') { - continue; - } - $state = json_decode($json, true); - if (is_array($state) && is_string($state['url'] ?? null) && $state['url'] !== '') { - yield self::normalise($state); + $state = self::readState($directory, $directory); + if ($state !== null) { + yield $state; } } } diff --git a/configure.phtml b/configure.phtml index 204278a..c4f50eb 100644 --- a/configure.phtml +++ b/configure.phtml @@ -2,6 +2,7 @@ declare(strict_types=1); /** @var RssCloudExtension $this */ $callback = $this->callbackUrl(); + $subscriptions = $this->subscriptions(); ?>
@@ -87,3 +88,42 @@
+ +

+

+ + +

+ + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + +
+ + 		0 ? timestamptodate($state['lease_start']) : _t('ext.rsscloud.status.never') ?> 0 ? timestamptodate($state['last_notify']) : _t('ext.rsscloud.status.never') ?>
+ diff --git a/extension.php b/extension.php index 9fced78..97af8eb 100644 --- a/extension.php +++ b/extension.php @@ -50,6 +50,18 @@ final class RssCloudExtension extends Minz_Extension { /** Upper bound on `pleaseNotify` calls issued in a single refresh cycle, so cron cannot stall. */ public const MAX_SUBSCRIPTIONS_PER_RUN = 10; + /** A cloud server is registered and believed to be notifying us. */ + public const STATUS_ACTIVE = 'active'; + + /** The last `pleaseNotify` was rejected or could not be made; `error_message` says why. */ + public const STATUS_ERROR = 'error'; + + /** Discovered, but no registration has been attempted yet. */ + public const STATUS_PENDING = 'pending'; + + /** Registered successfully, but the lease is older than the renewal window and may have lapsed. */ + public const STATUS_STALE = 'stale'; + /** Feed attribute holding the resource URL this feed is (or should be) subscribed under. */ private const FEED_ATTRIBUTE = 'rssCloud'; @@ -149,6 +161,46 @@ public function renewSeconds(): int { return min(self::MAX_RENEW_HOURS, max(1, $hours)) * 3600; } + /** + * Every known subscription with the state the configuration screen displays, resource-sorted + * because {@see RssCloud_Registry::all()} yields in hash order. + * + * Collected rather than streamed: the view needs to know whether there is anything at all + * before it commits to drawing a table, which a generator cannot answer. + * + * @return list}> + */ + public function subscriptions(): array { + $registry = $this->registry(); + $renewSeconds = $this->renewSeconds(); + + $rows = []; + foreach ($registry->all() as $state) { + $rows[] = [ + 'state' => $state, + 'status' => self::statusOf($state, $renewSeconds), + 'subscribers' => $registry->subscribers($state['url']), + ]; + } + usort($rows, static fn(array $a, array $b): int => strcmp($a['state']['url'], $b['state']['url'])); + + return $rows; + } + + /** + * @param RssCloudState $state + * @return self::STATUS_* + */ + private static function statusOf(array $state, int $renewSeconds): string { + if ($state['error']) { + return self::STATUS_ERROR; + } + if ($state['lease_start'] <= 0) { + return self::STATUS_PENDING; + } + return RssCloud_Subscriber::isHealthy($state, $renewSeconds) ? self::STATUS_ACTIVE : self::STATUS_STALE; + } + public function maxStalenessSeconds(): int { return max(1, $this->getSystemConfigurationInt('max_staleness_hours') ?? self::DEFAULT_MAX_STALENESS_HOURS) * 3600; } diff --git a/i18n/en/ext.php b/i18n/en/ext.php index 01ca433..325ed4d 100644 --- a/i18n/en/ext.php +++ b/i18n/en/ext.php @@ -20,5 +20,22 @@ 'renew_hours.help' => 'rssCloud does not negotiate a lease duration. Subscriptions expire after 25 hours and are meant to be renewed every 24, so the default of 23 leaves a margin. Anything above 24 would simply lapse, and is capped.', 'skip_polling' => 'Skip polling covered resources', 'skip_polling.help' => 'Stop polling a resource on a timer while its cloud subscription is healthy. It is still polled if it goes stale, or when refreshed individually.', + 'status' => 'Subscriptions', + 'status.active' => 'Active', + 'status.endpoint' => 'Cloud server', + 'status.error' => 'Failed', + 'status.help' => 'One row per resource this instance has discovered a cloud server for. "Renewed" is when registration was last attempted, not when it last succeeded; the state column says whether it did.', + 'status.kind' => 'Type', + 'status.kind.feed' => 'Feed', + 'status.kind.opml' => 'Dynamic OPML', + 'status.never' => 'Never', + 'status.none' => 'No cloud servers have been discovered yet. Resources are registered as they are refreshed, so this fills in once feeds advertising a cloud have been fetched at least once.', + 'status.notified' => 'Last notified', + 'status.pending' => 'Pending', + 'status.renewed' => 'Renewal attempted', + 'status.resource' => 'Resource', + 'status.stale' => 'Stale', + 'status.state' => 'State', + 'status.subscribers' => 'Users', ), ); diff --git a/tests/RssCloud/RegistryTest.php b/tests/RssCloud/RegistryTest.php new file mode 100644 index 0000000..e3de648 --- /dev/null +++ b/tests/RssCloud/RegistryTest.php @@ -0,0 +1,215 @@ +basePath = sys_get_temp_dir() . '/rsscloud-registry-test-' . getmypid() . '-' . uniqid(); + mkdir($this->basePath . '/resources', 0770, true); + $this->registry = new RssCloud_Registry($this->basePath); + @unlink(RSSCLOUD_LOG); + } + + #[\Override] + protected function tearDown(): void { + recursive_unlink($this->basePath); + @rmdir($this->basePath); + @unlink(RSSCLOUD_LOG); + } + + /** Write a state file straight to disk, bypassing save(), so malformed content can be staged. */ + private function writeRaw(string $resourceUrl, string $json): void { + $directory = $this->registry->directory($resourceUrl); + mkdir($directory, 0770, true); + if ($json !== '') { + file_put_contents($directory . '/!cloud.json', $json); + } + } + + private function log(): string { + return (string)@file_get_contents(RSSCLOUD_LOG); + } + + /** @param array $state */ + private function writeState(string $resourceUrl, array $state): void { + $this->writeRaw($resourceUrl, (string)json_encode($state + ['url' => $resourceUrl])); + } + + public function test_load_returnsStoredState(): void { + $this->writeState('https://a.example/feed', [ + 'kind' => 'opml', + 'endpoint' => 'https://rpc.example/pleaseNotify', + 'registerProcedure' => '', + 'lease_start' => 1000, + 'last_notify' => 2000, + 'error' => false, + 'error_message' => '', + ]); + + $state = $this->registry->load('https://a.example/feed'); + + self::assertIsArray($state); + self::assertSame('https://a.example/feed', $state['url']); + self::assertSame('opml', $state['kind']); + self::assertSame('https://rpc.example/pleaseNotify', $state['endpoint']); + self::assertSame(1000, $state['lease_start']); + self::assertSame(2000, $state['last_notify']); + self::assertFalse($state['error']); + } + + public function test_load_reportsMalformedJson(): void { + $this->writeRaw('https://b.example/feed', '{not json'); + + self::assertNull($this->registry->load('https://b.example/feed')); + self::assertStringContainsString('invalid state JSON', $this->log()); + } + + /** An empty url used to be accepted here and rejected in all(); both now reject it. */ + public function test_load_reportsEmptyUrl(): void { + $this->writeRaw('https://c.example/feed', (string)json_encode(['url' => ''])); + + self::assertNull($this->registry->load('https://c.example/feed')); + self::assertStringContainsString('invalid state JSON', $this->log()); + } + + /** + * A directory without a state file is ordinary: save() creates the directory before writing, + * and addSubscriber() can create one on its own. It must not be logged as a fault. + */ + public function test_load_isSilentWhenFileAbsent(): void { + $this->writeRaw('https://d.example/feed', ''); + + self::assertNull($this->registry->load('https://d.example/feed')); + self::assertSame('', $this->log()); + } + + public function test_load_constrainsUnknownKindToFeed(): void { + $this->writeState('https://e.example/feed', ['kind' => 'something-else']); + + $state = $this->registry->load('https://e.example/feed'); + + self::assertIsArray($state); + self::assertSame(RssCloud_Registry::KIND_FEED, $state['kind']); + } + + /** Absence of the flag must read as broken, not as fine, like core's WebSub code. */ + public function test_load_treatsMissingErrorFlagAsError(): void { + $this->writeState('https://f.example/feed', []); + + $state = $this->registry->load('https://f.example/feed'); + + self::assertIsArray($state); + self::assertTrue($state['error']); + } + + public function test_all_yieldsOnlyUsableStateAndReportsTheRest(): void { + $this->writeState('https://a.example/feed', ['endpoint' => 'https://rpc.example/x']); + $this->writeRaw('https://b.example/feed', '{not json'); + $this->writeRaw('https://c.example/feed', (string)json_encode(['url' => ''])); + $this->writeRaw('https://d.example/feed', ''); + + $urls = []; + foreach ($this->registry->all() as $state) { + $urls[] = $state['url']; + } + + self::assertSame(['https://a.example/feed'], $urls); + // The two unusable files are reported; the absent one is not. + self::assertSame(2, substr_count($this->log(), 'invalid state JSON')); + } + + /** Documented as single-pass, so callers know not to count it without collecting it first. */ + public function test_all_isAGenerator(): void { + self::assertInstanceOf(Generator::class, $this->registry->all()); + } + + public function test_all_isEmptyWhenNothingIsStored(): void { + $states = []; + foreach ($this->registry->all() as $state) { + $states[] = $state; + } + + self::assertSame([], $states); + } + + public function test_subscribers_roundTrip(): void { + $this->writeState('https://a.example/feed', []); + + $this->registry->addSubscriber('https://a.example/feed', 'alice'); + $this->registry->addSubscriber('https://a.example/feed', 'bob'); + self::assertSame(['alice', 'bob'], $this->sortedSubscribers('https://a.example/feed')); + + $this->registry->removeSubscriber('https://a.example/feed', 'alice'); + self::assertSame(['bob'], $this->sortedSubscribers('https://a.example/feed')); + } + + /** @return list */ + private function sortedSubscribers(string $resourceUrl): array { + $subscribers = $this->registry->subscribers($resourceUrl); + sort($subscribers); + return $subscribers; + } + + public function test_saveThenLoad_roundTrips(): void { + $this->registry->init(); + $this->registry->save('https://g.example/feed', [ + 'kind' => RssCloud_Registry::KIND_OPML, + 'endpoint' => 'https://rpc.example/pleaseNotify', + 'lease_start' => 42, + 'error' => false, + ]); + + $state = $this->registry->load('https://g.example/feed'); + + self::assertIsArray($state); + self::assertSame('https://g.example/feed', $state['url']); + self::assertSame(RssCloud_Registry::KIND_OPML, $state['kind']); + self::assertSame(42, $state['lease_start']); + self::assertFalse($state['error']); + } + + /** Moving to a different cloud server invalidates the lease rather than carrying it over. */ + public function test_remember_resetsLeaseWhenEndpointChanges(): void { + $this->registry->init(); + $this->registry->save('https://h.example/feed', [ + 'endpoint' => 'https://old.example/pleaseNotify', + 'lease_start' => 999, + 'error' => false, + ]); + + $endpoint = RssCloud_Endpoint::fromUrl('https://new.example/pleaseNotify'); + self::assertInstanceOf(RssCloud_Endpoint::class, $endpoint); + $moved = $this->registry->remember('https://h.example/feed', $endpoint, RssCloud_Registry::KIND_FEED); + + self::assertSame(0, $moved['lease_start']); + self::assertTrue($moved['error']); + } + + public function test_remember_keepsLeaseWhenEndpointIsUnchanged(): void { + $this->registry->init(); + $this->registry->save('https://i.example/feed', [ + 'endpoint' => 'https://same.example/pleaseNotify', + 'lease_start' => 999, + 'error' => false, + ]); + + $endpoint = RssCloud_Endpoint::fromUrl('https://same.example/pleaseNotify'); + self::assertInstanceOf(RssCloud_Endpoint::class, $endpoint); + $kept = $this->registry->remember('https://i.example/feed', $endpoint, RssCloud_Registry::KIND_FEED); + + self::assertSame(999, $kept['lease_start']); + self::assertFalse($kept['error']); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..ce42cef --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,29 @@ +