Skip to content

Commit 2486b3b

Browse files
AlexVanGogenAlexVanGogen
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 2486b3b

File tree

4 files changed

+199
-29
lines changed

4 files changed

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

35131
/**
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.
132+
* The default implementation of [ImageSourceResolver] that can resolve image links in Markdown files according to
133+
* provided [resolveCapabilities].
39134
*
135+
* @param resolveCapabilities A list of [ImageSourceResolver.ResolveCapability]s that this resolver can support.
136+
* @param logResolveFailure Whether to log any failures to resolve image sources.
40137
* @see ImageSourceResolver
41138
*/
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) {
139+
internal class DefaultImageSourceResolver(
140+
private val resolveCapabilities: Set<ImageSourceResolver.ResolveCapability> =
141+
ImageSourceResolver.defaultCapabilities,
142+
private val logResolveFailure: Boolean = true,
143+
) : ImageSourceResolver {
144+
override fun resolve(rawDestination: String): String? {
145+
val result = resolveCapabilities.firstNotNullOfOrNull { it.resolve(rawDestination) }
146+
if (result == null && logResolveFailure) {
50147
myLogger()
51148
.warn(
52-
"Markdown image '$rawDestination' expected at classpath '$rawDestination' but not found. " +
53-
"Please ensure it's in your 'src/main/resources/' folder."
149+
"Failed to resolve image source: $rawDestination. Supported capabilities: ${resolveCapabilities.joinToString()}"
54150
)
55-
return rawDestination // This will cause Coil to fail and not render anything.
56151
}
57-
58-
return resourceUrl.toExternalForm()
152+
return result
59153
}
60154
}
61155

@@ -84,5 +178,5 @@ internal object DefaultImageSourceResolver : ImageSourceResolver {
84178
@ExperimentalJewelApi
85179
public val LocalMarkdownImageSourceResolver: ProvidableCompositionLocal<ImageSourceResolver> =
86180
staticCompositionLocalOf {
87-
DefaultImageSourceResolver
181+
ImageSourceResolver.create()
88182
}

‎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)