From 89e018a4f09df825153dc3c734ef7dc36e92349b Mon Sep 17 00:00:00 2001
From: Franz Liedke <franz@develophp.org>
Date: Tue, 13 Nov 2018 22:23:52 +0100
Subject: [PATCH] Simplify PrerequisiteInterface

I went with a return type of Collection, because it is easier to call
methods such as isEmpty() directly on those objects.
---
 src/Install/Console/InstallCommand.php        | 25 +++++++------
 src/Install/Controller/IndexController.php    | 10 +++---
 .../Prerequisite/AbstractPrerequisite.php     | 24 -------------
 src/Install/Prerequisite/Composite.php        | 17 ++++-----
 src/Install/Prerequisite/PhpExtensions.php    | 17 +++++----
 src/Install/Prerequisite/PhpVersion.php       | 12 ++++---
 .../Prerequisite/PrerequisiteInterface.php    | 15 ++++++--
 src/Install/Prerequisite/WritablePaths.php    | 35 ++++++++++++++-----
 views/install/app.php                         | 12 +++----
 views/install/errors.php                      | 14 --------
 views/install/problems.php                    | 14 ++++++++
 11 files changed, 98 insertions(+), 97 deletions(-)
 delete mode 100644 src/Install/Prerequisite/AbstractPrerequisite.php
 delete mode 100644 views/install/errors.php
 create mode 100644 views/install/problems.php

diff --git a/src/Install/Console/InstallCommand.php b/src/Install/Console/InstallCommand.php
index e429d62d8..246dbc0ce 100644
--- a/src/Install/Console/InstallCommand.php
+++ b/src/Install/Console/InstallCommand.php
@@ -80,21 +80,16 @@ class InstallCommand extends AbstractCommand
     {
         $this->init();
 
-        $prerequisites = $this->installation->prerequisites();
-        $prerequisites->check();
-        $errors = $prerequisites->getErrors();
+        $problems = $this->installation->prerequisites()->problems();
 
-        if (empty($errors)) {
+        if ($problems->isEmpty()) {
             $this->info('Installing Flarum...');
 
             $this->install();
 
             $this->info('DONE.');
         } else {
-            $this->output->writeln(
-                '<error>Please fix the following errors before we can continue with the installation.</error>'
-            );
-            $this->showErrors($errors);
+            $this->showProblems($problems);
         }
     }
 
@@ -178,13 +173,17 @@ class InstallCommand extends AbstractCommand
             ->run();
     }
 
