Repository: spatie/laravel-backup (php)

Issue:
BackupDestination does not check write result
The class BackupDestination with function 'write' writes the backup file towards for example S3, but when f.e. S3MultipartUploadException occurs, the flysystem class returns 'false' instead of the exception. The backup is still marked as succesful. 

Pull request:
adds: exception to throw when flysystem returns false
fix #1326 

I have done research on this and I would like to note it here.
league/flysystem - v1: Methods in League\Flysystem\FilesystemInterface doesn't throw exceptions instead returns true, false, resource, array. In flysystem v2 there are [exceptions which will be thrown](https://flysystem.thephpleague.com/v2/docs/usage/exception-handling/) instead of returning false.

In this package only Backup@delete checks for its result for rest of the methods(writeStream, readStream) I've added checking the result and throwing exception.

Even though I tried to add tests for these cases, I find it difficult to achieve 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 25464b0..1b96de3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,14 @@
 
 All notable changes to `laravel-backup` will be documented in this file.
 
+## 7.6.2 - 2021-07-19
+
+- fix retrieving translation string (#1353)
+
+## 7.6.1 - 2021-07-13
+
+- fix Spanish Translations (#1350)
+
 ## 7.6.0 - 2021-06-01
 
 - translate notification properties
diff --git a/composer.json b/composer.json
index 4815b17..ed8119c 100644
--- a/composer.json
+++ b/composer.json
@@ -19,7 +19,6 @@
     ],
     "require": {
         "php": "^8.0",
-        "ext-pcntl": "*",
         "ext-zip": "^1.14.0",
         "illuminate/console": "^8.36",
         "illuminate/contracts": "^8.36",
@@ -32,7 +31,7 @@
         "spatie/laravel-package-tools": "^1.6.2",
         "spatie/laravel-signal-aware-command": "^1.1",
         "spatie/temporary-directory": "^2.0",
-        "symfony/console": "^5.2.6",
+        "symfony/console": "^5.2.12",
         "symfony/finder": "^5.2"
     },
     "require-dev": {
diff --git a/docs/_index.md b/docs/_index.md
index c3592f2..d627603 100644
--- a/docs/_index.md
+++ b/docs/_index.md
@@ -2,5 +2,5 @@
 title: v7
 slogan: One day you'll thank us for this
 githubUrl: https://github.com/spatie/laravel-backup
-branch: master
+branch: main
 ---
diff --git a/resources/lang/es/notifications.php b/resources/lang/es/notifications.php
index 2aa0d95..e707876 100644
--- a/resources/lang/es/notifications.php
+++ b/resources/lang/es/notifications.php
@@ -40,6 +40,6 @@ return [
     'newest_backup_size' => 'Tamaño de copia de seguridad más reciente',
     'number_of_backups' => 'Número de copias de seguridad',
     'total_storage_used' => 'Almacenamiento total utilizado',
-    'newest_backup_date' => 'Tamaño de copia de seguridad más reciente',
-    'oldest_backup_date' => 'Tamaño de copia de seguridad más antiguo',
+    'newest_backup_date' => 'Fecha de la copia de seguridad más reciente',
+    'oldest_backup_date' => 'Fecha de la copia de seguridad más antigua',
 ];
diff --git a/src/BackupDestination/Backup.php b/src/BackupDestination/Backup.php
index de3284b..79c6f9a 100644
--- a/src/BackupDestination/Backup.php
+++ b/src/BackupDestination/Backup.php
@@ -5,6 +5,7 @@ namespace Spatie\Backup\BackupDestination;
 use Carbon\Carbon;
 use Illuminate\Contracts\Filesystem\Filesystem;
 use InvalidArgumentException;
+use Spatie\Backup\Exceptions\InvalidBackupFile;
 use Spatie\Backup\Tasks\Backup\BackupJob;
 
 class Backup
@@ -70,7 +71,10 @@ class Backup
 
     public function stream()
     {
-        return $this->disk->readStream($this->path);
+        return throw_unless(
+            $this->disk->readStream($this->path),
+            InvalidBackupFile::readError($this)
+        );
     }
 
     public function delete(): void
diff --git a/src/BackupDestination/BackupDestination.php b/src/BackupDestination/BackupDestination.php
index 5e04712..ec0d889 100644
--- a/src/BackupDestination/BackupDestination.php
+++ b/src/BackupDestination/BackupDestination.php
@@ -7,6 +7,7 @@ use Exception;
 use Illuminate\Contracts\Filesystem\Factory;
 use Illuminate\Contracts\Filesystem\Filesystem;
 use Spatie\Backup\Exceptions\InvalidBackupDestination;
+use Spatie\Backup\Exceptions\InvalidBackupFile;
 
 class BackupDestination
 {
@@ -81,7 +82,7 @@ class BackupDestination
 
         $handle = fopen($file, 'r+');
 
-        $this->disk->getDriver()->writeStream(
+        $hasWritten = $this->disk->getDriver()->writeStream(
             $destination,
             $handle,
             $this->getDiskOptions()
@@ -90,6 +91,10 @@ class BackupDestination
         if (is_resource($handle)) {
             fclose($handle);
         }
+
+        if (! $hasWritten) {
+            throw InvalidBackupFile::writeError($this->backupName());
+        }
     }
 
     public function backupName(): string
diff --git a/src/Commands/BackupCommand.php b/src/Commands/BackupCommand.php
index 067795b..42b60a4 100644
--- a/src/Commands/BackupCommand.php
+++ b/src/Commands/BackupCommand.php
@@ -51,6 +51,10 @@ class BackupCommand extends BaseCommand
                 $backupJob->disableNotifications();
             }
 
+            if (! $this->getSubscribedSignals()) {
+                $backupJob->disableSignals();
+            }
+
             $backupJob->run();
 
             consoleOutput()->comment('Backup completed!');
diff --git a/src/Commands/BaseCommand.php b/src/Commands/BaseCommand.php
index 5b736e8..b99751a 100644
--- a/src/Commands/BaseCommand.php
+++ b/src/Commands/BaseCommand.php
@@ -6,6 +6,7 @@ use Spatie\Backup\Helpers\ConsoleOutput;
 use Spatie\SignalAwareCommand\SignalAwareCommand;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\SignalRegistry\SignalRegistry;
 
 abstract class BaseCommand extends SignalAwareCommand
 {
@@ -13,7 +14,7 @@ abstract class BaseCommand extends SignalAwareCommand
 
     public function __construct()
     {
-        if (PHP_OS_FAMILY !== 'Windows' && $this->runningInConsole() && defined('SIGINT')) {
+        if ($this->runningInConsole() && SignalRegistry::isSupported()) {
             $this->handlesSignals[] = SIGINT;
         }
 
@@ -31,4 +32,9 @@ abstract class BaseCommand extends SignalAwareCommand
     {
         return in_array(php_sapi_name(), ['cli', 'phpdbg']);
     }
+
+    public function getSubscribedSignals(): array
+    {
+        return $this->handlesSignals;
+    }
 }
diff --git a/src/Exceptions/InvalidBackupFile.php b/src/Exceptions/InvalidBackupFile.php
new file mode 100644
index 0000000..e10f36f
--- /dev/null
+++ b/src/Exceptions/InvalidBackupFile.php
@@ -0,0 +1,21 @@
+<?php
+
+namespace Spatie\Backup\Exceptions;
+
+use Exception;
+use Spatie\Backup\BackupDestination\Backup;
+
+class InvalidBackupFile extends Exception
+{
+    public static function writeError(string $backupName): self
+    {
+        return new static("There has been an error writing file for the backup named `{$backupName}`.");
+    }
+
+    public static function readError(Backup $backup): self
+    {
+        $path = $backup->path();
+
+        return new static("There has been an error reading the backup `{$path}`");
+    }
+}
diff --git a/src/Tasks/Backup/BackupJob.php b/src/Tasks/Backup/BackupJob.php
index 60fc67c..f02defc 100644
--- a/src/Tasks/Backup/BackupJob.php
+++ b/src/Tasks/Backup/BackupJob.php
@@ -37,6 +37,8 @@ class BackupJob
 
     protected bool $sendNotifications = true;
 
+    protected bool $signals = true;
+
     public function __construct()
     {
         $this
@@ -77,6 +79,13 @@ class BackupJob
         return $this;
     }
 
+    public function disableSignals(): self
+    {
+        $this->signals = false;
+
+        return $this;
+    }
+
     public function setDefaultFilename(): self
     {
         $this->filename = Carbon::now()->format(static::FILENAME_FORMAT);
@@ -135,11 +144,13 @@ class BackupJob
             ->create()
             ->empty();
 
-        Signal::handle(SIGINT, function (Command $command) {
-            $command->info('Cleaning up temporary directory...');
+        if ($this->signals) {
+            Signal::handle(SIGINT, function (Command $command) {
+                $command->info('Cleaning up temporary directory...');
 
-            $this->temporaryDirectory->delete();
-        });
+                $this->temporaryDirectory->delete();
+            });
+        }
 
         try {
             if (! count($this->backupDestinations)) {
@@ -167,7 +178,9 @@ class BackupJob
 
         $this->temporaryDirectory->delete();
 
-        Signal::clearHandlers(SIGINT);
+        if ($this->signals) {
+            Signal::clearHandlers(SIGINT);
+        }
     }
 
     protected function createBackupManifest(): Manifest
diff --git a/src/Tasks/Monitor/HealthChecks/IsReachable.php b/src/Tasks/Monitor/HealthChecks/IsReachable.php
index 3440aaa..211194e 100644
--- a/src/Tasks/Monitor/HealthChecks/IsReachable.php
+++ b/src/Tasks/Monitor/HealthChecks/IsReachable.php
@@ -11,7 +11,7 @@ class IsReachable extends HealthCheck
     {
         $this->failUnless(
             $backupDestination->isReachable(),
-            trans('backup::notification.unhealthy_backup_found_not_reachable', [
+            trans('backup::notifications.unhealthy_backup_found_not_reachable', [
                 'error' => $backupDestination->connectionError,
             ])
         );


Test patch:
diff --git a/tests/BackupDestination/BackupTest.php b/tests/BackupDestination/BackupTest.php
index f04b5e5..3fc8c3a 100644
--- a/tests/BackupDestination/BackupTest.php
+++ b/tests/BackupDestination/BackupTest.php
@@ -3,9 +3,16 @@
 namespace Spatie\Backup\Tests\BackupDestination;
 
 use Carbon\Carbon;
+use Illuminate\Filesystem\FilesystemAdapter;
 use Illuminate\Support\Facades\Storage;
+use League\Flysystem\Adapter\Local;
+use League\Flysystem\Filesystem;
+use Mockery as m;
+use Mockery\MockInterface;
 use Spatie\Backup\BackupDestination\Backup;
+use Spatie\Backup\BackupDestination\BackupDestination;
 use Spatie\Backup\BackupDestination\BackupDestinationFactory;
+use Spatie\Backup\Exceptions\InvalidBackupFile;
 use Spatie\Backup\Tests\TestCase;
 
 class BackupTest extends TestCase
@@ -30,6 +37,47 @@ class BackupTest extends TestCase
         $this->assertSame("mysite.com/{$fileName}", $backup->path());
     }
 
+    /** @test */
+    public function it_can_get_backup_as_stream_resource()
+    {
+        $fileName = 'test.zip';
+
+        $backup = $this->getBackupForFile($fileName);
+
+        $this->assertIsResource($backup->stream());
+    }
+
+    /** @test */
+    public function when_its_unable_to_read_the_stream_throws_exception()
+    {
+        $path = 'mysite.com/test.zip';
+
+        $filesystem = m::mock(FilesystemAdapter::class);
+        $filesystem->shouldReceive('readStream')->once()->with($path)->andReturn(false);
+
+        $backup = new Backup($filesystem, $path);
+
+        $this->expectException(InvalidBackupFile::class);
+        $backup->stream();
+    }
+
+    /** @test */
+    public function when_its_unable_to_write_to_stream_throws_exception()
+    {
+        $mock = $this->partialMock(Local::class, function (MockInterface $mock) {
+            $mock->shouldReceive('writeStream')->once()->andReturn(false);
+        });
+        $adapter = new FilesystemAdapter(new Filesystem($mock));
+
+        $path = Storage::disk($diskName = 'local')->path(
+            $this->createFileOnDisk($diskName, $filePath = 'mysite.com/file.zip', now())
+        );
+        $backupDestination = new BackupDestination($adapter, $backupName = 'mysite', $diskName);
+
+        $this->expectException(InvalidBackupFile::class);
+        $backupDestination->write($path);
+    }
+
     /** @test */
     public function it_can_delete_itself()
     {