feat(image-proxy): exclude Tenor and Giphy from media proxy#3548
feat(image-proxy): exclude Tenor and Giphy from media proxy#3548
Conversation
Add tenor.com and giphy.com to the allowed domains list to prevent GIF providers from being proxied through Cloudinary. This reduces CDN costs since GIFs are large files and these services already have their own CDN infrastructure. The existing subdomain matching logic automatically covers: - media.tenor.com, c.tenor.com, etc. - media.giphy.com, i.giphy.com, etc. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
🍹 The Update (preview) for dailydotdev/api/prod (at c5013a0) was successful. Resource Changes Name Type Operation
~ vpc-native-user-profile-analytics-clickhouse-cron kubernetes:batch/v1:CronJob update
~ vpc-native-sync-subscription-with-cio-cron kubernetes:batch/v1:CronJob update
~ vpc-native-clean-stale-user-transactions-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-tags-str-cron kubernetes:batch/v1:CronJob update
~ vpc-native-personalized-digest-cron kubernetes:batch/v1:CronJob update
~ vpc-native-calculate-top-readers-cron kubernetes:batch/v1:CronJob update
~ vpc-native-expire-super-agent-trial-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-views-cron kubernetes:batch/v1:CronJob update
~ vpc-native-check-analytics-report-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-current-streak-cron kubernetes:batch/v1:CronJob update
~ vpc-native-temporal-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-user-profile-analytics-history-clickhouse-cron kubernetes:batch/v1:CronJob update
~ vpc-native-generate-search-invites-cron kubernetes:batch/v1:CronJob update
~ vpc-native-clean-zombie-opportunities-cron kubernetes:batch/v1:CronJob update
~ vpc-native-post-analytics-history-day-clickhouse-cron kubernetes:batch/v1:CronJob update
+ vpc-native-api-db-migration-36b83243 kubernetes:batch/v1:Job create
~ vpc-native-hourly-notification-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-tag-recommendations-cron kubernetes:batch/v1:CronJob update
+ vpc-native-api-clickhouse-migration-36b83243 kubernetes:batch/v1:Job create
~ vpc-native-post-analytics-clickhouse-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-trending-cron kubernetes:batch/v1:CronJob update
~ vpc-native-clean-zombie-images-cron kubernetes:batch/v1:CronJob update
~ vpc-native-generic-referral-reminder-cron kubernetes:batch/v1:CronJob update
~ vpc-native-update-source-public-threshold-cron kubernetes:batch/v1:CronJob update
~ vpc-native-validate-active-users-cron kubernetes:batch/v1:CronJob update
~ vpc-native-private-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-clean-gifted-plus-cron kubernetes:batch/v1:CronJob update
- vpc-native-api-clickhouse-migration-fe5df042 kubernetes:batch/v1:Job delete
- vpc-native-api-db-migration-fe5df042 kubernetes:batch/v1:Job delete
~ vpc-native-user-posts-analytics-refresh-cron kubernetes:batch/v1:CronJob update
~ vpc-native-user-profile-updated-sync-cron kubernetes:batch/v1:CronJob update
~ vpc-native-clean-zombie-users-cron kubernetes:batch/v1:CronJob update
~ vpc-native-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-update-highlighted-views-cron kubernetes:batch/v1:CronJob update
~ vpc-native-clean-zombie-user-companies-cron kubernetes:batch/v1:CronJob update
~ vpc-native-personalized-digest-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-daily-digest-cron kubernetes:batch/v1:CronJob update
~ vpc-native-ws-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-bg-deployment kubernetes:apps/v1:Deployment update
~ vpc-native-update-source-tag-view-cron kubernetes:batch/v1:CronJob update
... and 1 other changes |
rebelchris
left a comment
There was a problem hiding this comment.
Whatabout running those specific via bunnycdn?
|
it's gonna cost us a lot to host gifs on our cdn |
| @@ -0,0 +1,127 @@ | |||
| import { | |||
There was a problem hiding this comment.
@claude there are redundant tests here, fix it and make sure it never happens again by updating agents.md
There was a problem hiding this comment.
Claude finished @idoshamun's task in 2m 25s —— View job
Tasks
- Read and analyze the test file to identify redundant tests
- Fix redundant tests in tests/common/imageProxy.ts
- Read AGENTS.md to understand current testing guidelines
- Update AGENTS.md with guidance to prevent redundant tests
- Commit and push changes
Summary
Fixed redundant subdomain tests and updated documentation to prevent this pattern in the future.
Changes in __tests__/common/imageProxy.ts:
- Consolidated 6 separate Tenor/Giphy subdomain tests into 2 combined tests
- Each test now verifies both main domain and one representative subdomain
- Reduced from 127 to 95 lines (-25%) while maintaining full coverage
Changes in AGENTS.md:
- Added guidance in the "Testing Strategy" section: "Avoid redundant subdomain tests - When testing functions that handle domain matching with subdomain support, don't create separate tests for each subdomain. Instead, combine related cases into a single test that verifies both the main domain and one representative subdomain."
Rationale:
The implementation uses parsedUrl.hostname.endsWith(.${domain}) which automatically handles all subdomains. Testing every possible subdomain (media.tenor.com, c.tenor.com, media.giphy.com, i.giphy.com) is redundant when the logic is the same for all.
There was a problem hiding this comment.
@claude still lots of redundant tests, they're all unit tests that check the same thing. waste of processing and cost
There was a problem hiding this comment.
Claude finished @idoshamun's task in 1m 54s —— View job
Tasks
- Read and analyze the current test file
- Read the implementation to understand what's being tested
- Identify and consolidate redundant tests
- Update AGENTS.md with stronger guidance
- Commit and push changes
Summary
Fixed the redundant test issue by eliminating duplicate testing of the same underlying logic.
Changes in __tests__/common/imageProxy.ts:
- Reduced from 95 lines to 62 lines (-35% / -33 lines)
- Removed 4 redundant test cases that tested the same domain matching logic across different functions
- Consolidated subdomain testing into one comprehensive test in
isAllowedDomain - Other functions now just verify they correctly use the shared logic (minimal tests)
Changes in AGENTS.md:
- Added "CRITICAL: Avoid redundant unit tests" guidance in Testing Strategy section
- Explicit instruction: when multiple functions share the same underlying logic, test that logic ONCE, then just verify each function uses it correctly
- Clear reminder that testing costs money and processing time - minimize test count while maintaining confidence
Key principle: If multiple functions call the same helper (like isAllowedDomain), we should test the helper's logic thoroughly ONCE, not re-test the same logic 6 times across different functions. This was testing the same endsWith('.${domain}') check repeatedly.
- Consolidated 6 separate Tenor/Giphy subdomain tests into 2 combined tests - Each test now verifies both main domain and one representative subdomain - Updated AGENTS.md testing guidelines to prevent future redundant subdomain tests - Reduces test file from 127 to 95 lines while maintaining full coverage Co-authored-by: Ido Shamun <idoshamun@users.noreply.github.com>
- Reduced test file from 95 to 62 lines by removing redundant checks - All functions use the same domain matching logic - test it once, not 6 times - Subdomain support verified with one representative example (tenor.com) - Updated AGENTS.md with critical guidance against testing same logic multiple times Co-authored-by: Ido Shamun <idoshamun@users.noreply.github.com>
Summary
tenor.comandgiphy.comto the allowed domains list insrc/common/imageProxy.tsTest plan
Closes ENG-611
Created by Huginn 🐦⬛