Skip to content

Commit 13c6fcf

Browse files
AlexVanGogenAlexVanGogen
committed
[JEWEL-1058] Improve DefaultImageSourceResolver
- Support relative paths - Abstract different approaches to resolve a given string - Provide an option to not log resolve failures -- we don't want to pollute logs in editor+preview mode when resolve works on each source file edit - Make `ImageSourceResolver.resolve` return null if a given string cannot be resolved Making Coil try to render something by an obscure string can lead to unexpected behavior (exceptions at least)
1 parent 34da811 commit 13c6fcf

File tree

4 files changed

+168
-29
lines changed

4 files changed

+168
-29
lines changed

‎platform/jewel/markdown/core/api-dump-experimental.txt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,31 @@ f:org.jetbrains.jewel.markdown.processing.ProcessingUtilKt
355355
- render(org.jetbrains.jewel.markdown.MarkdownBlock,Z,kotlin.jvm.functions.Function1,kotlin.jvm.functions.Function0,androidx.compose.ui.Modifier,androidx.compose.runtime.Composer,I):V
356356
- renderThematicBreak(org.jetbrains.jewel.markdown.rendering.MarkdownStyling$ThematicBreak,Z,androidx.compose.ui.Modifier,androidx.compose.runtime.Composer,I):V
357357
*:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver
358+
- *sf:Companion:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$Companion
358359
- a:resolve(java.lang.String):java.lang.String
360+
*f:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$Companion
361+
- f:create(java.nio.file.Path,Z):org.jetbrains.jewel.markdown.rendering.ImageSourceResolver
362+
- f:create(java.util.List,Z):org.jetbrains.jewel.markdown.rendering.ImageSourceResolver
363+
- bs:create$default(org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$Companion,java.util.List,Z,I,java.lang.Object):org.jetbrains.jewel.markdown.rendering.ImageSourceResolver
364+
*:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability
365+
- getDisplayName():java.lang.String
366+
- a:resolve(java.lang.String):java.lang.String
367+
*f:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability$PlainUri
368+
- org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability
369+
- sf:$stable:I
370+
- sf:INSTANCE:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability$PlainUri
371+
- resolve(java.lang.String):java.lang.String
372+
*f:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability$RelativePath
373+
- org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability
374+
- sf:$stable:I
375+
- <init>(java.nio.file.Path):V
376+
- getDisplayName():java.lang.String
377+
- resolve(java.lang.String):java.lang.String
378+
*f:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability$RelativePathInResources
379+
- org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability
380+
- sf:$stable:I
381+
- sf:INSTANCE:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability$RelativePathInResources
382+
- resolve(java.lang.String):java.lang.String
359383
f:org.jetbrains.jewel.markdown.rendering.ImageSourceResolverKt
360384
- *sf:getLocalMarkdownImageSourceResolver():androidx.compose.runtime.ProvidableCompositionLocal
361385
*:org.jetbrains.jewel.markdown.rendering.InlineMarkdownRenderer

‎platform/jewel/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/ImageSourceResolver.kt‎

Lines changed: 102 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ package org.jetbrains.jewel.markdown.rendering
44
import androidx.compose.runtime.ProvidableCompositionLocal
55
import androidx.compose.runtime.staticCompositionLocalOf
66
import java.net.URI
7+
import java.nio.file.Path
78
import org.jetbrains.annotations.ApiStatus
9+
import org.jetbrains.annotations.VisibleForTesting
810
import org.jetbrains.jewel.foundation.ExperimentalJewelApi
911
import org.jetbrains.jewel.foundation.util.myLogger
1012

