Skip to content

Commit b7d189b

Browse files
committed
[JEWEL-1058] Improve DefaultImageSourceResolver
- Support absolute and 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 b7d189b

File tree

4 files changed

+206
-29
lines changed

4 files changed

+206
-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: 120 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,134 @@ 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 =
40+
setOf(
41+
ResolveCapability.PlainUri,
42+
ResolveCapability.RelativePathInResources(),
43+
ResolveCapability.AbsolutePath,
44+
)
45+
46+
/**
47+
* Creates [ImageSourceResolver] that can resolve image links in Markdown files if they are either:
48+
* - plain URIs, e.g., `https://example.com/image.png` or `file:///image.png`
49+
* - absolute paths, e.g., `/image.png`
50+
* - relative paths in the current classloader's resources, e.g., `/images/my-image.png`
51+
* - relative paths relative to a given root directory [rootDir], e.g., `../images/my-image.png`
52+
*
53+
* If [logResolveFailure] is true, logs any failures to resolve image sources.
54+
*/
55+
public fun create(rootDir: Path, logResolveFailure: Boolean): ImageSourceResolver =
56+
create(
57+
buildSet {
58+
addAll(defaultCapabilities)
59+
add(ResolveCapability.RelativePath(rootDir))
60+
},
61+
logResolveFailure,
62+
)
63+
64+
/**
65+
* Creates [ImageSourceResolver] that can resolve image links in Markdown files according to provided
66+
* [resolveCapabilities].
67+
*
68+
* If [logResolveFailure] is true, logs any failures to resolve image sources.
69+
*/
70+
public fun create(
71+
resolveCapabilities: Set<ResolveCapability> = defaultCapabilities,
72+
logResolveFailure: Boolean = true,
73+
): ImageSourceResolver = DefaultImageSourceResolver(resolveCapabilities, logResolveFailure)
74+
}
75+
76+
/** Provides a list of capabilities that the default [ImageSourceResolver] implementation supports. */
77+
@ApiStatus.Experimental
78+
@ExperimentalJewelApi
79+
public sealed interface ResolveCapability {
80+
/** Resolves a raw image destination string from a Markdown file into a fully-qualified, loadable path. */
81+
public fun resolve(rawDestination: String): String?
82+
83+
/** Represents the ability to resolve plain URIs as-is. */
84+
@ApiStatus.Experimental
85+
@ExperimentalJewelApi
86+
public object PlainUri : ResolveCapability {
87+
override fun toString(): String = "PlainUri"
88+
89+
override fun resolve(rawDestination: String): String? {
90+
val uri = runCatching { URI.create(rawDestination) }.getOrNull() ?: return null
91+
return if (uri.isAbsolute) rawDestination else null
92+
}
93+
}
94+
95+
/**
96+
* Represents the ability to resolve relative paths in the [resourceClass] classloader's resources, or in the
97+
* current classloader's resources if [resourceClass] is `null`.
98+
*/
99+
@ApiStatus.Experimental
100+
@ExperimentalJewelApi
101+
public class RelativePathInResources(val resourceClass: Class<*>? = null) : ResolveCapability {
102+
override fun toString(): String = "RelativePathInResources"
103+
104+
override fun resolve(rawDestination: String): String? =
105+
(resourceClass ?: javaClass).classLoader.getResource(rawDestination.removePrefix("/"))?.toExternalForm()
106+
}
107+
108+
/** Represents the ability to resolve absolute paths as-is. */
109+
public object AbsolutePath : ResolveCapability {
110+
override fun resolve(rawDestination: String): String? {
111+
val rawPath = Path.of(rawDestination)
112+
if (rawPath.isAbsolute) return rawDestination
113+
return null
114+
}
115+
}
116+
117+
/** Represents the ability to resolve relative paths relative to a given root directory [rootDir]. */
118+
@ApiStatus.Experimental
119+
@ExperimentalJewelApi
120+
public class RelativePath(private val rootDir: Path) : ResolveCapability {
121+
override fun resolve(rawDestination: String): String? {
122+
val rawPath = Path.of(rawDestination)
123+
// don't resolve absolute paths, it's not this resolver's capability
124+
if (rawPath.isAbsolute) return null
125+
126+
val normalizedRoot = runCatching { rootDir.toAbsolutePath().normalize() }.getOrNull() ?: return null
127+
128+
val resolved = runCatching { normalizedRoot.resolve(rawPath).normalize() }.getOrNull() ?: return null
129+
130+
return resolved.toString()
131+
}
132+
133+
override fun toString(): String = "RelativePath(rootDir=$rootDir)"
134+
}
135+
}
33136
}
34137

