Implement CDN support#1998
Open
andrewroth wants to merge 6 commits into
Open
Conversation
Adds MOBILE_CONTENT_API_CDN_HOST env var. When set, the translation files redirect and Translation#s3_url return URLs on that host (e.g. a CloudFront distribution) instead of the S3 bucket. When unset, behavior is unchanged. Supports KR1 — improving tool download speed in low-bandwidth areas by serving content through a CDN. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds a `rails_public_blob` direct route that generates ActiveStorage blob URLs against MOBILE_CONTENT_API_CDN_HOST when set, falling back to the standard Rails blob URL otherwise. The attachment serializer and download controller now use this helper so mobile clients hit CloudFront directly instead of going through Rails for every file. This requires DevOps to configure CloudFront so it can serve the bucket's blob keys (via public-read or Origin Access Control); the API generates unsigned URLs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Verifies that the serializer returns CDN-based URLs when MOBILE_CONTENT_API_CDN_HOST is configured and falls back to standard Rails blob URLs otherwise. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
knutsenm
requested changes
May 29, 2026
| end | ||
|
|
||
| def self.content_host | ||
| ENV["MOBILE_CONTENT_API_CDN_HOST"].presence || |
Contributor
There was a problem hiding this comment.
Although optional, this new ENV var should get a mention in the .env file.
Contributor
There was a problem hiding this comment.
While we're on the topic, here's Claude on other missing ENV vars:
The new one (this branch)
MOBILE_CONTENT_API_CDN_HOST— introduced by the CDN routing work, not yet in.env.
App-config vars used in code but missing from .env
These are real application settings (not runtime/framework noise), so they're the ones worth documenting:
| Var | Why it matters |
|---|---|
AWS_ACCESS_KEY_ID |
S3 credentials (only the bucket + region are in .env) |
AWS_SECRET_ACCESS_KEY |
S3 credentials |
AWS_S3_CONFIG_BUCKET |
Second bucket, separate from MOBILE_CONTENT_API_BUCKET |
JSON_WEB_TOKEN_SECRET |
Auth token signing — notable omission |
JSON_WEB_TOKEN_SECRET_TEST |
Test variant of the above |
GOOGLE_ANALYTICS_VIEW_ID |
GA reporting integration |
GOOGLE_API_USE_RAILS_LOGGER |
Google API logging toggle |
DB_POOL |
DB connection pool size |
ROLLBAR_ENV |
Rollbar environment tag (token is already there) |
ENVIRONMENT |
App environment indicator |
PROJECT_NAME |
Used in config |
TEST_DB_HOST / TEST_DB_NAME / TEST_DB_USER / TEST_DB_PASSWORD / TEST_DB_PORT |
Test DB connection (parallel to the DB_* set that is documented) |
Deliberately not worth adding
These showed up in the grep but are runtime/platform-provided, not things you'd set in a dev .env:
RAILS_ENV, RAILS_MASTER_KEY, RAILS_LOG_LEVEL, RAILS_MAX_THREADS, PORT, PIDFILE, BUNDLE_GEMFILE, CI, GIT_COMMIT, AWS_EXECUTION_ENV.
Priorities
The standouts to prioritize:
MOBILE_CONTENT_API_CDN_HOST(this branch)- The AWS credential pair +
AWS_S3_CONFIG_BUCKET JSON_WEB_TOKEN_SECRET
Those are functional gaps a new dev would hit, whereas the others are mostly nice-to-document.
There was a problem hiding this comment.
Pull request overview
This PR adds CDN support to the Mobile Content API by rewriting generated asset URLs (ActiveStorage blobs and translation ZIP downloads) to use a configurable CDN host when MOBILE_CONTENT_API_CDN_HOST is set, while preserving existing behavior when it is not.
Changes:
- Add a
direct :rails_public_blobroute helper to generate CDN-backed blob URLs, and switch attachment serialization/download redirects to use it. - Route translation ZIP download redirects through a shared
Package.content_host, and addPackage.public_urlto rewrite S3 object URLs to the CDN host. - Add/extend request/model/serializer specs to cover CDN and non-CDN behaviors.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
config/routes.rb |
Introduces rails_public_blob direct route and switches translation file redirect to Package.content_host. |
app/serializers/attachment_serializer.rb |
Uses rails_public_blob_url for attachment file URLs. |
app/controllers/attachments_controller.rb |
Redirects downloads via rails_public_blob_url (allowing other host for CDN). |
app/models/translation.rb |
Uses Package.public_url for translation S3 URLs. |
app/lib/package.rb |
Adds content_host and public_url helpers to support CDN host rewriting. |
spec/routes_spec.rb |
Adds request spec coverage for CDN redirect behavior. |
spec/models/translation_spec.rb |
Adds model spec coverage for CDN host rewriting of S3 URLs. |
spec/serializers/attachment_serializer_spec.rb |
Adds serializer spec coverage for CDN vs standard blob URL generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use simpler and safer string join for url paths Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Add MOBILE_CONTENT_API_CDN_HOST and other in-use-but-undocumented env vars (AWS credentials, AWS_S3_CONFIG_BUCKET, JSON_WEB_TOKEN_SECRET) to .env as stubs, per review feedback. Fix attachment serializer spec to upload wall.jpg as image/jpeg and use create! to fail fast. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I see you added the "On Staging" label, I'll get this merged to the staging branch! |
|
Merge conflict attempting to merge this into staging. Please fix manually. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement CDN support