1
1
Fork 0
mirror of https://code.mensbeam.com/MensBeam/Arsse.git synced 2024-12-22 21:22:40 +00:00

Remove root field from folders table

The field is no longer required with the use of recursive common table expressions, and  presents a possible loss of referential integrity
This commit is contained in:
J. King 2017-04-01 14:49:31 -04:00
parent a111bcc231
commit 1e1b848c62
4 changed files with 31 additions and 39 deletions

View file

@ -294,16 +294,10 @@ class Database {
if($parent===0) { if($parent===0) {
// if no parent is specified, do nothing // if no parent is specified, do nothing
$parent = null; $parent = null;
$root = null;
} else { } else {
// if a parent is specified, make sure it exists and belongs to the user; get its root (first-level) folder if it's a nested folder // if a parent is specified, make sure it exists and belongs to the user; get its root (first-level) folder if it's a nested folder
$p = $this->db->prepare("SELECT id,root from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $parent)->getRow(); $p = $this->db->prepare("SELECT id from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $parent)->getValue();
if(!$p) { if(!$p) throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]);
throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]);
} else {
// if the parent does not have a root specified (because it is a first-level folder) use the parent ID as the root ID
$root = $p['root']===null ? $parent : $p['root'];
}
} }
// check if a folder by the same name already exists, because nulls are wonky in SQL // check if a folder by the same name already exists, because nulls are wonky in SQL
// FIXME: How should folder name be compared? Should a Unicode normalization be applied before comparison and insertion? // FIXME: How should folder name be compared? Should a Unicode normalization be applied before comparison and insertion?
@ -311,7 +305,7 @@ class Database {
throw new Db\ExceptionInput("constraintViolation"); // FIXME: There needs to be a practical message here throw new Db\ExceptionInput("constraintViolation"); // FIXME: There needs to be a practical message here
} }
// actually perform the insert (!) // actually perform the insert (!)
return $this->db->prepare("INSERT INTO arsse_folders(owner,parent,root,name) values(?,?,?,?)", "str", "int", "int", "str")->run($user, $parent, $root, $data['name'])->lastId(); return $this->db->prepare("INSERT INTO arsse_folders(owner,parent,name) values(?,?,?)", "str", "int", "str")->run($user, $parent, $data['name'])->lastId();
} }
public function folderList(string $user, int $parent = null, bool $recursive = true): Db\Result { public function folderList(string $user, int $parent = null, bool $recursive = true): Db\Result {
@ -372,24 +366,20 @@ class Database {
if($parent===0) { if($parent===0) {
// if no parent is specified, do nothing // if no parent is specified, do nothing
$parent = null; $parent = null;
$root = null;
} else { } else {
// if a parent is specified, make sure it exists and belongs to the user; get its root (first-level) folder if it's a nested folder // if a parent is specified, make sure it exists and belongs to the user; get its root (first-level) folder if it's a nested folder
$p = $this->db->prepare( $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) ". "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,root,(id not in (select id from folders)) as valid from arsse_folders where owner is ? and id is ?", "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(); "str", "int", "str", "int")->run($user, $id, $user, $parent)->getRow();
if(!$p) { if(!$p) {
throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]); throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]);
} else { } else {
// if using the desired parent would create a circular dependence, throw a constraint violation // 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(!$p['valid']) throw new Db\ExceptionInput("circularDependence", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]);
// if the parent does not have a root specified (because it is a first-level folder) use the parent ID as the root ID
$root = $p['root']===null ? $parent : $p['root'];
} }
} }
$data['parent'] = $parent; $data['parent'] = $parent;
$data['root'] = $root;
// check to make sure the target folder name/location would not create a duplicate (we must di this check because null is not distinct in SQL) // check to make sure the target folder name/location would not create a duplicate (we must di 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(); $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) { if(!is_null($existing) && $existing != $id) {
@ -398,7 +388,6 @@ class Database {
$valid = [ $valid = [
'name' => "str", 'name' => "str",
'parent' => "int", 'parent' => "int",
'root' => "int",
]; ];
$data = $this->processUpdate($data, $valid, ['owner' => [$user, "str"], 'id' => [$id, "int"]]); $data = $this->processUpdate($data, $valid, ['owner' => [$user, "str"], 'id' => [$id, "int"]]);
extract($data); extract($data);

View file

@ -54,7 +54,6 @@ create table arsse_folders(
id integer primary key not null, -- sequence number id integer primary key not null, -- sequence number
owner TEXT not null references arsse_users(id) on delete cascade on update cascade, -- owner of folder owner TEXT not null references arsse_users(id) on delete cascade on update cascade, -- owner of folder
parent integer references arsse_folders(id) on delete cascade, -- parent folder id parent integer references arsse_folders(id) on delete cascade, -- parent folder id
root integer references arsse_folders(id) on delete cascade, -- first-level folder (NextCloud folder)
name TEXT not null, -- folder name name TEXT not null, -- folder name
modified datetime not null default CURRENT_TIMESTAMP, -- modified datetime not null default CURRENT_TIMESTAMP, --
unique(owner,name,parent) -- cannot have multiple folders with the same name under the same parent for the same owner unique(owner,name,parent) -- cannot have multiple folders with the same name under the same parent for the same owner

View file

@ -13,7 +13,6 @@ trait SeriesFolder {
'id' => "int", 'id' => "int",
'owner' => "str", 'owner' => "str",
'parent' => "int", 'parent' => "int",
'root' => "int",
'name' => "str", 'name' => "str",
], ],
/* Layout translates to: /* Layout translates to:
@ -27,12 +26,12 @@ trait SeriesFolder {
Politics Politics
*/ */
'rows' => [ 'rows' => [
[1, "john.doe@example.com", null, null, "Technology"], [1, "john.doe@example.com", null, "Technology"],
[2, "john.doe@example.com", 1, 1, "Software"], [2, "john.doe@example.com", 1, "Software"],
[3, "john.doe@example.com", 1, 1, "Rocketry"], [3, "john.doe@example.com", 1, "Rocketry"],
[4, "jane.doe@example.com", null, null, "Politics"], [4, "jane.doe@example.com", null, "Politics"],
[5, "john.doe@example.com", null, null, "Politics"], [5, "john.doe@example.com", null, "Politics"],
[6, "john.doe@example.com", 2, 1, "Politics"], [6, "john.doe@example.com", 2, "Politics"],
] ]
] ]
]; ];
@ -45,8 +44,8 @@ trait SeriesFolder {
$user = "john.doe@example.com"; $user = "john.doe@example.com";
$this->assertSame(7, Data::$db->folderAdd($user, ['name' => "Entertainment"])); $this->assertSame(7, Data::$db->folderAdd($user, ['name' => "Entertainment"]));
Phake::verify(Data::$user)->authorize($user, "folderAdd"); Phake::verify(Data::$user)->authorize($user, "folderAdd");
$state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'root', 'name']]); $state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'name']]);
$state['arsse_folders']['rows'][] = [7, $user, null, null, "Entertainment"]; $state['arsse_folders']['rows'][] = [7, $user, null, "Entertainment"];
$this->compareExpectations($state); $this->compareExpectations($state);
} }
@ -59,8 +58,8 @@ trait SeriesFolder {
$user = "john.doe@example.com"; $user = "john.doe@example.com";
$this->assertSame(7, Data::$db->folderAdd($user, ['name' => "GNOME", 'parent' => 2])); $this->assertSame(7, Data::$db->folderAdd($user, ['name' => "GNOME", 'parent' => 2]));
Phake::verify(Data::$user)->authorize($user, "folderAdd"); Phake::verify(Data::$user)->authorize($user, "folderAdd");
$state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'root', 'name']]); $state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'name']]);
$state['arsse_folders']['rows'][] = [7, $user, 2, 1, "GNOME"]; $state['arsse_folders']['rows'][] = [7, $user, 2, "GNOME"];
$this->compareExpectations($state); $this->compareExpectations($state);
} }
@ -156,14 +155,14 @@ trait SeriesFolder {
function testRemoveAFolder() { function testRemoveAFolder() {
$this->assertTrue(Data::$db->folderRemove("john.doe@example.com", 6)); $this->assertTrue(Data::$db->folderRemove("john.doe@example.com", 6));
$state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'root', 'name']]); $state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'name']]);
array_pop($state['arsse_folders']['rows']); array_pop($state['arsse_folders']['rows']);
$this->compareExpectations($state); $this->compareExpectations($state);
} }
function testRemoveAFolderTree() { function testRemoveAFolderTree() {
$this->assertTrue(Data::$db->folderRemove("john.doe@example.com", 1)); $this->assertTrue(Data::$db->folderRemove("john.doe@example.com", 1));
$state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'root', 'name']]); $state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'name']]);
foreach([0,1,2,5] as $index) { foreach([0,1,2,5] as $index) {
unset($state['arsse_folders']['rows'][$index]); unset($state['arsse_folders']['rows'][$index]);
} }
@ -223,16 +222,15 @@ trait SeriesFolder {
function testRenameAFolder() { function testRenameAFolder() {
$this->assertTrue(Data::$db->folderPropertiesSet("john.doe@example.com", 6, ['name' => "Opinion"])); $this->assertTrue(Data::$db->folderPropertiesSet("john.doe@example.com", 6, ['name' => "Opinion"]));
$state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'root', 'name']]); $state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'name']]);
$state['arsse_folders']['rows'][5][4] = "Opinion"; $state['arsse_folders']['rows'][5][3] = "Opinion";
$this->compareExpectations($state); $this->compareExpectations($state);
} }
function testMoveAFolder() { function testMoveAFolder() {
$this->assertTrue(Data::$db->folderPropertiesSet("john.doe@example.com", 6, ['parent' => 5])); $this->assertTrue(Data::$db->folderPropertiesSet("john.doe@example.com", 6, ['parent' => 5]));
$state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'root', 'name']]); $state = $this->primeExpectations($this->data, ['arsse_folders' => ['id','owner', 'parent', 'name']]);
$state['arsse_folders']['rows'][5][2] = 5; // parent should have changed $state['arsse_folders']['rows'][5][2] = 5; // parent should have changed
$state['arsse_folders']['rows'][5][3] = 5; // root should also have changed
$this->compareExpectations($state); $this->compareExpectations($state);
} }

View file

@ -69,12 +69,18 @@ trait Setup {
function compareExpectations(array $expected): bool { function compareExpectations(array $expected): bool {
foreach($expected as $table => $info) { foreach($expected as $table => $info) {
$cols = implode(",", array_keys($info['columns'])); $cols = implode(",", array_keys($info['columns']));
foreach($this->drv->prepare("SELECT $cols from $table")->run() as $num => $row) { $data = $this->drv->prepare("SELECT $cols from $table")->run()->getAll();
$row = array_values($row); $cols = array_keys($info['columns']);
$this->assertGreaterThan(0, sizeof($info['rows']), "Expectations contain fewer rows than the database table $table"); foreach($info['rows'] as $index => $values) {
$exp = array_shift($info['rows']); $row = [];
$this->assertSame($exp, $row, "Row ".($num+1)." of table $table does not match expectations at array index $num."); foreach($values as $key => $value) {
$row[$cols[$key]] = $value;
}
$found = array_search($row, $data);
$this->assertNotSame(false, $found, "Table $table does not contain record at array index $index.");
unset($data[$found]);
} }
$this->assertSame([], $data);
} }
return true; return true;
} }