chore: refactor cli command, drop regions, introduce cache header (#3)

* chore: refactor cli command, drop regions, introduce cache header * Apply fixes from StyleCI * chore: remove tmp notes * Apply fixes from StyleCI * adjust comment * remove debug code --------- Co-authored-by: StyleCI Bot <[email protected]>
FriendsOfFlarum · Nov 23, 2024 · 4fe09e6 · 4fe09e6
1 parent de1e181
commit 4fe09e6
Show file tree

Hide file tree

Showing 11 changed files with 136 additions and 177 deletions.
diff --git a/README.md b/README.md
@@ -24,12 +24,13 @@ The S3 (or compatible) bucket can be configured either by environment variables
 #### Environment variables
 - `FOF_S3_ACCESS_KEY_ID` - your access key ID *
 - `FOF_S3_SECRET_ACCESS_KEY` - your secret *
-- `FOF_S3_DEFAULT_REGION` - the region *
+- `FOF_S3_REGION` - the region *
 - `FOF_S3_BUCKET` - the bucket name *
 - `FOF_S3_URL` - the public facing base URL of the bucket
 - `FOF_S3_ENDPOINT` - the ARN
 - `FOF_S3_ACL` - The ACL, if any, that should be applied to the uploaded object. For possible values, see [AWS Docs](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl) 
 - `FOF_S3_PATH_STYLE_ENDPOINT` - boolean value
+- `FOF_S3_CACHE_CONTROL` - Optional. Specify the `max-age` header files should be served with, for example `3153600` (1 year). `0` or not set = no caching.
 
 `*` denotes the minimum requirements for using S3 on AWS. S3-compatible services will require more.
 
@@ -39,10 +40,10 @@ If you plan to setup the S3 configuration using the environment variables, pleas
 
 After your new bucket is configured, any exisiting files, will not exist there (ie uploaded avatars, profile covers, etc).
 
-Use the provided command to start moving these files:
+Use the provided command to start copying these files. An optional additional paramater `--move` will delete the files from your local filesystem after a successful copy.
 
 ```php
-php flarum fof:s3:move
+php flarum fof:s3:copy --move
 ```
 
 ## Links

diff --git a/extend.php b/extend.php
@@ -28,7 +28,7 @@
         ->register(Provider\S3DiskProvider::class),
 
     (new Extend\Console())
-        ->command(Console\MoveAssetsCommand::class),
+        ->command(Console\CopyAssetsCommand::class),
 
     (new Extend\Filesystem())
         ->driver('s3', Driver\S3Driver::class)

diff --git a/js/src/admin/components/S3SettingsPage.tsx b/js/src/admin/components/S3SettingsPage.tsx
@@ -1,6 +1,7 @@
 import app from 'flarum/admin/app';
 import ExtensionPage from 'flarum/admin/components/ExtensionPage';
 import ItemList from 'flarum/common/utils/ItemList';
+import Placeholder from 'flarum/common/components/Placeholder';
 import type Mithril from 'mithril';
 
 type AwsRegion = {
@@ -86,6 +87,11 @@ export default class S3SettingsPage extends ExtensionPage {
         })
       );
 
+    // If there are no items, add a placeholder.
+    if (items.toArray().length === 0 && this.s3SetByEnv) {
+      items.add('setByEnv', <Placeholder text={app.translator.trans('fof-s3-assets.admin.settings.general.configured_by_environment')} />);
+    }
+
     return items;
   }
 
