Skip to content

Commit 4c2b75a

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 4c2b75a

File tree

4 files changed

+175
-29
lines changed

4 files changed

+175
-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: 107 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,121 @@ 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 debugName: String
75+
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 val debugName: String
84+
get() = "PlainUri"
85+
86+
override fun resolve(rawDestination: String): String? {
87+
val uri = runCatching { URI.create(rawDestination) }.getOrNull() ?: return null
88+
return if (uri.isAbsolute) rawDestination else null
89+
}
90+
}
91+
92+
/** Represents the ability to resolve relative paths in the current classloader's resources. */
93+
@ApiStatus.Experimental
94+
@ExperimentalJewelApi
95+
public object RelativePathInResources : ResolveCapability {
96+
override val debugName: String
97+
get() = "RelativePathInResources"
98+
99+
override fun resolve(rawDestination: String): String? =
100+
javaClass.classLoader.getResource(rawDestination.removePrefix("/"))?.toExternalForm()
101+
}
102+
103+
/** Represents the ability to resolve relative paths relative to a given root directory [rootDir]. */
104+
@ApiStatus.Experimental
105+
@ExperimentalJewelApi
106+
public class RelativePath(private val rootDir: Path) : ResolveCapability {
107+
override fun resolve(rawDestination: String): String? {
108+
// don't resolve absolute paths, it's not this resolver's capability
109+
if (rawDestination.startsWith("/")) return null
110+
111+
val normalizedRoot = runCatching { rootDir.toAbsolutePath().normalize() }.getOrNull() ?: return null
112+
113+
val resolved =
114+
runCatching { normalizedRoot.resolve(rawDestination).normalize() }.getOrNull() ?: return null
115+
116+
return resolved.toString()
117+
}
118+
119+
override val debugName: String
120+
get() = "RelativePath(rootDir=$rootDir)"
121+
}
122+
}
33123
}
34124

35125
/**
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.
126+
* The default implementation of [ImageSourceResolver] that can resolve image links in Markdown files according to
127+
* provided [resolveCapabilities].
39128
*
129+
* @param resolveCapabilities A list of [ImageSourceResolver.ResolveCapability]s that this resolver can support.
130+
* @param logResolveFailure Whether to log any failures to resolve image sources.
40131
* @see ImageSourceResolver
41132
*/
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) {
133+
internal class DefaultImageSourceResolver(
134+
private val resolveCapabilities: List<ImageSourceResolver.ResolveCapability> =
135+
ImageSourceResolver.defaultCapabilities,
136+
private val logResolveFailure: Boolean = true,
137+
) : ImageSourceResolver {
138+
override fun resolve(rawDestination: String): String? {
139+
val result = resolveCapabilities.firstNotNullOfOrNull { it.resolve(rawDestination) }
140+
if (result == null && logResolveFailure) {
50141
myLogger()
51142
.warn(
52-
"Markdown image '$rawDestination' expected at classpath '$rawDestination' but not found. " +
53-
"Please ensure it's in your 'src/main/resources/' folder."
143+
"Failed to resolve image source: $rawDestination. Supported capabilities: ${resolveCapabilities.joinToString()}"
54144
)
55-
return rawDestination // This will cause Coil to fail and not render anything.
56145
}
57-
58-
return resourceUrl.toExternalForm()
146+
return result
59147
}
60148
}
61149

@@ -84,5 +172,5 @@ internal object DefaultImageSourceResolver : ImageSourceResolver {
84172
@ExperimentalJewelApi
85173
public val LocalMarkdownImageSourceResolver: ProvidableCompositionLocal<ImageSourceResolver> =
86174
staticCompositionLocalOf {
87-
DefaultImageSourceResolver
175+
ImageSourceResolver.create()
88176
}

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