- Issue created by @xurizaemon
- Merge request !76Replace the Replace use of Drupal\Core\Asset\CssCollectionOptimizer with... โ (Open) created by prem suthar
- ๐ณ๐ฟNew Zealand xurizaemon ลtepoti, Aotearoa ๐
@prem.suthar, I wonder if you ran the tests with these changes before submitting your MR?
There is existing WIP in ๐ Establish automated test coverage for Flysystem Active for these, please hold off further effort here until the maintainer has a chance to give input.
Setting to "postponed" to hopefully prevent duplicated effort and/or additional work.
- ๐ณ๐ฟNew Zealand xurizaemon ลtepoti, Aotearoa ๐
My reading of https://git.drupalcode.org/project/flysystem/blob/cecd7d4e2c48e757323db3... is that it validates the behaviour of FlySystem's collection optimizer with regard to Drupal core's stale_file_threshold. That configuration is deprecated in 10.1.x and removed in 11.x. Therefore it appears that we may be able to remove the existing test coverage in that file as part of this issue.
- ๐ณ๐ฟNew Zealand xurizaemon ลtepoti, Aotearoa ๐
Rather than deleting the CollectionOptimizerTest entirely, I looked at reworking it to ensure that the change in FlySystem's CollectionOptimizerLazy overrides.
FlySystem's collection optimizers catch the exception if a CollectionOptimizerLazy->deleteAll() call fails, and log the exception.
In https://git.drupalcode.org/project/flysystem/-/merge_requests/75/diffs?c... I hoped to validate this behaviour, by setting permissions on the path to make it not writable, then trying the delete, and verifying that the file is not deleted. However that doesn't work as desired (the file is deleted anyway).
Linking the change here for discussion, keen for input.
- bc56a1f0 committed on 2.3.x
Issue #3497516: Remove Flysystem-specific AssetDumper,...
- bc56a1f0 committed on 2.3.x
- a3326d6d committed on 2.3.x
Issue #3497516: Factor out SerializationStopperTrait, updated Trait...
- a3326d6d committed on 2.3.x
- ๐บ๐ธUnited States lhridley
@xurizaemon Pursuant to your original posts about removing the CollectionOptimizer tests, I have spent a few hours reviewing the implementaiton of the Lazy asset optimizers in Drupal Core, and their intended use vs the Flysystem asset optimizers, and have reached the same conclusion, that the Lazy optimizers are not needed once the core Lazy optimizers are implemented, as their sole intention is to consolidate and write optimized css and js files (with "optimized" meaning minified and aggregated css and js files) to the "assets://" stream wrapper. Defining a replacement stream wrapper for "assets://" is within the purview of Flysystem, and perhaps a better use of effort would. be to evaluate whether a Flysystem adapter can be used as a replacement for Drupal Core's Asset Stream Wrapper.
I'm still in the camp of removing the Lazy optimizers and any associated tests from Flysystem, as they don't appear to be needed. However, a functional test that replaces the core "assets://" stream wrapper with a Flysystem stream wrapper does seem to be a more relevant set of tests to me, to evaluate that in fact replacing the core stream wrapper does in fact work properly.
Therefore, I'm closing this issue again, and opening an issue to create a functional test for replacing the defined core "assets://" stream wrapper with a Flysystem implementation using a Flysystem adapter.
- ๐บ๐ธUnited States lhridley
https://www.drupal.org/project/flysystem/issues/3499242 ๐ Create a Functional test for replacing Drupal Core's "assets://" stream wrapper with a Flysystem implementation. Active