@@ -96,7 +102,7 @@ export default class S3SettingsPage extends ExtensionPage {
       'awsS3Key',
       this.buildSettingComponent({
         setting: `${this.settingPrefix}awsS3Key`,
-        type: 'password',
+        type: 'string',
         label: app.translator.trans('fof-s3-assets.admin.settings.s3key.label'),
         help: app.translator.trans('fof-s3-assets.admin.settings.s3key.help'),
       })
@@ -106,7 +112,7 @@ export default class S3SettingsPage extends ExtensionPage {
       'awsS3Secret',
       this.buildSettingComponent({
         setting: `${this.settingPrefix}awsS3Secret`,
-        type: 'password',
+        type: 'string',
         label: app.translator.trans('fof-s3-assets.admin.settings.s3secret.label'),
         help: app.translator.trans('fof-s3-assets.admin.settings.s3secret.help'),
       })
@@ -116,14 +122,7 @@ export default class S3SettingsPage extends ExtensionPage {
       'awsS3Region',
       this.buildSettingComponent({
         setting: `${this.settingPrefix}awsS3Region`,
-        type: 'select',
-        options: this.awsRegions.reduce(
-          (options, region) => {
-            options[region.value] = `${region.label} (${region.value})`;
-            return options;
-          },
-          {} as Record<string, string>
-        ),
+        type: 'string',
         label: app.translator.trans('fof-s3-assets.admin.settings.s3region.label'),
         help: app.translator.trans('fof-s3-assets.admin.settings.s3region.help'),
       })
@@ -149,6 +148,17 @@ export default class S3SettingsPage extends ExtensionPage {
       })
     );
 
+    items.add(
+      'awsS3CacheControl',
+      this.buildSettingComponent({
+        setting: `${this.settingPrefix}awsS3CacheControl`,
+        type: 'number',
+        min: 0,
+        label: app.translator.trans('fof-s3-assets.admin.settings.aws-s3.cache-control.label'),
+        help: app.translator.trans('fof-s3-assets.admin.settings.aws-s3.cache-control.help'),
+      })
+    );
+
     return items;
   }
 

diff --git a/locale/en.yml b/locale/en.yml
@@ -2,19 +2,25 @@ fof-s3-assets:
   admin:
     settings:
       general:
+        configured_by_environment: Your bucket settings have been pre-configured, nothing to see here.
         heading: General Settings
         help: General settings for the assets.
         shareWithFoFUpload:
           label: Use FoF Upload S3 settings.
           help: Re-use the S3 settings from the FoF Upload extension.
       aws-s3:
+        cache-control:
+          label: Cache-Control
+          help: Sets the Cache-Control header for the uploaded files. 0 means no cache.
         heading: S3 Settings
         help: Settings for AWS S3 and/or S3-compatible buckets. If you are using AWS S3, you only need to configure these settings.
+
+
       aws-s3-compatible:
         heading: S3-Compatible Settings
         help: Settings needed for S3-compatible buckets only. Leave these settings blank if you are using AWS S3.
 
-      configured_by_environment: Your bucket settings have been pre-configured, nothing to see here.
+
       aws-section: These settings are required for both AWS S3 and S3-compatible buckets
       s3-compatible-section: These settings are only required for S3-compatible buckets
       s3-compatible-section-help: Leave these settings blank if using AWS S3

diff --git a/src/Console/CopyAssetsCommand.php b/src/Console/CopyAssetsCommand.php
@@ -0,0 +1,80 @@
+<?php
+
+/*
+ * This file is part of fof/s3-assets.
+ *
+ * Copyright (c) FriendsOfFlarum
+ *
+ * For the full copyright and license information, please view the LICENSE.md
+ * file that was distributed with this source code.
+ */
+
+namespace FoF\S3Assets\Console;
+
+use Flarum\Foundation\Console\AssetsPublishCommand;
+use Flarum\Foundation\Paths;
+use Illuminate\Console\Command;
+use Illuminate\Contracts\Container\Container;
+use Illuminate\Contracts\Filesystem\Cloud;
+use Illuminate\Contracts\Filesystem\Factory;
+use Illuminate\Filesystem\Filesystem;
+use Symfony\Component\Console\Input\ArrayInput;
+use Symfony\Component\Console\Output\ConsoleOutput;
+
+class CopyAssetsCommand extends Command
+{
+    protected $signature = 'fof:s3:copy 
+        {--move : Delete local files after moving to the S3 disk}';
+
+    protected $description = 'Copy assets to the S3 disk';
+
+    public function handle(Container $container, Factory $factory, Paths $paths, AssetsPublishCommand $publishCommand)
+    {
+        /** @var Filesystem $localFilesystem */
+        $localFilesystem = $container->make('files');
+
+        // Determine if files should be deleted after moving
+        $deleteAfterMove = $this->option('move');
+
+        // Move assets
+        $this->info($deleteAfterMove ? 'Moving assets...' : 'Copying assets...');
+        $this->moveFilesToDisk($localFilesystem, $paths->public.'/assets', $factory->disk('flarum-assets'), $deleteAfterMove);
+
+        $publishCommand->run(
+            new ArrayInput([]),
+            new ConsoleOutput()
+        );
+    }
+
+    /**
+     * Get the registered disks.
+     *
+     * @return array
+     */
+    protected function getFlarumDisks(): array
+    {
+        return resolve('flarum.filesystem.disks');
+    }
+
+    protected function moveFilesToDisk(Filesystem $localFilesystem, string $localPath, Cloud $disk, bool $deleteAfterMove): void
+    {
+        $count = count($localFilesystem->allFiles($localPath));
+        $this->output->progressStart($count);
+
+        foreach ($localFilesystem->allFiles($localPath) as $file) {
+            /** @var \Symfony\Component\Finder\SplFileInfo $file */
+            $written = $disk->put($file->getRelativePathname(), $file->getContents());
+
+            if ($written) {
+                if ($deleteAfterMove) {
+                    $localFilesystem->delete($file->getPathname());
+                }
+                $this->output->progressAdvance();
+            } else {
+                throw new \Exception('File did not copy');
+            }
+        }
+
+        $this->output->progressFinish();
+    }
+}
diff --git a/src/Console/MoveAssetsCommand.php b/src/Console/MoveAssetsCommand.php
diff --git a/src/Content/AdminPayload.php b/src/Content/AdminPayload.php
@@ -30,7 +30,6 @@ public function __construct(
     public function __invoke(Document $document)
     {
         $document->payload['s3SetByEnv'] = $this->s3Config->shouldUseEnv();
-        $document->payload['FoFS3Regions'] = $this->s3->getAwsRegions();
         $document->payload['FoFS3ShareWithFoFUpload'] = $this->settings->get('fof-s3-assets.share_s3_config_with_fof_upload');
     }
 }
