Refactor to use more laravel logic and improve compatibility with old…

…er PHP versions (#206) * Fix @param namespaces for PHPDocs in ServerPolicy * Reduce permission check duplication in ServerPolicy This introduces a new checkPermission method to reduce code duplication when checking for permissions. * Simplify logic to list accessible servers for the user We can directly use the pluck function that laravel collections provide to simplify the logic. * Fix pagination issue when databases/servers exceed 20 Laravels strips out the currently selected tab (or any GET query for that matter) by default when using pagination. the appends() methods helps with keeping that information. * Refactor unnecessary array_merge calls We can just append to the array instead of constantly merging a new copy. * Fix accessing “API Access” on some versions of PHP The “new” word is reserved and should not be used as a method name. http://stackoverflow.com/questions/9575590/why-am-i-getting-an-unexpected-t-new-error-in-php * Fix revoking API keys on older versions of php (5.6) “string” was not a valid function argument type yet, so revoking keys results in an error on older installations. * Fix issues with API due to methods named “list” “list” is yet another reserved keyword in PHP and messes up older installations of PHP (5.6). This renames all methods named “list” to “lists”. The API route names are left untouched (e.g. still called “api.admin.users.list”). * Refactor and shorten some API logic Used laravel collection methods where applicable to directly transform the values instead of converting back and forth. This also removes some dead variables that were never used as well as getting rid of a n+1 problem in the Service API (loading service variables afterwards, not during the model creation). * Return model save status in repositories where applicable * Fix typo in ServicePolicy#powerStart * Apply StyleCI corrections
pterodactyl · Dec 12, 2016 · a85ac87 · a85ac87
1 parent c3abb32
commit a85ac87
Show file tree

Hide file tree

Showing 21 changed files with 199 additions and 335 deletions.
diff --git a/app/Http/Controllers/API/LocationController.php b/app/Http/Controllers/API/LocationController.php
@@ -47,17 +47,13 @@ public function __construct()
      * @Versions({"v1"})
      * @Response(200)
      */
-    public function list(Request $request)
+    public function lists(Request $request)
     {
-        $locations = Location::select('locations.*', DB::raw('GROUP_CONCAT(nodes.id) as nodes'))
+        return Location::select('locations.*', DB::raw('GROUP_CONCAT(nodes.id) as nodes'))
             ->join('nodes', 'locations.id', '=', 'nodes.location')
             ->groupBy('locations.id')
-            ->get();
-
-        foreach ($locations as &$location) {
-            $location->nodes = explode(',', $location->nodes);
-        }
-
-        return $locations->toArray();
+            ->get()->each(function ($location) {
+                $location->nodes = explode(',', $location->nodes);
+            })->all();
     }
 }
diff --git a/app/Http/Controllers/API/NodeController.php b/app/Http/Controllers/API/NodeController.php
@@ -56,7 +56,7 @@ public function __construct()
      * })
      * @Response(200)
      */
-    public function list(Request $request)
+    public function lists(Request $request)
     {
         return Models\Node::all()->toArray();
     }

diff --git a/app/Http/Controllers/API/ServerController.php b/app/Http/Controllers/API/ServerController.php
@@ -57,7 +57,7 @@ public function __construct()
      * })
      * @Response(200)
      */
-    public function list(Request $request)
+    public function lists(Request $request)
     {
         return Models\Server::all()->toArray();
     }

diff --git a/app/Http/Controllers/API/ServiceController.php b/app/Http/Controllers/API/ServiceController.php
@@ -38,7 +38,7 @@ public function __construct()
         //
     }
 
-    public function list(Request $request)
+    public function lists(Request $request)
     {
         return Models\Service::all()->toArray();
     }
@@ -50,14 +50,12 @@ public function view(Request $request, $id)
             throw new NotFoundHttpException('No service by that ID was found.');
         }
 
-        $options = Models\ServiceOptions::select('id', 'name', 'description', 'tag', 'docker_image')->where('parent_service', $service->id)->get();
-        foreach ($options as &$opt) {
-            $opt->variables = Models\ServiceVariables::where('option_id', $opt->id)->get();
-        }
-
         return [
             'service' => $service,
-            'options' => $options,
+            'options' => Models\ServiceOptions::select('id', 'name', 'description', 'tag', 'docker_image')
+                ->where('parent_service', $service->id)
+                ->with('variables')
+                ->get(),
         ];
     }
 }
diff --git a/app/Http/Controllers/API/User/InfoController.php b/app/Http/Controllers/API/User/InfoController.php
@@ -32,11 +32,8 @@ class InfoController extends BaseController
 {
     public function me(Request $request)
     {
-        $servers = Models\Server::getUserServers();
-        $response = [];
-
-        foreach ($servers as &$server) {
-            $response = array_merge($response, [[
+        return Models\Server::getUserServers()->map(function ($server) {
+            return [
                 'id' => $server->uuidShort,
                 'uuid' => $server->uuid,
                 'name' => $server->name,
@@ -48,9 +45,7 @@ public function me(Request $request)
                 'port' => $server->port,
                 'service' => $server->a_serviceName,
                 'option' => $server->a_serviceOptionName,
-            ]]);
-        }
-
-        return $response;
+            ];
+        })->all();
     }
 }
