Problem/Motivation

I'm trying to use the ArchiverManager to create a new Zip archive. It doesn't work.

Steps to reproduce

I'm creating a new FileDownloadAction with the following code (hopefully this will be added to core or Views Bulk Operations, see #3367286: Bulk file download: Create an archive of selected files):

  /**
   * {@inheritdoc}
   */
  public function execute($entity = NULL) {
    if ($entity) {
      $this->executeMultiple([$entity]);
    }
  }

  /**
   * {@inheritdoc}
   */
  public function executeMultiple(array $entities) {
    $destination = $this->getDestination();

    /** @var \Drupal\Core\Archiver\ArchiverInterface $archiver */
    $archive = $this->archiverManager->getInstance(['filepath' => $destination]);

    /** @var \Drupal\file\FileInterface $file */
    foreach ($entities as $file) {
      $archive->add($file->getFileUri());
    }
  }

The code errors.

Drupal\Core\Archiver\ArchiverException: Cannot open '/var/www/html/private/vbo_archive.zip' in Drupal\Core\Archiver\Zip->__construct() (line 32 of /var/www/html/web/core/lib/Drupal/Core/Archiver/Zip.php).

There is this code in \Drupal\Core\Archiver\Zip

  /**
   * Constructs a Zip object.
   *
   * @param string $file_path
   *   The full system path of the archive to manipulate. Only local files
   *   are supported. If the file does not yet exist, it will be created if
   *   appropriate.
   *
   * @throws \Drupal\Core\Archiver\ArchiverException
   */
  public function __construct($file_path) {
    $this->zip = new \ZipArchive();
    if ($this->zip->open($file_path) !== TRUE) {
      throw new ArchiverException("Cannot open '$file_path'");
    }
  }

However, it won't actually create a new archive if it doesn't exist. For that to work, a flag is needed:

    if ($this->zip->open($file_path, \ZipArchive::CREATE) !== TRUE) {

Proposed resolution

It seems odd to me that Drupal wraps \ZipArchive yet it doesn't have a way to pass any flags. Ideally, the ArchiverManager should probably allow for additional options. Currently, it has this:

  /**
   * {@inheritdoc}
   */
  public function createInstance($plugin_id, array $configuration = []) {
    $plugin_definition = $this->getDefinition($plugin_id);
    $plugin_class = DefaultFactory::getPluginClass($plugin_id, $plugin_definition, 'Drupal\Core\Archiver\ArchiverInterface');
    return new $plugin_class($this->fileSystem->realpath($configuration['filepath']));
  }

Perhaps the $configuration array could be passed to the $plugin_class() constructor, and each wrapper can check for and handle additional args?

Alternatively, this single case could be fixed by adding the CREATE flag, which allows the function to work as stated (I tested it and it works if the flag is there.)

If the file does not yet exist, it will be created

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

solideogloria created an issue. See original summary.

solideogloria’s picture

Issue summary: View changes
cilefen’s picture

I feel like there may already be an issue for this.

solideogloria’s picture

I checked and I couldn't find one.

longwave’s picture

cilefen’s picture

Status: Active » Postponed (maintainer needs more info)

@longwave That's the one I half-remembered. Can we verify?

solideogloria’s picture

Ah, I was looking for open issues. I'm actually still on 9.5.x, and the fix wasn't applied to 9.5. Yes, the patch in that issue works.

solideogloria’s picture

Status: Postponed (maintainer needs more info) » Closed (duplicate)