diff --git a/src/Driver/Config.php b/src/Driver/Config.php
@@ -52,7 +52,7 @@ public function config(): array
         return $config;
     }
 
-    protected function buildConfigArray(string $key, string $secret, string $region, string $bucket, string $cdnUrl, ?string $endpoint, ?bool $pathStyle, ?string $acl, bool $setByEnv = false, string $driver = 's3'): array
+    protected function buildConfigArray(string $key, string $secret, string $region, string $bucket, string $cdnUrl, ?string $endpoint, ?bool $pathStyle, ?string $acl, ?int $cache = null, bool $setByEnv = false, string $driver = 's3'): array
     {
         // These are the required values for AWS S3.
         // Some S3-compatible services may require additional values, so we check if any of these are set below.
@@ -64,6 +64,7 @@ protected function buildConfigArray(string $key, string $secret, string $region,
             'bucket'                  => $bucket,
             'url'                     => $cdnUrl,
             'set_by_environment'      => $setByEnv,
+            'options'                 => [],
         ];
 
         // These values are generally only required for S3-compatible services.
@@ -77,9 +78,11 @@ protected function buildConfigArray(string $key, string $secret, string $region,
         }
 
         if ($acl) {
-            $config['options'] = [
-                'ACL' => $acl,
-            ];
+            $config['options']['ACL'] = $acl;
+        }
+
+        if ($cache) {
+            $config['options']['CacheControl'] = "max-age=$cache";
         }
 
         return $config;
@@ -93,6 +96,7 @@ protected function buildConfigFromEnv(): array
         $endpoint = env('FOF_S3_ENDPOINT');
         $pathStyle = (bool) env('FOF_S3_PATH_STYLE_ENDPOINT', false);
         $acl = env('FOF_S3_ACL');
+        $cache = env('FOF_S3_CACHE_CONTROL');
 
         return $this->buildConfigArray(
             key: env('FOF_S3_ACCESS_KEY_ID'),
@@ -103,6 +107,7 @@ protected function buildConfigFromEnv(): array
             endpoint: $endpoint,
             pathStyle: $pathStyle,
             acl: $acl,
+            cache: $cache,
             setByEnv: true
         );
     }
@@ -115,6 +120,7 @@ protected function buildConfigFromSettings(): array
         $endpoint = $this->getSetting('awsS3Endpoint');
         $pathStyle = (bool) $this->getSetting('awsS3UsePathStyleEndpoint', false);
         $acl = $this->getSetting('awsS3ACL');
+        $cache = $this->getSetting('awsS3CacheControl');
 
         return $this->buildConfigArray(
             key: $this->getSetting('awsS3Key', ''),
@@ -125,6 +131,7 @@ protected function buildConfigFromSettings(): array
             endpoint: $endpoint,
             pathStyle: $pathStyle,
             acl: $acl,
+            cache: $cache,
             setByEnv: false
         );
     }