FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

increase cache times of content · devhttps/frontend@9fc7afe · GitHub

/ frontend Public
forked from guardian/frontend

Commit 9fc7afe

Browse files
Grant Klopper
committed
increase cache times of content
1 parent afbd56a commit 9fc7afe

6 files changed

Lines changed: 107 additions & 82 deletions

File tree

‎article/app/controllers/ArticleController.scala‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,6 @@ object ArticleController extends Controller with RendersItemResponse with Loggin
195195
}
196196

197197
def createLiveBlogModel(liveBlog: Article, response: ItemResponse, maybeRequiredBlockId: Option[String]) = {
198-
import conf.switches.Switches.LongCacheSwitch
199198

200199
val pageSize = if (liveBlog.content.tags.tags.map(_.id).contains("sport/sport")) 30 else 10
201200
val liveBlogPageModel = LiveBlogCurrentPage(
@@ -207,18 +206,14 @@ object ArticleController extends Controller with RendersItemResponse with Loggin
207206
case Some(pageModel) =>
208207

209208
val cacheTime =
210-
if (!pageModel.currentPage.isArchivePage && liveBlog.fields.isLive) liveBlog.metadata.cacheTime
211-
else {
212-
if (LongCacheSwitch.isSwitchedOn) {
213-
if (liveBlog.fields.lastModified > DateTime.now(liveBlog.fields.lastModified.getZone) - 1.hour) CacheTime.RecentlyUpdatedPurgable
214-
else if (liveBlog.fields.lastModified > DateTime.now(liveBlog.fields.lastModified.getZone) - 24.hours) CacheTime.LastDayUpdatedPurgable
215-
else CacheTime.NotRecentlyUpdatedPurgable
216-
} else {
217-
if (liveBlog.fields.lastModified > DateTime.now(liveBlog.fields.lastModified.getZone) - 1.hour) CacheTime.RecentlyUpdated
218-
else if (liveBlog.fields.lastModified > DateTime.now(liveBlog.fields.lastModified.getZone) - 24.hours) CacheTime.LastDayUpdated
219-
else CacheTime.NotRecentlyUpdated
220-
}
221-
}
209+
if (!pageModel.currentPage.isArchivePage && liveBlog.fields.isLive)
210+
liveBlog.metadata.cacheTime
211+
else if (liveBlog.fields.lastModified > DateTime.now(liveBlog.fields.lastModified.getZone) - 1.hour)
212+
CacheTime.RecentlyUpdated
213+
else if (liveBlog.fields.lastModified > DateTime.now(liveBlog.fields.lastModified.getZone) - 24.hours)
214+
CacheTime.LastDayUpdated
215+
else
216+
CacheTime.NotRecentlyUpdated
222217

223218
val liveBlogCache = liveBlog.copy(
224219
content = liveBlog.content.copy(

‎common/app/conf/switches/PerformanceSwitches.scala‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,6 @@ trait PerformanceSwitches {
6969
exposeClientSide = true
7070
)
7171

72-
val DoubleCacheTimesSwitch = Switch(
73-
SwitchGroup.Performance,
74-
"double-cache-times",
75-
"Doubles the cache time of every endpoint. Turn on to help handle exceptional load.",
76-
safeState = On,
77-
sellByDate = never,
78-
exposeClientSide = false
79-
)
80-
8172
val RelatedContentSwitch = Switch(
8273
SwitchGroup.Performance,
8374
"related-content",

‎common/app/dev/DevAssetsController.scala‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@ package dev
33
import common.Assets.AssetNotFoundException
44
import common.ExecutionContexts
55
import java.io.File
6+
import model.{NoCache, Cached}
7+
import model.Cached.WithoutRevalidationResult
8+
import play.api.Play
69
import play.api.libs.MimeTypes
710
import play.api.mvc._
811
import play.api.libs.iteratee.Enumerator
12+
import play.api.Play.current
913

1014
object DevAssetsController extends Controller with ExecutionContexts {
1115

@@ -42,10 +46,18 @@ object DevAssetsController extends Controller with ExecutionContexts {
4246
if (MimeTypes.isText(mime)) s"$mime; charset=utf-8" else mime
4347
} getOrElse BINARY
4448

45-
Result(
46-
ResponseHeader(OK, Map(CONTENT_TYPE -> contentType)),
47-
Enumerator.fromStream(resolved.openStream())
48-
)
49+
val result = Result(
50+
ResponseHeader(OK, Map(CONTENT_TYPE -> contentType)),
51+
Enumerator.fromStream(resolved.openStream())
52+
)
53+
54+
// WebDriver caches during tests. Caching CSS during tests might speed some things up.
55+
if (Play.isTest) {
56+
Cached(84000)(WithoutRevalidationResult(result))
57+
} else {
58+
// but we don't want caching during development...
59+
NoCache(result)
60+
}
4961
}
5062

5163
def surveys(file: String): Action[AnyContent] =

‎common/app/model/Cached.scala‎

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,30 @@
11
package model
22

3-
import conf.switches.Switches
43
import conf.switches.Switches._
54
import org.joda.time.DateTime
65
import org.scala_tools.time.Imports._
76
import play.api.http.Writeable
87
import play.api.mvc._
9-
import play.twirl.api.Html
8+
import scala.math.{min, max}
9+
10+
import scala.concurrent.ExecutionContext.Implicits.global
1011
import scala.concurrent.Future
1112
import scala.concurrent.duration.Duration
12-
import scala.concurrent.ExecutionContext.Implicits.global
1313

1414
case class CacheTime(cacheSeconds: Int)
1515
object CacheTime {
1616

17-
object LiveBlogActive extends CacheTime(5)
18-
object RecentlyUpdated extends CacheTime(10)
19-
object LastDayUpdated extends CacheTime(30)
20-
object NotRecentlyUpdated extends CacheTime(300)
17+
// 3800 seems slightly arbitrary, but our CDN caches to disk if above 3700
18+
// https://community.fastly.com/t/why-isnt-serve-stale-working-as-expected/369
19+
private def extended(cacheTime: Int) = if (LongCacheSwitch.isSwitchedOn) 3800 else cacheTime
20+
2121
object Default extends CacheTime(60)
22-
object RecentlyUpdatedPurgable extends CacheTime(300)
23-
object LastDayUpdatedPurgable extends CacheTime(1200)
24-
object NotRecentlyUpdatedPurgable extends CacheTime(1800)
22+
object LiveBlogActive extends CacheTime(5)
23+
object RecentlyUpdated extends CacheTime(60)
2524

25+
def LastDayUpdated = CacheTime(extended(60))
26+
def NotRecentlyUpdated = CacheTime(extended(300))
27+
def NotRecentlyUpdatedPurgable = CacheTime(extended(1800))
2628
}
2729

2830
object Cached extends implicits.Dates {
@@ -80,19 +82,32 @@ object Cached extends implicits.Dates {
8082
cacheableResult.result
8183
}
8284

83-
private def cacheHeaders(seconds: Int, result: Result, maybeHash: Option[(Hash, Option[String])]) = {
85+
/*
86+
NOTE, if you change these headers make sure they are compatible with our Edge Cache
87+
88+
see
89+
http://tools.ietf.org/html/rfc5861
90+
http://www.fastly.com/blog/stale-while-revalidate
91+
http://docs.fastly.com/guides/22966608/40347813
92+
93+
This explains Surrogate-Control vs Cache-Control
94+
TLDR Surrogate-Control is used by the CDN, Cache-Control by the browser - do *not* add `private` to Cache-Control
95+
https://docs.fastly.com/guides/tutorials/cache-control-tutorial
96+
*/
97+
private def cacheHeaders(maxAge: Int, result: Result, maybeHash: Option[(Hash, Option[String])]) = {
8498
val now = DateTime.now
85-
val expiresTime = now + seconds.seconds
86-
val maxAge = if (DoubleCacheTimesSwitch.isSwitchedOn) seconds * 2 else seconds
99+
val expiresTime = if (LongCacheSwitch.isSwitchedOn) now + min(maxAge, 60).seconds else now + maxAge.seconds
87100

88-
// NOTE, if you change these headers make sure they are compatible with our Edge Cache
101+
val staleWhileRevalidateSeconds = max(maxAge / 10, 1)
102+
val surrogateCacheControl = s"max-age=$maxAge, stale-while-revalidate=$staleWhileRevalidateSeconds, stale-if-error=$tenDaysInSeconds"
89103

90-
// see
91-
// http://tools.ietf.org/html/rfc5861
92-
// http://www.fastly.com/blog/stale-while-revalidate
93-
// http://docs.fastly.com/guides/22966608/40347813
94-
val staleWhileRevalidateSeconds = math.max(maxAge / 10, 1)
95-
val cacheControl = s"max-age=$maxAge, stale-while-revalidate=$staleWhileRevalidateSeconds, stale-if-error=$tenDaysInSeconds"
104+
val cacheControl = if (LongCacheSwitch.isSwitchedOn) {
105+
val browserMaxAge = min(maxAge, 60)
106+
val browserStaleWhileRevalidateSeconds = max(browserMaxAge / 10, 1)
107+
s"max-age=$browserMaxAge, stale-while-revalidate=$browserStaleWhileRevalidateSeconds, stale-if-error=$tenDaysInSeconds"
108+
} else {
109+
surrogateCacheControl
110+
}
96111

97112
val (etagHeaderString, validatedResult): (String, Result) = maybeHash.map { case (hash, maybeHashToMatch) =>
98113
val etag = s"""W/"hash${hash.string}""""
@@ -106,13 +121,15 @@ object Cached extends implicits.Dates {
106121
)
107122

108123
validatedResult.withHeaders(
109-
"Surrogate-Control" -> cacheControl,
124+
125+
// the cache headers used by the CDN
126+
"Surrogate-Control" -> surrogateCacheControl,
127+
// the cache headers that make their way through to the browser
110128
"Cache-Control" -> cacheControl,
129+
111130
"Expires" -> expiresTime.toHttpDateTimeString,
112131
"Date" -> now.toHttpDateTimeString,
113132
"ETag" -> etagHeaderString)
114-
115-
116133
}
117134
}
118135

‎common/app/model/content.scala‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -389,13 +389,6 @@ object Article {
389389
analyticsName = s"GFE:$section:$contentType:${id.substring(id.lastIndexOf("/") + 1)}",
390390
adUnitSuffix = section + "/" + contentType.toLowerCase,
391391
schemaType = Some(ArticleSchemas(content.tags)),
392-
cacheTime = if (!fields.isLive && LongCacheSwitch.isSwitchedOn) {
393-
if (fields.lastModified > DateTime.now(fields.lastModified.getZone) - 1.hour) CacheTime.RecentlyUpdatedPurgable
394-
else if (fields.lastModified > DateTime.now(fields.lastModified.getZone) - 24.hours) CacheTime.LastDayUpdatedPurgable
395-
else CacheTime.NotRecentlyUpdatedPurgable
396-
} else {
397-
content.metadata.cacheTime
398-
},
399392
iosType = Some("Article"),
400393
javascriptConfigOverrides = javascriptConfig,
401394
opengraphPropertiesOverrides = opengraphProperties,

‎common/test/model/CachedTest.scala‎

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ package model
22

33
import com.gu.contentapi.client.model.v1.{Content => ApiContent, ContentFields}
44
import com.gu.contentapi.client.utils.CapiModelEnrichment.RichJodaDateTime
5-
import conf.switches.Switches
6-
import conf.switches.Switches.DoubleCacheTimesSwitch
5+
import conf.switches.Switches.LongCacheSwitch
76
import model.Cached.{WithoutRevalidationResult, RevalidatableResult}
87
import org.joda.time.DateTime
98
import org.scala_tools.time.Imports._
@@ -13,7 +12,8 @@ import play.api.mvc.Results
1312
class CachedTest extends FlatSpec with Matchers with Results with implicits.Dates {
1413

1514
"Cached" should "cache live content for 5 seconds" in {
16-
Switches.DoubleCacheTimesSwitch.switchOff()
15+
LongCacheSwitch.switchOff()
16+
1717

1818
val modified = new DateTime(2001, 5, 20, 12, 3, 4, 555)
1919
val liveContent = content(lastModified = modified, live = true)
@@ -26,22 +26,22 @@ class CachedTest extends FlatSpec with Matchers with Results with implicits.Date
2626
headers("Cache-Control") should be("max-age=5, stale-while-revalidate=1, stale-if-error=864000")
2727
}
2828

29-
it should "cache content less than 1 hour old for 10 seconds" in {
30-
Switches.DoubleCacheTimesSwitch.switchOff()
29+
it should "cache content less than 1 hour old for 60 seconds" in {
30+
LongCacheSwitch.switchOff()
3131

3232
val modifiedAlmost1HourAgo = DateTime.now - 58.minutes
3333
val liveContent = content(lastModified = modifiedAlmost1HourAgo, live = false)
3434

35-
liveContent.metadata.cacheTime.cacheSeconds should be(10)
35+
liveContent.metadata.cacheTime.cacheSeconds should be(60)
3636

37-
val result = Cached(10, WithoutRevalidationResult(Ok("foo")), None)
37+
val result = Cached(60, WithoutRevalidationResult(Ok("foo")), None)
3838
val headers = result.header.headers
3939

40-
headers("Cache-Control") should be("max-age=10, stale-while-revalidate=1, stale-if-error=864000")
40+
headers("Cache-Control") should be("max-age=60, stale-while-revalidate=6, stale-if-error=864000")
4141
}
4242

4343
it should "cache older content for 5 minutes" in {
44-
Switches.DoubleCacheTimesSwitch.switchOff()
44+
LongCacheSwitch.switchOff()
4545

4646
val modifiedLongAgo = DateTime.now - 25.hours
4747
val liveContent = content(lastModified = modifiedLongAgo, live = false)
@@ -55,7 +55,7 @@ class CachedTest extends FlatSpec with Matchers with Results with implicits.Date
5555
}
5656

5757
it should "cache other things for 1 minute" in {
58-
Switches.DoubleCacheTimesSwitch.switchOff()
58+
LongCacheSwitch.switchOff()
5959

6060
val page = SimplePage(MetaData.make(
6161
id = "",
@@ -71,18 +71,8 @@ class CachedTest extends FlatSpec with Matchers with Results with implicits.Date
7171
headers("Cache-Control") should be("max-age=60, stale-while-revalidate=6, stale-if-error=864000")
7272
}
7373

74-
it should "double the cache time if DoubleCacheTimesSwitch is switched on" in {
75-
76-
DoubleCacheTimesSwitch.switchOn()
77-
78-
val result = Cached(10, WithoutRevalidationResult(Ok("foo")), None)
79-
val headers = result.header.headers
80-
81-
headers("Cache-Control") should be("max-age=20, stale-while-revalidate=2, stale-if-error=864000")
82-
}
83-
8474
it should "have at least 1 second stale-while-revalidate" in {
85-
DoubleCacheTimesSwitch.switchOff()
75+
LongCacheSwitch.switchOff()
8676

8777
val result = Cached(5, WithoutRevalidationResult(Ok("foo")), None)
8878
val headers = result.header.headers
@@ -91,7 +81,7 @@ class CachedTest extends FlatSpec with Matchers with Results with implicits.Date
9181
}
9282

9383
it should "set Surrogate-Control the same as Cache-Control" in {
94-
Switches.DoubleCacheTimesSwitch.switchOff()
84+
LongCacheSwitch.switchOff()
9585

9686
val result = Cached(60, WithoutRevalidationResult(Ok("foo")), None)
9787
val headers = result.header.headers
@@ -100,8 +90,35 @@ class CachedTest extends FlatSpec with Matchers with Results with implicits.Date
10090
headers("Cache-Control") should equal (headers("Surrogate-Control"))
10191
}
10292

103-
it should "etag should be added" in {
104-
DoubleCacheTimesSwitch.switchOff()
93+
"Longer cache control" should "be applied to SurrogateControl if enabled" in {
94+
LongCacheSwitch.switchOn()
95+
96+
val modifiedLongAgo = DateTime.now - 25.hours
97+
val liveContent = content(lastModified = modifiedLongAgo, live = false)
98+
99+
liveContent.metadata.cacheTime.cacheSeconds should be(3800)
100+
101+
val result = Cached(3800, WithoutRevalidationResult(Ok("foo")), None)
102+
val headers = result.header.headers
103+
104+
headers("Surrogate-Control") should be("max-age=3800, stale-while-revalidate=380, stale-if-error=864000")
105+
}
106+
107+
it should "limit the max-age to 60" in {
108+
LongCacheSwitch.switchOn()
109+
110+
val modifiedLongAgo = DateTime.now - 25.hours
111+
val liveContent = content(lastModified = modifiedLongAgo, live = false)
112+
113+
liveContent.metadata.cacheTime.cacheSeconds should be(3800)
114+
115+
val result = Cached(3800, WithoutRevalidationResult(Ok("foo")), None)
116+
val headers = result.header.headers
117+
118+
headers("Cache-Control") should be("max-age=60, stale-while-revalidate=6, stale-if-error=864000")
119+
}
120+
121+
"ETags" should "should be added" in {
105122

106123
val result = Cached(5, RevalidatableResult(Ok("foo"), "A"), None)
107124
val headers = result.header.headers
@@ -112,7 +129,7 @@ class CachedTest extends FlatSpec with Matchers with Results with implicits.Date
112129
}
113130

114131
it should "wrong etag should be ignored" in {
115-
DoubleCacheTimesSwitch.switchOff()
132+
LongCacheSwitch.switchOff()
116133

117134
val result = Cached(5, RevalidatableResult(Ok("foo"), "A"), Some("""W/"hasheroo""""))
118135
val headers = result.header.headers
@@ -123,7 +140,7 @@ class CachedTest extends FlatSpec with Matchers with Results with implicits.Date
123140
}
124141

125142
it should "correct etag should not be ignored" in {
126-
DoubleCacheTimesSwitch.switchOff()
143+
LongCacheSwitch.switchOff()
127144

128145
val result = Cached(5, RevalidatableResult(Ok("foo"), "A"), Some("""W/"hash96""""))
129146
val headers = result.header.headers

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL