From c393dfc42b95f46c626d866bef178eea94001378 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 5 Sep 2017 19:35:14 -0400 Subject: [PATCH] Sundry fixes - Make use of PHP 7's null coalescing operator - remove use of static property in Lang class - Improve code coverage slightly --- lib/CLI.php | 8 ++------ lib/Db/AbstractDriver.php | 8 ++------ lib/Db/SQLite3/Driver.php | 11 ++++------- lib/Db/SQLite3/Result.php | 2 +- lib/Db/SQLite3/Statement.php | 2 +- lib/Lang.php | 15 ++++++++------- lib/REST/NextCloudNews/V1_2.php | 4 +++- lib/REST/Request.php | 12 +++--------- tests/Lang/testLangComplex.php | 2 +- tests/REST/NextCloudNews/TestNCNV1_2.php | 2 ++ 10 files changed, 27 insertions(+), 39 deletions(-) diff --git a/lib/CLI.php b/lib/CLI.php index b62e4f4b..56dfbad1 100644 --- a/lib/CLI.php +++ b/lib/CLI.php @@ -23,9 +23,7 @@ USAGE_TEXT; } public function __construct(array $argv = null) { - if (is_null($argv)) { - $argv = array_slice($_SERVER['argv'], 1); - } + $argv = $argv ?? array_slice($_SERVER['argv'], 1); $this->args = \Docopt::handle($this->usage(), [ 'argv' => $argv, 'help' => true, @@ -46,9 +44,7 @@ USAGE_TEXT; public function dispatch(array $args = null): int { // act on command line - if (is_null($args)) { - $args = $this->args; - } + $args = $args ?? $this->args; if ($this->command("daemon", $args)) { $this->loadConf(); return $this->daemon(); diff --git a/lib/Db/AbstractDriver.php b/lib/Db/AbstractDriver.php index 7092aa37..b49ceb14 100644 --- a/lib/Db/AbstractDriver.php +++ b/lib/Db/AbstractDriver.php @@ -36,9 +36,7 @@ abstract class AbstractDriver implements Driver { } public function savepointRelease(int $index = null): bool { - if (is_null($index)) { - $index = $this->transDepth; - } + $index = $index ?? $this->transDepth; if (array_key_exists($index, $this->transStatus)) { switch ($this->transStatus[$index]) { case self::TR_PEND: @@ -83,9 +81,7 @@ abstract class AbstractDriver implements Driver { } public function savepointUndo(int $index = null): bool { - if (is_null($index)) { - $index = $this->transDepth; - } + $index = $index ?? $this->transDepth; if (array_key_exists($index, $this->transStatus)) { switch ($this->transStatus[$index]) { case self::TR_PEND: diff --git a/lib/Db/SQLite3/Driver.php b/lib/Db/SQLite3/Driver.php index 02f96003..b47339e6 100644 --- a/lib/Db/SQLite3/Driver.php +++ b/lib/Db/SQLite3/Driver.php @@ -20,12 +20,9 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { // check to make sure required extension is loaded if (!class_exists("SQLite3")) { throw new Exception("extMissing", self::driverName()); // @codeCoverageIgnore - } - $dbFile = Arsse::$conf->dbSQLite3File; - if (is_null($dbFile)) { - // if no database file is specified in the configuration, use a suitable default - $dbFile = \JKingWeb\Arsse\BASE."arsse.db"; - } + } + // if no database file is specified in the configuration, use a suitable default + $dbFile = Arsse::$conf->dbSQLite3File ?? \JKingWeb\Arsse\BASE."arsse.db"; $mode = \SQLITE3_OPEN_READWRITE | \SQLITE3_OPEN_CREATE; $timeout = Arsse::$conf->dbSQLite3Timeout * 1000; try { @@ -66,7 +63,7 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { public function __destruct() { try { $this->db->close(); - } catch (\Exception $e) { + } catch (\Exception $e) { // @codeCoverageIgnore } unset($this->db); } diff --git a/lib/Db/SQLite3/Result.php b/lib/Db/SQLite3/Result.php index 477e4604..53b79795 100644 --- a/lib/Db/SQLite3/Result.php +++ b/lib/Db/SQLite3/Result.php @@ -54,7 +54,7 @@ class Result implements \JKingWeb\Arsse\Db\Result { public function __destruct() { try { $this->set->finalize(); - } catch (\Throwable $e) { + } catch (\Throwable $e) { // @codeCoverageIgnore } unset($this->set); } diff --git a/lib/Db/SQLite3/Statement.php b/lib/Db/SQLite3/Statement.php index 6ca47875..ddf879e3 100644 --- a/lib/Db/SQLite3/Statement.php +++ b/lib/Db/SQLite3/Statement.php @@ -36,7 +36,7 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { public function __destruct() { try { $this->st->close(); - } catch (\Throwable $e) { + } catch (\Throwable $e) { // @codeCoverageIgnore } unset($this->st); } diff --git a/lib/Lang.php b/lib/Lang.php index 60411cae..c1fdf972 100644 --- a/lib/Lang.php +++ b/lib/Lang.php @@ -16,7 +16,7 @@ class Lang { ]; public $path; // path to locale files; this is a public property to facilitate unit testing - protected static $requirementsMet = false; // whether the Intl extension is loaded + protected $requirementsMet = false; // whether the Intl extension is loaded protected $synched = false; // whether the wanted locale is actually loaded (lazy loading is used by default) protected $wanted = self::DEFAULT; // the currently requested locale protected $locale = ""; // the currently loaded locale @@ -29,8 +29,8 @@ class Lang { public function set(string $locale, bool $immediate = false): string { // make sure the Intl extension is loaded - if (!static::$requirementsMet) { - static::checkRequirements(); + if (!$this->requirementsMet) { + $this->checkRequirements(); } // if requesting the same locale as already wanted, just return (but load first if we've requested an immediate load) if ($locale==$this->wanted) { @@ -121,14 +121,15 @@ class Lang { return \Locale::lookup($list, $locale, true, $default); } - protected static function checkRequirements(): bool { + protected function checkRequirements(): bool { if (!extension_loaded("intl")) { throw new ExceptionFatal("The \"Intl\" extension is required, but not loaded"); } - static::$requirementsMet = true; + $this->requirementsMet = true; return true; } + /** @codeCoverageIgnore */ protected function globFiles(string $path): array { // we wrap PHP's glob function in this method so that unit tests may override it return glob($path."*.php"); @@ -148,8 +149,8 @@ class Lang { } protected function load(): bool { - if (!self::$requirementsMet) { - self::checkRequirements(); + if (!$this->requirementsMet) { + $this->checkRequirements(); } // if we've requested no locale (""), just load the fallback strings and return if ($this->wanted=="") { diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index fdcc8a77..f17a298d 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -73,11 +73,12 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { return new Response(405, "", "", ["Allow: ".$e->getMessage()]); } if (!method_exists($this, $func)) { - return new Response(501); + return new Response(501); // @codeCoverageIgnore } // dispatch try { return $this->$func($req->paths, $data); + // @codeCoverageIgnoreStart } catch (Exception $e) { // if there was a REST exception return 400 return new Response(400); @@ -85,6 +86,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // if there was any other Arsse exception return 500 return new Response(500); } + // @codeCoverageIgnoreEnd } protected function chooseCall(array $url, string $method): string { diff --git a/lib/REST/Request.php b/lib/REST/Request.php index b767b567..bdd3a39f 100644 --- a/lib/REST/Request.php +++ b/lib/REST/Request.php @@ -12,15 +12,9 @@ class Request { public $body = ""; public function __construct(string $method = null, string $url = null, string $body = null, string $contentType = null) { - if (is_null($method)) { - $method = $_SERVER['REQUEST_METHOD']; - } - if (is_null($url)) { - $url = $_SERVER['REQUEST_URI']; - } - if (is_null($body)) { - $body = file_get_contents("php://input"); - } + $method = $method ?? $_SERVER['REQUEST_METHOD']; + $url = $url ?? $_SERVER['REQUEST_URI']; + $body = $body ?? file_get_contents("php://input"); if (is_null($contentType)) { if (isset($_SERVER['HTTP_CONTENT_TYPE'])) { $contentType = $_SERVER['HTTP_CONTENT_TYPE']; diff --git a/tests/Lang/testLangComplex.php b/tests/Lang/testLangComplex.php index 99c6b548..137d0016 100644 --- a/tests/Lang/testLangComplex.php +++ b/tests/Lang/testLangComplex.php @@ -47,7 +47,7 @@ class TestLangComplex extends Test\AbstractTest { } public function testFetchAMessage() { - $this->l->set("de", true); + $this->l->set("de"); $this->assertEquals('und der Stein der Weisen', $this->l->msg('Test.presentText')); } diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index bd78bb9c..a137df97 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -737,6 +737,8 @@ class TestNCNV1_2 extends Test\AbstractTest { Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), $this->anything())->thenReturn(true); Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->editions([]))->thenThrow(new ExceptionInput("tooShort")); // data model function requires one valid integer for multiples Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->editions($in[1]))->thenThrow(new ExceptionInput("tooLong")); // data model function limited to 50 items for multiples + Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->articles([]))->thenThrow(new ExceptionInput("tooShort")); // data model function requires one valid integer for multiples + Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->articles($in[1]))->thenThrow(new ExceptionInput("tooLong")); // data model function limited to 50 items for multiples $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple"))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple")));