fix(api): thumbnails not updating properly

incorrect cache control could prevent updated thumbnails to show up
use shallow etags for thumbnails
This commit is contained in:
Gauthier Roebroeck 2020-04-06 15:19:08 +08:00
parent 54d269bea7
commit a5bd9087df
4 changed files with 143 additions and 20 deletions

View File

@ -0,0 +1,21 @@
package org.gotson.komga.infrastructure.web
import org.springframework.boot.web.servlet.FilterRegistrationBean
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.web.filter.ShallowEtagHeaderFilter
@Configuration
class EtagFilterConfiguration {
@Bean
fun shallowEtagHeaderFilter(): FilterRegistrationBean<ShallowEtagHeaderFilter> =
FilterRegistrationBean(ShallowEtagHeaderFilter())
.also {
it.addUrlPatterns(
"/api/*",
"/opds/*"
)
it.setName("etagFilter")
}
}

View File

@ -186,19 +186,10 @@ class BookController(
@PathVariable bookId: Long
): ResponseEntity<ByteArray> =
bookRepository.findByIdOrNull(bookId)?.let { book ->
val etag = book.id.toString()
if (request.checkNotModified(etag, getBookLastModified(book))) {
return@let ResponseEntity
.status(HttpStatus.NOT_MODIFIED)
.eTag(etag)
.setNotModified(book)
.body(ByteArray(0))
}
if (!principal.user.canAccessBook(book)) throw ResponseStatusException(HttpStatus.UNAUTHORIZED)
if (book.media.thumbnail != null) {
ResponseEntity.ok()
.eTag(etag)
.setNotModified(book)
.setCachePrivate()
.body(book.media.thumbnail)
} else throw ResponseStatusException(HttpStatus.NOT_FOUND)
} ?: throw ResponseStatusException(HttpStatus.NOT_FOUND)
@ -392,11 +383,14 @@ class BookController(
bookRepository.save(book).toDto(includeFullUrl = true)
} ?: throw ResponseStatusException(HttpStatus.NOT_FOUND)
private fun ResponseEntity.BodyBuilder.setNotModified(book: Book) =
private fun ResponseEntity.BodyBuilder.setCachePrivate() =
this.cacheControl(CacheControl.maxAge(0, TimeUnit.SECONDS)
.cachePrivate()
.mustRevalidate()
).lastModified(getBookLastModified(book))
)
private fun ResponseEntity.BodyBuilder.setNotModified(book: Book) =
this.setCachePrivate().lastModified(getBookLastModified(book))
private fun getBookLastModified(book: Book) =
book.media.lastModifiedDate!!.toInstant(ZoneOffset.UTC).toEpochMilli()

View File

