Last of subscription tests

- Also tweaked SQL dateformat() function to output proper HTTP dates
- Also introduced method to set a default output date format

lib/Database.php

@@ -10,12 +10,11 @@ class Database {
     const FORMAT_DATE    = "Y-m-d";
     const FORMAT_TIME    = "h:i:s";
 
-    protected $data;
     public    $db;
-    private   $driver;
+    protected $dateFormatDefault = "sql";
 
     public function __construct() {
-        $this->driver = $driver = Data::$conf->dbDriver;
+        $driver = Data::$conf->dbDriver;
         $this->db = new $driver(INSTALL);
         $ver = $this->db->schemaVersion();
         if(!INSTALL && $ver < self::SCHEMA_VERSION) {
@@ -23,6 +22,19 @@ class Database {
         }
     }
 
+    protected function caller(): string {
+        return debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3)[2]['function'];
+    }
+
+    function dateFormatDefault(string $set = null): string {
+        if(is_null($set)) return $this->dateFormatDefault;
+        $set = strtolower($set);
+        if(in_array($set, ["sql", "iso8601", "unix", "http"])) {
+            $this->dateFormatDefault = $set;
+        }
+        return $this->dateFormatDefault;
+    }
+    
     static public function listDrivers(): array {
         $sep = \DIRECTORY_SEPARATOR;
         $path = __DIR__.$sep."Db".$sep;
@@ -341,33 +353,17 @@ class Database {
 
     public function folderPropertiesSet(string $user, int $id, array $data): bool {
         if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]);
-        // layer the existing folder properties onto the new desired ones; this also has the effect of checking whether the folder is valid
+        // validate the folder ID and, if specified, the parent to move it to
-        $data = array_merge($this->folderPropertiesGet($user, $id), $data);
+        if(array_key_exists("parent", $data)) {
-        // if the desired folder name is missing or invalid, throw an exception
+            $f = $this->folderValidateId($user, $id, $data['parent']);
-        if($data['name']=="") {
-            throw new Db\ExceptionInput("missing", ["action" => __FUNCTION__, "field" => "name"]);
-        } else if(!strlen(trim($data['name']))) {
-            throw new Db\ExceptionInput("whitespace", ["action" => __FUNCTION__, "field" => "name"]);
-        }
-        // normalize folder's parent, if there is one
-        $parent = array_key_exists("parent", $data) ? (int) $data['parent'] : 0;
-        if($parent===0) {
-            // if no parent is specified, do nothing
-            $parent = null;
         } else {
-            // if a parent is specified, make sure it exists and belongs to the user
+            $f = $this->folderValidateId($user, $id);
-            $p = $this->db->prepare(
-                "WITH RECURSIVE folders(id) as (SELECT id from arsse_folders where owner is ? and id is ? union select arsse_folders.id from arsse_folders join folders on arsse_folders.parent=folders.id) ".
-                "SELECT id,(id not in (select id from folders)) as valid from arsse_folders where owner is ? and id is ?",
-            "str", "int", "str", "int")->run($user, $id, $user, $parent)->getRow();
-            if(!$p) {
-                throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]);
-            } else {
-                // if using the desired parent would create a circular dependence, throw an exception
-                if(!$p['valid']) throw new Db\ExceptionInput("circularDependence", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]);
         }
+        // if a new name is specified, validate it
+        if(array_key_exists("name", $data)) {
+            $this->folderValidateName($data['name']);
         }
-        $data['parent'] = $parent;
+        $data = array_merge($f, $data);
         // check to make sure the target folder name/location would not create a duplicate (we must do this check because null is not distinct in SQL)
         $existing = $this->db->prepare("SELECT id from arsse_folders where owner is ? and parent is ? and name is ?", "str", "int", "str")->run($user, $data['parent'], $data['name'])->getValue();
         if(!is_null($existing) && $existing != $id) {
@@ -381,6 +377,47 @@ class Database {
         return (bool) $this->db->prepare("UPDATE arsse_folders set $setClause where owner is ? and id is ?", $setTypes, "str", "int")->run($setValues, $user, $id)->changes();
     }
 
+    protected function folderValidateId(string $user, int $id = null, int $parent = null): array {
+        if(is_null($id)) {
+            // if no ID is specified this is a no-op, unless a parent is specified, which is always a circular dependence
+            if(!is_null($parent)) {
+                throw new Db\ExceptionInput("circularDependence", ["action" => $this->caller(), "field" => "parent", 'id' => $parent]);
+            }
+            return [name => null, parent => null];
+        }
+        // check whether the folder exists and is owned by the user
+        $f = $this->db->prepare("SELECT name,parent from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->getRow();
+        if(!$f) throw new Db\ExceptionInput("idMissing", ["action" => $this->caller(), "field" => "folder", 'id' => $parent]);
+        // if we're moving a folder to a new parent, check that the parent is valid
+        if(!is_null($parent)) {
+            // make sure both that the parent exists, and that the parent is not either the folder itself or one of its children (a circular dependence)
+            $p = $this->db->prepare(
+                "WITH RECURSIVE folders(id) as (SELECT id from arsse_folders where owner is ? and id is ? union select arsse_folders.id from arsse_folders join folders on arsse_folders.parent=folders.id) ".
+                "SELECT id,(id not in (select id from folders)) as valid from arsse_folders where owner is ? and id is ?",
+                "str", "int", "str", "int"
+            )->run($user, $id, $user, $parent)->getRow();
+            if(!$p) {
+                // if the parent doesn't exist or doesn't below to the user, throw an exception
+                throw new Db\ExceptionInput("idMissing", ["action" => $this->caller(), "field" => "parent", 'id' => $parent]);
+            } else {
+                // if using the desired parent would create a circular dependence, throw a different exception
+                if(!$p['valid']) throw new Db\ExceptionInput("circularDependence", ["action" => $this->caller(), "field" => "parent", 'id' => $parent]);
+            }
+        }
+        return $f;
+    }
+
+    protected function folderValidateName($name): bool {
+        $name = (string) $name;
+        if($name=="") {
+            throw new Db\ExceptionInput("missing", ["action" => $this->caller(), "field" => "name"]);
+        } else if(!strlen(trim($name))) {
+            throw new Db\ExceptionInput("whitespace", ["action" => $this->caller(), "field" => "name"]);
+        } else {
+            return true;
+        }
+    }
+
     public function subscriptionAdd(string $user, string $url, string $fetchUser = "", string $fetchPassword = ""): int {
         if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]);
         // check to see if the feed exists
@@ -407,23 +444,25 @@ class Database {
         $query = 
             "SELECT 
                 arsse_subscriptions.id,
-                url,favicon,source,folder,added,pinned,err_count,err_msg,order_type,
+                url,favicon,source,folder,pinned,err_count,err_msg,order_type,
+                DATEFORMAT(?, added) as added,
                 CASE WHEN arsse_subscriptions.title is not null THEN arsse_subscriptions.title ELSE arsse_feeds.title END as title,
-                (SELECT count(*) from arsse_articles where feed is arsse_subscriptions.feed) - (SELECT count(*) from (SELECT article,feed from arsse_marks join arsse_articles on article = arsse_articles.id where owner is ? and feed is arsse_feeds.id and read is 1)) as unread
+                (SELECT count(*) from arsse_articles where feed is arsse_subscriptions.feed) - (SELECT count(*) from arsse_marks join arsse_articles on article = arsse_articles.id where owner is ? and feed is arsse_feeds.id and read is 1) as unread
              from arsse_subscriptions join arsse_feeds on feed = arsse_feeds.id where owner is ?";
+        $queryOrder = "order by pinned desc, title";
+        $queryTypes = ["str", "str", "str"];
+        $queryValues = [$this->dateFormatDefault, $user, $user];
         if(!is_null($folder)) {
-            if(!$this->db->prepare("SELECT count(*) from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $folder)->getValue()) {
+            $this->folderValidateId($user, $folder);
-                throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $folder]);
-            }
             return $this->db->prepare(
-                "WITH RECURSIVE folders(folder) as (SELECT ? union select id from arsse_folders join folders on parent is folder) $query and folder in (select folder from folders)", 
+                "WITH RECURSIVE folders(folder) as (SELECT ? union select id from arsse_folders join folders on parent is folder) $query and folder in (select folder from folders) $queryOrder", 
-                "int", "str", "str"
+                "int", $queryTypes
-            )->run($folder, $user, $user);
+            )->run($folder, $queryValues);
         } else if(!is_null($id)) {
             // this condition facilitates the implementation of subscriptionPropertiesGet, which would otherwise have to duplicate the complex query
-            return $this->db->prepare("$query and arsse_subscriptions.id is ?", "str", "str", "int")->run($user, $user, $id);
+            return $this->db->prepare("$query and arsse_subscriptions.id is ? $queryOrder", $queryTypes, "int")->run($queryValues, $id);
         } else {
-            return $this->db->prepare($query, "str", "str")->run($user, $user);
+            return $this->db->prepare("$query $queryOrder", $queryTypes)->run($queryValues);
         }
     }
 
