From 58ce95a255c04e1b6ecac0d794bc4dbeb23871de Mon Sep 17 00:00:00 2001 From: Romain d'Alverny Date: Sun, 16 Jan 2022 17:01:53 +0100 Subject: Add test coverage, add tests, refactor OPML code --- Makefile | 3 ++- app/classes/CSRF.php | 4 +-- app/classes/Opml.php | 11 +++++++- app/classes/OpmlManager.php | 65 +++++++++++++++++++++++++++++---------------- phpunit.xml | 27 ++++++++++++++++--- tests/CSRFTest.php | 36 +++++++++++++++++++++++++ tests/HelpersTest.php | 11 -------- tests/OpmlManagerTest.php | 53 ++++++++++++++++++++++++++++++++++++ tests/OpmlTest.php | 53 ++++++++++++++++++++++++++++++++++++ tests/PlanetConfigTest.php | 7 +++++ tests/PlanetTest.php | 11 +++++--- tests/opml/test-empty.opml | 0 tests/opml/test-valid.opml | 16 +++++++++++ 13 files changed, 251 insertions(+), 46 deletions(-) create mode 100644 tests/CSRFTest.php delete mode 100644 tests/HelpersTest.php create mode 100644 tests/OpmlManagerTest.php create mode 100644 tests/OpmlTest.php create mode 100644 tests/opml/test-empty.opml create mode 100644 tests/opml/test-valid.opml diff --git a/Makefile b/Makefile index 17647f0..00506fa 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,11 @@ VENDOR=./vendor/bin/ +PHPUNIT=php -dxdebug.enabled=1 -dxdebug.mode=coverage ./vendor/bin/phpunit --coverage-text test: { php -S 127.0.0.1:8081 >& /dev/null & }; \ PID=$$!; \ - $(VENDOR)phpunit; \ + $(PHPUNIT); \ RES=$$?; \ kill $$PID; \ exit $$RES diff --git a/app/classes/CSRF.php b/app/classes/CSRF.php index 9a700cf..cf9fc1e 100644 --- a/app/classes/CSRF.php +++ b/app/classes/CSRF.php @@ -3,7 +3,7 @@ class CSRF { /** @var string */ - const HMAC_ALGORITHM = 'sha1'; + const HMAC_ALGORITHM = 'sha256'; /** @var string */ const SESSION_KEY_NAME = '_csrf_key'; @@ -48,7 +48,7 @@ class CSRF public static function getKey() { if (empty($_SESSION[self::SESSION_KEY_NAME])) { - $_SESSION[self::SESSION_KEY_NAME] = random_bytes(16); + $_SESSION[self::SESSION_KEY_NAME] = bin2hex(random_bytes(16)); } return $_SESSION[self::SESSION_KEY_NAME]; } diff --git a/app/classes/Opml.php b/app/classes/Opml.php index c5f185f..b91b43e 100644 --- a/app/classes/Opml.php +++ b/app/classes/Opml.php @@ -9,6 +9,9 @@ class Opml public string $ownerEmail = ''; public string $ownerId = ''; + public string $dateCreated = ''; + public string $dateModified = ''; + public string $title = ''; /** @var array */ @@ -21,7 +24,7 @@ class Opml 'TITLE' => 'name', 'XMLURL' => 'feed', 'DESCRIPTION' => 'description', - 'ISDOWN' => 'isDown' + 'ISDOWN' => 'isDown', ); @@ -81,6 +84,12 @@ class Opml case 'OWNERID': $this->ownerId = $cdata; break; + case 'DATECREATED': + $this->dateCreated = $cdata; + break; + case 'DATEMODIFIED': + $this->dateModified = $cdata; + break; } } diff --git a/app/classes/OpmlManager.php b/app/classes/OpmlManager.php index cd3d685..679d1c4 100644 --- a/app/classes/OpmlManager.php +++ b/app/classes/OpmlManager.php @@ -3,7 +3,7 @@ class OpmlManager { - public static function load($file) + public static function load(string $file) : Opml { if (!file_exists($file)) { throw new Exception('OPML file not found!'); @@ -22,44 +22,63 @@ class OpmlManager return $opml; } - /** - * @param Opml $opml - * @param string $file - */ - public static function save($opml, $file) + public static function format(Opml $opml, $freezeDateModified = false) : string { - $out = ''."\n"; - $out.= ''."\n"; - $out.= ''."\n"; - $out.= ''.htmlspecialchars($opml->getTitle()).''."\n"; - $out.= ''.gmdate('c').''."\n"; - $out.= ''.gmdate('c').''."\n"; + $owner = ''; if ($opml->ownerName != '') { - $out.= ''.htmlspecialchars($opml->ownerName).''."\n"; + $owner .= ''.htmlspecialchars($opml->ownerName).''."\n"; } if ($opml->ownerEmail != '') { - $out.= ''.htmlspecialchars($opml->ownerEmail).''."\n"; + $owner .= ''.htmlspecialchars($opml->ownerEmail).''."\n"; } if ($opml->ownerId != '') { - $out.= ''.htmlspecialchars($opml->ownerId).''."\n"; + $owner .= ''.htmlspecialchars($opml->ownerId).''."\n"; } - $out.= 'http://opml.org/spec2.opml'."\n"; - $out.= ''."\n"; - $out.= ''."\n"; + $entries = ''; foreach ($opml->entries as $person) { - $out .= sprintf( - '', + $entries .= sprintf( + "\t" . '', htmlspecialchars($person['name'], ENT_QUOTES), htmlspecialchars($person['website'], ENT_QUOTES), htmlspecialchars($person['feed'], ENT_QUOTES), htmlspecialchars($person['isDown'] ?? '', ENT_QUOTES) ) . "\n"; } - $out.= ''."\n"; - $out.= ''; - file_put_contents($file, $out); + $template = << + + + %s + %s + %s + %s + http://opml.org/spec2.opml + + +%s + + +XML; + + return sprintf( + $template, + htmlspecialchars($opml->getTitle()), + $opml->dateCreated, + $freezeDateModified ? $opml->dateModified : date_format(date_create('now', new DateTimeZone('UTC')), DateTimeInterface::ATOM), + $owner, + $entries + ); + } + + /** + * @param Opml $opml + * @param string $file + */ + public static function save(Opml $opml, string $file) : int|bool + { + return file_put_contents($file, self::format($opml)); } public static function backup($file) diff --git a/phpunit.xml b/phpunit.xml index 334431c..8d463d4 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -7,13 +7,32 @@ beStrictAboutOutputDuringTests="true" beStrictAboutTestsThatDoNotTestAnything="true" beStrictAboutTodoAnnotatedTests="true" + cacheResultFile="tmp/.phpunit.cache/test-results" + colors="true" + convertErrorsToExceptions="true" + convertNoticesToExceptions="true" + convertWarningsToExceptions="true" verbose="true"> - + + + . + + vendor + + + + + + + - - tests - + + + + tests + + diff --git a/tests/CSRFTest.php b/tests/CSRFTest.php new file mode 100644 index 0000000..39fda58 --- /dev/null +++ b/tests/CSRFTest.php @@ -0,0 +1,36 @@ +temp_key = CSRF::getKey(); + $this->assertIsString($this->temp_key); + $this->assertEquals(32, strlen($this->temp_key)); + } + + public function testGenerate() + { + $token = CSRF::generate("some-action"); + $this->assertIsString($token); + $this->assertEquals(64, strlen($token)); + + $this->expectException(InvalidArgumentException::class); + CSRF::generate(); + CSRF::generate(12); + CSRF::generate(null); + } + + public function testVerify() + { + $token = CSRF::generate("some-action"); + $this->assertEquals(CSRF::verify($token, "some-action"), true); + $this->assertEquals(CSRF::verify($token, "other-action"), false); + $this->assertEquals(CSRF::verify("anything-else", "some-action"), false); + $this->assertEquals(CSRF::verify(1, "string"), false); + $this->assertEquals(CSRF::verify("string", 2), false); + $this->assertEquals(CSRF::verify(null, null), false); + } +} diff --git a/tests/HelpersTest.php b/tests/HelpersTest.php deleted file mode 100644 index a21ef1b..0000000 --- a/tests/HelpersTest.php +++ /dev/null @@ -1,11 +0,0 @@ -assertTrue(true); - } -} diff --git a/tests/OpmlManagerTest.php b/tests/OpmlManagerTest.php new file mode 100644 index 0000000..718e2f0 --- /dev/null +++ b/tests/OpmlManagerTest.php @@ -0,0 +1,53 @@ +fixtures = [ + [file_get_contents('./tests/opml/test-empty.opml'), [], '', '', '', ''], + [ + file_get_contents('./tests/opml/test-valid.opml'), + [ + [ + 'website' => 'https://blog.example.com/', + 'name' => 'text 1', + 'feed' => 'https://some.other.example.com/feed/path', + 'isDown' => '', + ], + [ + 'website' => 'https://blog2.example.com', + 'name' => 'text 2', + 'feed' => 'https://blog2.example.com/rss.xml', + 'isDown' => '', + ] + ], + 'Test OPML', + 'user name', + 'user@example.com', + 'http://user.example.com/' + ] + ]; + } + + public function testLoadValidFile() + { + $mngr = OpmlManager::load('tests/opml/test-valid.opml'); + $this->assertInstanceOf('Opml', $mngr); + } + + public function testLoadAbsentFile() + { + $this->expectException('Exception'); + OpmlManager::load('/some/where'); + } + + public function testFormat() + { + $file = 'tests/opml/test-valid.opml'; + $opml = OpmlManager::load($file); + $this->assertXmlStringEqualsXmlFile($file, OpmlManager::format($opml, true)); + } +} diff --git a/tests/OpmlTest.php b/tests/OpmlTest.php new file mode 100644 index 0000000..571fdaf --- /dev/null +++ b/tests/OpmlTest.php @@ -0,0 +1,53 @@ +fixtures = [ + [file_get_contents('./tests/opml/test-empty.opml'), [], '', '', '', ''], + [ + file_get_contents('./tests/opml/test-valid.opml'), + [ + [ + 'website' => 'https://blog.example.com/', + 'name' => 'text 1', + 'feed' => 'https://some.other.example.com/feed/path', + 'isDown' => '', + ], + [ + 'website' => 'https://blog2.example.com', + 'name' => 'text 2', + 'feed' => 'https://blog2.example.com/rss.xml', + 'isDown' => '', + ] + ], + 'Test OPML', + 'user name', + 'user@example.com', + 'http://user.example.com/' + ] + ]; + } + + public function testParse() + { + foreach ($this->fixtures as $data) { + $given = $data[0]; + $entries = $data[1]; + + $opml = new Opml(); + $entries = $opml->parse($given); + + $this->assertEquals($data[1], $entries); + $this->assertEquals($data[1], $opml->entries); + $this->assertEquals($data[1], $opml->getPeople()); + $this->assertEquals($data[2], $opml->getTitle()); + $this->assertEquals($data[3], $opml->ownerName); + $this->assertEquals($data[4], $opml->ownerEmail); + $this->assertEquals($data[5], $opml->ownerId); + } + } +} diff --git a/tests/PlanetConfigTest.php b/tests/PlanetConfigTest.php index 4db6e90..ddf7373 100644 --- a/tests/PlanetConfigTest.php +++ b/tests/PlanetConfigTest.php @@ -70,4 +70,11 @@ class PlanetConfigTest extends TestCase $conf = new PlanetConfig(['foo' => 'bar'], false); $this->assertEquals("---\nfoo: bar\n", $conf->toYaml()); } + + public function testConfigLoad() + { + $conf = PlanetConfig::load("."); + $default = new PlanetConfig(); + $this->assertEquals($default, $conf); + } } diff --git a/tests/PlanetTest.php b/tests/PlanetTest.php index d4b87b8..1bc699b 100644 --- a/tests/PlanetTest.php +++ b/tests/PlanetTest.php @@ -22,10 +22,12 @@ class FoolItem { protected $categories; - public function __construct($categories) + public function __construct($categories = null) { - foreach ($categories as $c) { - $this->categories[] = new FoolCategory($c); + if (is_array($categories)) { + foreach ($categories as $c) { + $this->categories[] = new FoolCategory($c); + } } } @@ -49,7 +51,8 @@ class PlanetTest extends TestCase new FoolItem(array('catA', 'catB', 'catC')), new FoolItem(array('catB')), new FoolItem(array('catA')), - new FoolItem(array('catC')) + new FoolItem(array('catC')), + new FoolItem(), ); } diff --git a/tests/opml/test-empty.opml b/tests/opml/test-empty.opml new file mode 100644 index 0000000..e69de29 diff --git a/tests/opml/test-valid.opml b/tests/opml/test-valid.opml new file mode 100644 index 0000000..686d19e --- /dev/null +++ b/tests/opml/test-valid.opml @@ -0,0 +1,16 @@ + + + +Test OPML +2022-01-14T17:15:05+00:00 +2022-01-14T17:15:05+00:00 +user name +user@example.com +http://user.example.com/ +http://opml.org/spec2.opml + + + + + + \ No newline at end of file -- cgit v1.2.1