@@ -27,35 +29,116 @@ public interface ImageSourceResolver {
2729
*
2830
* @param rawDestination The raw destination string from the Markdown, e.g., "my-image.png" or
2931
* "https://example.com/image.png".
30-
* @return A fully-qualified, loadable path to the image, which can be consumed by an image loader.
32+
* @return A fully-qualified, loadable path to the image, which can be consumed by an image loader, or `null` if the
33+
* image could not be resolved.
3134
*/
32-
public fun resolve(rawDestination: String): String
35+
public fun resolve(rawDestination: String): String?
36+
37+
public companion object {
38+
@VisibleForTesting
39+
internal val defaultCapabilities = listOf(ResolveCapability.PlainUri, ResolveCapability.RelativePathInResources)
40+
41+
/**
42+
* Creates [ImageSourceResolver] that can resolve image links in Markdown files if they are either:
43+
* - plain URIs, e.g., `https://example.com/image.png`
44+
* - relative paths in the current classloader's resources, e.g., `/images/my-image.png`
45+
* - relative paths relative to a given root directory [rootDir], e.g., `../images/my-image.png`
46+
*
47+
* If [logResolveFailure] is true, logs any failures to resolve image sources.
48+
*/
49+
public fun create(rootDir: Path, logResolveFailure: Boolean): ImageSourceResolver =
50+
create(
51+
buildList {
52+
addAll(defaultCapabilities)
53+
add(ResolveCapability.RelativePath(rootDir))
54+
},
55+
logResolveFailure,
56+
)
57+
58+
/**
59+
* Creates [ImageSourceResolver] that can resolve image links in Markdown files according to provided
60+
* [resolveCapabilities].
61+
*
62+
* If [logResolveFailure] is true, logs any failures to resolve image sources.
63+
*/
64+
public fun create(
65+
resolveCapabilities: List<ResolveCapability> = defaultCapabilities,
66+
logResolveFailure: Boolean = true,
67+
): ImageSourceResolver = DefaultImageSourceResolver(resolveCapabilities)
68+
}
69+
70+
/** Provides a list of capabilities that the default [ImageSourceResolver] implementation supports. */
71+
@ApiStatus.Experimental
72+
@ExperimentalJewelApi
73+
public sealed interface ResolveCapability {
74+
public val displayName: String
75+
get() = this::class.simpleName!!
76+
77+
/** Resolves a raw image destination string from a Markdown file into a fully-qualified, loadable path. */
78+
public fun resolve(rawDestination: String): String?
79+
80+
/** Represents the ability to resolve plain URIs as-is. */
81+
@ApiStatus.Experimental
82+
@ExperimentalJewelApi
83+
public object PlainUri : ResolveCapability {
84+
override fun resolve(rawDestination: String): String? {
85+
val uri = runCatching { URI.create(rawDestination) }.getOrNull() ?: return null
86+
return if (uri.isAbsolute) rawDestination else null
87+
}
88+
}
89+
90+
/** Represents the ability to resolve relative paths in the current classloader's resources. */
91+
@ApiStatus.Experimental
92+
@ExperimentalJewelApi
93+
public object RelativePathInResources : ResolveCapability {
94+
override fun resolve(rawDestination: String): String? =
95+
javaClass.classLoader.getResource(rawDestination.removePrefix("/"))?.toExternalForm()
96+
}
97+
98+
/** Represents the ability to resolve relative paths relative to a given root directory [rootDir]. */
99+
@ApiStatus.Experimental
100+
@ExperimentalJewelApi
101+
public class RelativePath(private val rootDir: Path) : ResolveCapability {
102+
override fun resolve(rawDestination: String): String? {
103+
// don't resolve absolute paths, it's not this resolver's capability
104+
if (rawDestination.startsWith("/")) return null
105+
106+
val normalizedRoot = runCatching { rootDir.toAbsolutePath().normalize() }.getOrNull() ?: return null
107+
108+
val resolved =
109+
runCatching { normalizedRoot.resolve(rawDestination).normalize() }.getOrNull() ?: return null
110+
111+
return resolved.toString()
112+
}
113+
114+
override val displayName: String
115+
get() = "RelativePath(rootDir=$rootDir)"
116+
}
117+
}
33118
}
34119