35138
/**
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.
139+
* The default implementation of [ImageSourceResolver] that can resolve image links in Markdown files according to
140+
* provided [resolveCapabilities].
39141
*
142+
* @param resolveCapabilities A list of [ImageSourceResolver.ResolveCapability]s that this resolver can support.
143+
* @param logResolveFailure Whether to log any failures to resolve image sources.
40144
* @see ImageSourceResolver
41145
*/
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) {
146+
internal class DefaultImageSourceResolver(
147+
private val resolveCapabilities: Set<ImageSourceResolver.ResolveCapability> =
148+
ImageSourceResolver.defaultCapabilities,
149+
private val logResolveFailure: Boolean = true,
150+
) : ImageSourceResolver {
151+
override fun resolve(rawDestination: String): String? {
152+
val result = resolveCapabilities.firstNotNullOfOrNull { it.resolve(rawDestination) }
153+
if (result == null && logResolveFailure) {
50154
myLogger()
51155
.warn(
52-
"Markdown image '$rawDestination' expected at classpath '$rawDestination' but not found. " +
53-
"Please ensure it's in your 'src/main/resources/' folder."
156+
"Failed to resolve image source: $rawDestination. Supported capabilities: ${resolveCapabilities.joinToString()}"
54157
)
55-
return rawDestination // This will cause Coil to fail and not render anything.
56158
}
57-
58-
return resourceUrl.toExternalForm()
159+
return result
59160
}
60161
}
61162

@@ -84,5 +185,5 @@ internal object DefaultImageSourceResolver : ImageSourceResolver {
84185
@ExperimentalJewelApi
85186
public val LocalMarkdownImageSourceResolver: ProvidableCompositionLocal<ImageSourceResolver> =
86187
staticCompositionLocalOf {
87-
DefaultImageSourceResolver
188+
ImageSourceResolver.create()
88189
}

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: 52 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,41 @@ public class DefaultImageSourceResolverTest {
4154
}
4255

4356
@Test
44-
public fun `resolveImageSource returns raw destination for non-existent resource`() {
57+
public fun `resolves absolute paths`() {
58+
val absolutePath = "/absolute/path/to/image.png"
59+
60+
val result = DefaultImageSourceResolver().resolve(absolutePath)
61+
62+
TestCase.assertEquals(absolutePath, result)
63+
}
64+
65+
@Test
66+
public fun `doesn't resolve non-existent resource`() {
4567
val nonExistentResource = "this_file_does_not_exist.jpg"
4668

47-
val result = DefaultImageSourceResolver.resolve(nonExistentResource)
69+
val result = DefaultImageSourceResolver().resolve(nonExistentResource)
70+
assertNull(result)
71+
}
72+
73+
@Test
74+
public fun `doesn't resolve URIs without capability`() {
75+
val fullUri = "https://example.com/image.png"
76+
77+
val result =
78+
DefaultImageSourceResolver(
79+
resolveCapabilities = setOf(ImageSourceResolver.ResolveCapability.RelativePathInResources())
80+
)
81+
.resolve(fullUri)
82+
assertNull(result)
83+
}
84+
85+
@Test
86+
public fun `doesn't resolve existing classpath resource without capability`() {
87+
val resourceName = "/test-image.svg"
4888

49-
// It should return the original string, which will later cause Coil to fail.
50-
TestCase.assertEquals(nonExistentResource, result)
89+
val result =
90+
DefaultImageSourceResolver(resolveCapabilities = setOf(ImageSourceResolver.ResolveCapability.PlainUri))
91+
.resolve(resourceName)
92+
assertNull(result)
5193
}
5294
}

0 commit comments

Comments
 (0)