From 0aac35db01e545a84ac1b00946839af72fdf85ba Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Wed, 13 Jul 2022 07:33:22 +0000
Subject: [PATCH] fix code for restapi routes, fixes #1303

Closes #1303 and #1302

Merge request studip/studip!801
---
 app/routes/Feedback.php                       |  78 ++++----
 app/routes/FileSystem.php                     |  28 +--
 app/routes/ResourceBooking.php                |   2 +-
 app/routes/ResourceCategories.php             | 170 +++++++-----------
 app/routes/ResourcePermissions.php            | 126 ++++---------
 app/routes/ResourceProperties.php             |  58 ++----
 app/routes/ResourceRequest.php                |  30 +---
 app/routes/Resources.php                      |   2 +-
 app/routes/RoomClipboard.php                  |  36 ++--
 app/routes/User.php                           |   2 +-
 app/routes/Wiki.php                           |  12 +-
 lib/models/FeedbackElement.php                |  23 +--
 lib/models/FeedbackEntry.php                  |  18 +-
 .../resources/ResourceCategory.class.php      |  28 +--
 .../ResourcePropertyDefinition.class.php      |  37 ++--
 15 files changed, 254 insertions(+), 396 deletions(-)

diff --git a/app/routes/Feedback.php b/app/routes/Feedback.php
index c92e85ce421..52bbac70ef9 100644
--- a/app/routes/Feedback.php
+++ b/app/routes/Feedback.php
@@ -183,22 +183,13 @@ class Feedback extends \RESTAPI\RouteMap
             'user_id'       => $GLOBALS['user']->id
         ]);
 
-        if($feedback->commentable === 1) {
-            $entry->comment = $this->data['comment'];
-        }
+        $entry->rating = $this->getRating(
+            $feedback->mode,
+            (int) $this->data['rating']
+        );
 
-        if($feedback->mode !== 0) {
-            $rating = intval($this->data['rating']);
-            if ($rating === 0) {
-                $rating = 1;
-            }
-            if ($feedback->mode === 1) {
-                $rating = ($rating > 5 ? 5 : $rating);
-            }
-            if ($feedback->mode === 2) {
-                $rating = ($rating > 10 ? 10 : $rating);
-            }
-            $entry->rating = $rating;
+        if ($feedback->commentable) {
+            $entry->comment = $this->data['comment'];
         }
 
         $entry->store();
@@ -213,28 +204,25 @@ class Feedback extends \RESTAPI\RouteMap
      */
     public function editFeedbackEntry($entry_id)
     {
-        if (!$entry = \FeedbackEntry::find($entry_id)) {
-            $this->error(404);
+        $entry = \FeedbackEntry::find($entry_id);
+
+        if (!$entry) {
+            $this->notFound();
         }
+
         if (!$entry->isEditable()) {
             $this->error(403);
         }
-        if($feedback->mode !== 0) {
-            $rating = intval($this->data['rating']);
-            if ($rating === 0) {
-                $rating = 1;
-            }
-            if ($feedback->mode === 1) {
-                $rating = ($rating > 5 ? 5 : $rating);
-            }
-            if ($feedback->mode === 2) {
-                $rating = ($rating > 10 ? 10 : $rating);
-            }
-            $entry->rating = $rating;
-        }
-        if($feedback->commentable === 1) {
-            $entry->comment = $this->data['comment'] !== null ? $this->data['comment'] : $entry->comment;
+
+        $entry->rating = $this->getRating(
+            $entry->feedback->mode,
+            (int) $this->data['rating']
+        );
+
+        if ($entry->feedback->commentable) {
+            $entry->comment = $this->data['comment'] ?? $entry->comment;
         }
+
         $entry->store();
         return $entry->toArray();
     }
@@ -254,4 +242,30 @@ class Feedback extends \RESTAPI\RouteMap
             $this->halt(200);
         }
     }
+
+    /**
+     * @param int $mode
+     * @param int $rating
+     * @return int
+     */
+    private function getRating(int $mode, int $rating): int
+    {
+        if ($mode === 0) {
+            return 0;
+        }
+
+        if ($rating === 0) {
+            return 1;
+        }
+
+        if ($mode === 1) {
+            return min(5, $rating);
+        }
+
+        if ($mode === 2) {
+            return min(10, $rating);
+        }
+
+        throw new \InvalidArgumentException("Invalid mode {$mode}");
+    }
 }