@@ -451,6 +490,17 @@ class Database {
             // if the ID doesn't exist or doesn't belong to the user, throw an exception
             throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]);
         }
+        if(array_key_exists("folder", $data)) {
+            // ensure the target folder exists and belong to the user
+            $this->folderValidateId($user, $data['folder']);
+        }
+        if(array_key_exists("title", $data)) {
+            // if the title is effectively an empty string, change it to null so that the feed title is used instead
+            $title = (string) $data['title'];
+            $title = trim($title);
+            if($title==="") $title = null;
+            $data['title'] = $title;
+        }
         $valid = [
             'title'      => "str",
             'folder'     => "int",

lib/Db/SQLite3/CustomFunctions.php

@@ -5,22 +5,22 @@ namespace JKingWeb\Arsse\Db\SQLite3;
 class CustomFunctions {
     protected static $tz;
     
-    // Converts from SQLite3's date format to a specified standard date format.
+    // Converts from SQL date format to a specified standard date format.
     public static function dateFormat(string $format, $date) {
+        $format = strtolower($format);
+        if($format=="sql") return $date;
         settype($date, "string");
         if($date=="") return null;
         if(is_null(self::$tz)) self::$tz = new \DateTimeZone("UTC");
         $date = \DateTime::createFromFormat('Y-m-d H:i:s', $date, self::$tz);
-        $format = strtolower($format);
         switch ($format) {
             case 'unix': 
                 return $date->getTimestamp();
-            case 'rfc822':
             case 'http': 
-                return $date->format(\DateTime::RFC822);
+                return $date->format("D, d M Y H:i:s \G\M\T");
             case 'iso8601':
             default: 
-                return $date->format(\DateTime::ISO8601);
+                return $date->format(\DateTime::ATOM);
         }
     }
 }

tests/lib/Database/SeriesSubscription.php

@@ -31,11 +31,13 @@ trait SeriesSubscription {
                     'feed'       => "int",
                     'title'      => "str",
                     'folder'     => "int",
+                    'pinned'     => "bool",
+                    'order_type' => "int",
                 ],
                 'rows' => [
-                    [1,"john.doe@example.com",2,null,null],
+                    [1,"john.doe@example.com",2,null,null,1,2],
-                    [2,"jane.doe@example.com",2,null,null],
+                    [2,"jane.doe@example.com",2,null,null,0,0],
-                    [3,"john.doe@example.com",3,"Ook",2],
+                    [3,"john.doe@example.com",3,"Ook",2,0,1],
                 ]
             ],
             'arsse_articles' => [
@@ -66,17 +68,17 @@ trait SeriesSubscription {
                     'starred' => "bool",
                 ],
                 'rows' => [
-                    [1,1,"jane.doe@example.com",true,false],
+                    [1,1,"jane.doe@example.com",1,0],
-                    [2,2,"jane.doe@example.com",true,false],
+                    [2,2,"jane.doe@example.com",1,0],
-                    [3,3,"jane.doe@example.com",true,false],
+                    [3,3,"jane.doe@example.com",1,0],
-                    [4,4,"jane.doe@example.com",true,false],
+                    [4,4,"jane.doe@example.com",1,0],
-                    [5,5,"jane.doe@example.com",true,false],
+                    [5,5,"jane.doe@example.com",1,0],
-                    [6,6,"jane.doe@example.com",true,false],
+                    [6,6,"jane.doe@example.com",1,0],
-                    [7,7,"jane.doe@example.com",true,false],
+                    [7,7,"jane.doe@example.com",1,0],
-                    [8,8,"jane.doe@example.com",true,false],
+                    [8,8,"jane.doe@example.com",1,0],
-                    [9, 1,"john.doe@example.com",true,false],
+                    [9, 1,"john.doe@example.com",1,0],
-                    [10,7,"john.doe@example.com",true,false],
+                    [10,7,"john.doe@example.com",1,0],
-                    [11,8,"john.doe@example.com",false,false],
+                    [11,8,"john.doe@example.com",0,0],
                 ]
             ],
         ];
@@ -85,51 +87,49 @@ trait SeriesSubscription {
         $this->primeDatabase($this->data);
         // initialize a partial mock of the Database object to later manipulate the feedUpdate method
         Data::$db = Phake::PartialMock(Database::class, $this->drv);
+        $this->user = "john.doe@example.com";
     }
 
     function testAddASubscriptionToAnExistingFeed() {
-        $user = "john.doe@example.com";
         $url = "http://example.com/feed1";
         $subID = $this->nextID("arsse_subscriptions");
         Phake::when(Data::$db)->feedUpdate->thenReturn(true);
-        $this->assertSame($subID,Data::$db->subscriptionAdd($user, $url));
+        $this->assertSame($subID,Data::$db->subscriptionAdd($this->user, $url));
-        Phake::verify(Data::$user)->authorize($user, "subscriptionAdd");
+        Phake::verify(Data::$user)->authorize($this->user, "subscriptionAdd");
         Phake::verify(Data::$db, Phake::times(0))->feedUpdate(1, true);
         $state = $this->primeExpectations($this->data, [
             'arsse_feeds'         => ['id','url','username','password'],
             'arsse_subscriptions' => ['id','owner','feed'],
         ]);
-        $state['arsse_subscriptions']['rows'][] = [$subID,$user,1];
+        $state['arsse_subscriptions']['rows'][] = [$subID,$this->user,1];
         $this->compareExpectations($state);
     }
 
     function testAddASubscriptionToANewFeed() {
-        $user = "john.doe@example.com";
         $url = "http://example.org/feed1";
         $feedID = $this->nextID("arsse_feeds");
         $subID = $this->nextID("arsse_subscriptions");
         Phake::when(Data::$db)->feedUpdate->thenReturn(true);
-        $this->assertSame($subID,Data::$db->subscriptionAdd($user, $url));
+        $this->assertSame($subID,Data::$db->subscriptionAdd($this->user, $url));
-        Phake::verify(Data::$user)->authorize($user, "subscriptionAdd");
+        Phake::verify(Data::$user)->authorize($this->user, "subscriptionAdd");
         Phake::verify(Data::$db)->feedUpdate($feedID, true);
         $state = $this->primeExpectations($this->data, [
             'arsse_feeds'         => ['id','url','username','password'],
             'arsse_subscriptions' => ['id','owner','feed'],
         ]);
         $state['arsse_feeds']['rows'][] = [$feedID,$url,"",""];
-        $state['arsse_subscriptions']['rows'][] = [$subID,$user,$feedID];
+        $state['arsse_subscriptions']['rows'][] = [$subID,$this->user,$feedID];
         $this->compareExpectations($state);
     }
 
     function testAddASubscriptionToAnInvalidFeed() {
-        $user = "john.doe@example.com";
         $url = "http://example.org/feed1";
         $feedID = $this->nextID("arsse_feeds");
         Phake::when(Data::$db)->feedUpdate->thenThrow(new FeedException($url, new \PicoFeed\Client\InvalidUrlException()));
         try {
-            Data::$db->subscriptionAdd($user, $url);
+            Data::$db->subscriptionAdd($this->user, $url);
         } catch(FeedException $e) {
-            Phake::verify(Data::$user)->authorize($user, "subscriptionAdd");
+            Phake::verify(Data::$user)->authorize($this->user, "subscriptionAdd");
             Phake::verify(Data::$db)->feedUpdate($feedID, true);
             $state = $this->primeExpectations($this->data, [
                 'arsse_feeds'         => ['id','url','username','password'],
@@ -142,24 +142,21 @@ trait SeriesSubscription {
     }
 
     function testAddADuplicateSubscription() {
-        $user = "john.doe@example.com";
         $url = "http://example.com/feed2";
         $this->assertException("constraintViolation", "Db", "ExceptionInput");
-        Data::$db->subscriptionAdd($user, $url);
+        Data::$db->subscriptionAdd($this->user, $url);
     }
 
     function testAddASubscriptionWithoutAuthority() {
-        $user = "john.doe@example.com";
         $url = "http://example.com/feed1";
         Phake::when(Data::$user)->authorize->thenReturn(false);
         $this->assertException("notAuthorized", "User", "ExceptionAuthz");
-        Data::$db->subscriptionAdd($user, $url);
+        Data::$db->subscriptionAdd($this->user, $url);
     }
 
     function testRemoveASubscription() {
-        $user = "john.doe@example.com";
+        $this->assertTrue(Data::$db->subscriptionRemove($this->user, 1));
-        $this->assertTrue(Data::$db->subscriptionRemove($user, 1));
+        Phake::verify(Data::$user)->authorize($this->user, "subscriptionRemove");
-        Phake::verify(Data::$user)->authorize($user, "subscriptionRemove");
         $state = $this->primeExpectations($this->data, [
             'arsse_feeds'         => ['id','url','username','password'],
             'arsse_subscriptions' => ['id','owner','feed'],
@@ -169,51 +166,121 @@ trait SeriesSubscription {
     }
 
     function testRemoveAMissingSubscription() {
-        $user = "john.doe@example.com";
         $this->assertException("idMissing", "Db", "ExceptionInput");
-        Data::$db->subscriptionRemove($user, 2112);
+        Data::$db->subscriptionRemove($this->user, 2112);
     }
 
     function testRemoveASubscriptionForTheWrongOwner() {
-        $user = "jane.doe@example.com";
+        $this->user = "jane.doe@example.com";
         $this->assertException("idMissing", "Db", "ExceptionInput");
-        Data::$db->subscriptionRemove($user, 1);
+        Data::$db->subscriptionRemove($this->user, 1);
     }
 
     function testRemoveASubscriptionWithoutAuthority() {
         Phake::when(Data::$user)->authorize->thenReturn(false);
         $this->assertException("notAuthorized", "User", "ExceptionAuthz");
-        Data::$db->subscriptionRemove("john.doe@example.com", 1);
+        Data::$db->subscriptionRemove($this->user, 1);
     }
 
     function testListSubscriptions() {
-        $user = "john.doe@example.com";
         $exp = [
             [
                 'url'        => "http://example.com/feed2",
                 'title'      => "Eek",
                 'folder'     => null,
                 'unread'     => 4,
+                'pinned'     => 1,
+                'order_type' => 2,
             ],
             [
                 'url'        => "http://example.com/feed3",
                 'title'      => "Ook",
                 'folder'     => 2,
                 'unread'     => 2,
+                'pinned'     => 0,
+                'order_type' => 1,
             ],
         ];
-        $this->assertResult($exp, Data::$db->subscriptionList($user));
+        $this->assertResult($exp, Data::$db->subscriptionList($this->user));
+        Phake::verify(Data::$user)->authorize($this->user, "subscriptionList");
+        $this->assertArraySubset($exp[0], Data::$db->subscriptionPropertiesGet($this->user, 1));
+        Phake::verify(Data::$user)->authorize($this->user, "subscriptionPropertiesGet");
+        $this->assertArraySubset($exp[1], Data::$db->subscriptionPropertiesGet($this->user, 3));
     }
 
     function testListSubscriptionsInAFolder() {
-        $user = "john.doe@example.com";
         $exp = [
             [
                 'url'        => "http://example.com/feed3",
                 'title'      => "Ook",
                 'folder'     => 2,
+                'unread'     => 2,
+                'pinned'     => 0,
+                'order_type' => 1,
             ],
         ];
-        $this->assertResult($exp, Data::$db->subscriptionList($user, 2));
+        $this->assertResult($exp, Data::$db->subscriptionList($this->user, 2));
+    }
+
+    function testListSubscriptionsWithDifferentDateFormats() {
+        Data::$db->dateFormatDefault("iso8601");
+        $d1 = Data::$db->subscriptionList($this->user, 2)->getRow()['added'];
+        Data::$db->dateFormatDefault("http");
+        $d2 = Data::$db->subscriptionList($this->user, 2)->getRow()['added'];
+        $this->assertNotEquals($d1, $d2);
+    }
+
+    function testListSubscriptionsInAMissingFolder() {
+        $this->assertException("idMissing", "Db", "ExceptionInput");
+        Data::$db->subscriptionList($this->user, 4);
+    }
+
+    function testListSubscriptionsWithoutAuthority() {
+        Phake::when(Data::$user)->authorize->thenReturn(false);
+        $this->assertException("notAuthorized", "User", "ExceptionAuthz");
+        Data::$db->subscriptionList($this->user);
+    }
+
+    function testSetThePropertiesOfASubscription() {
+        Data::$db->subscriptionPropertiesSet($this->user, 1,[
+            'title' => "Ook Ook",
+            'folder' => 3,
+            'pinned' => false,
+            'order_type' => 0,
+        ]);
+        Phake::verify(Data::$user)->authorize($this->user, "subscriptionPropertiesSet");
+        $state = $this->primeExpectations($this->data, [
+            'arsse_feeds'         => ['id','url','username','password','title'],
+            'arsse_subscriptions' => ['id','owner','feed','title','folder','pinned','order_type'],
+        ]);
+        $state['arsse_subscriptions']['rows'][0] = [1,"john.doe@example.com",2,"Ook Ook",3,0,0];
+        $this->compareExpectations($state);
+        Data::$db->subscriptionPropertiesSet($this->user, 1,[
+            'title' => "          ",
+        ]);
+        $state['arsse_subscriptions']['rows'][0] = [1,"john.doe@example.com",2,null,3,0,0];
+        $this->compareExpectations($state);
+    }
+
+    function testMoveSubscriptionToAMissingFolder() {
+        $this->assertException("idMissing", "Db", "ExceptionInput");
+        Data::$db->subscriptionPropertiesSet($this->user, 1,[
+            'folder' => 4,
+        ]);
+    }
+
+    function testSetThePropertiesOfAMissingSubscription() {
+        $this->assertException("idMissing", "Db", "ExceptionInput");
+        Data::$db->subscriptionPropertiesSet($this->user, 2112,[
+            'folder' => null,
+        ]);
+    }
+
+    function testSetThePropertiesOfASubscriptionWithoutAuthority() {
+        Phake::when(Data::$user)->authorize->thenReturn(false);
+        $this->assertException("notAuthorized", "User", "ExceptionAuthz");
+        Data::$db->subscriptionPropertiesSet($this->user, 1,[
+            'folder' => null,
+        ]);
     }
 }

tests/lib/Database/Setup.php

@@ -103,7 +103,7 @@ trait Setup {
                 foreach($values as $key => $value) {
                     $row[$cols[$key]] = $value;
                 }
-                $found = array_search($row, $data);
+                $found = array_search($row, $data, true);
                 $this->assertNotSame(false, $found, "Table $table does not contain record at array index $index.");
                 unset($data[$found]);
             }