Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
10 changes: 9 additions & 1 deletion app/Http/Controllers/Api/AttachmentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,19 @@ public function tmpDownload(Request $request, Attachment $attachment): ?Streamed

public function createTemporaryUrl(DownloadAttachmentRequest $request, Attachment $attachment): string
{
// 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