35120
/**
36-
* The default implementation of [ImageSourceResolver].
37-
*
38-
* Resolves full URIs as-is and attempts to find relative paths in the current classloader's resources.
121+
* The default implementation of [ImageSourceResolver] that can resolve image links in Markdown files according to
122+
* provided [resolveCapabilities].
39123
*
124+
* @param resolveCapabilities A list of [ImageSourceResolver.ResolveCapability]s that this resolver can support.
125+
* @param logResolveFailure Whether to log any failures to resolve image sources.
40126
* @see ImageSourceResolver
41127
*/
42-
internal object DefaultImageSourceResolver : ImageSourceResolver {
43-
override fun resolve(rawDestination: String): String {
44-
val uri = URI.create(rawDestination)
45-
if (uri.scheme != null) return rawDestination
46-
47-
val resourceUrl = javaClass.classLoader.getResource(rawDestination.removePrefix("/"))
48-
49-
if (resourceUrl == null) {
128+
internal class DefaultImageSourceResolver(
129+
private val resolveCapabilities: List<ImageSourceResolver.ResolveCapability> =
130+
ImageSourceResolver.defaultCapabilities,
131+
private val logResolveFailure: Boolean = true,
132+
) : ImageSourceResolver {
133+
override fun resolve(rawDestination: String): String? {
134+
val result = resolveCapabilities.firstNotNullOfOrNull { it.resolve(rawDestination) }
135+
if (result == null && logResolveFailure) {
50136
myLogger()
51137
.warn(
52-
"Markdown image '$rawDestination' expected at classpath '$rawDestination' but not found. " +
53-
"Please ensure it's in your 'src/main/resources/' folder."
138+
"Failed to resolve image source: $rawDestination. Supported capabilities: ${resolveCapabilities.joinToString()}"
54139
)
55-
return rawDestination // This will cause Coil to fail and not render anything.
56140
}
57-
58-
return resourceUrl.toExternalForm()
141+
return result
59142
}
60143
}
61144

@@ -84,5 +167,5 @@ internal object DefaultImageSourceResolver : ImageSourceResolver {
84167
@ExperimentalJewelApi
85168
public val LocalMarkdownImageSourceResolver: ProvidableCompositionLocal<ImageSourceResolver> =
86169
staticCompositionLocalOf {
87-
DefaultImageSourceResolver
170+
ImageSourceResolver.create()
88171
}

‎platform/jewel/markdown/core/src/test/kotlin/org/jetbrains/jewel/markdown/TestUtils.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package org.jetbrains.jewel.markdown
22

3+
import kotlin.contracts.ExperimentalContracts
4+
import kotlin.contracts.contract
35
import org.jetbrains.jewel.foundation.code.MimeType
46
import org.jetbrains.jewel.markdown.MarkdownBlock.BlockQuote
57
import org.jetbrains.jewel.markdown.MarkdownBlock.CodeBlock
@@ -15,6 +17,12 @@ import org.jetbrains.jewel.markdown.MarkdownBlock.Paragraph
1517
import org.jetbrains.jewel.markdown.MarkdownBlock.ThematicBreak
1618
import org.junit.Assert.assertTrue
1719

20+
@OptIn(ExperimentalContracts::class)
21+
public fun <T> T?.assertNotNull() {
22+
contract { returns() implies (this@assertNotNull != null) }
23+
assertTrue("Expected non-null value, but got null", this != null)
24+
}
25+
1826
public fun List<MarkdownBlock>.assertEquals(vararg expected: MarkdownBlock) {
1927
val differences = findDifferences(expected.toList(), indentSize = 0)
2028
assertTrue(

‎platform/jewel/markdown/core/src/test/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultImageSourceResolverTest.kt‎

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,27 @@
22
package org.jetbrains.jewel.markdown.rendering
33

44
import junit.framework.TestCase
5+
import junit.framework.TestCase.assertNull
6+
import org.jetbrains.jewel.markdown.assertNotNull
57
import org.junit.Test
68

79
public class DefaultImageSourceResolverTest {
810
@Test
9-
public fun `resolveImageSource returns raw destination for full URI`() {
11+
public fun `resolves full URI as raw destination`() {
1012
val fullUri = "https://example.com/image.png"
1113

12-
val result = DefaultImageSourceResolver.resolve(fullUri)
14+
val result = DefaultImageSourceResolver().resolve(fullUri)
1315

1416
TestCase.assertEquals(fullUri, result)
1517
}
1618

1719
@Test
18-
public fun `resolveImageSource resolves existing classpath resource`() {
20+
public fun `resolves existing classpath resource`() {
1921
// This test requires a file named `test-image.svg` to exist in `src/test/resources/`.
2022
val resourceName = "test-image.svg"
2123

22-
val result = DefaultImageSourceResolver.resolve(resourceName)
24+
val result = DefaultImageSourceResolver().resolve(resourceName)
25+
result.assertNotNull()
2326

2427
assert(result.startsWith("file:/") || result.startsWith("jar:file:/")) {
2528
"Expected result to start with 'file:/' or 'jar:file:/', but got '$result'"
@@ -28,11 +31,12 @@ public class DefaultImageSourceResolverTest {
2831
}
2932

3033
@Test
31-
public fun `resolveImageSource with slash prefix resolves existing classpath resource`() {
34+
public fun `resolves existing classpath resource with slash prefix`() {
3235
// This test requires a file named `test-image.svg` to exist in `src/test/resources/`.
3336
val resourceName = "/test-image.svg"
3437

35-
val result = DefaultImageSourceResolver.resolve(resourceName)
38+
val result = DefaultImageSourceResolver().resolve(resourceName)
39+
result.assertNotNull()
3640

3741
assert(result.startsWith("file:/") || result.startsWith("jar:file:/")) {
3842
"Expected result to start with 'file:/' or 'jar:file:/', but got '$result'"
@@ -41,12 +45,32 @@ public class DefaultImageSourceResolverTest {
4145
}
4246

4347
@Test
44-
public fun `resolveImageSource returns raw destination for non-existent resource`() {
48+
public fun `doesn't resolve non-existent resource`() {
4549
val nonExistentResource = "this_file_does_not_exist.jpg"
4650

47-
val result = DefaultImageSourceResolver.resolve(nonExistentResource)
51+
val result = DefaultImageSourceResolver().resolve(nonExistentResource)
52+
assertNull(result)
53+
}
54+
55+
@Test
56+
public fun `doesn't resolve URIs without capability`() {
57+
val fullUri = "https://example.com/image.png"
58+
59+
val result =
60+
DefaultImageSourceResolver(
61+
resolveCapabilities = listOf(ImageSourceResolver.ResolveCapability.RelativePathInResources)
62+
)
63+
.resolve(fullUri)
64+
assertNull(result)
65+
}
66+
67+
@Test
68+
public fun `doesn't resolve existing classpath resource without capability`() {
69+
val resourceName = "/test-image.svg"
4870

49-
// It should return the original string, which will later cause Coil to fail.
50-
TestCase.assertEquals(nonExistentResource, result)
71+
val result =
72+
DefaultImageSourceResolver(resolveCapabilities = listOf(ImageSourceResolver.ResolveCapability.PlainUri))
73+
.resolve(resourceName)
74+
assertNull(result)
5175
}
5276
}

0 commit comments

Comments
 (0)