Allowed different storage types for images and attachments

- Added new env and config vars to allow this.
- Also added tests for awkward config logic including fallback for new
env vars.

Closes #1302

diff --git a/.env.example.complete b/.env.example.complete
index 37421a419e5485c3f4e6d612bf08236e3a0f2159..829a7509b2a3fa31229017180f776e2347679adb 100644 (file)
--- a/.env.example.complete
+++ b/.env.example.complete
@@ -95,6 +95,16 @@ QUEUE_DRIVER=sync
 # Can be 'local', 'local_secure' or 's3'
 STORAGE_TYPE=local
 
+# Image storage system to use
+# Defaults to the value of STORAGE_TYPE if unset.
+# Accepts the same values as STORAGE_TYPE.
+STORAGE_IMAGE_TYPE=local
+
+# Attachment storage system to use
+# Defaults to the value of STORAGE_TYPE if unset.
+# Accepts the same values as STORAGE_TYPE although 'local' will be forced to 'local_secure'.
+STORAGE_ATTACHMENT_TYPE=local_secure
+
 # Amazon S3 storage configuration
 STORAGE_S3_KEY=your-s3-key
 STORAGE_S3_SECRET=your-s3-secret

diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php
index e613642c4f3dd8699a7c3f0c37b2c5394e468700..6e875a1e7a35c9cec514ccd3e21316ef04980019 100644 (file)
--- a/app/Uploads/AttachmentService.php
+++ b/app/Uploads/AttachmentService.php
@@ -13,7 +13,7 @@ class AttachmentService extends UploadService
      */
     protected function getStorage()
     {
-        $storageType = config('filesystems.default');
+        $storageType = config('filesystems.attachments');
 
         // Override default location if set to local public to ensure not visible.
         if ($storageType === 'local') {

diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php
index 8eefbaf9dd3468521d388addb292c7285ffd9b16..71fd2cd4eab31f5c0f4545cc9c671d95090443f6 100644 (file)
--- a/app/Uploads/ImageService.php
+++ b/app/Uploads/ImageService.php
@@ -45,9 +45,9 @@ class ImageService extends UploadService
      */
     protected function getStorage($type = '')
     {
-        $storageType = config('filesystems.default');
+        $storageType = config('filesystems.images');
 
-        // Override default location if set to local public to ensure not visible.
+        // Ensure system images (App logo) are uploaded to a public space
         if ($type === 'system' && $storageType === 'local_secure') {
             $storageType = 'local';
         }
@@ -458,7 +458,7 @@ class ImageService extends UploadService
             // Get the standard public s3 url if s3 is set as storage type
             // Uses the nice, short URL if bucket name has no periods in otherwise the longer
             // region-based url will be used to prevent http issues.
-            if ($storageUrl == false && config('filesystems.default') === 's3') {
+            if ($storageUrl == false && config('filesystems.images') === 's3') {
                 $storageDetails = config('filesystems.disks.s3');
                 if (strpos($storageDetails['bucket'], '.') === false) {
                     $storageUrl = 'https://' . $storageDetails['bucket'] . '.s3.amazonaws.com';

diff --git a/config/filesystems.php b/config/filesystems.php
index 13198a5052e83b02ad0cf3f939b89229f48a93a0..bd7d28300abae17112857ead07d3d000c4fd823b 100644 (file)
--- a/config/filesystems.php
+++ b/config/filesystems.php
@@ -14,6 +14,12 @@ return [
     // Options: local, local_secure, s3
     'default' => env('STORAGE_TYPE', 'local'),
 
+    // Filesystem to use specifically for image uploads.
+    'images' => env('STORAGE_IMAGE_TYPE', env('STORAGE_TYPE', 'local')),
+
+    // Filesystem to use specifically for file attachments.
+    'attachments' => env('STORAGE_ATTACHMENT_TYPE', env('STORAGE_TYPE', 'local')),
+
     // Storage URL
     // This is the url to where the storage is located for when using an external
     // file storage service, such as s3, to store publicly accessible assets.

diff --git a/phpunit.xml b/phpunit.xml
index 804afcf5d3ed2ab681792aaffbea10e168a0e1ce..0e51f6af16b375b92fd612353a29306a4e06feef 100644 (file)
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -34,6 +34,8 @@
         <env name="AVATAR_URL" value=""/>
         <env name="LDAP_VERSION" value="3"/>
         <env name="STORAGE_TYPE" value="local"/>
+        <env name="ATTACHMENT_STORAGE_TYPE" value="local"/>
+        <env name="IMAGE_STORAGE_TYPE" value="local"/>
         <env name="GITHUB_APP_ID" value="aaaaaaaaaaaaaa"/>
         <env name="GITHUB_APP_SECRET" value="aaaaaaaaaaaaaa"/>
         <env name="GITHUB_AUTO_REGISTER" value=""/>

diff --git a/tests/Unit/ConfigTest.php b/tests/Unit/ConfigTest.php
new file mode 100644 (file)
index 0000000..45f7b07
--- /dev/null
+++ b/tests/Unit/ConfigTest.php
@@ -0,0 +1,54 @@
+<?php namespace Tests;
+
+/**
+ * Class ConfigTest
+ * Many of the tests here are to check on tweaks made
+ * to maintain backwards compatibility.
+ *
+ * @package Tests
+ */
+class ConfigTest extends TestCase
+{
+
+    public function test_filesystem_images_falls_back_to_storage_type_var()
+    {
+        putenv('STORAGE_TYPE=local_secure');
+
+        $this->checkEnvConfigResult('STORAGE_IMAGE_TYPE', 's3', 'filesystems.images', 's3');
+        $this->checkEnvConfigResult('STORAGE_IMAGE_TYPE', null, 'filesystems.images', 'local_secure');
+    }
+
+    public function test_filesystem_attachments_falls_back_to_storage_type_var()
+    {
+        putenv('STORAGE_TYPE=local_secure');
+
+        $this->checkEnvConfigResult('STORAGE_ATTACHMENT_TYPE', 's3', 'filesystems.attachments', 's3');
+        $this->checkEnvConfigResult('STORAGE_ATTACHMENT_TYPE', null, 'filesystems.attachments', 'local_secure');
+    }
+
+    public function test_app_url_blank_if_old_default_value()
+    {
+        $initUrl = 'https://example.com/docs';
+        $oldDefault = 'http://bookstack.dev';
+        $this->checkEnvConfigResult('APP_URL', $initUrl, 'app.url', $initUrl);
+        $this->checkEnvConfigResult('APP_URL', $oldDefault, 'app.url', '');
+    }
+
+    /**
+     * Set an environment variable of the given name and value
+     * then check the given config key to see if it matches the given result.
+     * Providing a null $envVal clears the variable.
+     * @param string $envName
+     * @param string|null $envVal
+     * @param string $configKey
+     * @param string $expectedResult
+     */
+    protected function checkEnvConfigResult(string $envName, $envVal, string $configKey, string $expectedResult)
+    {
+        $envString = $envName . (is_null($envVal) ? '' : '=') . ($envVal ?? '');
+        putenv($envString);
+        $this->refreshApplication();
+        $this->assertEquals($expectedResult, config($configKey));
+    }
+
+}
\ No newline at end of file

diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php
index 01bf23d5b2b3ba2ab9a631d9ef151e101ff94ada..f9265337889c94ef7f049159e92da6bb3aada780 100644 (file)
--- a/tests/Uploads/ImageTest.php
+++ b/tests/Uploads/ImageTest.php
@@ -176,7 +176,7 @@ class ImageTest extends TestCase
 
     public function test_secure_images_uploads_to_correct_place()
     {
-        config()->set('filesystems.default', 'local_secure');
+        config()->set('filesystems.images', 'local_secure');
         $this->asEditor();
         $galleryFile = $this->getTestImage('my-secure-test-upload.png');
         $page = Page::first();
@@ -194,7 +194,7 @@ class ImageTest extends TestCase
 
     public function test_secure_images_included_in_exports()
     {
-        config()->set('filesystems.default', 'local_secure');
+        config()->set('filesystems.images', 'local_secure');
         $this->asEditor();
         $galleryFile = $this->getTestImage('my-secure-test-upload.png');
         $page = Page::first();
@@ -217,7 +217,7 @@ class ImageTest extends TestCase
 
     public function test_system_images_remain_public()
     {
-        config()->set('filesystems.default', 'local_secure');
+        config()->set('filesystems.images', 'local_secure');
         $this->asAdmin();
         $galleryFile = $this->getTestImage('my-system-test-upload.png');
         $expectedPath = public_path('uploads/images/system/' . Date('Y-m') . '/my-system-test-upload.png');