File migration fails when the files are at a remote URI

Yes I think we should try detect remote files.

Also for testing, I think it's time to introduce some unit tests for the file destination. It's one of our more complicate destinations and it's hard to get different sets of test data to test using the file migration. I think need to test public, private files using relative, absolute and remote paths plus tmp files with relative and absolute paths.

I'll take a stab at this in the next day or so unless you beat me to it.

OK, I started this off. Few things to note:

• I don't have anything using private:// destination
• I didn't use PHPUnit because the destination uses drupal_realpath
• I couldn't replicate the bug I think this issue described, there is a test with a remote source file copying to a local destination?

Added a couple of stub methods that should be filled in. Also, since we're fixing up the EntityFile destination, I wonder if we should remove the wrong assumption that temporary files are absolute at the same time?

OK, reproduced the bug in this issue, the key part is having a folder that doesn't exist that needs to be created. Failing patch attached.

OK, reproduced the bug in this issue, the key part is having a folder that doesn't exist that needs to be created. Failing patch attached.

Moving the prepare directory code to the top fixes the issue here.

Opened #2417975: EntityFile destination incorrectly assumes temporary files are absolute paths to handle the wrong assumptions with temporary files after this issue goes in with the new tests.

OK some final clean-up.

I discussed this issue with @chx, he wanted to use PHPUnit. We can't do that right now because of the use of drupal_realpath, file_prepare_directory and drupal_dirname in the EntityFile destination.

The more I look, I don't even know why Drupal needs custom implementations of realpath anyway, it should just work if the stream wrapper is registered with PHP? Because PHP's implementation can't support remote stream wrappers with these methods.

I opened #2417987: EntityFile needs better support for remote destinations which will take us closer to using PHPUnit by removing drupal_realpath.

Realpath for remote/virtual makes not a lot of sense and PHP instead of a passthrough decided to not support at all and we decided (very , very long ago #517814: File API Stream Wrapper Conversion ) to add drupal_realpath and a realpath to the stream wrapper. It was meant to be a crutch for D6->D7 conversion but it lingered oh well. Anyways, this is good as it is.

I changed the test so that $source_base_path is coming from the data provider similar to how it does from the FileUri process plugin in a real migration. This is going to mike life easier for #2417975: EntityFile destination incorrectly assumes temporary files are absolute paths which needs to test relative and absolute paths.

Even better!

Migration is not subject to beta evaluation. Committed 06aff1a and pushed to 8.0.x. Thanks!

diff --git a/core/modules/migrate/tests/src/Unit/destination/EntityFileTest.php b/core/modules/migrate/tests/src/Unit/destination/EntityFileTest.php
index 3cb5bc6..731c722 100644
--- a/core/modules/migrate/tests/src/Unit/destination/EntityFileTest.php
+++ b/core/modules/migrate/tests/src/Unit/destination/EntityFileTest.php
@@ -46,8 +46,6 @@ public function setUp() {
 
   /**
    * Test successful imports/copies.
-   *
-   * @throws \Drupal\migrate\MigrateException
    */
   public function testSuccessfulCopies() {
     foreach ($this->localFileDataProvider() as $data) {
@@ -118,7 +116,6 @@ protected function doImport($row_values, $destination_path, $source_base_path =
 
 }
 
-
 class TestEntityFile extends EntityFile {
 
   /**
@@ -129,7 +126,7 @@ class TestEntityFile extends EntityFile {
   public $mockEntity;
 
   /**
-   * Make this public for easy writing during tets.
+   * Make this public for easy writing during tests.
    *
    * @var array
    */
@@ -159,4 +156,5 @@ protected function getEntity(Row $row, array $old_destination_id_values) {
    * {@inheritdoc}
    */
   protected function save(ContentEntityInterface $entity, array $old_destination_id_values = array()) {}
+
 }

Minor fixes on commit.