Add failure cache to element loader

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 (file)
--- 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<String, Boolean>()
-       private val imageCache = CacheBuilder.newBuilder().build<String, LinkedImage>()
+       private val failureCache = CacheBuilder.newBuilder().ticker(ticker).expireAfterWrite(30, MINUTES).build<String, Boolean>()
+       private val imageCache = CacheBuilder.newBuilder().build<String, LinkedElement>()
        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 (file)
--- 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 (file)
--- 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<String, Any?>?) =
                        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 (file)
--- 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<Part>)
                                        ?.filterIsInstance<FreenetLinkPart>()
                                        ?.map { elementLoader.loadElement(it.link) }
+                                       ?.filter { !it.failed }
                                        ?: listOf<LinkedElement>()
 
 }

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 (file)
--- 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<FreenetInterface>()
-       private val elementLoader = DefaultElementLoader(freenetInterface)
+       private val ticker = mock<Ticker>()
+       private val elementLoader = DefaultElementLoader(freenetInterface, ticker)
        private val callback = capture<BackgroundFetchCallback>()
 
-
        @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 (file)
--- 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<String, Any?>()) as String
+               val html = filter.format(null, LinkedElement("KSK@gpl.png"), emptyMap<String, Any?>()) 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 (file)
--- 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<LinkedElement>(
-                               LinkedImage("KSK@link", true),
-                               LinkedImage("KSK@link.png")
+                               LinkedElement("KSK@loading.png", failed = false, loading = true),
+                               LinkedElement("KSK@link.png", failed = false, loading = false)
                ))
        }