diff --git a/api/IonElement.api b/api/IonElement.api index 03b3975..bd05261 100644 --- a/api/IonElement.api +++ b/api/IonElement.api @@ -388,18 +388,39 @@ public final class com/amazon/ionelement/api/IonElementLoaderException : com/ama } public final class com/amazon/ionelement/api/IonElementLoaderOptions { + public static final field Companion Lcom/amazon/ionelement/api/IonElementLoaderOptions$Companion; public fun ()V public fun (Z)V public synthetic fun (ZILkotlin/jvm/internal/DefaultConstructorMarker;)V + public static final fun builder ()Lcom/amazon/ionelement/api/IonElementLoaderOptions$Builder; public final fun component1 ()Z + public final fun copy ()Lcom/amazon/ionelement/api/IonElementLoaderOptions; public final fun copy (Z)Lcom/amazon/ionelement/api/IonElementLoaderOptions; public static synthetic fun copy$default (Lcom/amazon/ionelement/api/IonElementLoaderOptions;ZILjava/lang/Object;)Lcom/amazon/ionelement/api/IonElementLoaderOptions; + public final synthetic fun copyWith (Lkotlin/jvm/functions/Function1;)Lcom/amazon/ionelement/api/IonElementLoaderOptions; public fun equals (Ljava/lang/Object;)Z public final fun getIncludeLocationMeta ()Z + public final fun getUseRecursiveLoad ()Z public fun hashCode ()I + public final fun toBuilder ()Lcom/amazon/ionelement/api/IonElementLoaderOptions$Builder; public fun toString ()Ljava/lang/String; } +public final class com/amazon/ionelement/api/IonElementLoaderOptions$Builder { + public final fun build ()Lcom/amazon/ionelement/api/IonElementLoaderOptions; + public final fun getIncludeLocationMeta ()Z + public final fun getUseRecursiveLoad ()Z + public final synthetic fun setIncludeLocationMeta (Z)V + public final synthetic fun setUseRecursiveLoad (Z)V + public final fun withIncludeLocationMeta (Z)Lcom/amazon/ionelement/api/IonElementLoaderOptions$Builder; + public final fun withUseRecursiveLoad (Z)Lcom/amazon/ionelement/api/IonElementLoaderOptions$Builder; +} + +public final class com/amazon/ionelement/api/IonElementLoaderOptions$Companion { + public final fun builder ()Lcom/amazon/ionelement/api/IonElementLoaderOptions$Builder; + public final synthetic fun invoke (Lkotlin/jvm/functions/Function1;)Lcom/amazon/ionelement/api/IonElementLoaderOptions; +} + public abstract class com/amazon/ionelement/api/IonLocation { } diff --git a/src/main/kotlin/com/amazon/ionelement/api/IonElementLoader.kt b/src/main/kotlin/com/amazon/ionelement/api/IonElementLoader.kt index 8437086..d38248e 100644 --- a/src/main/kotlin/com/amazon/ionelement/api/IonElementLoader.kt +++ b/src/main/kotlin/com/amazon/ionelement/api/IonElementLoader.kt @@ -76,17 +76,145 @@ public interface IonElementLoader { /** * Specifies options for [IonElementLoader]. * - * While there is only one property here currently, new properties may be added to this class without breaking - * source compatibility with prior versions of this library. + * Java consumers must use [IonElementLoaderOptions.builder] to create an instance of [IonElementLoaderOptions]. + * ```java + * IonElementLoaderOptions loaderOpts = IonElementLoaderOptions.builder() + * .withIncludeLocationMetadata(true) + * .build(); + * ``` + * When calling from Kotlin, [IonElementLoaderOptions.invoke] is also available. + * ```kotlin + * val loaderOpts = IonElementLoaderOptions { + * includeLocationMetadata = true + * } + * ``` */ -public data class IonElementLoaderOptions( - /** - * Set to true to cause `IonLocation` to be stored in the [IonElement.metas] collection of all elements loaded. +public class IonElementLoaderOptions internal constructor( + val includeLocationMeta: Boolean, + val useRecursiveLoad: Boolean, +) { + /* + * Intentionally not a KDoc comment. + * + * IonElementLoaderOptions used to be a data class, but data classes have some inherent backwards compatibility + * issues. Namely, that the auto-generated `copy` function does not have Java overloads with lesser numbers of + * parameters. * - * This is `false` by default because it has a performance penalty. + * The `copy()` and `component1()` functions and the single arg constructor in this class exist for backwards + * compatibility. They should be removed in the next major version. + */ + + @Deprecated("Will be removed in the next major version. Replace with builder.") + @JvmOverloads + constructor(includeLocationMeta: Boolean = false) : this(includeLocationMeta, DEFAULT.useRecursiveLoad) + + @Deprecated("Will be removed in the next major version. Use toBuilder() to copy this to a new builder.") + @JvmOverloads + fun copy(includeLocationMeta: Boolean = this.includeLocationMeta): IonElementLoaderOptions { + return IonElementLoaderOptions(includeLocationMeta, useRecursiveLoad) + } + + @Deprecated("Will be removed in the next major version. Replace with getIncludeLocationMetadata().") + public operator fun component1() = includeLocationMeta + + /** + * Returns a new [Builder] that is prepopulated with the values in this [IonElementLoaderOptions]. + */ + fun toBuilder(): Builder = Builder(this) + + /** + * Creates a new copy of this [IonElementLoaderOptions] with the given changes. + */ + @JvmSynthetic + fun copyWith(block: Builder.() -> Unit) = toBuilder().apply(block).build() + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is IonElementLoaderOptions) return false + return includeLocationMeta == other.includeLocationMeta && + useRecursiveLoad == other.useRecursiveLoad + } + + override fun hashCode(): Int { + // We can treat all the boolean options as flags in a bitfield to guarantee no hash collisions. + return (if (includeLocationMeta) 1 else 0) + + (if (useRecursiveLoad) 2 else 0) + } + + override fun toString(): String { + return "IonElementLoaderOptions(" + + "includeLocationMeta=$includeLocationMeta," + + "useRecursiveLoad=$useRecursiveLoad," + + ")" + } + + companion object { + @JvmStatic + private val DEFAULT = IonElementLoaderOptions( + includeLocationMeta = false, + useRecursiveLoad = true, + ) + + @JvmStatic + fun builder() = Builder(DEFAULT) + + @JvmSynthetic + operator fun invoke(block: Builder.() -> Unit): IonElementLoaderOptions { + return builder().apply(block).build() + } + } + + /* + * Implementation Note: + * This builder includes `with*` methods for idiomatic usage from Java, and the properties have their setters marked + * with `@JvmSynthetic` rather than private so that they are visible from Kotlin for a DSL-like experience. */ - val includeLocationMeta: Boolean = false -) + class Builder internal constructor(startingValues: IonElementLoaderOptions) { + /** + * Set to `true` to cause `IonLocation` to be stored in the [IonElement.metas] collection of all elements loaded. + * + * This is `false` by default because it has a performance penalty. + */ + var includeLocationMeta: Boolean = startingValues.includeLocationMeta + @JvmSynthetic set + + /** + * Set to `false` to cause the [IonElementLoader] to use an iterative loader. Otherwise, the loader will use a + * recursive approach and fall back to the iterative loader if it steps into nested containers with a depth greater + * than 100. + * + * This is `true` by default. + * + * This does not affect the behavior of the [IonElementLoader] other than its performance characteristics. If + * performance is critical, users should benchmark both options for loading typical data and select the one with + * the desired performance characteristics. + */ + var useRecursiveLoad: Boolean = startingValues.useRecursiveLoad + @JvmSynthetic set + + /** + * Set to `true` to cause `IonLocation` to be stored in the [IonElement.metas] collection of all elements loaded. + * + * This is `false` by default because it has a performance penalty. + */ + fun withIncludeLocationMeta(value: Boolean) = apply { includeLocationMeta = value } + + /** + * Set to `false` to cause the [IonElementLoader] to use an iterative loader. Otherwise, the loader will use a + * recursive approach and fall back to the iterative loader if it steps into nested containers with a depth greater + * than 100. + * + * This is `true` by default. + * + * This does not affect the behavior of the [IonElementLoader] other than its performance characteristics. If + * performance is critical, users should benchmark both options for loading typical data and select the one with + * the desired performance characteristics. + */ + fun withUseRecursiveLoad(value: Boolean) = apply { useRecursiveLoad = value } + + fun build() = IonElementLoaderOptions(includeLocationMeta, useRecursiveLoad) + } +} /** Creates an [IonElementLoader] implementation with the specified [options]. */ @JvmOverloads diff --git a/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt index be046c9..c63c743 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt @@ -31,13 +31,12 @@ import kotlinx.collections.immutable.adapters.ImmutableListAdapter internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions) : IonElementLoader { - // TODO: It seems like some data can be read faster with a recursive approach, but other data is - // faster with an iterative approach. Consider making this configurable. It probably doesn't - // need to be finely-tune-able—just 0 or 100 (i.e. on/off) is probably sufficient. companion object { - private const val MAX_RECURSION_DEPTH: Int = 100 + private const val DEFAULT_MAX_RECURSION_DEPTH: Int = 100 } + private var maxRecursionDepth = if (options.useRecursiveLoad) DEFAULT_MAX_RECURSION_DEPTH else 0 + /** * Catches an [IonException] occurring in [block] and throws an [IonElementLoaderException] with * the current [IonLocation] of the fault, if one is available. Note that depending on the state of the @@ -143,7 +142,7 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions IonType.LIST -> { ionReader.stepIn() val listContent = ArrayList() - if (ionReader.depth < MAX_RECURSION_DEPTH) { + if (ionReader.depth < maxRecursionDepth) { while (ionReader.next() != null) { listContent.add(loadCurrentElementRecursively(ionReader)) } @@ -156,7 +155,7 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions IonType.SEXP -> { ionReader.stepIn() val sexpContent = ArrayList() - if (ionReader.depth < MAX_RECURSION_DEPTH) { + if (ionReader.depth < maxRecursionDepth) { while (ionReader.next() != null) { sexpContent.add(loadCurrentElementRecursively(ionReader)) } @@ -169,7 +168,7 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions IonType.STRUCT -> { val fields = ArrayList() ionReader.stepIn() - if (ionReader.depth < MAX_RECURSION_DEPTH) { + if (ionReader.depth < maxRecursionDepth) { while (ionReader.next() != null) { val fieldName = ionReader.fieldName val element = loadCurrentElementRecursively(ionReader) diff --git a/src/test/java/com/amazon/ionelement/demos/ElementLoaderDemo.java b/src/test/java/com/amazon/ionelement/demos/ElementLoaderDemo.java index 62d0887..05f9363 100644 --- a/src/test/java/com/amazon/ionelement/demos/ElementLoaderDemo.java +++ b/src/test/java/com/amazon/ionelement/demos/ElementLoaderDemo.java @@ -3,6 +3,7 @@ import com.amazon.ion.IonReader; import com.amazon.ionelement.api.AnyElement; import com.amazon.ionelement.api.ElementLoader; +import com.amazon.ionelement.api.IonElementLoaderOptions; import com.amazon.ionelement.util.TestUtils; import org.junit.jupiter.api.Test; @@ -17,6 +18,26 @@ public class ElementLoaderDemo extends DemoBase { private static final String ION_TEXT_STRUCT = "{ a_field: \"hello!\"}"; private static final String ION_TEXT_MULTIPLE_VALUES = "1 2 3"; + @Test + void createIonElementLoaderOptions() { + IonElementLoaderOptions opts = IonElementLoaderOptions.builder() + .withIncludeLocationMeta(true) + .withUseRecursiveLoad(false) + .build(); + + assertTrue(opts.getIncludeLocationMeta()); + assertFalse(opts.getUseRecursiveLoad()); + + IonElementLoaderOptions copy = opts.toBuilder() + .withUseRecursiveLoad(true) + .build(); + + // Unchanged + assertTrue(copy.getIncludeLocationMeta()); + // Changed + assertTrue(copy.getUseRecursiveLoad()); + } + @Test void loadSingleElement_text() { AnyElement value = ElementLoader.loadSingleElement(ION_TEXT_STRUCT); diff --git a/src/test/kotlin/com/amazon/ionelement/IonElementLoaderTests.kt b/src/test/kotlin/com/amazon/ionelement/IonElementLoaderTests.kt index ae957b7..6067045 100644 --- a/src/test/kotlin/com/amazon/ionelement/IonElementLoaderTests.kt +++ b/src/test/kotlin/com/amazon/ionelement/IonElementLoaderTests.kt @@ -24,6 +24,8 @@ import com.amazon.ionelement.util.IonElementLoaderTestCase import com.amazon.ionelement.util.convertToString import java.math.BigInteger import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest @@ -170,4 +172,24 @@ class IonElementLoaderTests { fun `loadSingleElement throws exception when more than one values in reader`() { assertThrows { loadSingleElement("a b") } } + + @Test + fun `create an IonElementLoaderOptions instance`() { + val opts = IonElementLoaderOptions { + includeLocationMeta = true + useRecursiveLoad = false + } + + assertTrue(opts.includeLocationMeta) + assertFalse(opts.useRecursiveLoad) + + val copy = opts.copyWith { + useRecursiveLoad = true + } + + // Unchanged + assertTrue(copy.includeLocationMeta) + // Changed + assertTrue(copy.useRecursiveLoad) + } } diff --git a/src/test/kotlin/com/amazon/ionelement/util/TestUtils.kt b/src/test/kotlin/com/amazon/ionelement/util/TestUtils.kt index 5ceb6af..ba68d84 100644 --- a/src/test/kotlin/com/amazon/ionelement/util/TestUtils.kt +++ b/src/test/kotlin/com/amazon/ionelement/util/TestUtils.kt @@ -36,4 +36,4 @@ fun convertToString(ionValue: IonValue): String { * * This is to support this somewhat commonly used scenario. */ -val INCLUDE_LOCATION_META = IonElementLoaderOptions(includeLocationMeta = true) +val INCLUDE_LOCATION_META = IonElementLoaderOptions { includeLocationMeta = true }