Return the backup file contents from Backup::getFile() (#37317)

[lbajsarowicz]

Description (*)

Magento\Backup\Model\Backup::getFile() is declared to return by reference but returns the result of a method call:

public function &getFile()
{
    if (!$this->exists()) {
        throw new \Magento\Framework\Exception\LocalizedException(__('The backup file does not exist.'));
    }

    return $this->varDirectory->read($this->_getFilePath());
}

PHP cannot bind a function call result to a reference, so this raises Only variable references should be returned by reference, as reported.

While fixing that, a second defect in the same statement turned up, and it explains why the notice is hard to observe in a running instance: $this->varDirectory->read() is Directory\Read::read(), which lists a directory. It is given the path of the backup file, and Driver\File::readDirectory() builds a FilesystemIterator over it:

Magento\Framework\Exception\FileSystemException:
FilesystemIterator::__construct(.../var/backups/1700000000_db_probe.sql): Failed to open directory: Not a directory

The exception is thrown before the return statement is reached, so the method can never return the contents its docblock promises (Return content of backup file) — it always fails for an existing backup file. Backup::output() reads the very same file correctly with readFile(), which is what getFile() was meant to do.

This PR:

• reads the backup with readFile(), so the method returns the file contents it documents;
• returns that through a local variable, so the by-reference return no longer raises the notice;
• corrects the @return docblock from array to string and documents the FileSystemException.

The & is deliberately kept. Removing it would be the cleaner long-term shape, but it changes the signature of a public method on an @api class; returning a variable satisfies the reported expected result ("Only a variable is returned by reference") with no backward compatibility impact.

Two pre-existing PHPCS warnings in the touched file (constant visibility/description on line 27, duplicated property short description on line 61) are cleaned up so the file passes the CI ruleset.

Related Pull Requests

None.

Fixed Issues (if relevant)

1. Fixes \Magento\Backup\Model\Backup::getFile() does not return a variable by reference #37317: \Magento\Backup\Model\Backup::getFile() does not return a variable by reference

Manual testing scenarios (*)

1. Create a database backup (System → Tools → Backups), or write a file into var/backups/ named like 1700000000_db_test.sql.
2. Load it through the model:

   $backup = $objectManager->create(\Magento\Backup\Model\Backup::class);
   $backup->setPath('backups')->setName('test')->setTime(1700000000);
   $backup->setType('db');
   var_dump($backup->getFile());

3. Before: Magento\Framework\Exception\FileSystemException: FilesystemIterator::__construct(...): Failed to open directory: Not a directory.
   After: the contents of the backup file are returned as a string, with no PHP notice.
4. Call getFile() for a backup that does not exist and verify it still throws LocalizedException with "The backup file does not exist."

Questions or comments

Covered by unit tests (contents returned, no notice raised on the by-reference return, exception for a missing file) and by an integration test that exercises the real var directory — the integration test is the one that fails on 2.4-develop with the FileSystemException above, since a mocked directory cannot distinguish read() from readFile().

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @lbajsarowicz. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

[lbajsarowicz]

@magento run all tests