From 9d5a84c3d877bbf3566e1cde3c96356a8d4aad11 Mon Sep 17 00:00:00 2001 From: Stuart Clark Date: Mon, 22 Jun 2026 03:56:10 +0000 Subject: [PATCH] feat(#2072237): allow disabling File (Field) Paths processing site-wide and per entity Add a site-wide "enabled" setting (config + settings form) and a transient `filefield_paths_settings` entity property so migrations and other programmatic saves can suppress or override processing for a single save without touching field configuration. Includes an update hook to default existing sites to enabled and a runtime requirements warning when disabled site-wide. Issue #2072237 by jose reyero, herved, trebormc, feng-shui, rudolfbyker, geek-merlin, vistree, deciphered. --- README.md | 39 +++- config/install/filefield_paths.settings.yml | 1 + config/schema/filefield_paths.schema.yml | 3 + filefield_paths.api.php | 36 ++++ filefield_paths.install | 60 ++++--- phpstan.neon | 12 ++ src/Form/SettingsForm.php | 8 + src/Hook/EntityWithFileField.php | 60 ++++++- src/Hook/FieldWidgetSingleElementForm.php | 7 +- .../EntityWithFileFieldOverrideTest.php | 166 ++++++++++++++++++ tests/src/Kernel/InstallFunctionsTest.php | 69 ++++++++ tests/src/Kernel/SettingsFormTest.php | 24 ++- tests/src/Unit/EntityWithFileFieldTest.php | 86 ++++++++- .../Unit/FieldWidgetSingleElementFormTest.php | 49 +++++- 14 files changed, 569 insertions(+), 51 deletions(-) create mode 100644 tests/src/Kernel/EntityWithFileFieldOverrideTest.php diff --git a/README.md b/README.md index 8e966fa..ae6bec0 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ Submit bug reports and feature suggestions, or track changes in the ## Table of contents - Requirements +- Recommended modules - Configuration - Features - FAQ @@ -30,24 +31,33 @@ Submit bug reports and feature suggestions, or track changes in the ## Requirements -This module requires the following modules: +This module requires the Drupal core File module. -- [Pathauto](https://www.drupal.org/project/pathauto) -- [Redirect](https://www.drupal.org/project/redirect) -- [Token](https://www.drupal.org/project/token) + +## Recommended modules + +These modules are optional, and are used when present: + +- [Pathauto](https://www.drupal.org/project/pathauto) - for filename/path + cleanup options. +- [Redirect](https://www.drupal.org/project/redirect) - to create a redirect + when a previously uploaded file is moved. +- [Token](https://www.drupal.org/project/token) - for the token browser when + building path/filename patterns. ## Configuration Once installed, File (Field) Paths needs to be configured for each file field -you wish to use. Settings can be found on the settings form of any supported -file based field. +you wish to use, on that field's settings form. For example, for an Image +field on the Article content type: -*Example:* - Administration > Structure > Content types > Article > Manage fields > Image -http://example.com/admin/structure/types/manage/article/fields/field_image +(admin/structure/types/manage/article/fields/field_image) +Module-wide settings, such as the temporary upload location, are at +Administration > Configuration > Media > File system > File (Field) Paths +(admin/config/media/file-system/filefield-paths). ## Features @@ -98,6 +108,17 @@ http://example.com/admin/structure/types/manage/article/fields/field_image servers so that you can make sure not to introduce any linking issues. +**Q: How do I disable File (Field) Paths?** + +**A:** At three levels: uncheck "Enable File (Field) Paths?" on a field's settings + form to disable it for that field, uncheck "Enable File (Field) Paths" on the + module's settings form to disable it site-wide, or set the + `filefield_paths_settings` property on an entity before saving it to disable + (or otherwise override) processing for that one save only, without changing + any configuration. See `filefield_paths.api.php` for the per-save override's + full API. + + ## Maintainers diff --git a/config/install/filefield_paths.settings.yml b/config/install/filefield_paths.settings.yml index ae96197..8f9450c 100644 --- a/config/install/filefield_paths.settings.yml +++ b/config/install/filefield_paths.settings.yml @@ -1 +1,2 @@ +enabled: true temp_location: 'public://filefield_paths' diff --git a/config/schema/filefield_paths.schema.yml b/config/schema/filefield_paths.schema.yml index 983393c..55504d1 100644 --- a/config/schema/filefield_paths.schema.yml +++ b/config/schema/filefield_paths.schema.yml @@ -4,6 +4,9 @@ filefield_paths.settings: type: config_object label: 'File (Field) Paths configuration settings' mapping: + enabled: + type: boolean + label: 'Enabled' temp_location: type: string label: 'Temporary file location' diff --git a/filefield_paths.api.php b/filefield_paths.api.php index 8feade1..fbf2172 100644 --- a/filefield_paths.api.php +++ b/filefield_paths.api.php @@ -3,6 +3,42 @@ /** * @file * Hooks provided by the File (Field) Paths module. + * + * @section sec_disabling Disabling processing + * + * File (Field) Paths offers three ways to suppress processing, at different + * levels of granularity: + * - Site-wide: disable the "Enable File (Field) Paths" checkbox on the + * module's settings form (filefield_paths.admin_settings), or set the + * `enabled` key of the `filefield_paths.settings` config object to FALSE. + * A persistent config change affecting every field on the site. + * - Per field, persistent: disable the "Enable File (Field) Paths?" checkbox + * on a specific field's settings form. A persistent config change + * affecting every entity that uses that field. + * - Per entity, transient: set the `filefield_paths_settings` property on an + * entity before saving it, to suppress (or otherwise override) processing + * for that single save only. This is a plain runtime property, never + * persisted to storage, and is the recommended approach for migrations and + * other programmatic imports where you don't want to touch field + * configuration (which would invalidate site-wide caches) just to skip + * processing for the rows being imported. For example: + * @code + * // Suppress every file field on this entity for this save only. + * $entity->filefield_paths_settings = ['enabled' => FALSE]; + * + * // Suppress just one field, leaving others on the entity untouched. + * $entity->filefield_paths_settings = ['field_image' => ['enabled' => FALSE]]; + * + * // Override any other settings key the same way, not just 'enabled' - + * // here the file still gets moved/renamed, but no redirect is created + * // for this save. + * $entity->filefield_paths_settings = ['redirect' => FALSE]; + * @endcode + * A key matching a field name on the entity is treated as a field-specific + * override (and takes precedence); any other key is applied to every + * field. This can only ever suppress or modify processing that is already + * enabled via field configuration - it cannot enable File (Field) Paths on + * a field where it isn't configured. */ declare(strict_types=1); diff --git a/filefield_paths.install b/filefield_paths.install index 9ba7d29..df781b8 100644 --- a/filefield_paths.install +++ b/filefield_paths.install @@ -73,33 +73,39 @@ function filefield_paths_requirements(string $phase): array { $requirements = []; if ($phase === 'runtime') { - $temporary_path = \Drupal::config('filefield_paths.settings')->get('temp_location'); + $config = \Drupal::config('filefield_paths.settings'); + $temporary_path = $config->get('temp_location'); + $temporary_scheme = $temporary_path ? substr((string) $temporary_path, 0, 9) : NULL; - // If it's not set, we don't need to do anything because the default will - // be secure. - if (!$temporary_path) { - return []; - } - - $temporary_scheme = substr((string) $temporary_path, 0, 9); - if ($temporary_scheme !== 'public://') { - return []; - } // If private files are supported, and the temporary scheme is 'public://' // then let the user know they need to change the temporary scheme in order - // to be secure. - $wrappers = \Drupal::service('stream_wrapper_manager')->getWrappers(); - $recommended_wrappers = ['private', 'temporary']; - foreach ($recommended_wrappers as $recommended_wrapper) { - if (in_array($recommended_wrapper, array_keys($wrappers), TRUE)) { - $requirements['filefield_paths'] = [ - 'title' => t('File (Field) Paths temporary path'), - 'value' => t('Insecure!'), - 'description' => t('This site supports private files but the File (Field) Paths temporary path is under public:// which could lead to private files being temporarily exposed publicly. Change the temporary path to be under temporary:// or private:// in order to secure your files.', [':url' => Url::fromRoute('filefield_paths.admin_settings')->toString()]), - 'severity' => DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '11.2.0', fn(): RequirementSeverity => RequirementSeverity::Error, fn() => REQUIREMENT_ERROR), - ]; + // to be secure. If temp_location is unset, the default will be secure. + if ($temporary_scheme === 'public://') { + $wrappers = \Drupal::service('stream_wrapper_manager')->getWrappers(); + $recommended_wrappers = ['private', 'temporary']; + foreach ($recommended_wrappers as $recommended_wrapper) { + if (in_array($recommended_wrapper, array_keys($wrappers), TRUE)) { + $requirements['filefield_paths'] = [ + 'title' => t('File (Field) Paths temporary path'), + 'value' => t('Insecure!'), + 'description' => t('This site supports private files but the File (Field) Paths temporary path is under public:// which could lead to private files being temporarily exposed publicly. Change the temporary path to be under temporary:// or private:// in order to secure your files.', [':url' => Url::fromRoute('filefield_paths.admin_settings')->toString()]), + 'severity' => DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '11.2.0', fn(): RequirementSeverity => RequirementSeverity::Error, fn() => REQUIREMENT_ERROR), + ]; + } } } + + // Use a strict comparison: an unset value means the site predates the + // 'enabled' setting and an update hook has not run yet, not that it was + // deliberately disabled. + if ($config->get('enabled') === FALSE) { + $requirements['filefield_paths_enabled'] = [ + 'title' => t('File (Field) Paths'), + 'value' => t('Disabled'), + 'description' => t('File (Field) Paths processing is disabled site-wide. If you do not intend to use it, consider uninstalling the module instead of leaving it disabled. Re-enable it if this was not intentional.', [':url' => Url::fromRoute('filefield_paths.admin_settings')->toString()]), + 'severity' => DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '11.2.0', fn(): RequirementSeverity => RequirementSeverity::Warning, fn() => REQUIREMENT_WARNING), + ]; + } } return $requirements; @@ -123,3 +129,13 @@ function filefield_paths_update_8001(): void { function filefield_paths_update_9001(): void { filefield_paths_update_temporary_location_configuration(); } + +/** + * Add the 'enabled' setting, defaulting existing sites to enabled. + */ +function filefield_paths_update_9002(): void { + $config = \Drupal::configFactory()->getEditable('filefield_paths.settings'); + if ($config->get('enabled') === NULL) { + $config->set('enabled', TRUE)->save(); + } +} diff --git a/phpstan.neon b/phpstan.neon index 406a478..af3a7aa 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -129,4 +129,16 @@ parameters: - '#Call to deprecated function _filefield_paths_#' reportUnmatched: false + - + # filefield_paths_settings is a plain runtime property (not a field), set directly on the + # entity per filefield_paths.api.php. PHPStan's Drupal extension assumes every property on a + # ContentEntityInterface is a field access and infers FieldItemListInterface for it. + # + # See the note above on ModuleDeprecatedWrappersTest.php for why this + # uses a leading wildcard instead of a path rooted at the module. + path: '*/EntityWithFileFieldOverrideTest.php' + messages: + - '#Property Drupal\\entity_test\\Entity\\EntityTest::\$filefield_paths_settings \(Drupal\\Core\\Field\\FieldItemListInterface\) does not accept#' + reportUnmatched: false + reportUnmatchedIgnoredErrors: false diff --git a/src/Form/SettingsForm.php b/src/Form/SettingsForm.php index 209878a..42c7a99 100644 --- a/src/Form/SettingsForm.php +++ b/src/Form/SettingsForm.php @@ -81,6 +81,13 @@ protected function getEditableConfigNames(): array { */ #[\Override] public function buildForm(array $form, FormStateInterface $form_state, ?Request $request = NULL) { + $form['enabled'] = [ + '#title' => $this->t('Enable File (Field) Paths'), + '#type' => 'checkbox', + '#default_value' => $this->config('filefield_paths.settings')->get('enabled') ?? TRUE, + '#description' => $this->t('Disable this to skip processing on all fields, site-wide. This can also be configured per field. Uninstall the module instead if you do not intend to use it.'), + ]; + $description = $this->t('The location that unprocessed files will be uploaded prior to being processed by File (Field) Paths.'); $description .= '
'; $description .= $this->t('It is recommended to use the temporary file system (temporary://) whenever possible, especially for files that do not require previewing before form submission. Alternatively, if your server configuration permits, the private file system (private://) is preferred for situations where file previews — such as image previews — are needed before the form is submitted, as it provides secure and appropriate access for this functionality.'); @@ -128,6 +135,7 @@ public function validateForm(array &$form, FormStateInterface $form_state): void public function submitForm(array &$form, FormStateInterface $form_state): void { $values = $form_state->getValues(); $this->config('filefield_paths.settings') + ->set('enabled', $values['enabled']) ->set('temp_location', $values['temp_location']) ->save(); } diff --git a/src/Hook/EntityWithFileField.php b/src/Hook/EntityWithFileField.php index 0faac51..876b167 100644 --- a/src/Hook/EntityWithFileField.php +++ b/src/Hook/EntityWithFileField.php @@ -2,8 +2,15 @@ declare(strict_types=1); +/** + * @file + * Hook implementation that processes an entity's file fields on save. + */ + namespace Drupal\filefield_paths\Hook; +use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Config\ImmutableConfig; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -21,10 +28,14 @@ * * @param \Closure $moduleHandlerClosure * The module handler closure. + * @param \Closure $configFactoryClosure + * The config factory closure. */ public function __construct( #[AutowireServiceClosure(ModuleHandlerInterface::class)] private \Closure $moduleHandlerClosure, + #[AutowireServiceClosure(ConfigFactoryInterface::class)] + private \Closure $configFactoryClosure, ) {} /** @@ -33,19 +44,42 @@ public function __construct( // @phpstan-ignore-next-line #[Hook('entity_insert'), Hook('entity_update')] public function handleProcessFile(EntityInterface $entity): void {// phpcs:ignore Squiz.WhiteSpace.FunctionSpacing.Before + if (!($this->getSettings()->get('enabled') ?? TRUE)) { + return; + } if (!$entity instanceof ContentEntityInterface) { return; } $module_handler = $this->getModuleHandler(); - foreach ($entity->getFields() as $field) { - if (FieldItem::hasConfigurationEnabled($field)) { - $settings = FieldItem::getConfiguration($field); - // Invoke hook_filefield_paths_process_file(). - $module_handler->invokeAll( - 'filefield_paths_process_file', - [$entity, $field, &$settings] - ); + $fields = $entity->getFields(); + // Lets calling code skip (or otherwise tweak) processing for a single + // save without touching field config, which would invalidate caches. + // See filefield_paths.api.php for the accepted shapes of this property. + $override = $entity->filefield_paths_settings ?? []; + // The property is untyped, so guard against a caller setting something + // other than an array. + if (!is_array($override)) { + $override = []; + } + // Anything that isn't a field name applies to every field; a field name + // key scopes the override to that field only and wins if both are set. + $flat_override = array_diff_key($override, $fields); + foreach ($fields as $field_name => $field) { + if (!FieldItem::hasConfigurationEnabled($field)) { + continue; + } + $settings = $flat_override + FieldItem::getConfiguration($field); + if (is_array($override[$field_name] ?? NULL)) { + $settings = $override[$field_name] + $settings; } + if (empty($settings['enabled'])) { + continue; + } + // Invoke hook_filefield_paths_process_file(). + $module_handler->invokeAll( + 'filefield_paths_process_file', + [$entity, $field, &$settings] + ); } } @@ -59,4 +93,14 @@ private function getModuleHandler(): ModuleHandlerInterface { return ($this->moduleHandlerClosure)(); } + /** + * Retrieves the configuration settings for filefield_paths. + * + * @return \Drupal\Core\Config\ImmutableConfig + * The configuration settings object. + */ + private function getSettings(): ImmutableConfig { + return ($this->configFactoryClosure)()->get('filefield_paths.settings'); + } + } diff --git a/src/Hook/FieldWidgetSingleElementForm.php b/src/Hook/FieldWidgetSingleElementForm.php index 2852943..2af87ec 100644 --- a/src/Hook/FieldWidgetSingleElementForm.php +++ b/src/Hook/FieldWidgetSingleElementForm.php @@ -34,7 +34,12 @@ public function __construct( #[Hook('field_widget_single_element_form_alter')] public function formAlter(array &$element, FormStateInterface $form_state, array $context): void {// phpcs:ignore Squiz.WhiteSpace.FunctionSpacing.Before // Force all File (Field) Paths uploads to go to the temporary file system - // prior to being processed. + // prior to being processed. Skipped entirely when disabled site-wide, so + // uploads fall back to the field's own upload location instead of being + // staged somewhere they will never be moved out of. + if (!($this->getSettings()->get('enabled') ?? TRUE)) { + return; + } if (FieldItem::hasConfigurationEnabled(FieldItem::getFromSupportedWidget($element, $context))) { $settings = $context['items']->getFieldDefinition() ->getThirdPartySettings('filefield_paths'); diff --git a/tests/src/Kernel/EntityWithFileFieldOverrideTest.php b/tests/src/Kernel/EntityWithFileFieldOverrideTest.php new file mode 100644 index 0000000..9946fd9 --- /dev/null +++ b/tests/src/Kernel/EntityWithFileFieldOverrideTest.php @@ -0,0 +1,166 @@ + + */ + protected static $modules = [ + 'system', + 'user', + 'field', + 'file', + 'entity_test', + 'filefield_paths', + ]; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + $this->installEntitySchema('user'); + $this->installEntitySchema('file'); + $this->installEntitySchema('entity_test'); + $this->installSchema('file', ['file_usage']); + $this->installConfig(['filefield_paths']); + + foreach (['field_file', 'field_file_two'] as $field_name) { + FieldStorageConfig::create([ + 'field_name' => $field_name, + 'entity_type' => 'entity_test', + 'type' => 'file', + 'cardinality' => FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED, + ])->save(); + FieldConfig::create([ + 'entity_type' => 'entity_test', + 'field_name' => $field_name, + 'bundle' => 'entity_test', + 'third_party_settings' => [ + 'filefield_paths' => [ + 'enabled' => TRUE, + 'file_path' => ['value' => 'new-dir', 'options' => ['transliterate' => FALSE]], + 'file_name' => ['value' => '', 'options' => ['transliterate' => FALSE]], + ], + ], + ])->save(); + } + } + + /** + * Creates a permanent file at the given public:// URI. + */ + protected function createFile(string $uri): File { + $file_system = $this->container->get('file_system'); + $directory = dirname($uri); + $file_system->prepareDirectory($directory, $file_system::CREATE_DIRECTORY); + file_put_contents($uri, 'contents'); + $file = File::create(['uri' => $uri]); + $file->setPermanent(); + $file->save(); + return $file; + } + + /** + * With no override, a new entity's file is processed as configured. + */ + public function testNoOverrideProcessesFile(): void { + $file = $this->createFile('public://original/example.txt'); + + $entity = EntityTest::create(['field_file' => [['target_id' => $file->id()]]]); + $entity->save(); + + $this->assertFileExists('public://new-dir/example.txt'); + } + + /** + * A non-array override is ignored rather than crashing the save. + */ + public function testNonArrayOverrideIsIgnored(): void { + $file = $this->createFile('public://original/example.txt'); + + $entity = EntityTest::create(['field_file' => [['target_id' => $file->id()]]]); + $entity->filefield_paths_settings = 'not-an-array'; + $entity->save(); + + $this->assertFileExists('public://new-dir/example.txt'); + } + + /** + * A whole-entity override suppresses processing for every field. + */ + public function testWholeEntityOverrideSuppressesProcessing(): void { + $file = $this->createFile('public://original/example.txt'); + + $entity = EntityTest::create(['field_file' => [['target_id' => $file->id()]]]); + $entity->filefield_paths_settings = ['enabled' => FALSE]; + $entity->save(); + + $this->assertFileExists('public://original/example.txt'); + $this->assertFileDoesNotExist('public://new-dir/example.txt'); + } + + /** + * A field-keyed override only suppresses processing for that field. + */ + public function testFieldSpecificOverrideSuppressesOnlyThatField(): void { + $suppressed_file = $this->createFile('public://original/suppressed.txt'); + $processed_file = $this->createFile('public://original/processed.txt'); + + $entity = EntityTest::create([ + 'field_file' => [['target_id' => $suppressed_file->id()]], + 'field_file_two' => [['target_id' => $processed_file->id()]], + ]); + $entity->filefield_paths_settings = ['field_file' => ['enabled' => FALSE]]; + $entity->save(); + + $this->assertFileExists('public://original/suppressed.txt'); + $this->assertFileDoesNotExist('public://new-dir/suppressed.txt'); + $this->assertFileExists('public://new-dir/processed.txt'); + } + + /** + * A field-specific override takes precedence over a flat override. + */ + public function testFieldSpecificOverrideTakesPrecedenceOverFlatOverride(): void { + $file = $this->createFile('public://original/example.txt'); + + $entity = EntityTest::create(['field_file' => [['target_id' => $file->id()]]]); + // Whole-entity override disables, field-specific override re-enables. + $entity->filefield_paths_settings = [ + 'enabled' => FALSE, + 'field_file' => ['enabled' => TRUE], + ]; + $entity->save(); + + $this->assertFileExists('public://new-dir/example.txt'); + } + +} diff --git a/tests/src/Kernel/InstallFunctionsTest.php b/tests/src/Kernel/InstallFunctionsTest.php index db1b176..ec2be6e 100644 --- a/tests/src/Kernel/InstallFunctionsTest.php +++ b/tests/src/Kernel/InstallFunctionsTest.php @@ -84,6 +84,75 @@ public function testRequirementsNoTempLocation(): void { $this->assertSame([], $requirements); } + /** + * Tests that requirements() flags the global toggle being disabled. + */ + public function testRequirementsGlobalDisabledIsWarning(): void { + $this->config('filefield_paths.settings') + ->set('enabled', FALSE) + ->save(); + + $requirements = filefield_paths_requirements('runtime'); + + $this->assertArrayHasKey('filefield_paths_enabled', $requirements); + $this->assertSame(DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '11.2.0', fn(): RequirementSeverity => RequirementSeverity::Warning, fn() => REQUIREMENT_WARNING), $requirements['filefield_paths_enabled']['severity']); + } + + /** + * Tests that requirements() passes when the global toggle is enabled. + */ + public function testRequirementsGlobalEnabledIsClean(): void { + $this->config('filefield_paths.settings') + ->set('enabled', TRUE) + ->save(); + + $requirements = filefield_paths_requirements('runtime'); + + $this->assertArrayNotHasKey('filefield_paths_enabled', $requirements); + } + + /** + * Tests that requirements() does not warn when 'enabled' is merely unset. + * + * An unset value means an existing site predates the setting and the + * update hook has not run yet, not that it was deliberately disabled. + */ + public function testRequirementsUnsetEnabledIsClean(): void { + $this->config('filefield_paths.settings') + ->clear('enabled') + ->save(); + + $requirements = filefield_paths_requirements('runtime'); + + $this->assertArrayNotHasKey('filefield_paths_enabled', $requirements); + } + + /** + * Tests that update_9002 defaults an unset 'enabled' setting to TRUE. + */ + public function testUpdate9002DefaultsToEnabled(): void { + $this->config('filefield_paths.settings') + ->clear('enabled') + ->save(); + + filefield_paths_update_9002(); + + $this->assertTrue($this->config('filefield_paths.settings')->get('enabled')); + } + + /** + * Tests that update_9002 leaves an explicit 'enabled' value untouched. + */ + public function testUpdate9002LeavesExplicitValueUntouched(): void { + $this->config('filefield_paths.settings') + ->set('enabled', FALSE) + ->save(); + + filefield_paths_update_9002(); + + $this->assertFalse($this->config('filefield_paths.settings')->get('enabled')); + } + /** * Tests that requirements() only checks during the runtime phase. */ diff --git a/tests/src/Kernel/SettingsFormTest.php b/tests/src/Kernel/SettingsFormTest.php index 3fccc72..32ff749 100644 --- a/tests/src/Kernel/SettingsFormTest.php +++ b/tests/src/Kernel/SettingsFormTest.php @@ -79,7 +79,7 @@ public function testBuildFormUsesStoredConfig(): void { */ public function testSubmitFormSavesValue(): void { $form_state = new FormState(); - $form_state->setValues(['temp_location' => 'temporary://my-path']); + $form_state->setValues(['enabled' => TRUE, 'temp_location' => 'temporary://my-path']); $form = []; $this->form->submitForm($form, $form_state); @@ -87,6 +87,28 @@ public function testSubmitFormSavesValue(): void { $this->assertSame('temporary://my-path', $this->config('filefield_paths.settings')->get('temp_location')); } + /** + * Tests that buildForm sets the default enabled value. + */ + public function testBuildFormSetsDefaultEnabled(): void { + $form = $this->form->buildForm([], new FormState()); + $this->assertArrayHasKey('enabled', $form); + $this->assertTrue($form['enabled']['#default_value']); + } + + /** + * Tests that submitForm persists the enabled value. + */ + public function testSubmitFormSavesEnabledValue(): void { + $form_state = new FormState(); + $form_state->setValues(['enabled' => FALSE, 'temp_location' => 'temporary://my-path']); + + $form = []; + $this->form->submitForm($form, $form_state); + + $this->assertFalse($this->config('filefield_paths.settings')->get('enabled')); + } + /** * Tests that validateForm rejects a missing scheme. */ diff --git a/tests/src/Unit/EntityWithFileFieldTest.php b/tests/src/Unit/EntityWithFileFieldTest.php index e2d0e92..b4f5ee0 100644 --- a/tests/src/Unit/EntityWithFileFieldTest.php +++ b/tests/src/Unit/EntityWithFileFieldTest.php @@ -5,8 +5,13 @@ namespace Drupal\Tests\filefield_paths\Unit; use PHPUnit\Framework\Attributes\Group; +use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Config\ImmutableConfig; +use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Extension\ModuleHandlerInterface; +use Drupal\field\FieldConfigInterface; +use Drupal\file\Plugin\Field\FieldType\FileFieldItemList; use Drupal\filefield_paths\Hook\EntityWithFileField; use Drupal\Tests\UnitTestCase; @@ -19,12 +24,52 @@ #[Group('filefield_paths')] class EntityWithFileFieldTest extends UnitTestCase { + /** + * Builds a handler with mocked module handler and config factory. + * + * @param \PHPUnit\Framework\MockObject\MockObject&\Drupal\Core\Extension\ModuleHandlerInterface $module_handler + * The mocked module handler, passed by reference so the caller can set + * expectations on it. + * @param bool $global_enabled + * The value the mocked `filefield_paths.settings:enabled` config returns. + * + * @return \Drupal\filefield_paths\Hook\EntityWithFileField + * The handler under test. + * + * @param-out \PHPUnit\Framework\MockObject\MockObject&\Drupal\Core\Extension\ModuleHandlerInterface $module_handler + */ + private function createHandler(?ModuleHandlerInterface &$module_handler = NULL, bool $global_enabled = TRUE): EntityWithFileField { + $module_handler = $this->createMock(ModuleHandlerInterface::class); + + $config = $this->createMock(ImmutableConfig::class); + $config->method('get')->with('enabled')->willReturn($global_enabled); + $config_factory = $this->createMock(ConfigFactoryInterface::class); + $config_factory->method('get')->with('filefield_paths.settings')->willReturn($config); + + return new EntityWithFileField( + fn (): ModuleHandlerInterface => $module_handler, + fn (): ConfigFactoryInterface => $config_factory, + ); + } + + /** + * Builds a mocked enabled FileFieldItemList field. + */ + private function createEnabledField(): FileFieldItemList { + $definition = $this->createMock(FieldConfigInterface::class); + $definition->method('getThirdPartySettings')->with('filefield_paths')->willReturn(['enabled' => TRUE]); + + $field = $this->createMock(FileFieldItemList::class); + $field->method('getFieldDefinition')->willReturn($definition); + + return $field; + } + /** * Tests handleProcessFile() skips non-content entities. */ public function testHandleProcessFileSkipsNonContentEntity(): void { - $module_handler = $this->createMock(ModuleHandlerInterface::class); - $handler = new EntityWithFileField(fn (): ModuleHandlerInterface => $module_handler); + $handler = $this->createHandler($module_handler); $entity = $this->createMock(EntityInterface::class); @@ -34,4 +79,41 @@ public function testHandleProcessFileSkipsNonContentEntity(): void { $handler->handleProcessFile($entity); } + /** + * Tests handleProcessFile() processes an enabled field with no override. + */ + public function testHandleProcessFileProcessesFieldWithoutOverride(): void { + $handler = $this->createHandler($module_handler); + + $field = $this->createEnabledField(); + $entity = $this->createMock(ContentEntityInterface::class); + $entity->method('getFields')->willReturn(['field_x' => $field]); + + $module_handler->expects($this->once())->method('invokeAll'); + + $handler->handleProcessFile($entity); + } + + /** + * Tests the global `enabled` config setting suppresses all processing. + */ + public function testHandleProcessFileSkipsAllFieldsWhenGloballyDisabled(): void { + $handler = $this->createHandler($module_handler, global_enabled: FALSE); + + $field = $this->createEnabledField(); + $entity = $this->createMock(ContentEntityInterface::class); + $entity->method('getFields')->willReturn(['field_x' => $field]); + + $module_handler->expects($this->never())->method('invokeAll'); + + $handler->handleProcessFile($entity); + } + + // Tests for the entity-level `filefield_paths_settings` transient override + // live in Kernel\EntityWithFileFieldOverrideTest: setting a dynamic + // property on a mocked ContentEntityInterface triggers a "creation of + // dynamic property" deprecation (the mock doesn't implement + // ContentEntityBase::__set()), whereas a real entity's magic setter + // handles it without deprecation, and exercising real file movement is a + // more meaningful assertion than mock call counts for this behavior. } diff --git a/tests/src/Unit/FieldWidgetSingleElementFormTest.php b/tests/src/Unit/FieldWidgetSingleElementFormTest.php index 5ee04af..15fe1dd 100644 --- a/tests/src/Unit/FieldWidgetSingleElementFormTest.php +++ b/tests/src/Unit/FieldWidgetSingleElementFormTest.php @@ -23,11 +23,31 @@ #[Group('filefield_paths')] class FieldWidgetSingleElementFormTest extends UnitTestCase { + /** + * Builds a hook instance with a mocked `filefield_paths.settings` config. + * + * @param bool $global_enabled + * The value the mocked `enabled` config key returns. + * @param string|null $temp_location + * The value the mocked `temp_location` config key returns. + */ + private function createHook(bool $global_enabled = TRUE, ?string $temp_location = NULL): FieldWidgetSingleElementForm { + $config = $this->createMock(ImmutableConfig::class); + $config->method('get')->willReturnMap([ + ['enabled', $global_enabled], + ['temp_location', $temp_location], + ]); + $config_factory = $this->createMock(ConfigFactoryInterface::class); + $config_factory->method('get')->with('filefield_paths.settings')->willReturn($config); + + return new FieldWidgetSingleElementForm(static fn (): MockObject => $config_factory); + } + /** * Tests that unsupported widgets are left untouched. */ public function testUnsupportedWidgetIsUntouched(): void { - $hook = new FieldWidgetSingleElementForm(static fn () => throw new \LogicException('Config factory should not be called.')); + $hook = $this->createHook(); $element = ['#type' => 'textfield']; $hook->formAlter($element, $this->createMock(FormStateInterface::class), []); @@ -39,7 +59,7 @@ public function testUnsupportedWidgetIsUntouched(): void { * Tests that the field-level temp location wins when set. */ public function testUsesFieldLevelTempLocation(): void { - $hook = new FieldWidgetSingleElementForm(static fn () => throw new \LogicException('Config factory should not be called.')); + $hook = $this->createHook(); $items = $this->buildFileFieldItemList(['enabled' => TRUE, 'temp_location' => 'private://custom']); $element = ['#type' => 'managed_file']; @@ -52,11 +72,7 @@ public function testUsesFieldLevelTempLocation(): void { * Tests that the global setting is used when no field-level value is set. */ public function testFallsBackToGlobalTempLocation(): void { - $config = $this->createMock(ImmutableConfig::class); - $config->method('get')->with('temp_location')->willReturn('temporary://filefield_paths'); - $config_factory = $this->createMock(ConfigFactoryInterface::class); - $config_factory->method('get')->with('filefield_paths.settings')->willReturn($config); - $hook = new FieldWidgetSingleElementForm(static fn (): MockObject => $config_factory); + $hook = $this->createHook(temp_location: 'temporary://filefield_paths'); $items = $this->buildFileFieldItemList(['enabled' => TRUE]); $element = ['#type' => 'managed_file']; @@ -69,7 +85,7 @@ public function testFallsBackToGlobalTempLocation(): void { * Tests that a disabled field is left untouched. */ public function testDisabledFieldIsUntouched(): void { - $hook = new FieldWidgetSingleElementForm(static fn () => throw new \LogicException('Config factory should not be called.')); + $hook = $this->createHook(); $items = $this->buildFileFieldItemList(['enabled' => FALSE]); $element = ['#type' => 'managed_file']; @@ -78,6 +94,23 @@ public function testDisabledFieldIsUntouched(): void { $this->assertArrayNotHasKey('#upload_location', $element); } + /** + * Tests that disabling File (Field) Paths site-wide skips the redirect. + * + * Otherwise an upload would be staged in the temporary location and never + * moved out of it, since EntityWithFileField::handleProcessFile() also + * skips processing entirely when disabled site-wide. + */ + public function testGloballyDisabledIsUntouched(): void { + $hook = $this->createHook(global_enabled: FALSE); + + $items = $this->buildFileFieldItemList(['enabled' => TRUE, 'temp_location' => 'private://custom']); + $element = ['#type' => 'managed_file']; + $hook->formAlter($element, $this->createMock(FormStateInterface::class), ['items' => $items]); + + $this->assertArrayNotHasKey('#upload_location', $element); + } + /** * Builds a mocked FileFieldItemList with the given filefield_paths settings. */