From 60c419593d790b1e06f89e916382efc50b620575 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Fri, 18 Sep 2026 17:13:03 -0300 Subject: [PATCH] chore: accept File API image on create/update via async FileProcessingJob --- Libs/Utils/FileUtils.php | 58 ++++ ...BadgeFeatureTypeValidationRulesFactory.php | 2 + .../Registration/SummitBadgeFeatureType.php | 4 + app/Services/FilePostProcessorService.php | 4 + .../Model/ISummitBadgeFeatureTypeService.php | 4 +- app/Services/Model/Imp/CompanyService.php | 55 +--- .../Imp/SummitBadgeFeatureTypeService.php | 113 ++++++- app/Swagger/SummitRegistrationSchemas.php | 2 + ...mmitBadgeFeatureTypeFileProcessingTest.php | 283 ++++++++++++++++++ .../OAuth2SummitBadgeFeatureTypeApiTest.php | 230 ++++++++++++++ 10 files changed, 694 insertions(+), 61 deletions(-) create mode 100644 tests/Unit/Services/SummitBadgeFeatureTypeFileProcessingTest.php diff --git a/Libs/Utils/FileUtils.php b/Libs/Utils/FileUtils.php index 36d593d74..c5b23c68d 100644 --- a/Libs/Utils/FileUtils.php +++ b/Libs/Utils/FileUtils.php @@ -13,6 +13,8 @@ **/ use App\Http\Utils\FileUploadInfo; +use App\Services\Model\FileInfoDTO; +use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use models\exceptions\ValidationException; @@ -139,6 +141,62 @@ public static function cleanLocalAndRemoteFile(string $localPath, string $remote unlink($localPath); } + /** + * Downloads a file from remote storage to a local temp path, verifies its MD5 (when provided), + * invokes $uploader($owner_entity_id, UploadedFile) to persist it, then cleans up. On failure the + * remote file is preserved so queue retries can re-download it. Cleanup errors after a successful + * upload are logged but not re-thrown - upload success determines job success, not storage housekeeping. + * @param FileInfoDTO $file_info_dto + * @param callable $uploader + * @return mixed whatever $uploader returns + * @throws ValidationException + */ + public static function processFileFromRemoteStorage(FileInfoDTO $file_info_dto, callable $uploader) + { + $localPath = self::getFileFromRemoteStorageOnTempStorage( + $file_info_dto->filename, + $file_info_dto->filepath + ); + $succeeded = false; + try { + if (!is_null($file_info_dto->md5)) { + $localHash = md5_file($localPath); + if ($localHash === false) + throw new ValidationException("File integrity check failed: unable to read local temp file."); + if ($localHash !== strtolower($file_info_dto->md5)) + throw new ValidationException("File integrity check failed: MD5 mismatch."); + } + $file = new UploadedFile( + path: $localPath, + originalName: $file_info_dto->filename, + mimeType: $file_info_dto->mime_type, + error: null, + test: true, + ); + $res = $uploader($file_info_dto->owner_entity_id, $file); + $succeeded = true; + } finally { + if ($succeeded) { + try { + self::cleanLocalAndRemoteFile($localPath, $file_info_dto->filepath); + } catch (\Throwable $e) { + // Upload succeeded; cleanup failure is non-fatal. Log and continue so the + // job does not retry and create duplicate File records. + Log::warning(sprintf( + "FileUtils::processFileFromRemoteStorage cleanup failed after successful upload (entity=%s member=%s filepath=%s): %s", + $file_info_dto->owner_entity_class, + $file_info_dto->owner_member_name, + $file_info_dto->filepath, + $e->getMessage() + )); + } + } else { + self::cleanLocalFile($localPath); + } + } + return $res; + } + /** * Deletes only the local temp file. Use in finally blocks where the remote * file must be preserved on failure so queue job retries can re-download it. diff --git a/app/Http/Controllers/Apis/Protected/Summit/Factories/Registration/SummitBadgeFeatureTypeValidationRulesFactory.php b/app/Http/Controllers/Apis/Protected/Summit/Factories/Registration/SummitBadgeFeatureTypeValidationRulesFactory.php index fc20c7068..6766a45ca 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/Factories/Registration/SummitBadgeFeatureTypeValidationRulesFactory.php +++ b/app/Http/Controllers/Apis/Protected/Summit/Factories/Registration/SummitBadgeFeatureTypeValidationRulesFactory.php @@ -30,12 +30,14 @@ public static function build(array $data, $update = false){ 'name' => 'sometimes|string', 'description' => 'sometimes|string', 'template_content' => 'nullable|string', + 'image' => 'sometimes|file_dto', ]; } return [ 'name' => 'required|string', 'description' => 'sometimes|string', 'template_content' => 'nullable|string', + 'image' => 'sometimes|file_dto', ]; } } \ No newline at end of file diff --git a/app/Models/Foundation/Summit/Registration/SummitBadgeFeatureType.php b/app/Models/Foundation/Summit/Registration/SummitBadgeFeatureType.php index d421117c9..f49f51b1c 100644 --- a/app/Models/Foundation/Summit/Registration/SummitBadgeFeatureType.php +++ b/app/Models/Foundation/Summit/Registration/SummitBadgeFeatureType.php @@ -27,6 +27,10 @@ class SummitBadgeFeatureType extends SilverstripeBaseModel { use SummitOwned; + public const ImageAllowedExtensions = ['png', 'jpg', 'jpeg', 'gif', 'svg']; + + public const ImageMaxFileSize = 10485760; // bytes + /** * @var string */ diff --git a/app/Services/FilePostProcessorService.php b/app/Services/FilePostProcessorService.php index c8eae3243..97eb0cf05 100644 --- a/app/Services/FilePostProcessorService.php +++ b/app/Services/FilePostProcessorService.php @@ -16,9 +16,11 @@ use App\Services\Model\ICompanyService; use App\Services\Model\IFilePostProcessorForChildEntity; use App\Services\Model\IFilePostProcessorService; +use App\Services\Model\ISummitBadgeFeatureTypeService; use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Log; use models\main\Company; +use models\summit\SummitBadgeFeatureType; use models\utils\IEntity; final class FilePostProcessorService implements IFilePostProcessorService @@ -33,6 +35,8 @@ private function locateService(string $className): ?IFilePostProcessorForChildEn switch($className){ case Company::class: return App::make(ICompanyService::class); + case SummitBadgeFeatureType::class: + return App::make(ISummitBadgeFeatureTypeService::class); } return null; } diff --git a/app/Services/Model/ISummitBadgeFeatureTypeService.php b/app/Services/Model/ISummitBadgeFeatureTypeService.php index cd9fedac0..2dcad2cc2 100644 --- a/app/Services/Model/ISummitBadgeFeatureTypeService.php +++ b/app/Services/Model/ISummitBadgeFeatureTypeService.php @@ -22,7 +22,7 @@ * Interface ISummitBadgeFeatureTypeService * @package App\Services\Model */ -interface ISummitBadgeFeatureTypeService +interface ISummitBadgeFeatureTypeService extends IFilePostProcessorForChildEntity { /** @@ -65,7 +65,7 @@ public function addFeatureImage Summit $summit, int $feature_id, UploadedFile $file, - int $max_file_size = 10485760 + int $max_file_size = SummitBadgeFeatureType::ImageMaxFileSize ):File; /** diff --git a/app/Services/Model/Imp/CompanyService.php b/app/Services/Model/Imp/CompanyService.php index 1e5d89f40..5493d7e92 100644 --- a/app/Services/Model/Imp/CompanyService.php +++ b/app/Services/Model/Imp/CompanyService.php @@ -240,70 +240,21 @@ public function processFileForChildEntity(FileInfoDTO $file_info_dto): IEntity { Log::debug(sprintf("CompanyService::processFileForChildEntity file_info_dto %s", $file_info_dto)); switch ($file_info_dto->owner_member_name) { case 'big_logo': - return $this->processLogoFile($file_info_dto, [$this, 'addCompanyBigLogo']); + return self::processFileFromRemoteStorage($file_info_dto, [$this, 'addCompanyBigLogo']); case 'logo': - return $this->processLogoFile($file_info_dto, [$this, 'addCompanyLogo']); + return self::processFileFromRemoteStorage($file_info_dto, [$this, 'addCompanyLogo']); default: Log::warning(sprintf("CompanyService::processFileForChildEntity unknown member name '%s'", $file_info_dto->owner_member_name)); throw new \InvalidArgumentException(sprintf("Unknown owner_member_name '%s' for entity class '%s'.", $file_info_dto->owner_member_name, $file_info_dto->owner_entity_class)); } } - /** - * Downloads a file from remote storage to a local temp path, verifies its MD5 (when provided), - * invokes $uploader to persist it, then cleans up. On failure the remote file is preserved - * so queue retries can re-download it. Cleanup errors after a successful upload are logged - * but not re-thrown - upload success determines job success, not storage housekeeping. - */ - private function processLogoFile(FileInfoDTO $file_info_dto, callable $uploader): IEntity - { - $localPath = self::getFileFromRemoteStorageOnTempStorage( - $file_info_dto->filename, - $file_info_dto->filepath - ); - $succeeded = false; - try { - if (!is_null($file_info_dto->md5)) { - $localHash = md5_file($localPath); - if ($localHash === false) - throw new ValidationException("File integrity check failed: unable to read local temp file."); - if ($localHash !== strtolower($file_info_dto->md5)) - throw new ValidationException("File integrity check failed: MD5 mismatch."); - } - $file = new UploadedFile( - path: $localPath, - originalName: $file_info_dto->filename, - mimeType: $file_info_dto->mime_type, - error: null, - test: true, - ); - $logo = $uploader($file_info_dto->owner_entity_id, $file); - $succeeded = true; - } finally { - if ($succeeded) { - try { - self::cleanLocalAndRemoteFile($localPath, $file_info_dto->filepath); - } catch (\Throwable $e) { - // Upload succeeded; cleanup failure is non-fatal. Log and continue so the - // job does not retry and create duplicate File records. - Log::warning(sprintf( - "CompanyService::processLogoFile cleanup failed after successful upload (filepath=%s): %s", - $file_info_dto->filepath, - $e->getMessage() - )); - } - } else { - self::cleanLocalFile($localPath); - } - } - return $logo; - } - private function dispatchLogoJob(Company $company, string $memberName, array $payload): void { $file_upload_info = FileUploadInfo::buildFromPayload($payload); if (is_null($file_upload_info)) return; + // TODO: also validate the size here (as SummitBadgeFeatureTypeService does) so an oversized logo returns 412 instead of failing the job silently if (!in_array($file_upload_info->getFileExt(), Company::LogoAllowedExtensions)) throw new ValidationException(sprintf( "%s file does not have a valid extension (%s).", diff --git a/app/Services/Model/Imp/SummitBadgeFeatureTypeService.php b/app/Services/Model/Imp/SummitBadgeFeatureTypeService.php index 921254872..652812f28 100644 --- a/app/Services/Model/Imp/SummitBadgeFeatureTypeService.php +++ b/app/Services/Model/Imp/SummitBadgeFeatureTypeService.php @@ -12,14 +12,22 @@ * limitations under the License. **/ +use App\Http\Utils\FileSizeUtil; +use App\Http\Utils\FileUploadInfo; use App\Http\Utils\IFileUploader; +use App\Jobs\FileProcessingJob; +use App\Jobs\Utils\JobDispatcher; use App\Models\Foundation\Summit\Factories\SummitBadgeFeatureTypeFactory; +use App\Models\Foundation\Summit\Repositories\ISummitBadgeFeatureTypeRepository; +use Illuminate\Support\Facades\Log; +use libs\utils\FileUtils; use libs\utils\ITransactionService; use models\exceptions\EntityNotFoundException; use models\exceptions\ValidationException; use models\main\File; use models\summit\Summit; use models\summit\SummitBadgeFeatureType; +use models\utils\IEntity; use Illuminate\Http\UploadedFile; /** * Class SummitBadgeFeatureTypeService @@ -28,25 +36,34 @@ final class SummitBadgeFeatureTypeService extends AbstractService implements ISummitBadgeFeatureTypeService { + use FileUtils; /** * @var IFileUploader */ private $file_uploader; + /** + * @var ISummitBadgeFeatureTypeRepository + */ + private $repository; + /** * SummitBadgeFeatureTypeService constructor. * @param IFileUploader $file_uploader + * @param ISummitBadgeFeatureTypeRepository $repository * @param ITransactionService $tx_service */ public function __construct ( IFileUploader $file_uploader, + ISummitBadgeFeatureTypeRepository $repository, ITransactionService $tx_service ) { parent::__construct($tx_service); $this->file_uploader = $file_uploader; + $this->repository = $repository; } /** @@ -58,7 +75,10 @@ public function __construct */ public function addBadgeFeatureType(Summit $summit, array $data): SummitBadgeFeatureType { - return $this->tx_service->transaction(function() use($summit, $data){ + // validated before the transaction so an invalid image means nothing is persisted + $image_upload_info = $this->buildImageUploadInfo($data); + + $feature = $this->tx_service->transaction(function() use($summit, $data){ $name = trim($data['name']); $former_feature = $summit->getFeatureTypeByName($name); if(!is_null($former_feature)){ @@ -72,6 +92,11 @@ public function addBadgeFeatureType(Summit $summit, array $data): SummitBadgeFea return $feature; }); + + if (!is_null($image_upload_info)) + $this->dispatchImageJob($feature, $image_upload_info); + + return $feature; } /** @@ -84,7 +109,10 @@ public function addBadgeFeatureType(Summit $summit, array $data): SummitBadgeFea */ public function updateBadgeFeatureType(Summit $summit, int $feature_id, array $data): SummitBadgeFeatureType { - return $this->tx_service->transaction(function() use($summit, $feature_id, $data){ + // validated before the transaction so an invalid image means nothing is changed + $image_upload_info = $this->buildImageUploadInfo($data); + + $feature = $this->tx_service->transaction(function() use($summit, $feature_id, $data){ $feature = $summit->getFeatureTypeById($feature_id); if(is_null($feature)) @@ -101,6 +129,11 @@ public function updateBadgeFeatureType(Summit $summit, int $feature_id, array $d return SummitBadgeFeatureTypeFactory::populate($feature, $data); }); + + if (!is_null($image_upload_info)) + $this->dispatchImageJob($feature, $image_upload_info); + + return $feature; } /** @@ -135,21 +168,19 @@ public function addFeatureImage Summit $summit, int $feature_id, UploadedFile $file, - int $max_file_size = 10485760 + int $max_file_size = SummitBadgeFeatureType::ImageMaxFileSize ):File { return $this->tx_service->transaction(function () use ($summit, $feature_id, $file, $max_file_size) { - $allowed_extensions = ['png', 'jpg', 'jpeg', 'gif', 'svg']; - $feature = $summit->getFeatureTypeById($feature_id); if (is_null($feature) || !$feature instanceof SummitBadgeFeatureType) { throw new EntityNotFoundException('feature type not found on summit!'); } - if (!in_array($file->extension(), $allowed_extensions)) { - throw new ValidationException("file does not has a valid extension ('png','jpg','jpeg','gif','pdf')."); + if (!in_array($file->extension(), SummitBadgeFeatureType::ImageAllowedExtensions)) { + throw new ValidationException(sprintf("file does not has a valid extension (%s).", implode(', ', SummitBadgeFeatureType::ImageAllowedExtensions))); } if ($file->getSize() > $max_file_size) { @@ -183,4 +214,72 @@ public function removeFeatureImage(Summit $summit, int $feature_id): void }); } + + /** + * @param FileInfoDTO $file_info_dto + * @return IEntity + * @throws EntityNotFoundException + * @throws ValidationException + */ + public function processFileForChildEntity(FileInfoDTO $file_info_dto): IEntity + { + Log::debug(sprintf("SummitBadgeFeatureTypeService::processFileForChildEntity file_info_dto %s", $file_info_dto)); + switch ($file_info_dto->owner_member_name) { + case 'image': + return self::processFileFromRemoteStorage($file_info_dto, function (int $feature_id, UploadedFile $file) { + // the DTO only carries the feature id; addFeatureImage needs its summit + $feature = $this->repository->getById($feature_id); + if (!$feature instanceof SummitBadgeFeatureType) + throw new EntityNotFoundException(sprintf("feature type %s not found.", $feature_id)); + return $this->addFeatureImage($feature->getSummit(), $feature_id, $file); + }); + default: + Log::warning(sprintf("SummitBadgeFeatureTypeService::processFileForChildEntity unknown member name '%s'", $file_info_dto->owner_member_name)); + throw new \InvalidArgumentException(sprintf("Unknown owner_member_name '%s' for entity class '%s'.", $file_info_dto->owner_member_name, $file_info_dto->owner_entity_class)); + } + } + + /** + * @param array $data + * @return FileUploadInfo|null null when no image payload was sent + * @throws ValidationException + */ + private function buildImageUploadInfo(array $data): ?FileUploadInfo + { + // a form with no file selected can send image as an empty string + if (!isset($data['image']) || !is_array($data['image'])) return null; + + $file_upload_info = FileUploadInfo::buildFromPayload($data['image']); + if (is_null($file_upload_info)) return null; + + if (!in_array($file_upload_info->getFileExt(), SummitBadgeFeatureType::ImageAllowedExtensions)) + throw new ValidationException(sprintf( + "Image file does not have a valid extension (%s).", + implode(',', SummitBadgeFeatureType::ImageAllowedExtensions) + )); + + // the job's addFeatureImage enforces the same limit; checking here returns 412 instead of a silent job failure + if ($file_upload_info->getSize(FileSizeUtil::B) > SummitBadgeFeatureType::ImageMaxFileSize) + throw new ValidationException(sprintf( + "Image file exceeds max file size (%s MB).", + (SummitBadgeFeatureType::ImageMaxFileSize / 1024) / 1024 + )); + + return $file_upload_info; + } + + private function dispatchImageJob(SummitBadgeFeatureType $feature, FileUploadInfo $file_upload_info): void + { + JobDispatcher::withDbFallback(job: new FileProcessingJob(new FileInfoDTO( + owner_entity_id: $feature->getId(), + owner_entity_class: SummitBadgeFeatureType::class, + owner_member_name: 'image', + filepath: $file_upload_info->getFilePath(), + filename: $file_upload_info->getFileName(), + size: $file_upload_info->getSize(), + md5: $file_upload_info->getMd5(), + mime_type: $file_upload_info->getMimeType(), + source_bucket: $file_upload_info->getSourceBucket() + ))); + } } \ No newline at end of file diff --git a/app/Swagger/SummitRegistrationSchemas.php b/app/Swagger/SummitRegistrationSchemas.php index 465b1b2e2..72fd2f408 100644 --- a/app/Swagger/SummitRegistrationSchemas.php +++ b/app/Swagger/SummitRegistrationSchemas.php @@ -1149,6 +1149,7 @@ class PaginatedSummitBadgeFeatureTypesResponseSchema new OA\Property(property: 'name', type: 'string', example: 'Speaker Ribbon'), new OA\Property(property: 'description', type: 'string', example: 'Special ribbon for speakers'), new OA\Property(property: 'template_content', type: 'string', example: '
{{name}}
'), + new OA\Property(property: 'image', nullable: true, ref: '#/components/schemas/FileDTO', description: 'Feature type image (File API payload), applied asynchronously'), ] )] class SummitBadgeFeatureTypeCreateRequestSchema @@ -1162,6 +1163,7 @@ class SummitBadgeFeatureTypeCreateRequestSchema new OA\Property(property: 'name', type: 'string', example: 'VIP Ribbon'), new OA\Property(property: 'description', type: 'string', example: 'VIP attendee designation'), new OA\Property(property: 'template_content', type: 'string', example: '
{{name}}
'), + new OA\Property(property: 'image', nullable: true, ref: '#/components/schemas/FileDTO', description: 'Feature type image (File API payload), applied asynchronously'), ] )] class SummitBadgeFeatureTypeUpdateRequestSchema diff --git a/tests/Unit/Services/SummitBadgeFeatureTypeFileProcessingTest.php b/tests/Unit/Services/SummitBadgeFeatureTypeFileProcessingTest.php new file mode 100644 index 000000000..afc2b8392 --- /dev/null +++ b/tests/Unit/Services/SummitBadgeFeatureTypeFileProcessingTest.php @@ -0,0 +1,283 @@ + FilePostProcessorService -> processFileForChildEntity). + * + * @package Tests\Unit\Services + */ +class SummitBadgeFeatureTypeFileProcessingTest extends TestCase +{ + private const RemotePath = 'badge-features/tmp/feature.png'; + + // 1x1 transparent PNG, so UploadedFile::extension() guesses 'png' from the content + private const PngBase64 = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg=='; + + private Container $app; + + protected function setUp(): void + { + parent::setUp(); + // Same minimal facade application as CompanyFileProcessingTest + Facade::clearResolvedInstances(); + $this->app = new Container(); + $this->app->singleton('log', fn() => new NullLogger()); + Container::setInstance($this->app); + Facade::setFacadeApplication($this->app); + } + + protected function tearDown(): void + { + Facade::setFacadeApplication(null); + Facade::clearResolvedInstances(); + Container::setInstance(null); + Mockery::close(); + parent::tearDown(); + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + private function pngContent(): string + { + return base64_decode(self::PngBase64); + } + + /** + * Binds a storage disk serving $content at RemotePath. Returns the disk mock so + * each test can set its own expectations on delete(). + */ + private function bindStorageFakes(string $content, ?callable $onSize = null): Mockery\MockInterface + { + $stream = fopen('php://temp', 'r+b'); + fwrite($stream, $content); + rewind($stream); + + $mockDisk = Mockery::mock(); + $mockDisk->shouldReceive('exists')->with(self::RemotePath)->andReturn(true); + $mockDisk->shouldReceive('readStream')->with(self::RemotePath)->andReturn($stream); + $mockDisk->shouldReceive('size')->with(self::RemotePath)->andReturnUsing(function () use ($content, $onSize) { + if (!is_null($onSize)) $onSize(); + return strlen($content); + }); + + $mockFsFactory = Mockery::mock(); + $mockFsFactory->shouldReceive('disk')->andReturn($mockDisk); + + $mockConfig = Mockery::mock(); + $mockConfig->shouldReceive('get')->with('file_upload.storage_driver')->andReturn('s3'); + $mockConfig->shouldReceive('get')->withAnyArgs()->andReturn(null); + + $this->app->singleton('filesystem', fn() => $mockFsFactory); + $this->app->singleton('config', fn() => $mockConfig); + + return $mockDisk; + } + + private function makeImageDto(string $memberName = 'image', ?string $md5 = null): FileInfoDTO + { + return new FileInfoDTO( + owner_entity_id: 1, + owner_entity_class: SummitBadgeFeatureType::class, + owner_member_name: $memberName, + filepath: self::RemotePath, + filename: 'feature.png', + size: 1, + md5: $md5, + mime_type: 'image/png', + ); + } + + /** + * Service with feature type #1 on a summit, and a file uploader mock the test configures. + */ + private function makeService(IFileUploader $fileUploader, ?SummitBadgeFeatureType &$feature = null): SummitBadgeFeatureTypeService + { + $summit = Mockery::mock(Summit::class); + $feature = Mockery::mock(SummitBadgeFeatureType::class); + $feature->shouldReceive('getSummit')->andReturn($summit); + $summit->shouldReceive('getFeatureTypeById')->with(1)->andReturn($feature); + + $repo = Mockery::mock(ISummitBadgeFeatureTypeRepository::class); + $repo->shouldReceive('getById')->with(1)->andReturn($feature); + + $tx = Mockery::mock(ITransactionService::class); + $tx->shouldReceive('transaction')->andReturnUsing(fn($cb) => $cb()); + + $ref = new \ReflectionClass(SummitBadgeFeatureTypeService::class); + $service = $ref->newInstanceWithoutConstructor(); + + foreach (['file_uploader' => $fileUploader, 'repository' => $repo] as $name => $value) { + $prop = $ref->getProperty($name); + $prop->setAccessible(true); + $prop->setValue($service, $value); + } + + $txProp = (new \ReflectionClass(AbstractService::class))->getProperty('tx_service'); + $txProp->setAccessible(true); + $txProp->setValue($service, $tx); + + return $service; + } + + // ------------------------------------------------------------------------- + // Tests + // ------------------------------------------------------------------------- + + public function testProcessFileForChildEntityThrowsForUnknownMemberName(): void + { + $service = $this->makeService(Mockery::mock(IFileUploader::class)); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessageMatches('/logo/'); + + $service->processFileForChildEntity($this->makeImageDto('logo')); + } + + public function testMd5MismatchFailsWithoutSettingImageAndKeepsRemoteFile(): void + { + $disk = $this->bindStorageFakes($this->pngContent()); + $disk->shouldNotReceive('delete'); + + $uploader = Mockery::mock(IFileUploader::class); + $uploader->shouldNotReceive('build'); + $service = $this->makeService($uploader, $feature); + $feature->shouldNotReceive('setImage'); + + try { + $service->processFileForChildEntity($this->makeImageDto('image', 'ffffffffffffffffffffffffffffffff')); + $this->fail('Expected ValidationException was not thrown.'); + } catch (ValidationException $e) { + $this->assertStringContainsString('MD5 mismatch', $e->getMessage()); + } + } + + public function testUnreadableLocalCopyIsReportedSeparatelyFromMd5Mismatch(): void + { + // Delete the downloaded temp copy right after the download so md5_file() cannot read it + $disk = $this->bindStorageFakes($this->pngContent(), function () { + $copies = glob(SummitBadgeFeatureTypeService::getLocalTmpStorage() . '/fproc_*') ?: []; + usort($copies, fn($a, $b) => filemtime($b) <=> filemtime($a)); + if (!empty($copies)) @unlink($copies[0]); + }); + $disk->shouldNotReceive('delete'); + + $uploader = Mockery::mock(IFileUploader::class); + $uploader->shouldNotReceive('build'); + $service = $this->makeService($uploader); + + // md5_file() warns before returning false; outside Laravel nothing converts that warning + set_error_handler(fn() => true, E_WARNING); + try { + $service->processFileForChildEntity($this->makeImageDto('image', md5($this->pngContent()))); + $this->fail('Expected ValidationException was not thrown.'); + } catch (ValidationException $e) { + $this->assertStringContainsString('unable to read', $e->getMessage()); + $this->assertStringNotContainsString('MD5 mismatch', $e->getMessage()); + } finally { + restore_error_handler(); + } + } + + public function testSuccessSetsImageAndCleansUpLocalAndRemoteFiles(): void + { + $disk = $this->bindStorageFakes($this->pngContent()); + $disk->shouldReceive('delete')->with(self::RemotePath)->once()->andReturn(true); + + $image = Mockery::mock(File::class); + $localPath = null; + $uploader = Mockery::mock(IFileUploader::class); + $uploader->shouldReceive('build') + ->once() + ->andReturnUsing(function (UploadedFile $file, string $folder) use ($image, &$localPath) { + $localPath = $file->getPathname(); + $this->assertSame('summit-event-images', $folder); + return $image; + }); + + $service = $this->makeService($uploader, $feature); + $feature->shouldReceive('setImage')->once()->with($image); + + $result = $service->processFileForChildEntity($this->makeImageDto('image', md5($this->pngContent()))); + + $this->assertSame($image, $result); + $this->assertNotNull($localPath); + $this->assertFileDoesNotExist($localPath); + } + + public function testUploadFailurePropagatesForRetryAndKeepsRemoteFile(): void + { + $disk = $this->bindStorageFakes($this->pngContent()); + $disk->shouldNotReceive('delete'); + + $localPath = null; + $uploader = Mockery::mock(IFileUploader::class); + $uploader->shouldReceive('build') + ->once() + ->andReturnUsing(function (UploadedFile $file) use (&$localPath) { + $localPath = $file->getPathname(); + throw new \RuntimeException('storage unavailable'); + }); + + $service = $this->makeService($uploader, $feature); + $feature->shouldNotReceive('setImage'); + + try { + $service->processFileForChildEntity($this->makeImageDto('image', md5($this->pngContent()))); + $this->fail('Expected RuntimeException was not thrown.'); + } catch (\RuntimeException $e) { + // not a ValidationException, so FileProcessingJob lets it propagate and the queue retries + $this->assertNotInstanceOf(ValidationException::class, $e); + } + + $this->assertFileDoesNotExist($localPath); + } + + public function testCleanupFailureAfterSuccessfulUploadDoesNotThrow(): void + { + $disk = $this->bindStorageFakes($this->pngContent()); + $disk->shouldReceive('delete')->andThrow(new \RuntimeException('delete failed')); + + $image = Mockery::mock(File::class); + $uploader = Mockery::mock(IFileUploader::class); + // a single build() call: the job succeeds, so no retry creates a second File record + $uploader->shouldReceive('build')->once()->andReturn($image); + + $service = $this->makeService($uploader, $feature); + $feature->shouldReceive('setImage')->once()->with($image); + + $this->assertSame($image, $service->processFileForChildEntity($this->makeImageDto('image', md5($this->pngContent())))); + } +} diff --git a/tests/oauth2/OAuth2SummitBadgeFeatureTypeApiTest.php b/tests/oauth2/OAuth2SummitBadgeFeatureTypeApiTest.php index 9b4d5fc3d..55a4562c0 100644 --- a/tests/oauth2/OAuth2SummitBadgeFeatureTypeApiTest.php +++ b/tests/oauth2/OAuth2SummitBadgeFeatureTypeApiTest.php @@ -11,8 +11,15 @@ * See the License for the specific language governing permissions and * limitations under the License. **/ +use App\Jobs\FileProcessingJob; +use App\Models\Foundation\Summit\Repositories\ISummitBadgeFeatureTypeRepository; use Illuminate\Http\UploadedFile; +use Illuminate\Support\Facades\App; +use Illuminate\Support\Facades\Config; +use Illuminate\Support\Facades\Queue; +use Illuminate\Support\Facades\Storage; use Mockery; +use models\summit\SummitBadgeFeatureType; /** * Class OAuth2SummitBadgeFeatureTypeApiTest */ @@ -306,4 +313,227 @@ public function testDeleteFeatureImage(){ $content = $response->getContent(); $this->assertResponseStatus(204); } + + // ------------------------------------------------------------------------- + // image as File API payload (async FileProcessingJob) + // ------------------------------------------------------------------------- + + private function jsonHeaders(): array + { + return [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json" + ]; + } + + /** + * Pins the File API storage to a fake local disk and stores $content at $remotePath. + */ + private function putRemoteFile(string $remotePath, string $content): void + { + Config::set('file_upload.storage_driver', 'local'); + Storage::fake('local'); + Storage::disk('local')->put($remotePath, $content); + } + + private function buildImagePayload(string $remotePath, string $filename, string $content, string $mime_type = 'image/png'): array + { + return [ + 'filepath' => $remotePath, + 'filename' => $filename, + 'md5' => md5($content), + 'size' => strlen($content), + 'mime_type' => $mime_type, + ]; + } + + private function fakePngContent(): string + { + return file_get_contents(UploadedFile::fake()->image('feature.png')->getRealPath()); + } + + private function postFeature(array $data) + { + return $this->action( + "POST", + "OAuth2SummitBadgeFeatureTypeApiController@add", + ['id' => self::$summit->getId()], + [], + [], + [], + $this->jsonHeaders(), + json_encode($data) + ); + } + + private function putFeature(int $feature_id, array $data) + { + return $this->action( + "PUT", + "OAuth2SummitBadgeFeatureTypeApiController@update", + ['id' => self::$summit->getId(), 'feature_id' => $feature_id], + [], + [], + [], + $this->jsonHeaders(), + json_encode($data) + ); + } + + private function assertImageJobPushedFor(int $feature_id): void + { + Queue::assertPushed(FileProcessingJob::class, 1); + Queue::assertPushed(FileProcessingJob::class, function (FileProcessingJob $job) use ($feature_id) { + return $job->fileInfoDTO->owner_entity_class === SummitBadgeFeatureType::class + && $job->fileInfoDTO->owner_member_name === 'image' + && $job->fileInfoDTO->owner_entity_id === $feature_id; + }); + } + + public function testAddBadgeFeatureTypeWithImageQueuesFileProcessingJob(): void + { + $content = $this->fakePngContent(); + $this->putRemoteFile('badge-features/tmp/feature.png', $content); + Queue::fake(); + + $response = $this->postFeature([ + 'name' => str_random(16) . '_feature_type', + 'image' => $this->buildImagePayload('badge-features/tmp/feature.png', 'feature.png', $content), + ]); + + $this->assertResponseStatus(201); + $feature = json_decode($response->getContent()); + $this->assertImageJobPushedFor($feature->id); + } + + public function testUpdateBadgeFeatureTypeWithImageQueuesFileProcessingJob(): void + { + $content = $this->fakePngContent(); + $this->putRemoteFile('badge-features/tmp/feature.png', $content); + Queue::fake(); + + $feature = $this->_testAddBadgeFeatureType(); + + // PUT goes through JsonController::updated(), which responds 201 + $this->putFeature($feature->id, [ + 'image' => $this->buildImagePayload('badge-features/tmp/feature.png', 'feature.png', $content), + ]); + + $this->assertResponseStatus(201); + $this->assertImageJobPushedFor($feature->id); + } + + public function testUpdateBadgeFeatureTypeWithoutImageQueuesNoJobAndKeepsImage(): void + { + $feature = $this->_testAddBadgeFeatureType(); + + $this->action( + "POST", + "OAuth2SummitBadgeFeatureTypeApiController@addFeatureImage", + ['id' => self::$summit->getId(), 'feature_id' => $feature->id], + [], + [], + ['file' => UploadedFile::fake()->image('feat.png')], + $this->jsonHeaders() + ); + $this->assertResponseStatus(201); + + Queue::fake(); + + $this->putFeature($feature->id, ['description' => 'updated without image']); + + $this->assertResponseStatus(201); + Queue::assertNotPushed(FileProcessingJob::class); + $entity = App::make(ISummitBadgeFeatureTypeRepository::class)->getById($feature->id); + $this->assertNotNull($entity->getImage()); + } + + public function testEmptyStringImageIsIgnoredOnAddAndUpdate(): void + { + Queue::fake(); + + $response = $this->postFeature([ + 'name' => str_random(16) . '_feature_type', + 'image' => '', + ]); + $this->assertResponseStatus(201); + $feature = json_decode($response->getContent()); + + $this->putFeature($feature->id, ['image' => '']); + $this->assertResponseStatus(201); + + Queue::assertNotPushed(FileProcessingJob::class); + } + + public function testImageWithInvalidExtensionReturns412AndPersistsNothing(): void + { + $this->putRemoteFile('badge-features/tmp/feature.bmp', 'fake-bmp-content'); + Queue::fake(); + $image = $this->buildImagePayload('badge-features/tmp/feature.bmp', 'feature.bmp', 'fake-bmp-content', 'image/bmp'); + + // POST: nothing is created, so the same name is still free afterwards + $name = str_random(16) . '_feature_type'; + $this->postFeature(['name' => $name, 'image' => $image]); + $this->assertResponseStatus(412); + $this->postFeature(['name' => $name]); + $this->assertResponseStatus(201); + + // PUT: the name change in the same request is not persisted + $feature = $this->_testAddBadgeFeatureType(); + $this->putFeature($feature->id, ['name' => str_random(16) . '_renamed', 'image' => $image]); + $this->assertResponseStatus(412); + + $response = $this->action( + "GET", + "OAuth2SummitBadgeFeatureTypeApiController@get", + ['id' => self::$summit->getId(), 'feature_id' => $feature->id], + [], + [], + [], + $this->jsonHeaders() + ); + $this->assertResponseStatus(200); + $this->assertEquals($feature->name, json_decode($response->getContent())->name); + + Queue::assertNotPushed(FileProcessingJob::class); + } + + public function testImageNotPresentInStorageReturns412AndPersistsNothing(): void + { + Config::set('file_upload.storage_driver', 'local'); + Storage::fake('local'); + Queue::fake(); + + $name = str_random(16) . '_feature_type'; + $this->postFeature([ + 'name' => $name, + 'image' => $this->buildImagePayload('badge-features/tmp/missing.png', 'missing.png', 'whatever'), + ]); + $this->assertResponseStatus(412); + + $this->postFeature(['name' => $name]); + $this->assertResponseStatus(201); + + Queue::assertNotPushed(FileProcessingJob::class); + } + + public function testImageOverMaxFileSizeReturns412AndPersistsNothing(): void + { + // valid extension, one byte over the limit enforced by addFeatureImage + $content = str_repeat('a', SummitBadgeFeatureType::ImageMaxFileSize + 1); + $this->putRemoteFile('badge-features/tmp/huge.png', $content); + Queue::fake(); + + $name = str_random(16) . '_feature_type'; + $this->postFeature([ + 'name' => $name, + 'image' => $this->buildImagePayload('badge-features/tmp/huge.png', 'huge.png', $content), + ]); + $this->assertResponseStatus(412); + + $this->postFeature(['name' => $name]); + $this->assertResponseStatus(201); + + Queue::assertNotPushed(FileProcessingJob::class); + } } \ No newline at end of file