-    protected function showErrors($errors)
+    protected function showProblems($problems)
     {
-        foreach ($errors as $error) {
-            $this->info($error['message']);
+        $this->output->writeln(
+            '<error>Please fix the following problems before we can continue with the installation.</error>'
+        );
 
-            if (isset($error['detail'])) {
-                $this->output->writeln('<comment>'.$error['detail'].'</comment>');
+        foreach ($problems as $problem) {
+            $this->info($problem['message']);
+
+            if (isset($problem['detail'])) {
+                $this->output->writeln('<comment>'.$problem['detail'].'</comment>');
             }
         }
     }
diff --git a/src/Install/Controller/IndexController.php b/src/Install/Controller/IndexController.php
index 03b1e2449..3d66789d2 100644
--- a/src/Install/Controller/IndexController.php
+++ b/src/Install/Controller/IndexController.php
@@ -46,14 +46,12 @@ class IndexController extends AbstractHtmlController
     {
         $view = $this->view->make('flarum.install::app')->with('title', 'Install Flarum');
 
-        $prerequisites = $this->installation->prerequisites();
-        $prerequisites->check();
-        $errors = $prerequisites->getErrors();
+        $problems = $this->installation->prerequisites()->problems();
 
-        if (count($errors)) {
-            $view->with('content', $this->view->make('flarum.install::errors')->with('errors', $errors));
-        } else {
+        if ($problems->isEmpty()) {
             $view->with('content', $this->view->make('flarum.install::install'));
+        } else {
+            $view->with('content', $this->view->make('flarum.install::problems')->with('problems', $problems));
         }
 
         return $view;
diff --git a/src/Install/Prerequisite/AbstractPrerequisite.php b/src/Install/Prerequisite/AbstractPrerequisite.php
deleted file mode 100644
index 06c6210b1..000000000
--- a/src/Install/Prerequisite/AbstractPrerequisite.php
+++ /dev/null
@@ -1,24 +0,0 @@
-<?php
-
-/*
- * This file is part of Flarum.
- *
- * (c) Toby Zerner <toby.zerner@gmail.com>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Flarum\Install\Prerequisite;
-
-abstract class AbstractPrerequisite implements PrerequisiteInterface
-{
-    protected $errors = [];
-
-    abstract public function check();
-
-    public function getErrors()
-    {
-        return $this->errors;
-    }
-}
diff --git a/src/Install/Prerequisite/Composite.php b/src/Install/Prerequisite/Composite.php
index ed0fb77e4..ed3a36e07 100644
--- a/src/Install/Prerequisite/Composite.php
+++ b/src/Install/Prerequisite/Composite.php
@@ -11,6 +11,8 @@
 
 namespace Flarum\Install\Prerequisite;
 
+use Illuminate\Support\Collection;
+
 class Composite implements PrerequisiteInterface
 {
     /**
@@ -25,21 +27,14 @@ class Composite implements PrerequisiteInterface
         }
     }
 
-    public function check()
+    public function problems(): Collection
     {
         return array_reduce(
             $this->prerequisites,
-            function ($previous, PrerequisiteInterface $prerequisite) {
-                return $prerequisite->check() && $previous;
+            function (Collection $errors, PrerequisiteInterface $condition) {
+                return $errors->concat($condition->problems());
             },
-            true
+            collect()
         );
     }
-
-    public function getErrors()
-    {
-        return collect($this->prerequisites)->map(function (PrerequisiteInterface $prerequisite) {
-            return $prerequisite->getErrors();
-        })->reduce('array_merge', []);
-    }
 }
diff --git a/src/Install/Prerequisite/PhpExtensions.php b/src/Install/Prerequisite/PhpExtensions.php
index 434eff927..bac5f0d47 100644
--- a/src/Install/Prerequisite/PhpExtensions.php
+++ b/src/Install/Prerequisite/PhpExtensions.php
@@ -11,7 +11,9 @@
 
 namespace Flarum\Install\Prerequisite;
 
-class PhpExtensions extends AbstractPrerequisite
+use Illuminate\Support\Collection;
+
+class PhpExtensions implements PrerequisiteInterface
 {
     protected $extensions;
 
@@ -20,14 +22,15 @@ class PhpExtensions extends AbstractPrerequisite
         $this->extensions = $extensions;
     }
 
-    public function check()
+    public function problems(): Collection
     {
-        foreach ($this->extensions as $extension) {
-            if (! extension_loaded($extension)) {
-                $this->errors[] = [
+        return collect($this->extensions)
+            ->reject(function ($extension) {
+                return extension_loaded($extension);
+            })->map(function ($extension) {
+                return [
                     'message' => "The PHP extension '$extension' is required.",
                 ];
-            }
-        }
+            });
     }
 }
diff --git a/src/Install/Prerequisite/PhpVersion.php b/src/Install/Prerequisite/PhpVersion.php
index b2960d372..59d21266f 100644
--- a/src/Install/Prerequisite/PhpVersion.php
+++ b/src/Install/Prerequisite/PhpVersion.php
@@ -11,7 +11,9 @@
 
 namespace Flarum\Install\Prerequisite;
 
-class PhpVersion extends AbstractPrerequisite
+use Illuminate\Support\Collection;
+
+class PhpVersion implements PrerequisiteInterface
 {
     protected $minVersion;
 
@@ -20,13 +22,15 @@ class PhpVersion extends AbstractPrerequisite
         $this->minVersion = $minVersion;
     }
 
-    public function check()
+    public function problems(): Collection
     {
         if (version_compare(PHP_VERSION, $this->minVersion, '<')) {
-            $this->errors[] = [
+            return collect()->push([
                 'message' => "PHP $this->minVersion is required.",
                 'detail' => 'You are running version '.PHP_VERSION.'. Talk to your hosting provider about upgrading to the latest PHP version.',
-            ];
+            ]);
         }
+
+        return collect();
     }
 }
diff --git a/src/Install/Prerequisite/PrerequisiteInterface.php b/src/Install/Prerequisite/PrerequisiteInterface.php
index 115e9f808..9b7907883 100644
--- a/src/Install/Prerequisite/PrerequisiteInterface.php
+++ b/src/Install/Prerequisite/PrerequisiteInterface.php
@@ -11,9 +11,18 @@
 
 namespace Flarum\Install\Prerequisite;
 
+use Illuminate\Support\Collection;
+
 interface PrerequisiteInterface
 {
-    public function check();
-
-    public function getErrors();
+    /**
+     * Verify that this prerequisite is fulfilled.
+     *
+     * If everything is okay, this method should return an empty Collection
+     * instance. When problems are detected, it should return a Collection of
+     * arrays, each having at least a "message" and optionally a "detail" key.
+     *
+     * @return Collection
+     */
+    public function problems(): Collection;
 }
diff --git a/src/Install/Prerequisite/WritablePaths.php b/src/Install/Prerequisite/WritablePaths.php
index 1f356ee6a..99beb2a3d 100644
--- a/src/Install/Prerequisite/WritablePaths.php
+++ b/src/Install/Prerequisite/WritablePaths.php
@@ -11,7 +11,9 @@
 
 namespace Flarum\Install\Prerequisite;
 
-class WritablePaths extends AbstractPrerequisite
+use Illuminate\Support\Collection;
+
+class WritablePaths implements PrerequisiteInterface
 {
     protected $paths;
 
@@ -20,21 +22,36 @@ class WritablePaths extends AbstractPrerequisite
         $this->paths = $paths;
     }
 
-    public function check()
+    public function problems(): Collection
     {
-        foreach ($this->paths as $path) {
-            if (! file_exists($path)) {
-                $this->errors[] = [
+        return $this->getMissingPaths()
+            ->concat($this->getNonWritablePaths());
+    }
+
+    private function getMissingPaths(): Collection
+    {
+        return collect($this->paths)
+            ->reject(function ($path) {
+                return file_exists($path);
+            })->map(function ($path) {
+                return [
                     'message' => 'The '.$this->getAbsolutePath($path).' directory doesn\'t exist',
                     'detail' => 'This directory is necessary for the installation. Please create the folder.',
                 ];
-            } elseif (! is_writable($path)) {
-                $this->errors[] = [
+            });
+    }
+
+    private function getNonWritablePaths(): Collection
+    {
+        return collect($this->paths)
+            ->filter(function ($path) {
+                return file_exists($path) && ! is_writable($path);
+            })->map(function ($path) {
+                return [
                     'message' => 'The '.$this->getAbsolutePath($path).' directory is not writable.',
                     'detail' => 'Please chmod this directory'.($path !== public_path() ? ' and its contents' : '').' to 0775.'
                 ];
-            }
-        }
+            });
     }
 
     private function getAbsolutePath($path)
diff --git a/views/install/app.php b/views/install/app.php
index 2e8601022..3e4f572de 100644
--- a/views/install/app.php
+++ b/views/install/app.php
@@ -129,30 +129,30 @@
         animation-name: fadeIn;
       }
 
-      .Errors {
+      .Problems {
         margin-top: 50px;
       }
-      .Errors .Error:first-child {
+      .Problems .Problem:first-child {
         border-top-left-radius: 4px;
         border-top-right-radius: 4px;
       }
-      .Errors .Error:last-child {
+      .Problems .Problem:last-child {
         border-bottom-left-radius: 4px;
         border-bottom-right-radius: 4px;
       }
-      .Error {
+      .Problem {
         background: #EDF2F7;
         margin: 0 0 1px;
         padding: 20px 25px;
         text-align: left;
       }
-      .Error-message {
+      .Problem-message {
         font-size: 16px;
         color: #3C5675;
         font-weight: normal;
         margin: 0;
       }
-      .Error-detail {
+      .Problem-detail {
         font-size: 13px;
         margin: 5px 0 0;
       }
diff --git a/views/install/errors.php b/views/install/errors.php
deleted file mode 100644
index bd73aeb43..000000000
--- a/views/install/errors.php
+++ /dev/null
@@ -1,14 +0,0 @@
-<h2>Hold Up!</h2>
-
-<p>These errors must be resolved before you can continue the installation. If you're having trouble, get help on the <a href="https://flarum.org/docs/install.html" target="_blank">Flarum website</a>.</p>
-
-<div class="Errors">
-  <?php foreach ($errors as $error): ?>
-    <div class="Error">
-      <h3 class="Error-message"><?php echo $error['message']; ?></h3>
-      <?php if (! empty($error['detail'])): ?>
-        <p class="Error-detail"><?php echo $error['detail']; ?></p>
-      <?php endif; ?>
-    </div>
-  <?php endforeach; ?>
-</div>
diff --git a/views/install/problems.php b/views/install/problems.php
new file mode 100644
index 000000000..36b8d6252
--- /dev/null
+++ b/views/install/problems.php
@@ -0,0 +1,14 @@
+<h2>Hold Up!</h2>
+
+<p>These problems must be resolved before you can continue the installation. If you're having trouble, get help on the <a href="https://flarum.org/docs/install.html" target="_blank">Flarum website</a>.</p>
+
+<div class="Problems">
+  <?php foreach ($problems as $problem): ?>
+    <div class="Problem">
+      <h3 class="Problem-message"><?php echo $problem['message']; ?></h3>
+      <?php if (! empty($problem['detail'])): ?>
+        <p class="Problem-detail"><?php echo $problem['detail']; ?></p>
+      <?php endif; ?>
+    </div>
+  <?php endforeach; ?>
+</div>