From b91ce252d94a8876097b939e129dc33264cef2f5 Mon Sep 17 00:00:00 2001 From: =?utf8?q?David=20=E2=80=98Bombe=E2=80=99=20Roden?= Date: Sun, 13 Nov 2016 11:06:10 +0100 Subject: [PATCH] Add failure cache to element loader --- .../sone/core/DefaultElementLoader.kt | 18 +++++++++------ .../net/pterodactylus/sone/core/ElementLoader.kt | 9 +------- .../sone/template/LinkedElementRenderFilter.kt | 7 +++--- .../sone/template/LinkedElementsFilter.kt | 1 + .../sone/core/DefaultElementLoaderTest.kt | 26 ++++++++++++++++------ .../sone/template/LinkedElementRenderFilterTest.kt | 4 ++-- .../sone/template/LinkedElementsFilterTest.kt | 11 ++++----- 7 files changed, 43 insertions(+), 33 deletions(-) diff --git a/src/main/kotlin/net/pterodactylus/sone/core/DefaultElementLoader.kt b/src/main/kotlin/net/pterodactylus/sone/core/DefaultElementLoader.kt index 956d73b..98a2caa 100644 --- a/src/main/kotlin/net/pterodactylus/sone/core/DefaultElementLoader.kt +++ b/src/main/kotlin/net/pterodactylus/sone/core/DefaultElementLoader.kt @@ -1,18 +1,21 @@ package net.pterodactylus.sone.core +import com.google.common.base.Ticker import com.google.common.cache.CacheBuilder import freenet.keys.FreenetURI import java.io.ByteArrayInputStream +import java.util.concurrent.TimeUnit.MINUTES import javax.imageio.ImageIO import javax.inject.Inject /** * [ElementLoader] implementation that uses a simple Guava [com.google.common.cache.Cache]. */ -class DefaultElementLoader @Inject constructor(private val freenetInterface: FreenetInterface) : ElementLoader { +class DefaultElementLoader @Inject constructor(private val freenetInterface: FreenetInterface, ticker: Ticker = Ticker.systemTicker()) : ElementLoader { private val loadingLinks = CacheBuilder.newBuilder().build() - private val imageCache = CacheBuilder.newBuilder().build() + private val failureCache = CacheBuilder.newBuilder().ticker(ticker).expireAfterWrite(30, MINUTES).build() + private val imageCache = CacheBuilder.newBuilder().build() private val callback = object : FreenetInterface.BackgroundFetchCallback { override fun cancelForMimeType(uri: FreenetURI, mimeType: String): Boolean { return !mimeType.startsWith("image/") @@ -25,12 +28,13 @@ class DefaultElementLoader @Inject constructor(private val freenetInterface: Fre ByteArrayInputStream(data).use { ImageIO.read(it) }?.let { - imageCache.get(uri.toString()) { LinkedImage(uri.toString()) } + imageCache.get(uri.toString()) { LinkedElement(uri.toString()) } } removeLoadingLink(uri) } override fun failed(uri: FreenetURI) { + failureCache.put(uri.toString(), true) removeLoadingLink(uri) } @@ -46,15 +50,15 @@ class DefaultElementLoader @Inject constructor(private val freenetInterface: Fre imageCache.getIfPresent(link)?.run { return this } + failureCache.getIfPresent(link)?.run { + return LinkedElement(link, failed = true) + } if (loadingLinks.getIfPresent(link) == null) { loadingLinks.put(link, true) freenetInterface.startFetch(FreenetURI(link), callback) } } - return object : LinkedElement { - override val link = link - override val loading = true - } + return LinkedElement(link, loading = true) } } diff --git a/src/main/kotlin/net/pterodactylus/sone/core/ElementLoader.kt b/src/main/kotlin/net/pterodactylus/sone/core/ElementLoader.kt index 2424cf1..95178ed 100644 --- a/src/main/kotlin/net/pterodactylus/sone/core/ElementLoader.kt +++ b/src/main/kotlin/net/pterodactylus/sone/core/ElementLoader.kt @@ -12,11 +12,4 @@ interface ElementLoader { } -interface LinkedElement { - - val link: String - val loading: Boolean - -} - -data class LinkedImage(override val link: String, override val loading: Boolean = false) : LinkedElement +data class LinkedElement(val link: String, val failed: Boolean = false, val loading: Boolean = false) diff --git a/src/main/kotlin/net/pterodactylus/sone/template/LinkedElementRenderFilter.kt b/src/main/kotlin/net/pterodactylus/sone/template/LinkedElementRenderFilter.kt index 6255f9d..6644ad8 100644 --- a/src/main/kotlin/net/pterodactylus/sone/template/LinkedElementRenderFilter.kt +++ b/src/main/kotlin/net/pterodactylus/sone/template/LinkedElementRenderFilter.kt @@ -1,7 +1,6 @@ package net.pterodactylus.sone.template import net.pterodactylus.sone.core.LinkedElement -import net.pterodactylus.sone.core.LinkedImage import net.pterodactylus.util.template.Filter import net.pterodactylus.util.template.TemplateContext import net.pterodactylus.util.template.TemplateContextFactory @@ -24,14 +23,14 @@ class LinkedElementRenderFilter(private val templateContextFactory: TemplateCont override fun format(templateContext: TemplateContext?, data: Any?, parameters: Map?) = when { data is LinkedElement && data.loading -> renderNotLoadedLinkedElement(data) - data is LinkedImage -> renderLinkedImage(data) + data is LinkedElement -> renderLinkedImage(data) else -> null } - private fun renderLinkedImage(linkedImage: LinkedImage) = + private fun renderLinkedImage(linkedElement: LinkedElement) = StringWriter().use { val templateContext = templateContextFactory.createTemplateContext() - templateContext["link"] = linkedImage.link + templateContext["link"] = linkedElement.link loadedImageTemplate.render(templateContext, it) it }.toString() diff --git a/src/main/kotlin/net/pterodactylus/sone/template/LinkedElementsFilter.kt b/src/main/kotlin/net/pterodactylus/sone/template/LinkedElementsFilter.kt index f95d6c0..ca94423 100644 --- a/src/main/kotlin/net/pterodactylus/sone/template/LinkedElementsFilter.kt +++ b/src/main/kotlin/net/pterodactylus/sone/template/LinkedElementsFilter.kt @@ -18,6 +18,7 @@ class LinkedElementsFilter(private val elementLoader: ElementLoader) : Filter { (data as? Iterable) ?.filterIsInstance() ?.map { elementLoader.loadElement(it.link) } + ?.filter { !it.failed } ?: listOf() } diff --git a/src/test/kotlin/net/pterodactylus/sone/core/DefaultElementLoaderTest.kt b/src/test/kotlin/net/pterodactylus/sone/core/DefaultElementLoaderTest.kt index 720cd67..5f8cb88 100644 --- a/src/test/kotlin/net/pterodactylus/sone/core/DefaultElementLoaderTest.kt +++ b/src/test/kotlin/net/pterodactylus/sone/core/DefaultElementLoaderTest.kt @@ -1,5 +1,6 @@ package net.pterodactylus.sone.core +import com.google.common.base.Ticker import com.google.common.io.ByteStreams import freenet.keys.FreenetURI import net.pterodactylus.sone.core.FreenetInterface.BackgroundFetchCallback @@ -7,13 +8,14 @@ import net.pterodactylus.sone.test.capture import net.pterodactylus.sone.test.mock import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.`is` -import org.hamcrest.Matchers.instanceOf import org.junit.Test import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.eq +import org.mockito.Mockito.`when` import org.mockito.Mockito.times import org.mockito.Mockito.verify import java.io.ByteArrayOutputStream +import java.util.concurrent.TimeUnit /** * Unit test for [DefaultElementLoaderTest]. @@ -26,10 +28,10 @@ class DefaultElementLoaderTest { } private val freenetInterface = mock() - private val elementLoader = DefaultElementLoader(freenetInterface) + private val ticker = mock() + private val elementLoader = DefaultElementLoader(freenetInterface, ticker) private val callback = capture() - @Test fun `image loader starts request for link that is not known`() { elementLoader.loadElement(IMAGE_ID) @@ -82,17 +84,27 @@ class DefaultElementLoaderTest { verify(freenetInterface).startFetch(eq(freenetURI), callback.capture()) callback.value.loaded(freenetURI, "image/png", read("/static/images/unknown-image-0.png")) val linkedElement = elementLoader.loadElement(IMAGE_ID) - assertThat(linkedElement.link, `is`(IMAGE_ID)) - assertThat(linkedElement.loading, `is`(false)) - assertThat(linkedElement, instanceOf(LinkedImage::class.java)) + assertThat(linkedElement, `is`(LinkedElement(IMAGE_ID))) } @Test - fun `image can be loaded again after it failed`() { + fun `image is not loaded again after it failed`() { elementLoader.loadElement(IMAGE_ID) verify(freenetInterface).startFetch(eq(freenetURI), callback.capture()) callback.value.failed(freenetURI) + assertThat(elementLoader.loadElement(IMAGE_ID).failed, `is`(true)) + verify(freenetInterface).startFetch(eq(freenetURI), callback.capture()) + } + + @Test + fun `image is loaded again after failure cache is expired`() { elementLoader.loadElement(IMAGE_ID) + verify(freenetInterface).startFetch(eq(freenetURI), callback.capture()) + callback.value.failed(freenetURI) + `when`(ticker.read()).thenReturn(TimeUnit.MINUTES.toNanos(31)) + val linkedElement = elementLoader.loadElement(IMAGE_ID) + assertThat(linkedElement.failed, `is`(false)) + assertThat(linkedElement.loading, `is`(true)) verify(freenetInterface, times(2)).startFetch(eq(freenetURI), callback.capture()) } diff --git a/src/test/kotlin/net/pterodactylus/sone/template/LinkedElementRenderFilterTest.kt b/src/test/kotlin/net/pterodactylus/sone/template/LinkedElementRenderFilterTest.kt index 20ee99a..07ef178 100644 --- a/src/test/kotlin/net/pterodactylus/sone/template/LinkedElementRenderFilterTest.kt +++ b/src/test/kotlin/net/pterodactylus/sone/template/LinkedElementRenderFilterTest.kt @@ -1,6 +1,6 @@ package net.pterodactylus.sone.template -import net.pterodactylus.sone.core.LinkedImage +import net.pterodactylus.sone.core.LinkedElement import net.pterodactylus.util.template.HtmlFilter import net.pterodactylus.util.template.TemplateContextFactory import org.hamcrest.MatcherAssert.assertThat @@ -23,7 +23,7 @@ class LinkedElementRenderFilterTest { @Test fun `filter can render linked images`() { - val html = filter.format(null, LinkedImage("KSK@gpl.png"), emptyMap()) as String + val html = filter.format(null, LinkedElement("KSK@gpl.png"), emptyMap()) as String val linkNode = Jsoup.parseBodyFragment(html).body().child(0) assertThat(linkNode.nodeName(), `is`("a")) assertThat(linkNode.attr("href"), `is`("/KSK@gpl.png")) diff --git a/src/test/kotlin/net/pterodactylus/sone/template/LinkedElementsFilterTest.kt b/src/test/kotlin/net/pterodactylus/sone/template/LinkedElementsFilterTest.kt index d519e41..1f6f681 100644 --- a/src/test/kotlin/net/pterodactylus/sone/template/LinkedElementsFilterTest.kt +++ b/src/test/kotlin/net/pterodactylus/sone/template/LinkedElementsFilterTest.kt @@ -2,7 +2,6 @@ package net.pterodactylus.sone.template import net.pterodactylus.sone.core.ElementLoader import net.pterodactylus.sone.core.LinkedElement -import net.pterodactylus.sone.core.LinkedImage import net.pterodactylus.sone.test.mock import net.pterodactylus.sone.text.FreenetLinkPart import net.pterodactylus.sone.text.LinkPart @@ -26,14 +25,16 @@ class LinkedElementsFilterTest { PlainTextPart("text"), LinkPart("http://link", "link"), FreenetLinkPart("KSK@link", "link", false), + FreenetLinkPart("KSK@loading.png", "link", false), FreenetLinkPart("KSK@link.png", "link", false) ) - `when`(imageLoader.loadElement("KSK@link")).thenReturn(LinkedImage("KSK@link", true)) - `when`(imageLoader.loadElement("KSK@link.png")).thenReturn(LinkedImage("KSK@link.png")) + `when`(imageLoader.loadElement("KSK@link")).thenReturn(LinkedElement("KSK@link", failed = true)) + `when`(imageLoader.loadElement("KSK@loading.png")).thenReturn(LinkedElement("KSK@loading.png", loading = true)) + `when`(imageLoader.loadElement("KSK@link.png")).thenReturn(LinkedElement("KSK@link.png")) val loadedImages = filter.format(null, parts, null) assertThat(loadedImages, contains( - LinkedImage("KSK@link", true), - LinkedImage("KSK@link.png") + LinkedElement("KSK@loading.png", failed = false, loading = true), + LinkedElement("KSK@link.png", failed = false, loading = false) )) } -- 2.7.4