@ -27,6 +27,7 @@ import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabas
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.data.repository.findByIdOrNull
import org.springframework.http.HttpHeaders
import org.springframework.http.MediaType
import org.springframework.jdbc.core.JdbcTemplate
import org.springframework.test.context.junit.jupiter.SpringExtension
@ -36,9 +37,8 @@ import org.springframework.test.web.servlet.get
import org.springframework.test.web.servlet.patch
import org.springframework.transaction.annotation.Transactional
import java.time.LocalDate
import java.time.LocalDateTime
import java.time.ZoneOffset
import javax.sql.DataSource
import kotlin.random.Random
@ExtendWith(SpringExtension::class)
@SpringBootTest
@ -327,16 +327,23 @@ class BookControllerTest(
inner class HttpCache {
@Test
@WithMockCustomUser
fun `given request with If-Modified-Since headers when getting thumbnail then returns 304 not modified`() {
fun `given request with cache headers when getting thumbnail then returns 304 not modified`() {
val series = makeSeries(
name = "series",
books = listOf(makeBook("1.cbr"))
books = listOf(makeBook("1.cbr").also {
it.media.thumbnail = Random.nextBytes(100)
})
).also { it.library = library }
seriesRepository.save(series)
mockMvc.get("/api/v1/books/${series.books.first().id}/thumbnail") {
val url = "/api/v1/books/${series.books.first().id}/thumbnail"
val response = mockMvc.get(url)
.andReturn().response
mockMvc.get(url) {
headers {
ifModifiedSince = LocalDateTime.now().toInstant(ZoneOffset.UTC).toEpochMilli()
ifNoneMatch = listOf(response.getHeader(HttpHeaders.ETAG)!!)
}
}.andExpect {
status { isNotModified }
@ -352,9 +359,14 @@ class BookControllerTest(
).also { it.library = library }
seriesRepository.save(series)
mockMvc.get("/api/v1/books/${series.books.first().id}/pages/1") {
val url = "/api/v1/books/${series.books.first().id}/pages/1"
val lastModified = mockMvc.get(url)
.andReturn().response.getHeader(HttpHeaders.LAST_MODIFIED)
mockMvc.get(url) {
headers {
ifModifiedSince = LocalDateTime.now().toInstant(ZoneOffset.UTC).toEpochMilli()
set(HttpHeaders.IF_MODIFIED_SINCE, lastModified!!)
}
}.andExpect {
status { isNotModified }
@ -362,6 +374,38 @@ class BookControllerTest(
}
}
//Not part of the above @Nested class because @Transactional fails
@Test
@WithMockCustomUser
@Transactional
fun `given request with cache headers and modified resource when getting thumbnail then returns 200 ok`() {
val book = makeBook("1.cbr").also {
it.media.thumbnail = Random.nextBytes(1)
}
val series = makeSeries(
name = "series",
books = listOf(book)
).also { it.library = library }
seriesRepository.save(series)
val url = "/api/v1/books/${series.books.first().id}/thumbnail"
val response = mockMvc.get(url)
.andReturn().response
Thread.sleep(100)
book.media.thumbnail = Random.nextBytes(1)
bookRepository.saveAndFlush(book)
mockMvc.get(url) {
headers {
ifNoneMatch = listOf(response.getHeader(HttpHeaders.ETAG)!!)
}
}.andExpect {
status { isOk }
}
}
@Nested
inner class MetadataUpdate {
@Test

View File

@ -7,6 +7,7 @@ import org.gotson.komga.domain.model.UserRoles
import org.gotson.komga.domain.model.makeBook
import org.gotson.komga.domain.model.makeLibrary
import org.gotson.komga.domain.model.makeSeries
import org.gotson.komga.domain.persistence.BookRepository
import org.gotson.komga.domain.persistence.LibraryRepository
import org.gotson.komga.domain.persistence.SeriesRepository
import org.hamcrest.Matchers
@ -23,6 +24,7 @@ import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabas
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.data.repository.findByIdOrNull
import org.springframework.http.HttpHeaders
import org.springframework.http.MediaType
import org.springframework.jdbc.core.JdbcTemplate
import org.springframework.test.context.junit.jupiter.SpringExtension
@ -32,6 +34,7 @@ import org.springframework.test.web.servlet.get
import org.springframework.test.web.servlet.patch
import org.springframework.transaction.annotation.Transactional
import javax.sql.DataSource
import kotlin.random.Random
@ExtendWith(SpringExtension::class)
@SpringBootTest
@ -40,6 +43,7 @@ import javax.sql.DataSource
class SeriesControllerTest(
@Autowired private val seriesRepository: SeriesRepository,
@Autowired private val libraryRepository: LibraryRepository,
@Autowired private val bookRepository: BookRepository,
@Autowired private val mockMvc: MockMvc
) {
@ -404,4 +408,64 @@ class SeriesControllerTest(
}
}
@Test
@WithMockCustomUser
fun `given request with cache headers when getting series thumbnail then returns 304 not modified`() {
val book = makeBook("1.cbr").also {
it.media.thumbnail = Random.nextBytes(1)
}
val series = makeSeries(
name = "series",
books = listOf(book)
).also { it.library = library }
seriesRepository.save(series)
val url = "/api/v1/series/${series.id}/thumbnail"
val response = mockMvc.get(url)
.andReturn().response
mockMvc.get(url) {
headers {
ifNoneMatch = listOf(response.getHeader(HttpHeaders.ETAG)!!)
}
}.andExpect {
status { isNotModified }
}
}
//Not part of the above @Nested class because @Transactional fails
@Test
@WithMockCustomUser
@Transactional
fun `given request with cache headers and modified first book when getting series thumbnail then returns 200 ok`() {
val book = makeBook("1.cbr").also {
it.media.thumbnail = Random.nextBytes(1)
}
val book2 = makeBook("2.cbr").also {
it.media.thumbnail = Random.nextBytes(1)
}
val series = makeSeries(
name = "series",
books = listOf(book, book2)
).also { it.library = library }
seriesRepository.save(series)
val url = "/api/v1/series/${series.id}/thumbnail"
val response = mockMvc.get(url)
.andReturn().response
book.metadata.numberSort = 3F
bookRepository.saveAndFlush(book)
mockMvc.get(url) {
headers {
ifNoneMatch = listOf(response.getHeader(HttpHeaders.ETAG)!!)
}
}.andExpect {
status { isOk }
}
}
}