diff --git a/app/Http/Controllers/API/User/ServerController.php b/app/Http/Controllers/API/User/ServerController.php
@@ -92,7 +92,6 @@ public function info(Request $request, $uuid)
     public function power(Request $request, $uuid)
     {
         $server = Models\Server::getByUUID($uuid);
-        $node = Models\Node::getByID($server->node);
         $client = Models\Node::guzzleRequest($server->node);
 
         Auth::user()->can('power-' . $request->input('action'), $server);

diff --git a/app/Http/Controllers/API/UserController.php b/app/Http/Controllers/API/UserController.php
@@ -55,7 +55,7 @@ public function __construct()
      * })
      * @Response(200)
      */
-    public function list(Request $request)
+    public function lists(Request $request)
     {
         return Models\User::all()->toArray();
     }

diff --git a/app/Http/Controllers/Base/APIController.php b/app/Http/Controllers/Base/APIController.php
@@ -48,7 +48,7 @@ public function index(Request $request)
         ]);
     }
 
-    public function new(Request $request)
+    public function create(Request $request)
     {
         return view('base.api.new');
     }
@@ -57,7 +57,7 @@ public function save(Request $request)
     {
         try {
             $repo = new APIRepository($request->user());
-            $secret = $repo->new($request->except(['_token']));
+            $secret = $repo->create($request->except(['_token']));
             Alert::success('An API Keypair has successfully been generated. The API secret for this public key is shown below and will not be shown again.<br /><br /><code>' . $secret . '</code>')->flash();
 
             return redirect()->route('account.api');

diff --git a/app/Http/Routes/APIRoutes.php b/app/Http/Routes/APIRoutes.php
@@ -55,7 +55,7 @@ public function map(Router $router)
              */
             $api->get('users', [
                 'as' => 'api.admin.users.list',
-                'uses' => 'Pterodactyl\Http\Controllers\API\UserController@list',
+                'uses' => 'Pterodactyl\Http\Controllers\API\UserController@lists',
             ]);
 
             $api->post('users', [
@@ -83,7 +83,7 @@ public function map(Router $router)
              */
             $api->get('servers', [
                 'as' => 'api.admin.servers.list',
-                'uses' => 'Pterodactyl\Http\Controllers\API\ServerController@list',
+                'uses' => 'Pterodactyl\Http\Controllers\API\ServerController@lists',
             ]);
 
             $api->post('servers', [
@@ -126,7 +126,7 @@ public function map(Router $router)
              */
             $api->get('nodes', [
                 'as' => 'api.admin.nodes.list',
-                'uses' => 'Pterodactyl\Http\Controllers\API\NodeController@list',
+                'uses' => 'Pterodactyl\Http\Controllers\API\NodeController@lists',
             ]);
 
             $api->post('nodes', [
@@ -164,15 +164,15 @@ public function map(Router $router)
              */
             $api->get('locations', [
                 'as' => 'api.admin.locations.list',
-                'uses' => 'Pterodactyl\Http\Controllers\API\LocationController@list',
+                'uses' => 'Pterodactyl\Http\Controllers\API\LocationController@lists',
             ]);
 
             /*
              * Service Routes
              */
             $api->get('services', [
                 'as' => 'api.admin.services.list',
-                'uses' => 'Pterodactyl\Http\Controllers\API\ServiceController@list',
+                'uses' => 'Pterodactyl\Http\Controllers\API\ServiceController@lists',
             ]);
 
             $api->get('services/{id}', [

diff --git a/app/Http/Routes/BaseRoutes.php b/app/Http/Routes/BaseRoutes.php
@@ -85,7 +85,7 @@ public function map(Router $router)
             ]);
             $router->get('/new', [
                 'as' => 'account.api.new',
-                'uses' => 'Base\APIController@new',
+                'uses' => 'Base\APIController@create',
             ]);
             $router->post('/new', [
                 'uses' => 'Base\APIController@save',

diff --git a/app/Models/ServiceOptions.php b/app/Models/ServiceOptions.php
@@ -50,4 +50,14 @@ class ServiceOptions extends Model
      protected $casts = [
          'parent_service' => 'integer',
      ];
+
+     /**
+      * Gets all variables associated with this service.
+      *
+      * @return \Illuminate\Database\Eloquent\Relations\HasMany
+      */
+     public function variables()
+     {
+         return $this->hasMany(ServiceVariables::class, 'option_id');
+     }
 }
diff --git a/app/Models/Subuser.php b/app/Models/Subuser.php
@@ -80,15 +80,8 @@ public function __construct()
      */
     public static function accessServers()
     {
-        $access = [];
-
         $union = self::select('server_id')->where('user_id', self::$user->id);
-        $select = Server::select('id')->where('owner', self::$user->id)->union($union)->get();
-
-        foreach ($select as &$select) {
-            $access = array_merge($access, [$select->id]);
-        }
 
-        return $access;
+        return Server::select('id')->where('owner', self::$user->id)->union($union)->pluck('id');
     }
 }