Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/Console/Commands/RotateScreenshots.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ class RotateScreenshots extends Command
*/
public function handle(): void
{
if (!StorageCleaner::isSupported()) {
$this->info('Screenshots are on object storage: use bucket lifecycle rules instead of rotation');
return;
}

if (!StorageCleaner::needThinning()) {
$this->info('Time for thinning hasn\'t come');
return;
Expand Down
11 changes: 11 additions & 0 deletions app/Contracts/AttachmentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@
use App\Models\Attachment;
use App\Models\Project;
use App\Models\Task;
use DateTimeInterface;
use Illuminate\Http\UploadedFile;

interface AttachmentService
{
/** Longest lifetime of a temporary download link: 7 days, the S3 (SigV4) presigning limit */
public const MAX_TEMPORARY_URL_SECONDS = 7 * 24 * 60 * 60;

public function storeFile(UploadedFile $file, Attachment $attachment): string|false;

public function handleProjectChange(Task $parent): void;
Expand All @@ -33,6 +37,13 @@ public function getPath(Attachment $attachment): string;

public function getFullPath(Attachment $attachment): string;

/**
* Presigned download URL when attachments are on object storage (S3); null for the local disk
* (the signed attachment.temporary-download route streams the file then).
* The expiration must not exceed MAX_TEMPORARY_URL_SECONDS.
*/
public function getTemporaryUrl(Attachment $attachment, DateTimeInterface $expiresAt): ?string;

public function getHashAlgo(): string;

public function getHashSum(Attachment|string $attachment): false|string;
Expand Down
54 changes: 45 additions & 9 deletions app/Contracts/ScreenshotService.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

namespace App\Contracts;

use App\Helpers\StorageHelper;
use App\Jobs\GenerateScreenshotThumbnail;
use App\Models\TimeInterval;
use Intervention\Image\ImageManagerStatic as Image;
use Intervention\Image\Constraint;
use Storage;
use URL;

abstract class ScreenshotService
{
Expand All @@ -15,6 +17,8 @@ abstract class ScreenshotService
public const THUMBS_FOLDER = 'thumbs/';
private const THUMB_WIDTH = 280;
private const QUALITY = 50;
/** Lifetime of signed / presigned screenshot URLs */
public const URL_TTL_MINUTES = 10;

/** Get screenshot path by interval */
abstract public function getScreenshotPath(TimeInterval|int $interval): string;
Expand All @@ -23,10 +27,8 @@ abstract public function getThumbPath(TimeInterval|int $interval): string;

public function saveScreenshot($file, $timeInterval): void
{
if (!Storage::exists(self::PARENT_FOLDER)) {
Storage::makeDirectory(self::PARENT_FOLDER);
}

// $file is a local path or UploadedFile; the result goes to the default disk (local or S3).
// Storage::put() creates missing directories itself, no makeDirectory() round-trips on S3.
$path = is_string($file) ? $file : $file->path();

$image = Image::make($path);
Expand All @@ -38,11 +40,7 @@ public function saveScreenshot($file, $timeInterval): void

public function createThumbnail(TimeInterval|int $timeInterval): void
{
if (!Storage::exists(self::PARENT_FOLDER . self::THUMBS_FOLDER)) {
Storage::makeDirectory(self::PARENT_FOLDER . self::THUMBS_FOLDER);
}

$image = Image::make(Storage::path($this->getScreenshotPath($timeInterval)));
$image = Image::make(Storage::get($this->getScreenshotPath($timeInterval)));

$thumb = $image->resize(self::THUMB_WIDTH, null, fn(Constraint $constraint) => $constraint->aspectRatio());

Expand All @@ -55,6 +53,44 @@ public function destroyScreenshot(TimeInterval|int $interval): void
Storage::delete($this->getThumbPath($interval));
}

/**
* Temporary URL of the screenshot (or its thumbnail) that browsers can load directly.
*
* Local disk: relative URL of a route protected by the "signed" middleware that streams the file.
* S3: presigned object URL.
*/
public function getTemporaryUrl(TimeInterval|int $interval, bool $thumbnail = false): string
{
$id = $interval instanceof TimeInterval ? $interval->id : $interval;
$expiresAt = now()->addMinutes(self::URL_TTL_MINUTES);

if (StorageHelper::isLocal()) {
return URL::temporarySignedRoute(
$thumbnail ? 'intervals.screenshot.thumb' : 'intervals.screenshot.original',
$expiresAt,
['interval' => $id],
absolute: false,
);
}

return Storage::temporaryUrl(
$thumbnail ? $this->getThumbPath($id) : $this->getScreenshotPath($id),
$expiresAt,
);
}

/**
* @return array{screenshot_url: string, thumbnail_url: string, expires_in: int}
*/
public function getTemporaryUrls(TimeInterval|int $interval): array
{
return [
'screenshot_url' => $this->getTemporaryUrl($interval),
'thumbnail_url' => $this->getTemporaryUrl($interval, true),
'expires_in' => self::URL_TTL_MINUTES * 60,
];
}

public static function getFullPath(): string
{
$fileSystemPath = config('filesystems.default');
Expand Down
36 changes: 34 additions & 2 deletions app/Helpers/StorageCleaner.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,45 @@
use Illuminate\Database\Eloquent\Builder;
use Storage;

/**
* Screenshot rotation by free space of the local disk (PVC).
*
* Object storage (FILESYSTEM_DISK=s3) has no "free space": rotation is a no-op there,
* retention is expected to be handled by bucket lifecycle rules.
*/
class StorageCleaner
{
public static function isSupported(): bool
{
return StorageHelper::isLocal();
}

public static function getFreeSpace(): float
{
return disk_free_space(Storage::path(ScreenshotService::PARENT_FOLDER));
if (!self::isSupported()) {
return 0;
}

$directory = Storage::path(ScreenshotService::PARENT_FOLDER);

return disk_free_space(is_dir($directory) ? $directory : Storage::path(''));
}

public static function getUsedSpace(): float
{
if (!self::isSupported()) {
return 0;
}

return config('cleaner.total_space') - self::getFreeSpace();
}

public static function needThinning(): bool
{
if (!self::isSupported()) {
return false;
}

return
self::getUsedSpace() * 100 / config('cleaner.total_space')
>= config('cleaner.threshold');
Expand All @@ -34,7 +58,10 @@
*/
public static function thin($force = false): void
{
if ((!$force && !self::needThinning()) || Cache::store('octane')->get('thinning_now')) {
if (!self::isSupported()
|| (!$force && !self::needThinning())
|| Cache::store('octane')->get('thinning_now')
) {
return;
}

Expand All @@ -54,8 +81,8 @@
}
}

Cache::store('octane')->set('thinning_now',false);

Check warning on line 84 in app/Helpers/StorageCleaner.php

View workflow job for this annotation

GitHub Actions / PHP_CodeSniffer (advisory)

No space found after comma in argument list
Cache::store('octane')->set('last_thin',now());

Check warning on line 85 in app/Helpers/StorageCleaner.php

View workflow job for this annotation

GitHub Actions / PHP_CodeSniffer (advisory)

No space found after comma in argument list
}

/**
Expand Down Expand Up @@ -91,6 +118,11 @@
*/
public static function countAvailableScreenshots(): int
{
// One existence check per interval is fine on a local disk, but would be a HEAD request each on S3.
if (!self::isSupported()) {
return 0;
}

$count = 0;

$service = app()->make(ScreenshotService::class);
Expand Down
19 changes: 19 additions & 0 deletions app/Helpers/StorageHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace App\Helpers;

class StorageHelper
{
/**
* Whether the disk keeps files on the local filesystem (PVC / storage/app)
* rather than on a remote object storage such as S3.
*
* Decided by the configured driver, so it stays correct under Storage::fake().
*/
public static function isLocal(?string $disk = null): bool
{
$disk ??= config('filesystems.default');

return config("filesystems.disks.$disk.driver") === 'local';
}
}
4 changes: 4 additions & 0 deletions app/Http/Controllers/Api/AboutController.php
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,9 @@
* @apiSuccess {Number} space.used The amount of space currently used (in bytes).
* @apiSuccess {Number} space.total The total amount of storage space available (in bytes).
*
* @apiSuccess {Number} threshold The storage usage threshold percentage before thinning is needed.

Check warning on line 205 in app/Http/Controllers/Api/AboutController.php

View workflow job for this annotation

GitHub Actions / PHP_CodeSniffer (advisory)

Line exceeds 120 characters; contains 128 characters
* @apiSuccess {Boolean} need_thinning Indicates if the storage requires thinning.
* @apiSuccess {Boolean} rotation_supported False on object storage (S3): space is not measured, rotation is a no-op.

Check warning on line 207 in app/Http/Controllers/Api/AboutController.php

View workflow job for this annotation

GitHub Actions / PHP_CodeSniffer (advisory)

Line exceeds 120 characters; contains 136 characters
* @apiSuccess {Number} screenshots_available The number of available screenshots in the storage.
*
* @apiSuccess {Object} thinning Information about the thinning process.
Expand All @@ -220,6 +221,7 @@
* },
* "threshold": 75,
* "need_thinning": true,
* "rotation_supported": true,
* "screenshots_available": 0,
* "thinning": {
* "now": null,
Expand All @@ -243,6 +245,8 @@
],
'threshold' => config('cleaner.threshold'),
'need_thinning' => StorageCleaner::needThinning(),
// false on object storage (S3): no free space to measure, rotation is a no-op
'rotation_supported' => StorageCleaner::isSupported(),
'screenshots_available' => StorageCleaner::countAvailableScreenshots(),
'thinning' => [
'now' => Cache::store('octane')->get('thinning_now'),
Expand Down
16 changes: 15 additions & 1 deletion app/Http/Controllers/Api/AttachmentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,25 @@ public function tmpDownload(Request $request, Attachment $attachment): ?Streamed

public function createTemporaryUrl(DownloadAttachmentRequest $request, Attachment $attachment): string
{
// Only verified files are downloadable, on every disk (streamDownloadLogic() checks the same for the
// local route; a presigned S3 URL bypasses the app, so the check has to happen before issuing it)
if ($attachment->status !== AttachmentStatus::GOOD) {
abort(404);
}

// query string values arrive as strings, Carbon 3 rejects them
$expiresAt = now()->addSeconds((int)($request->validated('seconds') ?? 3600));

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
// Object storage (S3): a presigned URL of the object itself, absolute
if ($presignedUrl = $this->attachmentService->getTemporaryUrl($attachment, $expiresAt)) {
return $presignedUrl;
}

// we do this because signature breaks if we use built in method for creating relative path
// also cannot determine request scheme when ran inside docker
$url = URL::temporarySignedRoute(
'attachment.temporary-download',
now()->addSeconds((int)($request->validated('seconds') ?? 3600)),
$expiresAt,
$attachment
);
$parsedUrl = parse_url($url);
Expand Down
Loading
Loading