Skip to content

Add asset endpoint to request binaries directly - #4645

Merged
ildyria merged 6 commits into
masterfrom
asset-v3
Aug 21, 2026
Merged

Add asset endpoint to request binaries directly#4645
ildyria merged 6 commits into
masterfrom
asset-v3

Conversation

@ildyria

@ildyria ildyria commented Aug 21, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added API v3 photo asset retrieval for album-scoped thumbnails and other size variants.
    • Supports authenticated access and secure, time-limited temporary links.
    • Delivers local assets directly, redirects stored assets through temporary URLs, and applies watermarking where configured.
    • Added album and photo access validation with standardized JSON error responses.
  • Documentation

    • Documented the API v3 asset endpoint, security model, response behavior, and implementation decisions.
  • Tests

    • Added coverage for authorization, signatures, missing assets, watermarking, and storage scenarios.

@ildyria
ildyria requested a review from a team as a code owner August 21, 2026 09:56
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 01a2af50-da87-4bc6-814e-e793c83ab3c7

📥 Commits

Reviewing files that changed from the base of the PR and between 7f3158c and 24b6d6c.

📒 Files selected for processing (2)
  • app/Http/Requests/Photo/GetPhotoAssetRequest.php
  • tests/ImageProcessing/Import/ImportFromServerBrowseTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/Http/Requests/Photo/GetPhotoAssetRequest.php

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

This change adds API v3 album-scoped photo asset retrieval. It introduces request validation, album authorization, temporary-link signing, watermark-aware local delivery, S3 redirects, feature tests, PHPUnit wiring, and architecture documentation.

Changes

API v3 asset retrieval

Layer / File(s) Summary
Endpoint contract and routing
app/Contracts/Http/Requests/RequestAttribute.php, app/Http/Middleware/EnsureJsonErrorResponses.php, app/Http/Kernel.php, app/Providers/RouteServiceProvider.php, routes/api_v3.php
Registers the API v3 route and JSON error middleware. The route accepts album, photo, and size-variant parameters while returning binary responses.
Request validation and authorization
app/Enum/SizeVariantAssetType.php, app/Http/Requests/Photo/GetPhotoAssetRequest.php, app/Services/TemporaryLinkSigner.php
Resolves album-scoped assets, validates variants and signatures, checks album membership, and returns distinct authorization outcomes.
Asset delivery and scenario coverage
app/Http/Controllers/Gallery/PhotoAssetController.php, tests/Feature_v3/*, tests/Unit/Services/TemporaryLinkSignerTest.php
Streams local assets, redirects S3 assets, resolves watermark variants, and tests route, authorization, signature, storage, and album scenarios.
Feature specification and architecture records
docs/specs/3-reference/api-design.md, docs/specs/4-architecture/features/056-api-v3-asset-retrieval/*, docs/specs/4-architecture/{knowledge-map.md,open-questions.md,roadmap.md}, docs/specs/6-decisions/*
Documents the endpoint contract, implementation plan, scenario matrix, architecture decisions, and completed feature records.
Test suite execution wiring
phpunit.xml, phpunit.ci.xml, .github/workflows/php_tests.yml, codecov.yml, tests/ImageProcessing/Import/ImportFromServerBrowseTest.php
Adds the Feature_v3 PHPUnit suite, excludes it on PHP 8.4, updates the Codecov build threshold, and updates the expected directory listing.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟠 High · up to 24b6d

The endpoint can fail for valid RAW and PLACEHOLDER assets, while guest requests may bypass required signature validation when temporary links are disabled, causing incorrect authorization behavior and potential unintended asset access. These issues should be fixed before merging.

Poem

A rabbit checked the routes at night,
Signed each link with moonlit light.
Small files streamed, S3 paths flew,
Tests hopped through Feature_v3 too.
Docs tucked the new API from view.

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.60% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 47 functions across 14 files. Write docstrings for the functions missing them to satisfy the coverage threshold.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/specs/4-architecture/features/056-api-v3-asset-retrieval/spec.md (1)

148-148: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add the required Markdown footer to each documentation file.

As per coding guidelines, every **/*.md file must end with an hr line followed by *Last updated: [date of the update]*.

  • docs/specs/4-architecture/features/056-api-v3-asset-retrieval/spec.md#L148-L148: append the footer.
  • docs/specs/4-architecture/features/056-api-v3-asset-retrieval/plan.md#L148-L148: append the footer.
  • docs/specs/4-architecture/features/056-api-v3-asset-retrieval/tasks.md#L138-L138: append the footer.
  • docs/specs/6-decisions/ADR-0008-v3-asset-endpoint-signing-and-authorization.md#L73-L73: append the footer.
  • docs/specs/6-decisions/ADR-0009-api-v3-response-shape-precedent.md#L44-L44: append the footer.

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5f346ffe-c506-4609-a9ec-c3e7528de212

📥 Commits

Reviewing files that changed from the base of the PR and between 27f51cf and 6b349d2.

📒 Files selected for processing (25)
  • .github/workflows/php_tests.yml
  • app/Contracts/Http/Requests/RequestAttribute.php
  • app/Http/Controllers/Gallery/PhotoAssetController.php
  • app/Http/Kernel.php
  • app/Http/Middleware/EnsureJsonErrorResponses.php
  • app/Http/Requests/Photo/GetPhotoAssetRequest.php
  • app/Providers/RouteServiceProvider.php
  • app/Rules/SizeVariantTypeNameRule.php
  • app/Services/TemporaryLinkSigner.php
  • codecov.yml
  • docs/specs/3-reference/api-design.md
  • docs/specs/4-architecture/features/056-api-v3-asset-retrieval/plan.md
  • docs/specs/4-architecture/features/056-api-v3-asset-retrieval/spec.md
  • docs/specs/4-architecture/features/056-api-v3-asset-retrieval/tasks.md
  • docs/specs/4-architecture/knowledge-map.md
  • docs/specs/4-architecture/open-questions.md
  • docs/specs/4-architecture/roadmap.md
  • docs/specs/6-decisions/ADR-0008-v3-asset-endpoint-signing-and-authorization.md
  • docs/specs/6-decisions/ADR-0009-api-v3-response-shape-precedent.md
  • phpunit.ci.xml
  • phpunit.xml
  • routes/api_v3.php
  • tests/Feature_v3/Base/BaseApiWithDataTest.php
  • tests/Feature_v3/Photo/PhotoAssetV3Test.php
  • tests/Unit/Services/TemporaryLinkSignerTest.php

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Comment thread app/Http/Controllers/Gallery/PhotoAssetController.php Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cbd4f27d-a4e4-4b83-9e2d-bef7182a95f6

📥 Commits

Reviewing files that changed from the base of the PR and between 6b349d2 and 7f3158c.

📒 Files selected for processing (5)
  • app/Enum/SizeVariantAssetType.php
  • app/Http/Controllers/Gallery/PhotoAssetController.php
  • app/Http/Requests/Photo/GetPhotoAssetRequest.php
  • routes/api_v3.php
  • tests/Feature_v3/Photo/PhotoAssetV3Test.php

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Comment thread app/Http/Requests/Photo/GetPhotoAssetRequest.php Outdated
@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.81481% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.20%. Comparing base (c8769e6) to head (24b6d6c).
⚠️ Report is 3 commits behind head on master.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ildyria ildyria added the v8 label Aug 21, 2026
@ildyria
ildyria merged commit 017edbe into master Aug 21, 2026
53 checks passed
@ildyria
ildyria deleted the asset-v3 branch August 21, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant