diff --git a/.env b/.env index 643c347..7dec6e4 100644 --- a/.env +++ b/.env @@ -114,8 +114,7 @@ FEATURE_SANITIZE_UPLOADS=false FEATURE_STAMP_INVOICES=false FEATURE_DATEV_EMAIL=false FEATURE_MYEP_LOGIN=false - -UPLOAD_REPLACEMENT_FILE=assets/pdf/chicken.pdf +FEATURE_UPLOAD_REPLACEMENT=false MAINTENANCE_MODE_ENABLED=false MAINTENANCE_MODE_IP_WHITELIST=188.245.62.21 diff --git a/config/packages/flagception.yaml b/config/packages/flagception.yaml index 4d658e4..45a3d27 100644 --- a/config/packages/flagception.yaml +++ b/config/packages/flagception.yaml @@ -13,3 +13,6 @@ flagception: myep_login: default: false env: FEATURE_MYEP_LOGIN + upload_replacement: + default: false + env: FEATURE_UPLOAD_REPLACEMENT diff --git a/config/services.yaml b/config/services.yaml index fc9836c..d214e22 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -50,6 +50,11 @@ parameters: teamer_inactive_period: '-2 years' + # The stand-in PDF served instead of real uploads while the upload_replacement feature is + # on. Relative to the project dir. Which file is a fixed fact of the repo; whether it is + # used at all is the feature flag's job alone. + upload_replacement_file: 'assets/pdf/chicken.pdf' + # The contract upload period, counted from the creation of the disposition. This is the # single definition: the reminder mail, the "Honorarvertrag fällig" flag on the teamer # dashboard, the admin list of overdue contracts and the wording of the mails all read it, @@ -262,8 +267,7 @@ services: arguments: $orphanageManager: '@oneup_uploader.orphanage_manager' $projectDir: '%kernel.project_dir%' - $environment: '%kernel.environment%' - $uploadReplacementFile: '%env(default::UPLOAD_REPLACEMENT_FILE)%' + $uploadReplacementFile: '%upload_replacement_file%' App\Service\Pdf\Sanitizer: arguments: diff --git a/src/Service/Upload/UploadHandler.php b/src/Service/Upload/UploadHandler.php index 7a3fced..03c29f3 100644 --- a/src/Service/Upload/UploadHandler.php +++ b/src/Service/Upload/UploadHandler.php @@ -6,6 +6,7 @@ use App\Entity\Upload; use App\Model\UploadDto; use App\Model\UploadSessionDto; use App\Service\Pdf\Sanitizer; +use Flagception\Manager\FeatureManagerInterface; use Oneup\UploaderBundle\Uploader\Orphanage\OrphanageManager; use Psr\Log\LoggerInterface; use Symfony\Component\Filesystem\Exception\IOException; @@ -21,12 +22,9 @@ class UploadHandler private readonly OrphanageManager $orphanageManager, private readonly Sanitizer $sanitizer, private readonly LoggerInterface $logger, + private readonly FeatureManagerInterface $featureManager, private readonly string $projectDir, - private readonly string $environment, - // Bound from %env(default::UPLOAD_REPLACEMENT_FILE)%, whose empty fallback parameter - // resolves to null when the variable is not set at all - so "unset", "set to empty" and - // "absent" all have to mean the same thing here. - private readonly ?string $uploadReplacementFile, + private readonly string $uploadReplacementFile, ) { } @@ -166,23 +164,22 @@ class UploadHandler } /** - * Returns the upload file path, substituting a replacement file in non-production environments - * when configured. Use this for read-only access (downloads, email attachments). + * Returns the upload file path, substituting the stand-in document while the + * upload_replacement feature is on. Use this for read-only access (downloads, email + * attachments); getUploadFilepath() stays the real location and is what writes target. */ public function getResolvedUploadFilepath(Upload $upload): string { - $originalPath = $this->getUploadFilepath($upload); - - if (null === $this->uploadReplacementFile - || '' === $this->uploadReplacementFile - || 'prod' === $this->environment) { - return $originalPath; + if (false === $this->featureManager->isActive('upload_replacement')) { + return $this->getUploadFilepath($upload); } $replacementPath = sprintf('%s/%s', $this->projectDir, $this->uploadReplacementFile); if (false === file_exists($replacementPath)) { - return $originalPath; + // Falling back to the real document here is what made the old behaviour opaque: the + // feature would look active and quietly serve the genuine file instead. + throw new \RuntimeException('Upload replacement file not found at '.$replacementPath); } return $replacementPath; diff --git a/tests/Service/Upload/UploadHandlerTest.php b/tests/Service/Upload/UploadHandlerTest.php new file mode 100644 index 0000000..f0785f4 --- /dev/null +++ b/tests/Service/Upload/UploadHandlerTest.php @@ -0,0 +1,105 @@ +featureManager = $this->createMock(FeatureManagerInterface::class); + } + + public function testTheRealDocumentIsServedWhileTheFeatureIsOff(): void + { + $this->featureManager->method('isActive')->with('upload_replacement')->willReturn(false); + + $this->assertSame( + '/srv/app/uploads/invoice/i/invoice-1.pdf', + $this->handler()->getResolvedUploadFilepath($this->upload()) + ); + } + + public function testTheStandInIsServedWhileTheFeatureIsOn(): void + { + $this->featureManager->method('isActive')->with('upload_replacement')->willReturn(true); + + // the real file of the repository, so the path resolves to something that exists + $handler = $this->handler(\dirname(__DIR__, 3), self::REPLACEMENT_FILE); + + $this->assertSame( + \dirname(__DIR__, 3).'/'.self::REPLACEMENT_FILE, + $handler->getResolvedUploadFilepath($this->upload()) + ); + } + + public function testAMissingStandInFailsInsteadOfServingTheRealDocument(): void + { + $this->featureManager->method('isActive')->with('upload_replacement')->willReturn(true); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('/srv/app/assets/pdf/nope.pdf'); + + $this->handler(self::PROJECT_DIR, 'assets/pdf/nope.pdf') + ->getResolvedUploadFilepath($this->upload()) + ; + } + + /** + * The real location is never substituted: it is what writes target. + */ + public function testTheUploadFilepathIgnoresTheFeature(): void + { + $this->featureManager->method('isActive')->with('upload_replacement')->willReturn(true); + + $this->assertSame( + '/srv/app/uploads/invoice/i/invoice-1.pdf', + $this->handler()->getUploadFilepath($this->upload()) + ); + } + + private function handler( + string $projectDir = self::PROJECT_DIR, + string $replacementFile = self::REPLACEMENT_FILE, + ): UploadHandler { + return new UploadHandler( + $this->createMock(RequestStack::class), + $this->createMock(OrphanageManager::class), + $this->createMock(Sanitizer::class), + new NullLogger(), + $this->featureManager, + $projectDir, + $replacementFile, + ); + } + + private function upload(): Upload + { + return (new Upload()) + ->setType(Upload::TYPE_INVOICE) + ->setFilename('invoice-1.pdf') + ; + } +}