Closed Bug 991955 Opened 10 years ago Closed 8 years ago

Put fireplace app API behind CDN

Categories

(Marketplace Graveyard :: API, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: clouserw, Unassigned)

References

Details

(Keywords: perf, Whiteboard: [see comment 2 and 3][marketplace-transition])

The only reason I can see is that Krupa and some developers probably expect any changes to the app to surface immediately here. With our current max-age of 3 minutes, well, you'd have to wait at most 3 minutes to see your change. I think the benefit outweighs that annoyance. I'm sure we can start coming up with a list of URLs to flush upon modification (I still see old code in AMO that used to do that). But yeah that's my 2¢.
Also, this API still uses user authentication, because we need to be able to display a private app if you are the developer or a reviewer. So users that are logged in wouldn't benefit from the CDN (it would actually probably make it slightly slower for them)
Blocks: 992365
Hmm, I wouldn't expect that use case a ton - maybe we could check if they are a logged in developer and if so do another request after to a non-CDN url.  I don't know if that's worth it, just thinking out loud.
Priority: -- → P2
FYI : https://marketplace.firefox.com/api/v1/fireplace/collection/<slug>/  should also go through the CDN.
Assignee: nobody → mpillard
Filed bug 1002471 for the collection API.
See Also: → 1002471
Assignee: mpillard → nobody
Whiteboard: [see comment 2 and 3]
Right now we're using the CDN to cache featured (homepage)/search/category pages. This bug is for the API response for an app detail page. If a user navigates to a detail page from the homepage, search results, or a category landing page, we don't hit this API because we already have the app data from the prior API endpoints. If a user is deeplinked to an app detail page, then we *do* use the data from this endpoint (in which case it should be cached on the CDN).

+1 to doing this but not super important.
No longer blocks: 992365
This is really important now that the feed can no longer uses model data from the homepage API response to populate the detail page.
This continues to be the slowest API response:
https://rpm.newrelic.com/accounts/315282/browser/2914756/bjax_requests#id=733240733&sort_by=response_time

––

> diff --git a/src/media/js/routes_api.js b/src/media/js/routes_api.js
> index 42d3994..1bfb0ab 100644
> --- a/src/media/js/routes_api.js
> +++ b/src/media/js/routes_api.js
> @@ -12,7 +12,7 @@ define('routes_api', [], function() {
>          'feed': '/api/v2/feed/get/?cache=21600&vary=0',
>          'feed-app': '/api/v2/fireplace/feed/apps/{0}/',
>          'feed-brand': '/api/v2/fireplace/feed/brands/{0}/',
> -        'feed-collection': '/api/v2/fireplace/feed/collections/{0}/',
> +        'feed-collection': '/api/v2/fireplace/feed/collections/{0}/?cache=21600&vary=0',
>          'feed-items': '/api/v2/feed/items/',
>          'feed-shelf': '/api/v2/fireplace/feed/shelves/{0}/',
>          'feedback': '/api/v2/account/feedback/',
> @@ -31,8 +31,8 @@ define('routes_api', [], function() {
>          'recommended': '/api/v2/apps/recommend/',
>          'record_free': '/api/v2/installs/record/',
>          'record_paid': '/api/v2/receipts/install/',
> -        'review': '/api/v2/apps/rating/{0}/',
> -        'reviews': '/api/v2/apps/rating/',
> +        'review': '/api/v2/apps/rating/{0}/?cache=1&vary=0',
> +        'reviews': '/api/v2/apps/rating/?cache=1&vary=0',
>          'search': '/api/v2/fireplace/search/?cache=1&vary=0',
>          'settings': '/api/v2/account/settings/mine/',
>          'site-config': '/api/v2/services/config/site/?cache=1&serializer=commonplace&vary=0',
> diff --git a/src/media/js/settings.js b/src/media/js/settings.js
> index 76116a6..d01d0de 100644
> --- a/src/media/js/settings.js
> +++ b/src/media/js/settings.js
> @@ -49,6 +49,8 @@ define('settings', ['l10n', 'settings_local', 'underscore'], function(l10n, sett
>          // These are the only API endpoints that should be served from the CDN
>          // (key: URL; value: max-age in seconds, but it's unused at the moment).
>          api_cdn_whitelist: {
> +            '/api/v2/fireplace/app/': 60 * 3,  // 3 minutes
> +            '/api/v2/fireplace/feed/collections/': 6 * 60 * 60,  // 6 hours
>              '/api/v2/fireplace/search/': 60 * 3,  // 3 minutes
>              '/api/v2/feed/get/': 6 * 60 * 60,  // 6 hours
>              '/api/v2/services/config/site/': 60 * 3  // 3 minutes
> @@ -76,8 +78,10 @@ define('settings', ['l10n', 'settings_local', 'underscore'], function(l10n, sett
>          // these TTLs apply to only when the app is first launched.
>          offline_cache_whitelist: {
>              '/api/v2/feed/get/': 60 * 60 * 24 * 7,  // 1 week
> -            '/api/v2/fireplace/consumer-info/': 60 * 60 * 24 * 7,  // 1 week
> -            '/api/v2/services/config/site/': 60 * 60 * 24 * 7  // 1 week
> +            '/api/v2/fireplace/app/': 60 * 60 * 24 * 7,  // 1 week
> +            '/api/v2/fireplace/feed/collections/': 60 * 60 * 24 * 7,  // 1 week
> +            '/api/v2/fireplace/search/': 60 * 60 * 24 * 7,  // 1 week
> +            '/api/v2/services/config/site/': 60 * 60 * 24 * 7,  // 1 week
>          },
>          offline_cache_enabled: offline_cache_enabled,
>          offline_cache_limit: 1024 * 1024 * 4,  // 4 MB
> 

––

Because of this line https://github.com/mozilla/marketplace-core-modules/blob/f5006da/urls.js#L107 the `baseurl` becomes `/api/v2/fireplace/app/quick-travel/`, which doesn't match the `/api/v2/fireplace/app/` in `settings.api_cdn_whitelist`. So we may need to go back to doing regexes. I'll leave that as an exercise to the assignee.
It's not so trivial - As the code is currently, this shouldn't just be cached on the CDN as it's an authenticated call.
(In reply to Mathieu Pillard [:mat] from comment #9)
> It's not so trivial - As the code is currently, this shouldn't just be
> cached on the CDN as it's an authenticated call.

Oh, I didn't realise that. When I load https://marketplace.firefox.com/app/7-minutos I didn't see the `?_user` in the URL:

     https://marketplace.firefox.com/api/v2/fireplace/app/7-minutos/?cache=1&lang=en-US&limit=25&region=us&vary=0
Depends on: 1162061
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Whiteboard: [see comment 2 and 3] → [see comment 2 and 3][marketplace-transition]
You need to log in before you can comment on or make changes to this bug.