From 4fe09e6af11391151cdcc5fdd43b288d42dc58bf Mon Sep 17 00:00:00 2001 From: IanM <16573496+imorland@users.noreply.github.com> Date: Sat, 23 Nov 2024 09:05:05 +0000 Subject: [PATCH] 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 --- README.md | 7 +- extend.php | 2 +- js/src/admin/components/S3SettingsPage.tsx | 30 ++++-- locale/en.yml | 8 +- src/Console/CopyAssetsCommand.php | 80 ++++++++++++++++ src/Console/MoveAssetsCommand.php | 102 --------------------- src/Content/AdminPayload.php | 1 - src/Driver/Config.php | 15 ++- src/Driver/S3Driver.php | 4 +- src/Repository/S3Repository.php | 45 --------- src/Validator/S3DiskConfigValidator.php | 19 ++-- 11 files changed, 136 insertions(+), 177 deletions(-) create mode 100644 src/Console/CopyAssetsCommand.php delete mode 100644 src/Console/MoveAssetsCommand.php diff --git a/README.md b/README.md index 16c94f8..5ee977c 100644 --- 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 index 6bf8156..ffffd8b 100644 --- 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 index 53a8795..30a0b87 100644 --- 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', ); + } + 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 - ), + 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 index 377c1d7..62108ff 100644 --- 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 new file mode 100644 index 0000000..c90f25b --- /dev/null +++ b/src/Console/CopyAssetsCommand.php @@ -0,0 +1,80 @@ +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 deleted file mode 100644 index fc7e905..0000000 --- a/src/Console/MoveAssetsCommand.php +++ /dev/null @@ -1,102 +0,0 @@ -avatarDisk = $factory->disk('flarum-avatars'); - - parent::__construct(); - } - - /** - * {@inheritdoc} - */ - protected function configure() - { - $this - ->setName('fof:s3:move') - ->setDescription('Move avatars, etc from local filesystem to S3 disks, then republish remaning assets'); - } - - /** - * {@inheritdoc} - */ - protected function fire() - { - /** @var Filesystem $localFilesystem */ - $localFilesystem = $this->container->make('files'); - - // Move avatars - $this->info('Moving avatars...'); - $this->moveFilesToDisk($localFilesystem, $this->paths->public.'/assets/avatars', $this->avatarDisk); - - // Move profile covers - if (Arr::has($this->getFlarumDisks(), 'sycho-profile-cover')) { - $this->info('Moving profile covers...'); - $coversDisk = $this->factory->disk('sycho-profile-cover'); - $this->moveFilesToDisk($localFilesystem, $this->paths->public.'/assets/covers', $coversDisk); - } - - $this->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): void - { - foreach ($localFilesystem->allFiles($localPath) as $file) { - /** @var \Symfony\Component\Finder\SplFileInfo $file */ - $this->info('Moving '.$file->getPathname()); - $written = $disk->put($file->getRelativePathname(), $file->getContents()); - - if ($written) { - //$localFilesystem->delete($file); - } else { - throw new \Exception('File did not move'); - } - } - } -} diff --git a/src/Content/AdminPayload.php b/src/Content/AdminPayload.php index e007bee..635a265 100644 --- 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 index 1e55eae..f5a7b7f 100644 --- 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 ); } diff --git a/src/Driver/S3Driver.php b/src/Driver/S3Driver.php index 30d40e0..bc7740f 100644 --- a/src/Driver/S3Driver.php +++ b/src/Driver/S3Driver.php @@ -47,9 +47,11 @@ public function build( $root = Arr::get($localConfig, 'root'); $root = str_replace($this->paths->public, '', $root); - return $this->manager->createS3Driver(array_merge( + $driver = $this->manager->createS3Driver(array_merge( $this->config->config(), ['root' => $root] )); + + return $driver; } } diff --git a/src/Repository/S3Repository.php b/src/Repository/S3Repository.php index 149319d..8a336ec 100644 --- a/src/Repository/S3Repository.php +++ b/src/Repository/S3Repository.php @@ -19,49 +19,4 @@ public function __construct( protected LoggerInterface $logger ) { } - - /** - * Get a list of AWS regions available for S3. - * - * @return array - */ - public function getAwsRegions(): array - { - // List of AWS regions supporting S3 - // Last verified: 22nd November 2024 - return [ - ['value' => 'us-east-2', 'label' => 'US East (Ohio)'], - ['value' => 'us-east-1', 'label' => 'US East (N. Virginia)'], - ['value' => 'us-west-1', 'label' => 'US West (N. California)'], - ['value' => 'us-west-2', 'label' => 'US West (Oregon)'], - ['value' => 'af-south-1', 'label' => 'Africa (Cape Town)'], - ['value' => 'ap-east-1', 'label' => 'Asia Pacific (Hong Kong)'], - ['value' => 'ap-south-2', 'label' => 'Asia Pacific (Hyderabad)'], - ['value' => 'ap-southeast-3', 'label' => 'Asia Pacific (Jakarta)'], - ['value' => 'ap-southeast-5', 'label' => 'Asia Pacific (Malaysia)'], - ['value' => 'ap-southeast-4', 'label' => 'Asia Pacific (Melbourne)'], - ['value' => 'ap-south-1', 'label' => 'Asia Pacific (Mumbai)'], - ['value' => 'ap-northeast-3', 'label' => 'Asia Pacific (Osaka)'], - ['value' => 'ap-northeast-2', 'label' => 'Asia Pacific (Seoul)'], - ['value' => 'ap-southeast-1', 'label' => 'Asia Pacific (Singapore)'], - ['value' => 'ap-southeast-2', 'label' => 'Asia Pacific (Sydney)'], - ['value' => 'ap-northeast-1', 'label' => 'Asia Pacific (Tokyo)'], - ['value' => 'ca-central-1', 'label' => 'Canada (Central)'], - ['value' => 'ca-west-1', 'label' => 'Canada West (Calgary)'], - ['value' => 'eu-central-1', 'label' => 'Europe (Frankfurt)'], - ['value' => 'eu-west-1', 'label' => 'Europe (Ireland)'], - ['value' => 'eu-west-2', 'label' => 'Europe (London)'], - ['value' => 'eu-south-1', 'label' => 'Europe (Milan)'], - ['value' => 'eu-west-3', 'label' => 'Europe (Paris)'], - ['value' => 'eu-south-2', 'label' => 'Europe (Spain)'], - ['value' => 'eu-north-1', 'label' => 'Europe (Stockholm)'], - ['value' => 'eu-central-2', 'label' => 'Europe (Zurich)'], - ['value' => 'il-central-1', 'label' => 'Israel (Tel Aviv)'], - ['value' => 'me-south-1', 'label' => 'Middle East (Bahrain)'], - ['value' => 'me-central-1', 'label' => 'Middle East (UAE)'], - ['value' => 'sa-east-1', 'label' => 'South America (São Paulo)'], - ['value' => 'us-gov-east-1', 'label' => 'AWS GovCloud (US-East)'], - ['value' => 'us-gov-west-1', 'label' => 'AWS GovCloud (US-West)'], - ]; - } } diff --git a/src/Validator/S3DiskConfigValidator.php b/src/Validator/S3DiskConfigValidator.php index 00acdd7..d8c56ab 100644 --- a/src/Validator/S3DiskConfigValidator.php +++ b/src/Validator/S3DiskConfigValidator.php @@ -16,15 +16,16 @@ class S3DiskConfigValidator extends AbstractValidator { protected $rules = [ - 'driver' => ['required', 'string', 'in:s3'], - 'key' => ['required', 'string'], - 'secret' => ['required', 'string'], - 'region' => ['required', 'string'], - 'bucket' => ['required', 'string'], - 'url' => ['url'], - //'endpoint' => ['url'], + 'driver' => ['required', 'string', 'in:s3'], + 'key' => ['required', 'string'], + 'secret' => ['required', 'string'], + 'region' => ['required', 'string'], + 'bucket' => ['required', 'string'], + 'url' => ['url'], + 'endpoint' => ['url'], 'use_path_style_endpoint' => ['required', 'bool'], - //'options.ACL' => ['string'], - 'set_by_environment' => ['required', 'bool'], + 'options.ACL' => ['string'], + 'options.CacheControl' => ['string'], + 'set_by_environment' => ['required', 'bool'], ]; }