D7 Private file migrations use of source file_private_path variable creates wrong directory structure

@sagannotcarl, the source path for files is set in the migration, d7_file_private. You should only have to edit the configured migration to set the correct path for your system. It is 'source_base_path'. See https://git.drupalcode.org/project/drupal/-/blob/8.8.x/core/modules/file...

I hope that helps.

Also seeing this on upgrade_d7_file_private.

I had to patch core/modules/file/src/Plugin/migrate/source/d7/File.php to get around this type of issue when migrating a D7 (multisite installation type) to D8 (also multisite installation type).

  protected function initializeIterator() {
    $this->publicPath = $this->variableGet('file_public_path', NULL);
    $this->privatePath = $this->variableGet('file_private_path', NULL);
    $this->temporaryPath = $this->variableGet('file_temporary_path', '/tmp');
    return parent::initializeIterator();
  }

file_public_path was properly set on my D7 site, but it wasn't reading it in for some reason and kept defaulting to 'sites/default/files'. Example error:

File '/var/www/drupal7//sites/default/files/picture_282.png' does not exist.

file_public_path in D7 = sites/example.com/files

im a dummy. forgot to remove the htaccess file from within the top level of the directory holding the private files.

@sagannotcarl i have not seen it mentioned anywhere in any documentation. i only did it while quadruple checking file permissions inside of terminal. but yea in the end private files are now migrating.

@sagannotcarl, did the suggestion by jim.shreds to remove .htaccess allow you to migrate the files?

Could it perhaps be the same problem as described here?
https://drupal.stackexchange.com/questions/283175/files-migration-issue

We're currently running into the same situation where the OLD source directory is prepended to the file path,

The link describes it perfectly! Or is that a different issue? This is the closest one I could find.
Example:

source:
  plugin: d7_file
  scheme: private
  constants:
    source_base_path: '../_migration/files/old-private-files-source'
process:
  fid:
    -
      plugin: get
      source: fid
  filename:
    -
      plugin: get
      source: filename
  source_full_path:
    -
      plugin: concat
      delimiter: /
      source:
        - constants/source_base_path
        - filepath

Expected:
"../_migration/files/old-private-files-source/dirXYZ/fileABC.pdf"

Actual / current result:
"../_migration/files/old-private-files-source//my-old-drupal7-private-file-directory-path-from-db-variables/dirXYZ/fileABC.pdf"

The OLD results from private:// should NOT be used here, as the directory is environment specific. And if someone needs it, it can be added to source_base_path! But removing it (without changing the source db or hacking core like in #7) seems problematic!?

Indeed the most simple, but dirty workaround is to delete the "file_privat_path" row from the copy of the source database variables table...
DELETE FROM variables WHERE name="file_private_path";

Tadaaa... problems gone! So I think this should be rated as major problem / bug for private files.

And like written in #7 the problem starts with these two functions in File.php:

  /**
   * {@inheritdoc}
   */
  protected function initializeIterator() {
    $this->publicPath = $this->variableGet('file_public_path', 'sites/default/files');
    $this->privatePath = $this->variableGet('file_private_path', NULL);
    return parent::initializeIterator();
  }

  /**
   * {@inheritdoc}
   */
  public function prepareRow(Row $row) {
    // Compute the filepath property, which is a physical representation of
    // the URI relative to the Drupal root.
    $path = str_replace(['public:/', 'private:/'], [$this->publicPath, $this->privatePath], $row->getSourceProperty('uri'));
    // At this point, $path could be an absolute path or a relative path,
    // depending on how the scheme's variable was set. So we need to shear out
    // the source_base_path in order to make them all relative.
    $path = preg_replace('#' . preg_quote($this->configuration['constants']['source_base_path']) . '#', '', $path, 1);
    $row->setSourceProperty('filepath', $path);
    return parent::prepareRow($row);
  }

In contrast to the private path, appending the old directory to public paths (or URL) is correct as that's still the folder to search in, while it's not for private files.

I'd suggest to make this configurable in the yml file by splitting the variables for example... something like this (especially for public files);

  source_full_path:
    -
      plugin: concat
      delimiter: /
      source:
        - constants/source_base_path
        - constants/source_base_files_directory
        - filepath

and removing the files directory variable from filepath!
Or simply documenting that source_base_path for private files should contain the /sites/default/files directory?

@quietone: Do you see the problem and what do you think?

Adding #3189876: Add documentation for file source plugins as related for documentation update after this was fixed.

Did the debugging with @Grevil and agree that for private files the combination with the private file path in "filepath" is a problem, while it leads to expected results for public files. Splitting the variables would be the most flexible way to solve this, I guess. "filepath" should never contain the replacement of the private:// or public:// scheme like it currently does.

Quite complex... especially not introducing a breaking change.

Currently I think it CAN only work if migration source and target are on the same machine, accessing the same directory structure or using an identical one for private files...

It's also worth reading the old issue: #2505283: Handle import of private files.

In #58 there was a constant "source_private_file_path" which got lost later on?

As alternative to the SQL we added a dirty workaround in the issue fork to never prepend the file_private_path to the filepath. Of course that's not a real solution.

One could also discuss to split "filepath" variable from "filedir" and put them together in source_full_path for a non-BC!

  source_full_path:
    -
      plugin: concat
      delimiter: /
      source:
        - constants/source_base_path
        - filedir
        - filepath

@quietone I guess we need your feedback :)

#2925899: MigrateUpgradeImportBatch does not use source_private_file_path & source_base_path correctly, making it impossible to have public & private files in separate locations also seems possibly related?

Just migrated another project and can confirm the issue still exists. The workaround in the MR fixes it. So we may proceed here with a cleaner fix, for example provide the private path in a separate migration variable or as source_base_path!

Just updating tags,

Here is a static patch for the time being.

I have applied patch 29 but am still seeing the path being built incorrectly by concatenating the public and private paths together after also applying the patch for https://www.drupal.org/project/migrate_upgrade/issues/2921617#comment-15... so that the private path can be applied while setting up a drush migration. I'm somewhat bewildered that I cannot determine a way to make this work properly considering how long it has been broken.

Rerolled MR!1367 againt 11.x in MR!6919. Verified both are the same and equal to #29.

Applies cleanly against 11.x - but still a quickfix as documented.

MR!6919 as patch attached!

Patch from #39 applies perfectly on 11.x, thx!