From 953b7c73028e8b1e1084de9f76cb21fd74b789a5 Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole Date: Fri, 10 Aug 2018 15:24:42 +0900 Subject: [PATCH] Issue #2729369 by quietone, Jo Fitzgerald, maxocub, mikeryan, heddn: Remove support for migrating temporary files --- .../src/Plugin/migrate/source/d6/File.php | 1 + .../src/Plugin/migrate/source/d7/File.php | 7 +- .../src/Kernel/Migrate/d6/MigrateFileTest.php | 35 +++------- .../src/Kernel/Migrate/d7/MigrateFileTest.php | 3 + .../Plugin/migrate/source/d6/FileTest.php | 37 ++++++++++- .../Plugin/migrate/source/d7/FileTest.php | 62 ++++++++++++++++-- .../migrate_drupal/tests/fixtures/drupal7.php | 11 ++++ .../src/Functional/d6/MigrateUpgrade6Test.php | 4 +- core/modules/simpletest/files/image-3.jpg | Bin 0 -> 1831 bytes 9 files changed, 121 insertions(+), 39 deletions(-) create mode 100644 core/modules/simpletest/files/image-3.jpg diff --git a/core/modules/file/src/Plugin/migrate/source/d6/File.php b/core/modules/file/src/Plugin/migrate/source/d6/File.php index 9ec4887671e..14a0a6d4cb5 100644 --- a/core/modules/file/src/Plugin/migrate/source/d6/File.php +++ b/core/modules/file/src/Plugin/migrate/source/d6/File.php @@ -42,6 +42,7 @@ class File extends DrupalSqlBase { public function query() { return $this->select('files', 'f') ->fields('f') + ->condition('filepath', '/tmp%', 'NOT LIKE') ->orderBy('timestamp') // If two or more files have the same timestamp, they'll end up in a // non-deterministic order. Ordering by fid (or any other unique field) diff --git a/core/modules/file/src/Plugin/migrate/source/d7/File.php b/core/modules/file/src/Plugin/migrate/source/d7/File.php index 5e88151229a..c0da770b70a 100644 --- a/core/modules/file/src/Plugin/migrate/source/d7/File.php +++ b/core/modules/file/src/Plugin/migrate/source/d7/File.php @@ -43,18 +43,21 @@ class File extends DrupalSqlBase { public function query() { $query = $this->select('file_managed', 'f') ->fields('f') + ->condition('uri', 'temporary://%', 'NOT LIKE') ->orderBy('f.timestamp'); // Filter by scheme(s), if configured. if (isset($this->configuration['scheme'])) { $schemes = []; + // Remove 'temporary' scheme. + $valid_schemes = array_diff((array) $this->configuration['scheme'], ['temporary']); // Accept either a single scheme, or a list. - foreach ((array) $this->configuration['scheme'] as $scheme) { + foreach ((array) $valid_schemes as $scheme) { $schemes[] = rtrim($scheme) . '://'; } $schemes = array_map([$this->getDatabase(), 'escapeLike'], $schemes); - // uri LIKE 'public://%' OR uri LIKE 'private://%' + // Add conditions, uri LIKE 'public://%' OR uri LIKE 'private://%'. $conditions = new Condition('OR'); foreach ($schemes as $scheme) { $conditions->condition('uri', $scheme . '%', 'LIKE'); diff --git a/core/modules/file/tests/src/Kernel/Migrate/d6/MigrateFileTest.php b/core/modules/file/tests/src/Kernel/Migrate/d6/MigrateFileTest.php index 989eb15900c..a3d4ba5e7c2 100644 --- a/core/modules/file/tests/src/Kernel/Migrate/d6/MigrateFileTest.php +++ b/core/modules/file/tests/src/Kernel/Migrate/d6/MigrateFileTest.php @@ -2,7 +2,6 @@ namespace Drupal\Tests\file\Kernel\Migrate\d6; -use Drupal\Component\Utility\Random; use Drupal\file\Entity\File; use Drupal\file\FileInterface; use Drupal\KernelTests\KernelTestBase; @@ -11,7 +10,7 @@ use Drupal\Tests\migrate\Kernel\MigrateDumpAlterInterface; use Drupal\Tests\migrate_drupal\Kernel\d6\MigrateDrupal6TestBase; /** - * file migration. + * Test file migration. * * @group migrate_drupal_6 */ @@ -68,8 +67,10 @@ class MigrateFileTest extends MigrateDrupal6TestBase implements MigrateDumpAlter public function testFiles() { $this->assertEntity(1, 'Image1.png', '39325', 'public://image-1.png', 'image/png', '1'); $this->assertEntity(2, 'Image2.jpg', '1831', 'public://image-2.jpg', 'image/jpeg', '1'); - $this->assertEntity(3, 'Image-test.gif', '183', 'public://image-test.gif', 'image/jpeg', '1'); + $this->assertEntity(3, 'image-3.jpg', '1831', 'public://image-3.jpg', 'image/jpeg', '1'); $this->assertEntity(4, 'html-1.txt', '24', 'public://html-1.txt', 'text/plain', '1'); + // Ensure temporary file was not migrated. + $this->assertNull(File::load(6)); $map_table = $this->getMigration('d6_file')->getIdMap()->mapTableName(); $map = \Drupal::database() @@ -81,10 +82,9 @@ class MigrateFileTest extends MigrateDrupal6TestBase implements MigrateDumpAlter // The 4 files from the fixture. 1 => '1', 2 => '2', + // The file updated in migrateDumpAlter(). 3 => '3', 5 => '4', - // The file updated in migrateDumpAlter(). - 6 => NULL, // The file created in migrateDumpAlter(). 7 => '4', ]; @@ -124,10 +124,9 @@ class MigrateFileTest extends MigrateDrupal6TestBase implements MigrateDumpAlter // The 4 files from the fixture. 1 => '5', 2 => '6', + // The file updated in migrateDumpAlter(). 3 => '7', 5 => '8', - // The file updated in migrateDumpAlter(). - 6 => NULL, // The files created in migrateDumpAlter(). 7 => '8', 8 => '8', @@ -142,33 +141,17 @@ class MigrateFileTest extends MigrateDrupal6TestBase implements MigrateDumpAlter $this->assertEquals(8, count(File::loadMultiple())); } - /** - * @return string - * A filename based upon the test. - */ - public static function getUniqueFilename() { - return static::$tempFilename; - } - /** * {@inheritdoc} */ public static function migrateDumpAlter(KernelTestBase $test) { - // Creates a random filename and updates the source database. - $random = new Random(); - $temp_directory = file_directory_temp(); - file_prepare_directory($temp_directory, FILE_CREATE_DIRECTORY); - static::$tempFilename = $test->getDatabasePrefix() . $random->name() . '.jpg'; - $file_path = $temp_directory . '/' . static::$tempFilename; - file_put_contents($file_path, ''); - $db = Database::getConnection('default', 'migrate'); $db->update('files') - ->condition('fid', 6) + ->condition('fid', 3) ->fields([ - 'filename' => static::$tempFilename, - 'filepath' => $file_path, + 'filename' => 'image-3.jpg', + 'filepath' => 'core/modules/simpletest/files/image-3.jpg', ]) ->execute(); diff --git a/core/modules/file/tests/src/Kernel/Migrate/d7/MigrateFileTest.php b/core/modules/file/tests/src/Kernel/Migrate/d7/MigrateFileTest.php index 38ef4ff3200..ea32df5e348 100644 --- a/core/modules/file/tests/src/Kernel/Migrate/d7/MigrateFileTest.php +++ b/core/modules/file/tests/src/Kernel/Migrate/d7/MigrateFileTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\file\Kernel\Migrate\d7; +use Drupal\file\Entity\File; use Drupal\Tests\migrate_drupal\Kernel\d7\MigrateDrupal7TestBase; /** @@ -43,6 +44,8 @@ class MigrateFileTest extends MigrateDrupal7TestBase { */ public function testFileMigration() { $this->assertEntity(1, 'cube.jpeg', 'public://cube.jpeg', 'image/jpeg', '3620', '1421727515', '1421727515', '1'); + // Ensure temporary file was not migrated. + $this->assertNull(File::load(4)); } } diff --git a/core/modules/file/tests/src/Kernel/Plugin/migrate/source/d6/FileTest.php b/core/modules/file/tests/src/Kernel/Plugin/migrate/source/d6/FileTest.php index cc8c9b36faf..33c430e2fdf 100644 --- a/core/modules/file/tests/src/Kernel/Plugin/migrate/source/d6/FileTest.php +++ b/core/modules/file/tests/src/Kernel/Plugin/migrate/source/d6/FileTest.php @@ -46,11 +46,42 @@ class FileTest extends MigrateSqlSourceTestBase { 'status' => 1, 'timestamp' => 1382255662, ], + [ + 'fid' => 3, + 'uid' => 1, + 'filename' => 'migrate-test-file-3.pdf', + 'filepath' => '/tmp/migrate-test-file-3.pdf', + 'filemime' => 'application/pdf', + 'filesize' => 304124, + 'status' => 1, + 'timestamp' => 1382277662, + ], ]; - // The expected results are identical to the source data. - $tests[0]['expected_data'] = $tests[0]['source_data']['files']; - + // The expected results are the same as the source data but excluding + // the temporary file. + $tests[0]['expected_data'] = [ + [ + 'fid' => 1, + 'uid' => 1, + 'filename' => 'migrate-test-file-1.pdf', + 'filepath' => 'sites/default/files/migrate-test-file-1.pdf', + 'filemime' => 'application/pdf', + 'filesize' => 890404, + 'status' => 1, + 'timestamp' => 1382255613, + ], + [ + 'fid' => 2, + 'uid' => 1, + 'filename' => 'migrate-test-file-2.pdf', + 'filepath' => 'sites/default/files/migrate-test-file-2.pdf', + 'filemime' => 'application/pdf', + 'filesize' => 204124, + 'status' => 1, + 'timestamp' => 1382255662, + ], + ]; return $tests; } diff --git a/core/modules/file/tests/src/Kernel/Plugin/migrate/source/d7/FileTest.php b/core/modules/file/tests/src/Kernel/Plugin/migrate/source/d7/FileTest.php index 8ae0ec41fa1..b588ad789ce 100644 --- a/core/modules/file/tests/src/Kernel/Plugin/migrate/source/d7/FileTest.php +++ b/core/modules/file/tests/src/Kernel/Plugin/migrate/source/d7/FileTest.php @@ -84,15 +84,14 @@ class FileTest extends MigrateSqlSourceTestBase { ], ]; - // The expected results will include only the first three files, since we - // are configuring the plugin to filter out the file with the null URI - // scheme. - $tests[0]['expected_data'] = array_slice($tests[0]['source_data']['file_managed'], 0, 3); + // The expected results will include only the first two files, since the + // plugin will filter out files with either the null URI scheme or the + // temporary scheme. + $tests[0]['expected_data'] = array_slice($tests[0]['source_data']['file_managed'], 0, 2); // The filepath property will vary by URI scheme. $tests[0]['expected_data'][0]['filepath'] = 'sites/default/files/cube.jpeg'; $tests[0]['expected_data'][1]['filepath'] = '/path/to/private/files/cube.jpeg'; - $tests[0]['expected_data'][2]['filepath'] = '/tmp/cube.jpeg'; // Do an automatic count. $tests[0]['expected_count'] = NULL; @@ -102,10 +101,61 @@ class FileTest extends MigrateSqlSourceTestBase { 'constants' => [ 'source_base_path' => '/path/to/files', ], - // Only return files which use one of these URI schemes. 'scheme' => ['public', 'private', 'temporary'], ]; + // Test getting only public files. + $tests[1]['source_data'] = $tests[0]['source_data']; + + $tests[1]['expected_data'] = [ + [ + 'fid' => '1', + 'uid' => '1', + 'filename' => 'cube.jpeg', + 'uri' => 'public://cube.jpeg', + 'filemime' => 'image/jpeg', + 'filesize' => '3620', + 'status' => '1', + 'timestamp' => '1421727515', + ], + ]; + // Do an automatic count. + $tests[1]['expected_count'] = NULL; + + // Set up plugin configuration. + $tests[1]['configuration'] = [ + 'constants' => [ + 'source_base_path' => '/path/to/files', + ], + 'scheme' => ['public'], + ]; + + // Test getting only public files when configuration scheme is not an array. + $tests[2]['source_data'] = $tests[0]['source_data']; + + $tests[2]['expected_data'] = [ + [ + 'fid' => '1', + 'uid' => '1', + 'filename' => 'cube.jpeg', + 'uri' => 'public://cube.jpeg', + 'filemime' => 'image/jpeg', + 'filesize' => '3620', + 'status' => '1', + 'timestamp' => '1421727515', + ], + ]; + // Do an automatic count. + $tests[2]['expected_count'] = NULL; + + // Set up plugin configuration. + $tests[2]['configuration'] = [ + 'constants' => [ + 'source_base_path' => '/path/to/files', + ], + 'scheme' => 'public', + ]; + return $tests; } diff --git a/core/modules/migrate_drupal/tests/fixtures/drupal7.php b/core/modules/migrate_drupal/tests/fixtures/drupal7.php index e76ee20f236..3624a4a92fb 100644 --- a/core/modules/migrate_drupal/tests/fixtures/drupal7.php +++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php @@ -13373,6 +13373,17 @@ $connection->insert('file_managed') 'status' => '1', 'timestamp' => '1486104045', )) +->values(array( + 'fid' => '4', + 'uid' => '1', + 'filename' => 'TerokNor.txt', + 'uri' => 'temporary://TerokNor.txt', + 'filemime' => 'text/plain', + 'filesize' => '2369', + 'status' => '1', + 'timestamp' => '1421747516', +)) + ->execute(); $connection->schema()->createTable('file_usage', array( diff --git a/core/modules/migrate_drupal_ui/tests/src/Functional/d6/MigrateUpgrade6Test.php b/core/modules/migrate_drupal_ui/tests/src/Functional/d6/MigrateUpgrade6Test.php index 96aa5579177..d8ffd029643 100644 --- a/core/modules/migrate_drupal_ui/tests/src/Functional/d6/MigrateUpgrade6Test.php +++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d6/MigrateUpgrade6Test.php @@ -69,7 +69,7 @@ class MigrateUpgrade6Test extends MigrateUpgradeExecuteTestBase { 'editor' => 2, 'field_config' => 89, 'field_storage_config' => 63, - 'file' => 8, + 'file' => 7, 'filter_format' => 7, 'image_style' => 5, 'language_content_settings' => 3, @@ -109,7 +109,7 @@ class MigrateUpgrade6Test extends MigrateUpgradeExecuteTestBase { $counts['comment'] = 7; $counts['entity_view_display'] = 55; $counts['entity_view_mode'] = 14; - $counts['file'] = 9; + $counts['file'] = 8; $counts['menu_link_content'] = 11; $counts['node'] = 18; $counts['taxonomy_term'] = 9; diff --git a/core/modules/simpletest/files/image-3.jpg b/core/modules/simpletest/files/image-3.jpg new file mode 100644 index 0000000000000000000000000000000000000000..ace07d078a00bee3b0ab02f7f077e39c23f2aa03 GIT binary patch literal 1831 zcmb7=c{rPi7RJBi%O=(kkyxzD}--uL|ZJMVel=g42krvbI2POeS> z1OfqEb_S4705kvrgCpQD1OkpgA`vLG8XB#jfX1q*DyeB<@pvsP4yUbWq_3?*)WzWl z`w7H7dreGDcIh85KVWQbWNc#m_YnvZiA1B&8fdhJu{KWI`2Uvt5r9Df6u=XN7y?iX z1jIn(?SKvdK%pQ6_^%L17#soxQ9JOk8UTfWP!ItkV90-O>;M=X0~jFGEsz>ihF`Q{ zY6X@^b}VSsJSZHX`4{qL-u=1b#6tjZ=LP?x9Ut`XKq6)*5DJCB!2du96aX;>Fm(&K zh9UK!pHMyyDD5 z^n}09s!`D4s?6Bt{#vijK(&u!=MOU#*S_t%f`8bSxw=)j4Xpn;Oc6gQk3+DXhy+yU z8sC?5C3xAfxr*+Ah-{B*+_M`=y~+VO%JXd@Fj@|L%dL-$nXho;|E^W-XhvJ zwqg3egl*~uAepNa`oP$Xpei$da1J6?lnNMQ(lc&9eeI}wTkxvN=U=(`!v2rnaBCzq z<^8G*vY2Qpy@ts71Ak%LK|k6Rkdh^5jU-){ucIl~w6;PO+T%&q!NZ-u5VjllthI4$ z&r1b-0}nUdrQ(x#@LgF(13!-^dS}=hLA&dx*}H8nW_CksT+K<`rq1GOQqyRcbqU{5 zwV`G1aEuEzN3XH6j@~obo76QOE+l{=mql@2!w>dWvh|}bi}MAReZ-h)NHa)Ub$6Jc`xRl>`1?!P z=>1VnE6T?j?8uMrU@S%C#4_qFEo@Zfz^Fs;0G(x4jh5-Q1Igm&3HQ1^8?92W<5uS> z`N=YltE+lOtpQk6m;3q6EP1f zVl@piJY#w8wsl&eE8$5PNk>{h>>d!Oqu1yyYVFZ+wVBE6(os|WIBaAMk~On343eA- zqkPTtoI`3BZvjPHk6C@`PWH*VodM-_Gu>-!#)g$w2$~=VP#xu0Kf+WVJ}j9|Wxe03 z%1@uNDGRt&Tl}nD#)yR%{)$PR{p1g9fZAnCzBk}$BGi{B6qZ*P!|cF{Za;Ihy|@2q zc^dvr^nzW<_^f8lEVgd1O`JLaRaG}{zkD1W1)n;?TgC0+-gKu21)hs^Z>bpCy2?>} zJb8M>Yt4B|5FD7C$q)j$87;nMqlP9=d6fm%HO66$Ni?>C_WdHZ?SqDea7i*EsWsj^gYcVbu1rjVm!?4^hZZSTmjt3W`zmMZYQ7Pqfg58O^Cl_9Z zl!PqoEhCv(yaoEJRL*@BSDX+!Qf#? zu#|gV2J7PFoOH;4e#K?Ul9z(YZOPtX*irvcsJeM{S`IX~_@V`bfUY+W?zLpE&wpZm zZ9gQT4Yllgjb_ifilScHOfCgxtDLxtEu`K#AkGl#%=y%_`lw72HcQX```_!x+@RM> zPSL5&wsTV6dO-!Rmi;IqK9XA`mSia>(%rpTX43Z3rFgV3IcFryOo+ex&!LnBQho6W z?-Fx0(Hb#gs;NT`Sk@o1O**9LBfEx16`)RB-j|?XFcQ43{qx&n4Wc-<(h>ZH2$gl~ zgoCjj0k;WSfraWxowMioDb7x0FHcJTnaHZ_^4gwVY4~))9r(4xghj()@}Td0b((NL zlfzto#t#aTWm&Zq*Kv=qU8AyJ_-2B{7dOp4B5s2nELMDEg~j+_yy(W5bF02tN*JQB o6n4ohkM2FL6DacE7?5a%wQIc@v_{sx0}~UmrCRqyPW_ literal 0 HcmV?d00001