Skip to content

Commit e7d7292

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 e7d7292

File tree

4 files changed

+179
-29
lines changed

4 files changed

+179
-29
lines changed

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,33 @@ 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+
- a:getDebugName():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+
- getDebugName():java.lang.String
372+
- resolve(java.lang.String):java.lang.String
373+
*f:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability$RelativePath
374+
- org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability
375+
- sf:$stable:I
376+
- <init>(java.nio.file.Path):V
377+
- getDebugName():java.lang.String
378+
- resolve(java.lang.String):java.lang.String
379+
*f:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability$RelativePathInResources
380+
- org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability
381+
- sf:$stable:I
382+
- sf:INSTANCE:org.jetbrains.jewel.markdown.rendering.ImageSourceResolver$ResolveCapability$RelativePathInResources
383+
- getDebugName():java.lang.String
384+
- resolve(java.lang.String):java.lang.String
359385
f:org.jetbrains.jewel.markdown.rendering.ImageSourceResolverKt
360386
- *sf:getLocalMarkdownImageSourceResolver():androidx.compose.runtime.ProvidableCompositionLocal
361387
*: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 = setOf(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+
buildSet {
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: Set<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+
/** Resolves a raw image destination string from a Markdown file into a fully-qualified, loadable path. */
75+
public fun resolve(rawDestination: String): String?
76+
77+
/** Represents the ability to resolve plain URIs as-is. */
78+
@ApiStatus.Experimental
79+
@ExperimentalJewelApi
80+
public object PlainUri : ResolveCapability {
81+
override fun toString(): String = "PlainUri"
82+
83+
override fun resolve(rawDestination: String): String? {
84+
val uri = runCatching { URI.create(rawDestination) }.getOrNull() ?: return null
85+
return if (uri.isAbsolute) rawDestination else null
86+
}
87+
}
88+
89+
/** Represents the ability to resolve relative paths in the current classloader's resources. */
90+
@ApiStatus.Experimental
91+
@ExperimentalJewelApi
92+
public object RelativePathInResources : ResolveCapability {
93+
override fun toString(): String = "RelativePathInResources"
94+
95+
override fun resolve(rawDestination: String): String? =
96+
javaClass.classLoader.getResource(rawDestination.removePrefix("/"))?.toExternalForm()
97+
}
98+
99+
/** Represents the ability to resolve relative paths relative to a given root directory [rootDir]. */
100+
@ApiStatus.Experimental
101+
@ExperimentalJewelApi
102+
public class RelativePath(private val rootDir: Path) : ResolveCapability {
103+
override fun resolve(rawDestination: String): String? {
104+
val rawPath = Path.of(rawDestination)
105+
// don't resolve absolute paths, it's not this resolver's capability
106+
if (rawPath.isAbsolute) return null
107+
108+
val normalizedRoot = runCatching { rootDir.toAbsolutePath().normalize() }.getOrNull() ?: return null
109+
110+
val resolved = runCatching { normalizedRoot.resolve(rawPath).normalize() }.getOrNull() ?: return null
111+
112+
return resolved.toString()
113+
}
114+
115+
override fun toString(): String = "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: Set<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: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,36 @@
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 full file URI as raw destination`() {
21+
val fullUri = "file:///image.png"
22+
23+
val result = DefaultImageSourceResolver().resolve(fullUri)
24+
25+
TestCase.assertEquals(fullUri, result)
26+
}
27+
28+
@Test
29+
public fun `resolves existing classpath resource`() {
1930
// This test requires a file named `test-image.svg` to exist in `src/test/resources/`.
2031
val resourceName = "test-image.svg"
2132

22-
val result = DefaultImageSourceResolver.resolve(resourceName)
33+
val result = DefaultImageSourceResolver().resolve(resourceName)
34+
result.assertNotNull()
2335

2436
assert(result.startsWith("file:/") || result.startsWith("jar:file:/")) {
2537
"Expected result to start with 'file:/' or 'jar:file:/', but got '$result'"
@@ -28,11 +40,12 @@ public class DefaultImageSourceResolverTest {
2840
}
2941

3042
@Test
31-
public fun `resolveImageSource with slash prefix resolves existing classpath resource`() {
43+
public fun `resolves existing classpath resource with slash prefix`() {
3244
// This test requires a file named `test-image.svg` to exist in `src/test/resources/`.
3345
val resourceName = "/test-image.svg"
3446

35-
val result = DefaultImageSourceResolver.resolve(resourceName)
47+
val result = DefaultImageSourceResolver().resolve(resourceName)
48+
result.assertNotNull()
3649

3750
assert(result.startsWith("file:/") || result.startsWith("jar:file:/")) {
3851
"Expected result to start with 'file:/' or 'jar:file:/', but got '$result'"
@@ -41,12 +54,32 @@ public class DefaultImageSourceResolverTest {
4154
}
4255

4356
@Test
44-
public fun `resolveImageSource returns raw destination for non-existent resource`() {
57+
public fun `doesn't resolve non-existent resource`() {
4558
val nonExistentResource = "this_file_does_not_exist.jpg"
4659

47-
val result = DefaultImageSourceResolver.resolve(nonExistentResource)
60+
val result = DefaultImageSourceResolver().resolve(nonExistentResource)
61+
assertNull(result)
62+
}
63+
64+
@Test
65+
public fun `doesn't resolve URIs without capability`() {
66+
val fullUri = "https://example.com/image.png"
67+
68+
val result =
69+
DefaultImageSourceResolver(
70+
resolveCapabilities = setOf(ImageSourceResolver.ResolveCapability.RelativePathInResources)
71+
)
72+
.resolve(fullUri)
73+
assertNull(result)
74+
}
75+
76+
@Test
77+
public fun `doesn't resolve existing classpath resource without capability`() {
78+
val resourceName = "/test-image.svg"
4879

49-
// It should return the original string, which will later cause Coil to fail.
50-
TestCase.assertEquals(nonExistentResource, result)
80+
val result =
81+
DefaultImageSourceResolver(resolveCapabilities = setOf(ImageSourceResolver.ResolveCapability.PlainUri))
82+
.resolve(resourceName)
83+
assertNull(result)
5184
}
5285
}

0 commit comments

Comments
 (0)