diff --git a/app/routes/FileSystem.php b/app/routes/FileSystem.php
index 591acfd9438..54e2851f01f 100644
--- a/app/routes/FileSystem.php
+++ b/app/routes/FileSystem.php
@@ -116,17 +116,17 @@ class FileSystem extends \RESTAPI\RouteMap
      */
     public function copyFileRef($file_ref_id, $destination_folder_id)
     {
-        $result = \FileManager::copyFileRef(
-            $this->requireFileRef($file_ref_id),
+        $result = \FileManager::copyFile(
+            $this->requireFileRef($file_ref_id)->getFileType(),
             $this->requireFolder($destination_folder_id)->getTypedFolder(),
             \User::findCurrent()
         );
 
-        if (!$result instanceof \FileRef) {
+        if (!($result instanceof \FileType)) {
             $this->error(500, 'Error while copying a file reference: ' . implode(' ', $result));
         }
 
-        return $this->filerefToJSON($result);
+        return $this->filerefToJSON($result->getFileRef());
     }
 
     /**
@@ -136,17 +136,17 @@ class FileSystem extends \RESTAPI\RouteMap
      */
     public function moveFileRef($file_ref_id, $destination_folder_id)
     {
-        $result = \FileManager::moveFileRef(
-            $this->requireFileRef($file_ref_id),
+        $result = \FileManager::moveFile(
+            $this->requireFileRef($file_ref_id)->getFileType(),
             $this->requireFolder($destination_folder_id)->getTypedFolder(),
              \User::findCurrent()
         );
 
-        if (!$result instanceof \FileRef) {
+        if (!($result instanceof \FileType)) {
             $this->error(500, 'Error while moving a file reference: ' . implode(' ', $result));
         }
 
-        return $this->filerefToJSON($result);
+        return $this->filerefToJSON($result->getFileRef());
     }
 
     /**
@@ -274,10 +274,10 @@ class FileSystem extends \RESTAPI\RouteMap
         $query = "folder_id = :folder_id ORDER BY name ASC";
         $parameters[':folder_id'] = $folder->id;
 
-        if ($limit || $offset) {
+        if ($this->limit || $this->offset) {
             $query .= " LIMIT :limit OFFSET :offset";
-            $parameters[':limit'] = $limit;
-            $parameters[':offset'] = $offset;
+            $parameters[':limit'] = $this->limit;
+            $parameters[':offset'] = $this->offset;
         }
 
         $file_refs = \FileRef::findAndMapBySql(function (\FileRef $ref) {
@@ -467,7 +467,7 @@ class FileSystem extends \RESTAPI\RouteMap
 
     /**
      * Requires a valid user object.
-     * @return User object
+     * @return \User object
      */
     private function requireUser()
     {
@@ -477,7 +477,7 @@ class FileSystem extends \RESTAPI\RouteMap
     /**
      * Requires a valid file reference object
      * @param  mixed $id_or_object Either a file reference id or object
-     * @return FileRef object
+     * @return \FileRef object
      */
     private function requireFileRef($id_or_object)
     {
@@ -512,7 +512,7 @@ class FileSystem extends \RESTAPI\RouteMap
 
     /**
      * Converts a file reference object to JSON.
-     * @param  FileRef $ref      File reference object
+     * @param  \FileRef $ref     File reference object
      * @param  boolean $extended Extended output? (includes folder, owner and terms of use)
      * @return array representation for json encoding
      */
diff --git a/app/routes/ResourceBooking.php b/app/routes/ResourceBooking.php
index d260c030f91..77b1e848d9a 100644
--- a/app/routes/ResourceBooking.php
+++ b/app/routes/ResourceBooking.php
@@ -125,7 +125,7 @@ class ResourceBooking extends \RESTAPI\RouteMap
         try {
             $booking->store();
             return $this->sendReturnData($booking->toRawArray());
-        } catch (Exception $e) {
+        } catch (\Exception $e) {
             $this->halt(500, $e->getMessage());
         }
     }
diff --git a/app/routes/ResourceCategories.php b/app/routes/ResourceCategories.php
index 05e66b4c5c0..70ca7700f2b 100644
--- a/app/routes/ResourceCategories.php
+++ b/app/routes/ResourceCategories.php
@@ -12,6 +12,16 @@ namespace RESTAPI\Routes;
  */
 class ResourceCategories extends \RESTAPI\RouteMap
 {
+    /**
+     * Validate access to each route.
+     */
+    public function before()
+    {
+        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
+            throw new \AccessDeniedException();
+        }
+    }
+
     /**
      * Returns all defined resource categories.
      *
@@ -19,20 +29,12 @@ class ResourceCategories extends \RESTAPI\RouteMap
      */
     public function getAllResourceCategories()
     {
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
-        }
-
-        $categories = \ResourceCategory::findBySql('TRUE ORDER BY name ASC');
-
-        $result = [];
-
-        if ($categories) {
-            foreach ($categories as $c) {
-                $result[] = $c->toRawArray();
-            }
-        }
-        return $result;
+        return \ResourceCategory::findAndMapBySql(
+            function (\ResourceCategory $category) {
+                return $category->toRawArray();
+            },
+            'TRUE ORDER BY name ASC'
+        );
     }
 
 
@@ -48,10 +50,6 @@ class ResourceCategories extends \RESTAPI\RouteMap
             $this->notFound('ResourceCategory object not found!');
         }
 
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
-        }
-
         return $category->toRawArray();
     }
 
@@ -63,23 +61,19 @@ class ResourceCategories extends \RESTAPI\RouteMap
      */
     public function addResourceCategory()
     {
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
-        }
-
-        $this->name = \Request::get('name');
-        $this->description = \Request::get('description');
-        $this->class_name = \Request::get('class_name');
-        $this->iconnr = \Request::int('iconnr');
+        $name = \Request::get('name');
+        $description = \Request::get('description');
+        $class_name = \Request::get('class_name');
+        $iconnr = \Request::int('iconnr');
 
         $properties_name = \Request::getArray('properties_name');
         $properties_type = \Request::getArray('properties_type');
         $properties_requestable = \Request::getArray('properties_requestable');
         $properties_protected = \Request::getArray('properties_protected');
 
-        $this->set_properties = [];
+        $set_properties = [];
         foreach ($properties_name as $key => $property_name) {
-            $this->set_properties[] = [
+            $set_properties[] = [
                 'name' => $property_name,
                 'type' => $properties_type[$key],
                 'requestable' => $properties_requestable[$key],
@@ -88,83 +82,70 @@ class ResourceCategories extends \RESTAPI\RouteMap
         }
 
         //validation:
-        if (!$this->name) {
+        if (!$name) {
             $this->halt(
                 400,
                 _('Der Name der Kategorie ist leer!')
             );
-            return;
         }
 
-        if (!is_a($this->class_name, 'Resource', true)) {
+        if (!is_a($class_name, 'Resource', true)) {
             $this->halt(
                 400,
                 _('Es wurde keine gültige Ressourcen-Datenklasse ausgewählt!')
             );
-            return;
         }
 
-        switch ($this->class_name) {
-            case 'Location': {
-                $this->category = \ResourceManager::createLocationCategory(
-                    $this->name,
-                    $this->description
+        switch ($class_name) {
+            case 'Location':
+                $category = \ResourceManager::createLocationCategory(
+                    $name,
+                    $description
                 );
                 break;
-            } case 'Building': {
-                $this->category = \ResourceManager::createBuildingCategory(
-                    $this->name,
-                    $this->description
+            case 'Building':
+                $category = \ResourceManager::createBuildingCategory(
+                    $name,
+                    $description
                 );
                 break;
-            } case 'Room': {
-                $this->category = \ResourceManager::createRoomCategory(
-                    $this->name,
-                    $this->description
+            case 'Room':
+                $category = \ResourceManager::createRoomCategory(
+                    $name,
+                    $description
                 );
                 break;
-            } default: {
-                $this->category = \ResourceManager::createCategory(
-                    $this->name,
-                    $this->description,
-                    $this->class_name,
+            default:
+                $category = \ResourceManager::createCategory(
+                    $name,
+                    $description,
+                    $class_name,
                     false,
-                    $this->iconnr
+                    $iconnr
                 );
-            }
-        }
-
-        $successfully_stored = false;
-        if ($this->category->isDirty()) {
-            $successfully_stored = $this->category->store();
-        } else {
-            $successfully_stored = true;
         }
 
-        if ($successfully_stored) {
-            //After we have stored the category we must store
-            //the properties or create them, if necessary:
-
-            $properties_successfully_stored = false;
-            foreach ($this->set_properties as $set_property) {
-                $this->category->addProperty(
-                    $set_property['name'],
-                    $set_property['type'],
-                    $set_property['requestable'],
-                    $set_property['protected']
-                );
-            }
-
-            $this->show_form = false;
-            return $this->category;
-        } else {
+        if ($category->store() === false) {
             $this->halt(
                 500,
                 _('Fehler beim Speichern der Kategorie!')
             );
         }
-    }
 
+        //After we have stored the category we must store
+        //the properties or create them, if necessary:
+
+        foreach ($set_properties as $set_property) {
+            $category->addProperty(
+                $set_property['name'],
+                $set_property['type'],
+                $set_property['requestable'],
+                $set_property['protected']
+            );
+        }
+
+        return $category->toRawArray();
+    }
 
     /**
      * Modifies a resource category.
@@ -178,11 +159,6 @@ class ResourceCategories extends \RESTAPI\RouteMap
             $this->notFound('ResourceCategory object not found!');
         }
 
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            $this->halt(403);
-            return;
-        }
-
         if ($category->system) {
             $this->halt(403, 'System categories must not be modified!');
             return;
@@ -203,16 +179,11 @@ class ResourceCategories extends \RESTAPI\RouteMap
             $category->iconnr = $iconnr;
         }
 
-        if ($category->isDirty()) {
-            if ($category->store()) {
-                return $category->toRawArray();
-            } else {
-                $this->halt(
-                    500,
-                    'Error while saving the category!'
-                );
-                return;
-            }
+        if ($category->store() === false) {
+            $this->halt(
+                500,
+                'Error while saving the category!'
+            );
         }
 
         return $category->toRawArray();
@@ -231,10 +202,6 @@ class ResourceCategories extends \RESTAPI\RouteMap
             $this->notFound('ResourceCategory object not found!');
         }
 
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            $this->halt(403);
-        }
-
         if ($category->system) {
             $this->halt(403,'System resource categories must not be deleted!');
             return;
@@ -263,10 +230,6 @@ class ResourceCategories extends \RESTAPI\RouteMap
             $this->notFound('ResourceCategory object not found!');
         }
 
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
-        }
-
         $result = [];
         $properties = \ResourceCategoryProperty::findBySql(
             'INNER JOIN resource_property_definitions rpd
@@ -305,10 +268,6 @@ class ResourceCategories extends \RESTAPI\RouteMap
             $this->notFound('ResourceCategory object not found!');
         }
 
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
-        }
-
         $offset = \Request::int('offset');
         $limit = \Request::int('limit');
         $with_full_name = \Request::get('with_full_name');
@@ -358,9 +317,6 @@ class ResourceCategories extends \RESTAPI\RouteMap
             $this->notFound('ResourceCategory object not found!');
         }
 
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
-        }
 
         $name = \Request::get('name');
         $description = \Request::get('description');
diff --git a/app/routes/ResourcePermissions.php b/app/routes/ResourcePermissions.php
index 89022b787b2..d69dceac000 100644
--- a/app/routes/ResourcePermissions.php
+++ b/app/routes/ResourcePermissions.php
@@ -37,7 +37,7 @@ class ResourcePermissions extends \RESTAPI\RouteMap
         $resource = $resource->getDerivedClassInstance();
 
         if (!$resource->userHasPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
+            throw new \AccessDeniedException();
         }
 
         $levels_str = \Request::get('levels');
@@ -82,7 +82,6 @@ class ResourcePermissions extends \RESTAPI\RouteMap
                     404,
                     'Resource not found!'
                 );
-                return;
             }
         }
 
@@ -92,24 +91,21 @@ class ResourcePermissions extends \RESTAPI\RouteMap
                 400,
                 'No user was provided!'
             );
-            return;
         }
 
         $current_user = \User::findCurrent();
 
         if (!\ResourceManager::userHasGlobalPermission($current_user, 'admin')) {
-            if ($resource_id != 'global') {
+            if ($resource_id !== 'global') {
                 $resource = \Resource::find($resource_id);
                 $resource = $resource->getDerivedClassInstance();
                 if (!$resource->userHasPermission($current_user, 'admin')) {
                     $this->halt(403);
-                    return;
                 }
             } else {
                 //$resource_id == 'global': One must be admin
                 //to perform this action!
                 $this->halt(403);
-                return;
             }
         }
 
@@ -151,24 +147,21 @@ class ResourcePermissions extends \RESTAPI\RouteMap
                 400,
                 'No user was provided!'
             );
-            return;
         }
 
         $current_user = \User::findCurrent();
 
         if (!\ResourceManager::userHasGlobalPermission($current_user, 'admin')) {
-            if ($resource_id != 'global') {
+            if ($resource_id !== 'global') {
                 $resource = \Resource::find($resource_id);
                 $resource = $resource->getDerivedClassInstance();
                 if (!$resource->userHasPermission($current_user, 'admin')) {
                     $this->halt(403);
-                    return;
                 }
             } else {
                 //$resource_id == 'global': One must be admin
                 //to perform this action!
                 $this->halt(403);
-                return;
             }
         }
 
@@ -180,7 +173,6 @@ class ResourcePermissions extends \RESTAPI\RouteMap
                 400,
                 'Invalid permission level specified!'
             );
-            return;
         }
 
         //Check if permissions are already present for the user.
@@ -201,15 +193,11 @@ class ResourcePermissions extends \RESTAPI\RouteMap
 
         $permission->perms = $perms;
 
-        if ($permission->isDirty()) {
-            if ($permission->store()) {
-                return $permission->toRawArray();
-            } else {
-                $this->halt(
-                    500,
-                    'Error while saving permissions!'
-                );
-            }
+        if ($permission->store() === false) {
+            $this->halt(
+                500,
+                'Error while saving permissions!'
+            );
         }
 
         return $permission->toRawArray();
@@ -221,14 +209,8 @@ class ResourcePermissions extends \RESTAPI\RouteMap
      */
     public function deletePermission($resource_id, $user_id)
     {
-        if ($resource_id !== 'global') {
-            if (!\Resource::exists($resource_id)) {
-                $this->halt(
-                    404,
-                    'Resource not found!'
-                );
-                return;
-            }
+        if ($resource_id !== 'global' && !\Resource::exists($resource_id)) {
+            $this->notFound('Resource not found!');
         }
 
         $user = \User::find($user_id);
@@ -237,24 +219,21 @@ class ResourcePermissions extends \RESTAPI\RouteMap
                 400,
                 'No user was provided!'
             );
-            return;
         }
 
         $current_user = \User::findCurrent();
 
         if (!\ResourceManager::userHasGlobalPermission($current_user, 'admin')) {
-            if ($resource_id != 'global') {
+            if ($resource_id !== 'global') {
                 $resource = \Resource::find($resource_id);
                 $resource = $resource->getDerivedClassInstance();
                 if (!$resource->userHasPermission($current_user, 'admin')) {
                     $this->halt(403);
-                    return;
                 }
             } else {
                 //$resource_id == 'global': One must be admin
                 //to perform this action!
                 $this->halt(403);
-                return;
             }
         }
 
@@ -309,7 +288,7 @@ class ResourcePermissions extends \RESTAPI\RouteMap
         $resource = $resource->getDerivedClassInstance();
 
         if (!$resource->userHasPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
+            throw new \AccessDeniedException();
         }
 
         $begin = \Request::get('begin');
@@ -344,19 +323,13 @@ class ResourcePermissions extends \RESTAPI\RouteMap
             $sql_array['levels'] = $levels;
         }
 
-        $permissions = \ResourceTemporaryPermission::findBySql(
+        return \ResourceTemporaryPermission::findAndMapBySql(
+            function (\ResourceTemporaryPermission $permission) {
+                return $permission->toRawArray();
+            },
             $sql,
             $sql_array
         );
-
-        $result = [];
-        if ($permissions) {
-            foreach ($permissions as $permission) {
-                $result[] = $permission->toRawArray();
-            }
-        }
-
-        return $result;
     }
 
 
@@ -369,11 +342,7 @@ class ResourcePermissions extends \RESTAPI\RouteMap
     {
         if ($resource_id !== 'global') {
             if (!\Resource::exists($resource_id)) {
-                $this->halt(
-                    404,
-                    'Resource not found!'
-                );
-                return;
+                $this->notFound('Resource not found!');
             }
         }
 
@@ -383,7 +352,6 @@ class ResourcePermissions extends \RESTAPI\RouteMap
                 400,
                 'No user was provided!'
             );
-            return;
         }
 
         $current_user = \User::findCurrent();
@@ -393,7 +361,7 @@ class ResourcePermissions extends \RESTAPI\RouteMap
         $begin = null;
         $end = null;
         $with_time_range = false;
-        if ($begin_str and $end_str) {
+        if ($begin_str && $end_str) {
             $with_time_range = true;
             $begin = new \DateTime();
             $begin->setTimestamp($begin_str);
@@ -402,18 +370,16 @@ class ResourcePermissions extends \RESTAPI\RouteMap
         }
 
         if (!\ResourceManager::userHasGlobalPermission($current_user, 'admin')) {
-            if ($resource_id != 'global') {
+            if ($resource_id !== 'global') {
                 $resource = \Resource::find($resource_id);
                 $resource = $resource->getDerivedClassInstance();
                 if (!$resource->userHasPermission($current_user, 'admin')) {
                     $this->halt(403);
-                    return;
                 }
             } else {
                 //$resource_id == 'global': One must be admin
                 //to perform this action!
                 $this->halt(403);
-                return;
             }
         }
 
@@ -467,41 +433,30 @@ class ResourcePermissions extends \RESTAPI\RouteMap
      */
     public function setTemporaryPermission($resource_id, $user_id)
     {
-        if (!\Resource::exists($resource_id)) {
-            $this->halt(
-                404,
-                'Resource not found!'
-            );
-            return;
+        $resource = \Resource::find($resource_id);
+        if (!$resource) {
+            $this->notFound('Resource not found!');
         }
 
         $user = \User::find($user_id);
         if (!$user) {
-            $this->halt(
-                400,
-                'No user was provided!'
-            );
-            return;
+            $this->notFound('User not found!');
         }
 
         $current_user = \User::findCurrent();
 
         if (!\ResourceManager::userHasGlobalPermission($current_user, 'admin')
-            and !$resource->userHasPermission($current_user, 'admin')) {
+            && !$resource->userHasPermission($current_user, 'admin')) {
             $this->halt(403);
-            return;
         }
 
         $begin_str = \Request::get('begin');
         $end_str = \Request::get('end');
-        $begin = null;
-        $end = null;
-        if (!$begin_str or !$end_str) {
+        if (!$begin_str || !$end_str) {
             $this->halt(
                 400,
                 'No time range specified for temporary permission!'
             );
-            return;
         }
 
         $begin = new \DateTime();
@@ -517,7 +472,6 @@ class ResourcePermissions extends \RESTAPI\RouteMap
                 400,
                 'Invalid permission level specified!'
             );
-            return;
         }
 
         //Check if permissions are already present for the user.
@@ -543,15 +497,11 @@ class ResourcePermissions extends \RESTAPI\RouteMap
 
         $permission->perms = $perms;
 
-        if ($permission->isDirty()) {
-            if ($permission->store()) {
-                return $permission->toRawArray();
-            } else {
-                $this->halt(
-                    500,
-                    'Error while saving permissions!'
-                );
-            }
+        if ($permission->store() === false) {
+            $this->halt(
+                500,
+                'Error while saving permissions!'
+            );
         }
 
         return $permission->toRawArray();
@@ -569,38 +519,28 @@ class ResourcePermissions extends \RESTAPI\RouteMap
     {
         if ($resource_id !== 'global') {
             if (!\Resource::exists($resource_id)) {
-                $this->halt(
-                    404,
-                    'Resource not found!'
-                );
-                return;
+                $this->notFound('Resource not found!');
             }
         }
 
         $user = \User::find($user_id);
         if (!$user) {
-            $this->halt(
-                400,
-                'No user was provided!'
-            );
-            return;
+            $this->notFound('User not found!');
         }
 
         $current_user = \User::findCurrent();
 
         if (!\ResourceManager::userHasGlobalPermission($current_user, 'admin')) {
-            if ($resource_id != 'global') {
+            if ($resource_id !== 'global') {
                 $resource = \Resource::find($resource_id);
                 $resource = $resource->getDerivedClassInstance();
                 if (!$resource->userHasPermission($current_user, 'admin')) {
                     $this->halt(403);
-                    return;
                 }
             } else {
                 //$resource_id == 'global': One must be admin
                 //to perform this action!
                 $this->halt(403);
-                return;
             }
         }
 
diff --git a/app/routes/ResourceProperties.php b/app/routes/ResourceProperties.php
index 14cbcbc6da2..8f00d0a18f7 100644
--- a/app/routes/ResourceProperties.php
+++ b/app/routes/ResourceProperties.php
@@ -12,6 +12,16 @@ namespace RESTAPI\Routes;
  */
 class ResourceProperties extends \RESTAPI\RouteMap
 {
+    /**
+     * Validate access to each route.
+     */
+    public function before()
+    {
+        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
+            throw new \AccessDeniedException();
+        }
+    }
+
     /**
      * Returns all resource property definitions.
      *
@@ -19,10 +29,6 @@ class ResourceProperties extends \RESTAPI\RouteMap
      */
     public function getAllResourcePropertyDefinitions()
     {
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
-        }
-
         $properties = \ResourcePropertyDefinition::findBySql('TRUE ORDER BY name ASC');
 
         $result = [];
@@ -44,55 +50,38 @@ class ResourceProperties extends \RESTAPI\RouteMap
      */
     public function addResourcePropertyDefinition()
     {
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
-        }
-
-        $defined_types = \ResourcePropertyDefinition::getDefinedTypes();
-
         $name = \Request::get('name');
         $description = \Request::i18n('description');
         $type = \Request::get('type');
         $write_permission_level = \Request::get('write_permission_level');
-        $options = \Request::get('options');
-        $range_search = \Request::get('range_search');
+        $options = \Request::get('options', '');
+        $range_search = \Request::bool('range_search');
 
         if (!$name) {
             $this->halt(
                 400,
                 'The field \'name\' must not be empty!'
             );
-            return;
         }
-        if (!in_array($type, $defined_types)) {
+        if (!in_array($type, \ResourcePropertyDefinition::getDefinedTypes())) {
             $this->halt(
                 400,
                 'Invalid property type specified!'
             );
-            return;
         }
         if (!in_array($write_permission_level, ['user', 'autor', 'tutor', 'admin'])) {
             $this->halt(
                 400,
                 'Invalid permission level in field \'write_permission_level\'!'
             );
-            return;
         }
 
         $property = new \ResourcePropertyDefinition();
         $property->name = $name;
         $property->description = $description;
         $property->type = $type;
-        if ($options) {
-            $property->options = $options;
-        } else {
-            $property->options = '';
-        }
-        $property->range_search = (
-            $range_search
-            ? '1'
-            : '0'
-        );
+        $property->options = $options ?: '';
+        $property->range_search = $range_search;
         $property->write_permission_level = $write_permission_level;
 
         if (!$property->store()) {
@@ -117,10 +106,6 @@ class ResourceProperties extends \RESTAPI\RouteMap
             $this->notFound('ResourcePropertyDefinition object not found!');
         }
 
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            throw new AccessDeniedException();
-        }
-
         return $property->toRawArray();
     }
 
@@ -137,17 +122,11 @@ class ResourceProperties extends \RESTAPI\RouteMap
             $this->notFound('ResourcePropertyDefinition object not found!');
         }
 
-        if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
-            $this->halt(403);
-            return;
-        }
-
         if ($property->system) {
             $this->halt(
                 403,
                 'System properties must not be edited!'
             );
-            return;
         }
 
         $name = $this->data['name'];
@@ -166,12 +145,11 @@ class ResourceProperties extends \RESTAPI\RouteMap
         }
 
         if ($type) {
-            if (!in_array($type, $defined_types)) {
+            if (!in_array($type, \ResourcePropertyDefinition::getDefinedTypes())) {
                 $this->halt(
                     400,
                     'Invalid property type specified!'
                 );
-                return;
             }
             $property->type = $type;
         }
@@ -182,7 +160,6 @@ class ResourceProperties extends \RESTAPI\RouteMap
                     400,
                     'Invalid permission level in field \'write_permission_level\'!'
                 );
-                return;
             }
             $property->write_permission_level = $write_permission_level;
         }
@@ -203,7 +180,6 @@ class ResourceProperties extends \RESTAPI\RouteMap
                     500,
                     'Error while saving the property!'
                 );
-                return;
             }
         }
 
@@ -225,7 +201,6 @@ class ResourceProperties extends \RESTAPI\RouteMap
 
         if (!\ResourceManager::userHasGlobalPermission(\User::findCurrent(), 'admin')) {
             $this->halt(403);
-            return;
         }
 
         //Check if the property is in use:
@@ -235,7 +210,6 @@ class ResourceProperties extends \RESTAPI\RouteMap
                 403,
                 'The property is in use and can therefore not be deleted!'
             );
-            return;
         }
 
         if ($property->delete()) {
diff --git a/app/routes/ResourceRequest.php b/app/routes/ResourceRequest.php
index 6d53b4671b3..b3d235d0a5b 100644
--- a/app/routes/ResourceRequest.php
+++ b/app/routes/ResourceRequest.php
@@ -68,7 +68,7 @@ class ResourceRequest extends \RESTAPI\RouteMap
         try {
             $request->store();
             return $this->sendReturnData($request->toRawArray());
-        } catch (Exception $e) {
+        } catch (\Exception $e) {
             $this->halt(500, $e->getMessage());
         }
     }
@@ -92,29 +92,17 @@ class ResourceRequest extends \RESTAPI\RouteMap
             throw new \AccessDeniedException();
         }
 
-        $quiet = \Request::get('quiet');
-        $new_reply_comment = \Request::get('reply_comment');
+        $request->reply_comment = \Request::get('reply_comment');
 
-        $request->reply_comment = $new_reply_comment;
-
-        if ($request->isDirty()) {
-            try {
-                $request->store();
-                if ($this->quiet) {
-                    return '';
-                } else {
-                    return $this->sendReturnData($request->toRawArray());
-                }
-            } catch (Exception $e) {
-                $this->halt(500, $e->getMessage());
-            }
-        } else {
-            if ($this->quiet) {
-                return '';
-            } else {
-                return $this->sendReturnData($request->toRawArray());
+        try {
+            if ($request->store() === false) {
+                throw new \RuntimeException('Could not store comment');
             }
+        } catch (\Exception $e) {
+            $this->halt(500, $e->getMessage());
         }
+
+        return $this->sendReturnData($request->toRawArray());
     }
 
 
diff --git a/app/routes/Resources.php b/app/routes/Resources.php
index 84b91105b38..bf826ba818a 100644
--- a/app/routes/Resources.php
+++ b/app/routes/Resources.php
@@ -82,7 +82,7 @@ class Resources extends \RESTAPI\RouteMap
             $resource->description = $description;
         }
         if ($parent_id) {
-            if (!Resource::exists($parent_id)) {
+            if (!\Resource::exists($parent_id)) {
                 $this->halt(
                     400,
                     'No resource exists with the ID \'' . $parent_id . '\'!'
diff --git a/app/routes/RoomClipboard.php b/app/routes/RoomClipboard.php
index 5335a6a0a8c..b3c611582b9 100644
--- a/app/routes/RoomClipboard.php
+++ b/app/routes/RoomClipboard.php
@@ -34,12 +34,12 @@ class RoomClipboard extends \RESTAPI\RouteMap
         $current_user = \User::findCurrent();
 
         //Permission check:
-        if ($clipboard->user_id != $current_user->id) {
-            throw new AccessDeniedException();
+        if ($clipboard->user_id !== $current_user->id) {
+            throw new \AccessDeniedException();
         }
 
-        $display_requests = \Request::get('display_requests');
-        $display_all_requests = \Request::get('display_all_requests');
+        $display_requests = \Request::bool('display_requests');
+        $display_all_requests = \Request::bool('display_all_requests');
 
         $begin_date = \Request::get('start');
         $end_date = \Request::get('end');
@@ -134,15 +134,7 @@ class RoomClipboard extends \RESTAPI\RouteMap
                             ]
                         ],
                         $booking_types,
-                        (
-                            $display_all_requests
-                            ? 'all'
-                            : (
-                                $display_requests
-                                ? true
-                                : false
-                            )
-                        )
+                        $display_all_requests ? 'all' : $display_requests
                     )
                 );
             }
@@ -173,12 +165,12 @@ class RoomClipboard extends \RESTAPI\RouteMap
         $current_user = \User::findCurrent();
 
         //Permission check:
-        if ($clipboard->user_id != $current_user->id) {
-            throw new AccessDeniedException();
+        if ($clipboard->user_id !== $current_user->id) {
+            throw new \AccessDeniedException();
         }
 
-        $display_requests = \Request::get('display_requests');
-        $display_all_requests = \Request::get('display_all_requests');
+        $display_requests = \Request::bool('display_requests');
+        $display_all_requests = \Request::bool('display_all_requests');
 
         $begin = new \DateTime();
         $end = new \DateTime();
@@ -257,15 +249,7 @@ class RoomClipboard extends \RESTAPI\RouteMap
                             ]
                         ],
                         $booking_types,
-                        (
-                            $display_all_requests
-                            ? 'all'
-                            : (
-                                $display_requests
-                                ? true
-                                : false
-                            )
-                        )
+                        $display_all_requests ? 'all' : $display_requests
                     )
                 );
             }
diff --git a/app/routes/User.php b/app/routes/User.php
index 4c194f1d026..89649fce423 100644
--- a/app/routes/User.php
+++ b/app/routes/User.php
@@ -260,7 +260,7 @@ class User extends \RESTAPI\RouteMap
      */
     public function patchCourseGroup($user_id, $course_id)
     {
-        $user = User::find($user_id);
+        $user = \User::find($user_id);
         if (!$user) {
             $this->notFound('User not found');
         }
diff --git a/app/routes/Wiki.php b/app/routes/Wiki.php
index daaa7ac35f8..5a4a8168dbe 100644
--- a/app/routes/Wiki.php
+++ b/app/routes/Wiki.php
@@ -122,12 +122,12 @@ class Wiki extends \RESTAPI\RouteMap
         $json = $page->toArray(words("range_id keyword chdate version"));
 
         // (pre-rendered) content
-        if (!in_array("content", $without)) {
+        if (!in_array('content', $without)) {
             $json['content']      = $page->body;
             $json['content_html'] = wikiReady($page->body);
         }
-        if (!in_array("user", $without)) {
-            if($page->author) {
+        if (!in_array('user', $without)) {
+            if ($page->author) {
                 $json['user'] = User::getMiniUser($this, $page->author);
             }
         }
@@ -139,9 +139,9 @@ class Wiki extends \RESTAPI\RouteMap
         }
 
         // string to int conversions as SORM does not know about ints
-        foreach (words("chdate mkdate filesize downloads") as $key) {
-            if (isset($result[$key])) {
-                $result[$key] = intval($result[$key]);
+        foreach (['chdate', 'mkdate', 'filesize', 'downloads'] as $key) {
+            if (isset($json[$key])) {
+                $json[$key] = (int) $json[$key];
             }
         }
 
diff --git a/lib/models/FeedbackElement.php b/lib/models/FeedbackElement.php
index f9f1ccb625c..08b5aa170ec 100644
--- a/lib/models/FeedbackElement.php
+++ b/lib/models/FeedbackElement.php
@@ -4,24 +4,27 @@
  *
  * @author Nils Gehrke <nils.gehrke@uni-goettingen.de>
  *
- * @property integer id database column
- * @property string user_id database column
- * @property string range_id database column
- * @property string range_type database column:
+ * @property int $id database column
+ * @property string $user_id database column
+ * @property string $range_id database column
+ * @property string $range_type database column:
  *                  name of class that implements FeedbackRange
  *
- * @property string course_id database column
- * @property string question database column
- * @property string description database column
- * @property integer mode database column:
+ * @property string $course_id database column
+ * @property string $question database column
+ * @property string $description database column
+ * @property int $mode database column:
  *                  0 without rating;
  *                  1 with star rating from 1 to 5;
  *                  2 with star rating from 1 to 10;
  *
- * @property boolean results_visible database column:
+ * @property boolean $results_visible database column:
  *                   show rating results to users after feedback submission
- * @property boolean commentable database column: users may comment ratings
+ * @property boolean $commentable database column: users may comment ratings
  *
+ * @property Feedbackentry[]|SimpleORMapCollection $entries
+ * @property Course $course
+ * @property User $user
  */
 
 class FeedbackElement extends SimpleORMap
diff --git a/lib/models/FeedbackEntry.php b/lib/models/FeedbackEntry.php
index dd2f86b2bb2..be370619162 100644
--- a/lib/models/FeedbackEntry.php
+++ b/lib/models/FeedbackEntry.php
@@ -4,12 +4,14 @@
  *
  * @author Nils Gehrke <nils.gehrke@uni-goettingen.de>
  *
- * @property integer id database column
- * @property integer feedback_id database column
- * @property string user_id database column
- * @property string comment database column
- * @property integer rating database column
+ * @property int $id database column
+ * @property int $feedback_id database column
+ * @property string $user_id database column
+ * @property string $comment database column
+ * @property int $rating database column
  *
+ * @property FeedbackElement $feedback
+ * @property User $user
  */
 
 class FeedbackEntry extends SimpleORMap
@@ -32,11 +34,7 @@ class FeedbackEntry extends SimpleORMap
 
     public function isEditable()
     {
-        $editable = false;
-        if ($this->user_id == $GLOBALS['user']->id) {
-            $editable = true;
-        }
-        return $editable;
+        return $this->user_id === $GLOBALS['user']->id;
     }
 
     public function isDeletable()
diff --git a/lib/models/resources/ResourceCategory.class.php b/lib/models/resources/ResourceCategory.class.php
index 2961ca2afba..90783737af6 100644
--- a/lib/models/resources/ResourceCategory.class.php
+++ b/lib/models/resources/ResourceCategory.class.php
@@ -3,6 +3,9 @@
 /**
  * ResourceCategory.class.php - model class for resource categories
  *
+ * The ResourceCategory class can be used as a Factory for
+ * Resource objects.
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
  * published by the Free Software Foundation; either version 2 of
@@ -15,21 +18,18 @@
  * @package     resources
  * @since       4.1
  *
- * @property string id database column
- * @property string name database column
- * @property string description database column
- * @property string class_name database column: The name of the SORM class
+ * @property string $id database column
+ * @property string $name database column
+ * @property string $description database column
+ * @property string $class_name database column: The name of the SORM class
  *     that handles the resource object, defaults to Resource.
- * @property string system database column
- * @property string iconnr database column
- * @property string mkdate database column
- * @property string chdate database column
- */
-
-
-/**
- * The ResourceCategory class can be used as a Factory for
- * Resource objects.
+ * @property bool $system database column
+ * @property int $iconnr database column
+ * @property int $mkdate database column
+ * @property int $chdate database column
+ *
+ * @property ResourcePropertyDefinition[]|SimpleORMapCollection $property_definitions
+ * @property ResourceCategoryProperty[]|SimpleORMapCollection $property_links
  */
 class ResourceCategory extends SimpleORMap
 {
diff --git a/lib/models/resources/ResourcePropertyDefinition.class.php b/lib/models/resources/ResourcePropertyDefinition.class.php
index b7d6b6eaa0a..c039459dad6 100644
--- a/lib/models/resources/ResourcePropertyDefinition.class.php
+++ b/lib/models/resources/ResourcePropertyDefinition.class.php
@@ -3,6 +3,9 @@
 /**
  * ResourcePropertyDefinition.class.php - model class for resource property definitions
  *
+ * The ResourcePropertyDefinition class can be used as a Factory
+ * for ResourceProperty objects.
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
  * published by the Free Software Foundation; either version 2 of
@@ -15,32 +18,30 @@
  * @package     resources
  * @since       4.1
  *
- * @property string property_id database column
- * @property string id alias for resource_id
- * @property string name database column The internal name of the property.
- * @property string display_name The display name of the property.
- * @property string description database column
- * @property string type database column ('bool', 'text', 'num', 'select',
+ * @property string $property_id database column
+ * @property string $id alias for resource_id
+ * @property string $name database column The internal name of the property.
+ * @property string $display_name The display name of the property.
+ * @property string $description database column
+ * @property string $type database column ('bool', 'text', 'num', 'select',
  *     'user', 'institute', 'position', 'fileref', 'url')
- * @property string options database column
- * @property string system database column
- * @property string info_label database column
- * @property string searchable database column
+ * @property string $options database column
+ * @property string $system database column
+ * @property string $info_label database column
+ * @property bool $searchable database column
  *     0 = not searchable, 1 = searchable
- * @property string range_search database column: Whether a search field
+ * @property string $range_search database column: Whether a search field
  *     for this property shall display a range selector (1) or not (0).
  *     Setting this attribute is only useful for the property types
  *     'num' and 'position'.
- * @property string write_permission_level database column
- * @property string mkdate database column
- * @property string chdate database column
+ * @property string $write_permission_level database column
+ * @property int $mkdate database column
+ * @property int $chdate database column
+ *
+ * @property ResourcePropertyGroup $group
  */
 
 
-/**
- * The ResourcePropertyDefinition class can be used as a Factory
- * for ResourceProperty objects.
- */
 class ResourcePropertyDefinition extends SimpleORMap
 {
     /**
-- 
GitLab