Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The follow code in \Drupal\config\Controller\ConfigController::downloadExport() means that overrides including those from settings.php will be exported into the tarball.
file_unmanaged_delete(file_directory_temp() . '/config.tar.gz');
$archiver = new ArchiveTar(file_directory_temp() . '/config.tar.gz', 'gz');
foreach (\Drupal::service('config.storage')->listAll() as $name) {
$archiver->addString("$name.yml", Yaml::encode(\Drupal::config($name)->get()));
}
$request = new Request(array('file' => 'config.tar.gz'));
return $this->fileDownloadController->download($request, 'temporary');
Proposed resolution
Fix this and add a test.
Remaining tasks
Write patch
Review
User interface changes
None
API changes
None
Comment | File | Size | Author |
---|---|---|---|
#11 | 2261131-raw-export-11.patch | 3.43 KB | Gábor Hojtsy |
#8 | 2261131-raw-export-8-test-only.patch | 2.34 KB | Gábor Hojtsy |
#8 | 2261131-raw-export-8.patch | 3.54 KB | Gábor Hojtsy |
#8 | interdiff.txt | 2.62 KB | Gábor Hojtsy |
#5 | 2261131-raw-export-5.patch | 2.77 KB | Gábor Hojtsy |
Comments
Comment #1
Gábor HojtsyCan it be as simple as this one (without tests for now)?
Comment #2
alexpottIt can! Perhaps a comment would be nice to say why we're using getRawData instead of deactivating overrides in the ConfigFactory - the reason is we *know* we just want the raw data and we're not doing further calls to config in a chain so disabling config overrides is unnecessary overhead.
Comment #3
alexpottIt is tempting to also get
replaced with
in this patch too. Slightly out of scope I know but could come under tidying up downloadExport() :)
Comment #4
Jose Reyero CreditAttribution: Jose Reyero commentedUpdated patch following suggestions in #3
plus, replaced too
by
which I think is the same case.
Comment #5
Gábor HojtsyNow with test coverage :) Totally not novice AFAIS.
The patch needed a reroll so no interdiff possible. The test-only patch is the new thing, the fix just includes minor comment fixes.
Comment #7
alexpottWe need a positive equivalent to confirm the the value is overridden if accessed through the config factory. This will make the test more robust and less susceptible to false positives.
I'm not a huge fan of affecting all the tests that install config_test.module
Comment #8
Gábor HojtsyI agree the positive test is useful I was in fact thinking to add it myself. Done now.
I think using a separate testing module just for this one line is silly and I believe reusing existing test modules for multiple purposes is not a sin. But whatever. Created yet another test module for this one line of test code then.
Comment #10
Gábor HojtsyUploaded in the wrong order. Ready for review.
Comment #11
Gábor HojtsyRerolled for PSR-4.
Comment #12
alexpottLooks great. Thanks @Gábor Hojtsy
Comment #13
vijaycs85Nice +1 RTBC
Comment #14
webchickExcellent, thanks all!
Committed and pushed to 8.x. w00t!
Comment #16
Gábor HojtsyYay, thanks!