From 5857a893f0ac396b49d1204c80dd026caf3dde3a Mon Sep 17 00:00:00 2001 From: Matthew Pope Date: Fri, 26 Jul 2024 14:40:30 -0700 Subject: [PATCH 1/4] Replaces kotlinx.collections.immutable dependency with custom immutable collections --- build.gradle.kts | 1 - .../kotlin/com/amazon/ionelement/api/Ion.kt | 123 ++++------- .../com/amazon/ionelement/api/IonMeta.kt | 7 +- .../ionelement/impl/BigIntIntElementImpl.kt | 11 +- .../amazon/ionelement/impl/BlobElementImpl.kt | 11 +- .../amazon/ionelement/impl/BoolElementImpl.kt | 11 +- .../amazon/ionelement/impl/ClobElementImpl.kt | 11 +- .../ionelement/impl/DecimalElementImpl.kt | 11 +- .../ionelement/impl/FloatElementImpl.kt | 11 +- .../ionelement/impl/IonElementLoaderImpl.kt | 35 ++-- .../amazon/ionelement/impl/ListElementImpl.kt | 13 +- .../com/amazon/ionelement/impl/ListView.kt | 3 + .../ionelement/impl/LongIntElementImpl.kt | 11 +- .../amazon/ionelement/impl/NullElementImpl.kt | 11 +- .../amazon/ionelement/impl/SeqElementBase.kt | 4 +- .../amazon/ionelement/impl/SexpElementImpl.kt | 13 +- .../ionelement/impl/StringElementImpl.kt | 11 +- .../ionelement/impl/StructElementImpl.kt | 26 ++- .../ionelement/impl/SymbolElementImpl.kt | 11 +- .../ionelement/impl/TimestampElementImpl.kt | 11 +- .../impl/collections/ImmutableList.kt | 92 +++++++++ .../impl/collections/ImmutableMap.kt | 96 +++++++++ .../amazon/ionelement/ImmutabilityTests.java | 1 - .../impl/collections/ImmutableListTest.kt | 162 +++++++++++++++ .../impl/collections/ImmutableMapTest.kt | 194 ++++++++++++++++++ 25 files changed, 690 insertions(+), 201 deletions(-) create mode 100644 src/main/kotlin/com/amazon/ionelement/impl/ListView.kt create mode 100644 src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableList.kt create mode 100644 src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableMap.kt create mode 100644 src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableListTest.kt create mode 100644 src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableMapTest.kt diff --git a/build.gradle.kts b/build.gradle.kts index 0529ac6..61ddff9 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -31,7 +31,6 @@ dependencies { api("com.amazon.ion:ion-java:[1.4.0,)") compileOnly("com.amazon.ion:ion-java:1.4.0") implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8") - implementation("org.jetbrains.kotlinx:kotlinx-collections-immutable:0.3.4") testImplementation("org.jetbrains.kotlin:kotlin-test") testImplementation("org.junit.jupiter:junit-jupiter-api:5.6.2") testImplementation("org.junit.jupiter:junit-jupiter-params:5.6.2") diff --git a/src/main/kotlin/com/amazon/ionelement/api/Ion.kt b/src/main/kotlin/com/amazon/ionelement/api/Ion.kt index c4bb4aa..27930f2 100644 --- a/src/main/kotlin/com/amazon/ionelement/api/Ion.kt +++ b/src/main/kotlin/com/amazon/ionelement/api/Ion.kt @@ -18,7 +18,6 @@ package com.amazon.ionelement.api import com.amazon.ion.Decimal import com.amazon.ion.Timestamp -import com.amazon.ionelement.impl.* import com.amazon.ionelement.impl.BigIntIntElementImpl import com.amazon.ionelement.impl.BlobElementImpl import com.amazon.ionelement.impl.BoolElementImpl @@ -34,12 +33,9 @@ import com.amazon.ionelement.impl.StructElementImpl import com.amazon.ionelement.impl.StructFieldImpl import com.amazon.ionelement.impl.SymbolElementImpl import com.amazon.ionelement.impl.TimestampElementImpl +import com.amazon.ionelement.impl.collections.* import java.math.BigInteger import java.util.function.Consumer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.persistentListOf -import kotlinx.collections.immutable.toPersistentList -import kotlinx.collections.immutable.toPersistentMap internal typealias Annotations = List @@ -127,8 +123,8 @@ public fun ionString( metas: MetaContainer = emptyMetaContainer() ): StringElement = StringElementImpl( value = s, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates a [StringElement] that represents an Ion `symbol`. */ public fun ionString( @@ -144,8 +140,8 @@ public fun ionSymbol( metas: MetaContainer = emptyMetaContainer() ): SymbolElement = SymbolElementImpl( value = s, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates a [SymbolElement] that represents an Ion `symbol`. */ @@ -162,8 +158,8 @@ public fun ionTimestamp( metas: MetaContainer = emptyMetaContainer() ): TimestampElement = TimestampElementImpl( timestampValue = Timestamp.valueOf(s), - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates a [TimestampElement] that represents an Ion `timestamp`. */ @@ -180,8 +176,8 @@ public fun ionTimestamp( metas: MetaContainer = emptyMetaContainer() ): TimestampElement = TimestampElementImpl( timestampValue = timestamp, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates a [TimestampElement] that represents an Ion `timestamp`. */ @@ -199,8 +195,8 @@ public fun ionInt( ): IntElement = LongIntElementImpl( longValue = l, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates an [IntElement] that represents an Ion `int`. */ @@ -217,8 +213,8 @@ public fun ionInt( metas: MetaContainer = emptyMetaContainer() ): IntElement = BigIntIntElementImpl( bigIntegerValue = bigInt, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates an [IntElement] that represents an Ion `BitInteger`. */ @@ -235,8 +231,8 @@ public fun ionBool( metas: MetaContainer = emptyMetaContainer() ): BoolElement = BoolElementImpl( booleanValue = b, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates a [BoolElement] that represents an Ion `bool`. */ @@ -253,8 +249,8 @@ public fun ionFloat( metas: MetaContainer = emptyMetaContainer() ): FloatElement = FloatElementImpl( doubleValue = d, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates a [FloatElement] that represents an Ion `float`. */ @@ -271,8 +267,8 @@ public fun ionDecimal( metas: MetaContainer = emptyMetaContainer() ): DecimalElement = DecimalElementImpl( decimalValue = bigDecimal, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates a [DecimalElement] that represents an Ion `decimall`. */ @@ -293,8 +289,8 @@ public fun ionBlob( metas: MetaContainer = emptyMetaContainer() ): BlobElement = BlobElementImpl( bytes = bytes.clone(), - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** @@ -322,8 +318,8 @@ public fun ionClob( metas: MetaContainer = emptyMetaContainer() ): ClobElement = ClobElementImpl( bytes = bytes.clone(), - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** * Creates a [ClobElement] that represents an Ion `clob`. @@ -346,9 +342,9 @@ public fun ionListOf( metas: MetaContainer = emptyMetaContainer() ): ListElement = ListElementImpl( - values = iterable.mapToEmptyOrPersistentList { it.asAnyElement() }, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + values = iterable.map { it.asAnyElement() }.toImmutableListUnsafe(), + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates a [ListElement] that represents an Ion `list`. */ @@ -395,9 +391,9 @@ public fun ionSexpOf( metas: MetaContainer = emptyMetaContainer() ): SexpElement = SexpElementImpl( - values = iterable.mapToEmptyOrPersistentList { it.asAnyElement() }, - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + values = iterable.map { it.asAnyElement() }.toImmutableListUnsafe(), + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates an [SexpElement] that represents an Ion `sexp`. */ @@ -437,9 +433,9 @@ public fun ionStructOf( metas: MetaContainer = emptyMetaContainer() ): StructElement = StructElementImpl( - allFields = fields.toEmptyOrPersistentList(), - annotations = annotations.toEmptyOrPersistentList(), - metas = metas.toPersistentMap() + allFields = fields.toImmutableList(), + annotations = annotations.toImmutableList(), + metas = metas.toImmutableMap() ) /** Creates a [StructElement] that represents an Ion `struct` with the specified fields. */ @@ -457,7 +453,7 @@ public fun ionStructOf( ): StructElement = ionStructOf( fields = fields.asIterable(), - annotations = annotations.toEmptyOrPersistentList(), + annotations = annotations.toImmutableList(), metas = metas ) @@ -523,55 +519,14 @@ public fun buildStruct( return ionStructOf(fields, annotations, metas) } -/** - * Converts an `Iterable` to a `PersistentList`. - * - * ### Why is this needed? - * `kotlinx.collections.immutable` <= 0.3.7 has a bug that causes it to unnecessarily allocate empty `PersistentList`s - * instead of using a singleton instance. The fix is in - * [kotlinx.collections.immutable#176](https://github.com/Kotlin/kotlinx.collections.immutable/pull/176), - * but we cannot use it because the version of `kotlinx.collections.immutable` that will have (or has) the fix - * requires Kotlin stdlib >=1.9.2, and `ion-element-kotlin` supports consumers using Kotlin >= 1.3.0. - */ -internal fun Iterable.toEmptyOrPersistentList(): PersistentList { - val isEmpty = if (this is Collection<*>) { - this.isEmpty() - } else { - !this.iterator().hasNext() - } - return if (isEmpty) EMPTY_PERSISTENT_LIST else toPersistentList() -} - -/** - * Converts an `Iterable` to a `PersistentList`, transforming each element using [block]. - * - * ### Why is this needed? - * `kotlinx.collections.immutable` <= 0.3.7 has a bug that causes it to unnecessarily allocate empty `PersistentList`s - * instead of using a singleton instance. The fix is in - * [kotlinx.collections.immutable#176](https://github.com/Kotlin/kotlinx.collections.immutable/pull/176), - * but we cannot use it because the version of `kotlinx.collections.immutable` that will have (or has) the fix - * requires Kotlin stdlib >=1.9.2, and `ion-element-kotlin` supports consumers using Kotlin >= 1.3.0. - */ -internal inline fun Iterable.mapToEmptyOrPersistentList(block: (T) -> R): PersistentList { - val isEmpty = if (this is Collection<*>) { - this.isEmpty() - } else { - !this.iterator().hasNext() - } - return if (isEmpty) EMPTY_PERSISTENT_LIST else mapTo(persistentListOf().builder(), block).build() -} - -// Memoized empty PersistentList for the memoized container types and null values -internal val EMPTY_PERSISTENT_LIST: PersistentList = persistentListOf() - // Memoized empty instances of our container types. -private val EMPTY_LIST = ListElementImpl(EMPTY_PERSISTENT_LIST, EMPTY_PERSISTENT_LIST, EMPTY_METAS) -private val EMPTY_SEXP = SexpElementImpl(EMPTY_PERSISTENT_LIST, EMPTY_PERSISTENT_LIST, EMPTY_METAS) -private val EMPTY_STRUCT = StructElementImpl(EMPTY_PERSISTENT_LIST, EMPTY_PERSISTENT_LIST, EMPTY_METAS) -private val EMPTY_BLOB = BlobElementImpl(ByteArray(0), EMPTY_PERSISTENT_LIST, EMPTY_METAS) -private val EMPTY_CLOB = ClobElementImpl(ByteArray(0), EMPTY_PERSISTENT_LIST, EMPTY_METAS) +private val EMPTY_LIST = ListElementImpl(ImmutableList.EMPTY, ImmutableList.EMPTY, EMPTY_METAS) +private val EMPTY_SEXP = SexpElementImpl(ImmutableList.EMPTY, ImmutableList.EMPTY, EMPTY_METAS) +private val EMPTY_STRUCT = StructElementImpl(ImmutableList.EMPTY, ImmutableList.EMPTY, EMPTY_METAS) +private val EMPTY_BLOB = BlobElementImpl(ByteArray(0), ImmutableList.EMPTY, EMPTY_METAS) +private val EMPTY_CLOB = ClobElementImpl(ByteArray(0), ImmutableList.EMPTY, EMPTY_METAS) // Memoized instances of all of our null values. private val ALL_NULLS = ElementType.values().map { - it to NullElementImpl(it, EMPTY_PERSISTENT_LIST, EMPTY_METAS) as IonElement + it to NullElementImpl(it, ImmutableList.EMPTY, EMPTY_METAS) as IonElement }.toMap() diff --git a/src/main/kotlin/com/amazon/ionelement/api/IonMeta.kt b/src/main/kotlin/com/amazon/ionelement/api/IonMeta.kt index 1fd067b..16821bf 100644 --- a/src/main/kotlin/com/amazon/ionelement/api/IonMeta.kt +++ b/src/main/kotlin/com/amazon/ionelement/api/IonMeta.kt @@ -16,13 +16,12 @@ @file: JvmName("IonMeta") package com.amazon.ionelement.api -import kotlinx.collections.immutable.PersistentMap -import kotlinx.collections.immutable.toPersistentHashMap +import com.amazon.ionelement.impl.collections.* public typealias MetaContainer = Map -internal typealias PersistentMetaContainer = PersistentMap +internal typealias ImmutableMetaContainer = ImmutableMap -internal val EMPTY_METAS = HashMap().toPersistentHashMap() +internal val EMPTY_METAS: ImmutableMetaContainer = emptyMap().toImmutableMap() public fun emptyMetaContainer(): MetaContainer = EMPTY_METAS diff --git a/src/main/kotlin/com/amazon/ionelement/impl/BigIntIntElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/BigIntIntElementImpl.kt index 0df64e2..a42f445 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/BigIntIntElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/BigIntIntElementImpl.kt @@ -17,16 +17,15 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer +import com.amazon.ionelement.api.ImmutableMetaContainer import com.amazon.ionelement.api.constraintError +import com.amazon.ionelement.impl.collections.* import java.math.BigInteger -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap internal class BigIntIntElementImpl( override val bigIntegerValue: BigInteger, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : AnyElementBase(), IntElement { override val type: ElementType get() = ElementType.INT @@ -42,7 +41,7 @@ internal class BigIntIntElementImpl( } override fun copy(annotations: List, metas: MetaContainer): BigIntIntElementImpl = - BigIntIntElementImpl(bigIntegerValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + BigIntIntElementImpl(bigIntegerValue, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): BigIntIntElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): BigIntIntElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/BlobElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/BlobElementImpl.kt index 159b42f..dcc1daa 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/BlobElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/BlobElementImpl.kt @@ -17,14 +17,13 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class BlobElementImpl( bytes: ByteArray, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : LobElementBase(bytes), BlobElement { override val type: ElementType get() = ElementType.BLOB @@ -34,7 +33,7 @@ internal class BlobElementImpl( override fun writeContentTo(writer: IonWriter) = writer.writeBlob(bytes) override fun copy(annotations: List, metas: MetaContainer): BlobElementImpl = - BlobElementImpl(bytes, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + BlobElementImpl(bytes, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): BlobElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): BlobElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/BoolElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/BoolElementImpl.kt index acb8daa..f22eaf2 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/BoolElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/BoolElementImpl.kt @@ -17,19 +17,18 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class BoolElementImpl( override val booleanValue: Boolean, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : AnyElementBase(), BoolElement { override val type: ElementType get() = ElementType.BOOL override fun copy(annotations: List, metas: MetaContainer): BoolElementImpl = - BoolElementImpl(booleanValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + BoolElementImpl(booleanValue, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): BoolElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): BoolElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/ClobElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/ClobElementImpl.kt index 2f8081b..1901969 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/ClobElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/ClobElementImpl.kt @@ -17,14 +17,13 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class ClobElementImpl( bytes: ByteArray, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : LobElementBase(bytes), ClobElement { override val type: ElementType get() = ElementType.CLOB @@ -33,7 +32,7 @@ internal class ClobElementImpl( override fun writeContentTo(writer: IonWriter) = writer.writeClob(bytes) override fun copy(annotations: List, metas: MetaContainer): ClobElementImpl = - ClobElementImpl(bytes, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + ClobElementImpl(bytes, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): ClobElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): ClobElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/DecimalElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/DecimalElementImpl.kt index 9cea505..d3536c2 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/DecimalElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/DecimalElementImpl.kt @@ -18,19 +18,18 @@ package com.amazon.ionelement.impl import com.amazon.ion.Decimal import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class DecimalElementImpl( override val decimalValue: Decimal, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : AnyElementBase(), DecimalElement { override val type get() = ElementType.DECIMAL override fun copy(annotations: List, metas: MetaContainer): DecimalElementImpl = - DecimalElementImpl(decimalValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + DecimalElementImpl(decimalValue, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): DecimalElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): DecimalElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/FloatElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/FloatElementImpl.kt index 56186fb..4b9d1c2 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/FloatElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/FloatElementImpl.kt @@ -17,19 +17,18 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class FloatElementImpl( override val doubleValue: Double, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : AnyElementBase(), FloatElement { override val type: ElementType get() = ElementType.FLOAT override fun copy(annotations: List, metas: MetaContainer): FloatElementImpl = - FloatElementImpl(doubleValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + FloatElementImpl(doubleValue, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): FloatElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): FloatElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt index fe2e18b..1cc72cd 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt @@ -24,7 +24,7 @@ import com.amazon.ion.SpanProvider import com.amazon.ion.TextSpan import com.amazon.ion.system.IonReaderBuilder import com.amazon.ionelement.api.* -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.impl.collections.* internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions) : IonElementLoader { @@ -45,6 +45,16 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions } } + private fun IonReader.getSpanProvider(): SpanProvider? = this.asFacet(SpanProvider::class.java) + + private fun SpanProvider.currentLocation(): IonLocation? { + return when (val currentSpan = currentSpan()) { + is TextSpan -> IonTextLocation(currentSpan.startLine, currentSpan.startColumn) + is OffsetSpan -> IonBinaryLocation(currentSpan.startOffset) + else -> null + } + } + private fun IonReader.currentLocation(): IonLocation? = when { // Can't attempt to get a SpanProvider unless we're on a value @@ -89,18 +99,13 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions return handleReaderException(ionReader) { val valueType = requireNotNull(ionReader.type) { "The IonReader was not positioned at an element." } - val annotations = ionReader.typeAnnotations!!.asList().toEmptyOrPersistentList() + val annotations = ionReader.typeAnnotations!!.toImmutableListUnsafe() - val metas = when { - options.includeLocationMeta -> { - val location = ionReader.currentLocation() - when { - location != null -> metaContainerOf(ION_LOCATION_META_TAG to location) - else -> emptyMetaContainer() - } - } - else -> emptyMetaContainer() - }.toPersistentMap() + var metas = EMPTY_METAS + if (options.includeLocationMeta) { + val location = ionReader.currentLocation() + if (location != null) metas = location.toMetaContainer() + } if (ionReader.isNullValue) { ionNull(valueType.toElementType(), annotations, metas) @@ -133,13 +138,13 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions IonType.BLOB -> BlobElementImpl(ionReader.newBytes(), annotations, metas) IonType.LIST -> { ionReader.stepIn() - val list = ListElementImpl(loadAllElements(ionReader).toEmptyOrPersistentList(), annotations, metas) + val list = ListElementImpl(loadAllElements(ionReader).toImmutableListUnsafe(), annotations, metas) ionReader.stepOut() list } IonType.SEXP -> { ionReader.stepIn() - val sexp = SexpElementImpl(loadAllElements(ionReader).toEmptyOrPersistentList(), annotations, metas) + val sexp = SexpElementImpl(loadAllElements(ionReader).toImmutableListUnsafe(), annotations, metas) ionReader.stepOut() sexp } @@ -155,7 +160,7 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions ) } ionReader.stepOut() - StructElementImpl(fields.toEmptyOrPersistentList(), annotations, metas) + StructElementImpl(fields.toImmutableListUnsafe(), annotations, metas) } IonType.DATAGRAM -> error("IonElementLoaderImpl does not know what to do with IonType.DATAGRAM") IonType.NULL -> error("IonType.NULL branch should be unreachable") diff --git a/src/main/kotlin/com/amazon/ionelement/impl/ListElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/ListElementImpl.kt index 87b0972..97f8465 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/ListElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/ListElementImpl.kt @@ -16,21 +16,20 @@ package com.amazon.ionelement.impl import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class ListElementImpl( - values: PersistentList, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + values: ImmutableList, + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : SeqElementBase(values), ListElement { override val type: ElementType get() = ElementType.LIST override val listValues: List get() = values override fun copy(annotations: List, metas: MetaContainer): ListElementImpl = - ListElementImpl(values, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + ListElementImpl(values, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): ListElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): ListElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/ListView.kt b/src/main/kotlin/com/amazon/ionelement/impl/ListView.kt new file mode 100644 index 0000000..92d9c5c --- /dev/null +++ b/src/main/kotlin/com/amazon/ionelement/impl/ListView.kt @@ -0,0 +1,3 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +package com.amazon.ionelement.impl diff --git a/src/main/kotlin/com/amazon/ionelement/impl/LongIntElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/LongIntElementImpl.kt index 7eaaa69..83e1129 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/LongIntElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/LongIntElementImpl.kt @@ -17,15 +17,14 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* import java.math.BigInteger -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap internal class LongIntElementImpl( override val longValue: Long, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : AnyElementBase(), IntElement { override val integerSize: IntElementSize get() = IntElementSize.LONG override val type: ElementType get() = ElementType.INT @@ -33,7 +32,7 @@ internal class LongIntElementImpl( override val bigIntegerValue: BigInteger get() = BigInteger.valueOf(longValue) override fun copy(annotations: List, metas: MetaContainer): LongIntElementImpl = - LongIntElementImpl(longValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + LongIntElementImpl(longValue, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): LongIntElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): LongIntElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/NullElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/NullElementImpl.kt index aeb6fb8..3cd9efb 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/NullElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/NullElementImpl.kt @@ -17,20 +17,19 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class NullElementImpl( override val type: ElementType = ElementType.NULL, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : AnyElementBase() { override val isNull: Boolean get() = true override fun copy(annotations: List, metas: MetaContainer): AnyElement = - NullElementImpl(type, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + NullElementImpl(type, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): AnyElement = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): AnyElement = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/SeqElementBase.kt b/src/main/kotlin/com/amazon/ionelement/impl/SeqElementBase.kt index 87fac2f..d960df7 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/SeqElementBase.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/SeqElementBase.kt @@ -19,10 +19,10 @@ import com.amazon.ion.IonWriter import com.amazon.ionelement.api.AnyElement import com.amazon.ionelement.api.MetaContainer import com.amazon.ionelement.api.SeqElement -import kotlinx.collections.immutable.PersistentList +import com.amazon.ionelement.impl.collections.* internal abstract class SeqElementBase( - override val values: PersistentList + override val values: ImmutableList ) : AnyElementBase(), SeqElement { override val containerValues: List get() = values diff --git a/src/main/kotlin/com/amazon/ionelement/impl/SexpElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/SexpElementImpl.kt index 75ea3f0..274296a 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/SexpElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/SexpElementImpl.kt @@ -16,21 +16,20 @@ package com.amazon.ionelement.impl import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class SexpElementImpl( - values: PersistentList, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + values: ImmutableList, + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : SeqElementBase(values), SexpElement { override val type: ElementType get() = ElementType.SEXP override val sexpValues: List get() = seqValues override fun copy(annotations: List, metas: MetaContainer): SexpElementImpl = - SexpElementImpl(values, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + SexpElementImpl(values, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): SexpElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): SexpElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/StringElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/StringElementImpl.kt index 9a73743..7ce7cae 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/StringElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/StringElementImpl.kt @@ -17,21 +17,20 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class StringElementImpl( value: String, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : TextElementBase(value), StringElement { override val type: ElementType get() = ElementType.STRING override val stringValue: String get() = textValue override fun copy(annotations: List, metas: MetaContainer): StringElementImpl = - StringElementImpl(textValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + StringElementImpl(textValue, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): StringElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): StringElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/StructElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/StructElementImpl.kt index 1c40147..6a236b8 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/StructElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/StructElementImpl.kt @@ -18,19 +18,16 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonType import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer +import com.amazon.ionelement.api.ImmutableMetaContainer import com.amazon.ionelement.api.constraintError +import com.amazon.ionelement.impl.collections.* import java.util.function.Consumer -import kotlinx.collections.immutable.PersistentCollection -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.PersistentMap -import kotlinx.collections.immutable.toPersistentMap // TODO: Consider creating a StructElement variant with optimizations that assume no duplicate field names. internal class StructElementImpl( - private val allFields: PersistentList, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + private val allFields: ImmutableList, + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : AnyElementBase(), StructElement { override val type: ElementType get() = ElementType.STRUCT @@ -38,11 +35,11 @@ internal class StructElementImpl( // Note that we are not using `by lazy` here because it requires 2 additional allocations and // has been demonstrated to significantly increase memory consumption! - private var valuesBackingField: PersistentCollection? = null + private var valuesBackingField: ImmutableList? = null override val values: Collection get() { if (valuesBackingField == null) { - valuesBackingField = fields.mapToEmptyOrPersistentList { it.value } + valuesBackingField = fields.map { it.value }.toImmutableListUnsafe() } return valuesBackingField!! } @@ -52,7 +49,7 @@ internal class StructElementImpl( // Note that we are not using `by lazy` here because it requires 2 additional allocations and // has been demonstrated to significantly increase memory consumption! - private var fieldsByNameBackingField: PersistentMap>? = null + private var fieldsByNameBackingField: ImmutableMap>? = null /** Lazily calculated map of field names and lists of their values. */ private val fieldsByName: Map> @@ -61,8 +58,9 @@ internal class StructElementImpl( fieldsByNameBackingField = fields .groupBy { it.name } - .map { structFieldGroup -> structFieldGroup.key to structFieldGroup.value.mapToEmptyOrPersistentList { it.value } } - .toMap().toPersistentMap() + .map { structFieldGroup -> structFieldGroup.key to structFieldGroup.value.map { it.value }.toImmutableListUnsafe() } + .toMap() + .toImmutableMapUnsafe() } return fieldsByNameBackingField!! } @@ -99,7 +97,7 @@ internal class StructElementImpl( override fun containsField(fieldName: String): Boolean = fieldsByName.containsKey(fieldName) override fun copy(annotations: List, metas: MetaContainer): StructElementImpl = - StructElementImpl(allFields, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + StructElementImpl(allFields, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): StructElementImpl = _withAnnotations(*additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/SymbolElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/SymbolElementImpl.kt index 6d56fb7..073cc06 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/SymbolElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/SymbolElementImpl.kt @@ -17,21 +17,20 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class SymbolElementImpl( value: String, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : TextElementBase(value), SymbolElement { override val type: ElementType get() = ElementType.SYMBOL override val symbolValue: String get() = textValue override fun copy(annotations: List, metas: MetaContainer): SymbolElementImpl = - SymbolElementImpl(textValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + SymbolElementImpl(textValue, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): SymbolElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): SymbolElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/TimestampElementImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/TimestampElementImpl.kt index ff559f4..d2dc1b4 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/TimestampElementImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/TimestampElementImpl.kt @@ -18,19 +18,18 @@ package com.amazon.ionelement.impl import com.amazon.ion.IonWriter import com.amazon.ion.Timestamp import com.amazon.ionelement.api.* -import com.amazon.ionelement.api.PersistentMetaContainer -import kotlinx.collections.immutable.PersistentList -import kotlinx.collections.immutable.toPersistentMap +import com.amazon.ionelement.api.ImmutableMetaContainer +import com.amazon.ionelement.impl.collections.* internal class TimestampElementImpl( override val timestampValue: Timestamp, - override val annotations: PersistentList, - override val metas: PersistentMetaContainer + override val annotations: ImmutableList, + override val metas: ImmutableMetaContainer ) : AnyElementBase(), TimestampElement { override val type: ElementType get() = ElementType.TIMESTAMP override fun copy(annotations: List, metas: MetaContainer): TimestampElementImpl = - TimestampElementImpl(timestampValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap()) + TimestampElementImpl(timestampValue, annotations.toImmutableList(), metas.toImmutableMap()) override fun withAnnotations(vararg additionalAnnotations: String): TimestampElementImpl = _withAnnotations(*additionalAnnotations) override fun withAnnotations(additionalAnnotations: Iterable): TimestampElementImpl = _withAnnotations(additionalAnnotations) diff --git a/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableList.kt b/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableList.kt new file mode 100644 index 0000000..e1bc006 --- /dev/null +++ b/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableList.kt @@ -0,0 +1,92 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +package com.amazon.ionelement.impl.collections + +import com.amazon.ionelement.impl.collections.ImmutableList.Companion.EMPTY + +/** + * A [List] that (unlike the standard library lists) cannot be modified from Java. + * + * We cannot use `List.of(...)` because those were introduced in JDK 9, and we still + * support JDK 8. + */ +// TODO: Mark as sealed once we're on a higher version of Kotlin. +internal interface ImmutableList : List { + companion object { + val EMPTY: ImmutableList = ListBackedImmutableList(emptyList()) + } +} + +/** + * Wraps any [List] as an [ImmutableList]. + */ +private class ListBackedImmutableList(private val list: List) : ImmutableList, List by list { + override fun toString(): String = list.toString() + override fun equals(other: Any?): Boolean = list == other + override fun hashCode(): Int = list.hashCode() +} + +/** + * Wraps an Array as an [ImmutableList]. + */ +private class ArrayBackedImmutableList(private val array: Array) : ImmutableList, AbstractList() { + override val size: Int get() = array.size + override fun get(index: Int): T = array[index] + + override fun toString(): String = array.contentDeepToString() + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is List<*>) return false + if (other.size != array.size) return false + for (i in array.indices) { + if (array[i] != other[i]) return false + } + return true + } + + override fun hashCode(): Int = array.contentDeepHashCode() +} + +/** + * Creates a [ImmutableList] for [this]. + * This function creates a defensive copy of [this] unless [this] is already a [ImmutableList]. + */ +internal fun Iterable.toImmutableList(): ImmutableList { + if (this is ImmutableList) return this + val isEmpty = if (this is Collection<*>) { + this.isEmpty() + } else { + !this.iterator().hasNext() + } + return if (isEmpty) EMPTY else ListBackedImmutableList(this.toList()) +} + +/** + * Creates a [ImmutableList] for [this]. + * This function creates a defensive copy of [this] unless [this] is already a [ImmutableList]. + */ +internal fun List.toImmutableList(): ImmutableList { + if (this is ImmutableList) return this + if (isEmpty()) return EMPTY + return ListBackedImmutableList(this.toList()) +} + +/** + * Creates a [ImmutableList] for [this] without making a defensive copy. + * Only call this method if you are sure that [this] cannot leak anywhere it could be mutated. + */ +internal fun List.toImmutableListUnsafe(): ImmutableList { + if (this is ImmutableList) return this + if (isEmpty()) return EMPTY + return ListBackedImmutableList(this) +} + +/** + * Creates a [ImmutableList] for [this] without making a defensive copy. + * Only call this method if you are sure that [this] cannot leak anywhere it could be mutated. + */ +internal fun Array.toImmutableListUnsafe(): ImmutableList { + if (isEmpty()) return EMPTY + return ArrayBackedImmutableList(this) +} diff --git a/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableMap.kt b/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableMap.kt new file mode 100644 index 0000000..5f838ee --- /dev/null +++ b/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableMap.kt @@ -0,0 +1,96 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +package com.amazon.ionelement.impl.collections + +import com.amazon.ionelement.api.* +import java.util.AbstractMap.SimpleImmutableEntry + +/** + * A [Map] that (unlike the standard library maps) cannot be modified from Java. + * + * We cannot use `Map.of(...)` because those were introduced in JDK 9, and we still + * support JDK 8. + */ +// TODO: Mark as sealed once we're on a higher version of Kotlin. +internal interface ImmutableMap : Map { + companion object { + val EMPTY: ImmutableMap = MapBackedImmutableMap(emptyMap()) + } +} + +/** + * Wraps any [Map] as an [ImmutableMap]. + */ +private class MapBackedImmutableMap(private val map: Map) : ImmutableMap, Map by map { + override fun toString(): String = map.toString() + override fun equals(other: Any?): Boolean = map == other + override fun hashCode(): Int = map.hashCode() +} + +/** + * Specialized implementation of [Map] that always has a size of 1 and contains only the key [ION_LOCATION_META_TAG]. + * + * This exists so that we can populate location metadata with as little overhead as possible. + * On 64-bit Hotspot JVM, this has an object size of only 16 bytes compared to [java.util.Collections.singletonMap] + * which creates a map with an object size of 40 bytes. + * + * We assume that by far the most common use case for this class is calling `get(ION_LOCATION_META_TAG)` + * rather than general `Map` operations. + */ +private class IonLocationBackedImmutableMap(private val value: IonLocation) : ImmutableMap { + override val size: Int get() = 1 + override fun isEmpty(): Boolean = false + + override fun get(key: String): IonLocation? = if (key == ION_LOCATION_META_TAG) value else null + override fun containsValue(value: IonLocation): Boolean = value == this.value + override fun containsKey(key: String): Boolean = key == ION_LOCATION_META_TAG + + override val keys: Set get() = KEY_SET + // We could memoize these values, but that would increase the memory footprint of this class. + override val values: Collection get() = setOf(value) + override val entries: Set> get() = setOf(SimpleImmutableEntry(ION_LOCATION_META_TAG, value)) + + override fun toString(): String = "{$ION_LOCATION_META_TAG=$value}" + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is Map<*, *>) return false + if (other.size != 1) return false + return other[ION_LOCATION_META_TAG] == value + } + override fun hashCode(): Int = ION_LOCATION_META_TAG.hashCode() xor value.hashCode() + + companion object { + private val KEY_SET = setOf(ION_LOCATION_META_TAG) + } +} + +/** + * Creates a [ImmutableMap] for [this] without making a defensive copy. + * Only call this method if you are sure that [this] cannot leak anywhere it could be mutated. + */ +internal fun Map.toImmutableMapUnsafe(): ImmutableMap { + if (this is ImmutableMap) return this + // Empty ImmutableMap can be safely cast to any `` because it is empty. + @Suppress("UNCHECKED_CAST") + if (isEmpty()) return (ImmutableMap.EMPTY as ImmutableMap) + return MapBackedImmutableMap(this) +} + +/** + * Creates a [ImmutableMap] for [this]. + * This function creates a defensive copy of [this] unless [this] is already a [ImmutableMap]. + */ +internal fun Map.toImmutableMap(): ImmutableMap { + if (this is ImmutableMap) return this + // Empty ImmutableMap can be safely cast to any `` because it is empty. + @Suppress("UNCHECKED_CAST") + if (isEmpty()) return (ImmutableMap.EMPTY as ImmutableMap) + return MapBackedImmutableMap(toMap()) +} + +/** + * Creates an [ImmutableMetaContainer] ([ImmutableMap]) that holds [this] [IonLocation] instance. + */ +internal fun IonLocation.toMetaContainer(): ImmutableMap { + return IonLocationBackedImmutableMap(this) +} diff --git a/src/test/kotlin/com/amazon/ionelement/ImmutabilityTests.java b/src/test/kotlin/com/amazon/ionelement/ImmutabilityTests.java index e911ea6..10d0597 100644 --- a/src/test/kotlin/com/amazon/ionelement/ImmutabilityTests.java +++ b/src/test/kotlin/com/amazon/ionelement/ImmutabilityTests.java @@ -4,7 +4,6 @@ import com.amazon.ion.Timestamp; import com.amazon.ionelement.api.*; import kotlin.Pair; -import kotlinx.collections.immutable.PersistentList; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; diff --git a/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableListTest.kt b/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableListTest.kt new file mode 100644 index 0000000..584be27 --- /dev/null +++ b/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableListTest.kt @@ -0,0 +1,162 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +package com.amazon.ionelement.impl.collections + +import kotlin.test.assertNotEquals +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test + +class ImmutableListTest { + + @Nested + inner class ListBackedImmutableListTest { + @Test + fun `equals() should be the same as a standard library list`() { + val list = listOf(1, 2, 3) + + val immutableListUnsafe = list.toImmutableListUnsafe() + assertEquals(list, immutableListUnsafe) + assertEquals(immutableListUnsafe, list) + assertEquals(immutableListUnsafe, immutableListUnsafe) + + val immutableList = list.toImmutableList() + assertEquals(list, immutableList) + assertEquals(immutableList, list) + assertEquals(immutableList, immutableList) + + assertNotEquals(immutableList, listOf(1)) + assertNotEquals(immutableList, listOf(2, 3, 4)) + assertNotEquals(immutableList, "foo") + } + + @Test + fun `hashCode() should return the same as a standard library list`() { + val list = listOf(1, 2, 3) + + val immutableListUnsafe = list.toImmutableListUnsafe() + assertEquals(list.hashCode(), immutableListUnsafe.hashCode()) + + val immutableList = list.toImmutableList() + assertEquals(list.hashCode(), immutableList.hashCode()) + } + + @Test + fun `toString() should return the same as a standard library list`() { + val list = listOf(1, 2, 3) + + val immutableListUnsafe = list.toImmutableListUnsafe() + assertEquals(list.toString(), immutableListUnsafe.toString()) + + val immutableList = list.toImmutableList() + assertEquals(list.toString(), immutableList.toString()) + } + + @Test + fun `toImmutableList() should return the singleton EMPTY when this is empty`() { + assertSame(ImmutableList.EMPTY, listOf().toImmutableList()) + // Check again to cover the same case in the overload for Iterable + assertSame(ImmutableList.EMPTY, sequenceOf().asIterable().toImmutableList()) + } + + @Test + fun `toImmutableListUnsafe() should return the singleton EMPTY when this is empty`() { + val immutableList = listOf().toImmutableListUnsafe() + assertSame(ImmutableList.EMPTY, immutableList) + } + + @Test + fun `toImmutableList() should return this when this is already a ImmutableList`() { + val list = listOf(1, 2, 3).toImmutableList() + assertSame(list, list.toImmutableList()) + // Check again to cover the same case in the overload for Iterable + assertSame(list, (list as Iterable).toImmutableList()) + } + + @Test + fun `toImmutableListUnsafe() should return this when this is already a ImmutableList`() { + val list = listOf(1, 2, 3).toImmutableList() + assertSame(list, list.toImmutableListUnsafe()) + } + + @Test + fun `toImmutableList() should create a defensive copy`() { + val list = listOf(1, 2, 3) + val mutableCopy = list.toMutableList() + val immutableList = mutableCopy.toImmutableList() + + // Add a value to `mutableCopy` and then make sure that ImmutableList is still [1, 2, 3] + mutableCopy.add(4) + assertEquals(list, immutableList) + Assertions.assertNotEquals(mutableCopy, immutableList) + } + + @Test + fun `toImmutableListUnsafe() should not create a defensive copy`() { + val list = listOf(1, 2, 3) + val mutableCopy = list.toMutableList() + val immutableList = mutableCopy.toImmutableListUnsafe() + + // Add a value to `mutableCopy` and then make sure that ImmutableList is now [1, 2, 3, 4] + mutableCopy.add(4) + Assertions.assertNotEquals(list, immutableList) + assertEquals(mutableCopy, immutableList) + } + } + + @Nested + inner class ArrayBackedImmutableListTest { + @Test + fun `equals() should be the same as a standard library list`() { + val array = arrayOf(1, 2, 3) + val list = listOf(*array) + + val immutableListUnsafe = array.toImmutableListUnsafe() + assertEquals(list, immutableListUnsafe) + assertEquals(immutableListUnsafe, list) + assertEquals(immutableListUnsafe, immutableListUnsafe) + + assertNotEquals(immutableListUnsafe, listOf(1)) + assertNotEquals(immutableListUnsafe, listOf(2, 3, 4)) + assertNotEquals(immutableListUnsafe, "foo") + } + + @Test + fun `hashCode() should return the same as a standard library list`() { + val array = arrayOf(1, 2, 3) + val list = listOf(*array) + + val immutableListUnsafe = array.toImmutableListUnsafe() + assertEquals(list.hashCode(), immutableListUnsafe.hashCode()) + } + + @Test + fun `toString() should return the same as a standard library list`() { + val array = arrayOf(1, 2, 3) + val list = listOf(*array) + + val immutableListUnsafe = array.toImmutableListUnsafe() + assertEquals(list.toString(), immutableListUnsafe.toString()) + } + + @Test + fun `toImmutableListUnsafe() should return the singleton EMPTY when this is empty`() { + val immutableList = arrayOf().toImmutableListUnsafe() + assertSame(ImmutableList.EMPTY, immutableList) + } + + @Test + fun `toImmutableListUnsafe() should not create a defensive copy`() { + val array = arrayOf(1, 2, 3) + val list = listOf(*array) + val immutableList = array.toImmutableListUnsafe() + + // Change a value in `array` and then make sure that ImmutableList is now [4, 2, 3] + array[0] = 4 + Assertions.assertNotEquals(list, immutableList) + assertEquals(array.asList(), immutableList) + } + } +} diff --git a/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableMapTest.kt b/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableMapTest.kt new file mode 100644 index 0000000..3de2c6c --- /dev/null +++ b/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableMapTest.kt @@ -0,0 +1,194 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +package com.amazon.ionelement.impl.collections + +import com.amazon.ionelement.api.* +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals +import kotlin.test.assertTrue +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test + +class ImmutableMapTest { + + @Nested + inner class MapBackedImmutableMapTest { + @Test + fun `equals() should be the same as a standard library map`() { + val map = mapOf( + 1 to "a", + 2 to "b", + 3 to "c", + ) + + val immutableMapUnsafe = map.toImmutableMapUnsafe() + assertEquals(map, immutableMapUnsafe) + assertEquals(immutableMapUnsafe, map) + assertEquals(immutableMapUnsafe, immutableMapUnsafe) + + val immutableMap = map.toImmutableMap() + assertEquals(map, immutableMap) + assertEquals(immutableMap, map) + assertEquals(immutableMap, immutableMap) + + assertNotEquals(immutableMap, mapOf(1 to "a")) + assertNotEquals(immutableMap, "foo") + } + + @Test + fun `hashCode() should be the same as a standard library map`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ) + + val immutableMapUnsafe = map.toImmutableMapUnsafe() + assertEquals(map.hashCode(), immutableMapUnsafe.hashCode()) + + val immutableMap = map.toImmutableMap() + assertEquals(map.hashCode(), immutableMap.hashCode()) + } + + @Test + fun `toString() should be the same as a standard library map`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ) + + val immutableMapUnsafe = map.toImmutableMapUnsafe() + assertEquals(map.toString(), immutableMapUnsafe.toString()) + + val immutableMap = map.toImmutableMap() + assertEquals(map.toString(), immutableMap.toString()) + } + + @Test + fun `toImmutableMap() should return the singleton EMPTY when this is empty`() { + val immutableMap = mapOf().toImmutableMap() + Assertions.assertSame(ImmutableMap.EMPTY, immutableMap) + } + + @Test + fun `toImmutableMapUnsafe() should return the singleton EMPTY when this is empty`() { + val immutableMap = mapOf().toImmutableMapUnsafe() + Assertions.assertSame(ImmutableMap.EMPTY, immutableMap) + } + + @Test + fun `toImmutableMap() should return this when this is already a ImmutableMap`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ).toImmutableMap() + Assertions.assertSame(map, map.toImmutableMap()) + } + + @Test + fun `toImmutableMapUnsafe() should return this when this is already a ImmutableMap`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ).toImmutableMap() + Assertions.assertSame(map, map.toImmutableMapUnsafe()) + } + + @Test + fun `toImmutableMap() should create a defensive copy`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ) + val mutableCopy = map.toMutableMap() + val immutableMap = mutableCopy.toImmutableMap() + + // Add a value to `mutableCopy` and then make sure that ImmutableMap is still [1, 2, 3] + mutableCopy["d"] = 4 + assertEquals(map, immutableMap) + Assertions.assertNotEquals(mutableCopy, immutableMap) + } + + @Test + fun `toImmutableMapUnsafe() should not create a defensive copy`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ) + val mutableCopy = map.toMutableMap() + val immutableMap = mutableCopy.toImmutableMapUnsafe() + + // Add a value to `mutableCopy` and then make sure that ImmutableMap now has "d" -> 4 + mutableCopy["d"] = 4 + Assertions.assertNotEquals(map, immutableMap) + assertEquals(mutableCopy, immutableMap) + } + } + + @Nested + inner class IonLocationBackedImmutableMapTest { + private val location = IonBinaryLocation(1) + private val locationMap: Map = location.toMetaContainer() + private val generalMap = mapOf(ION_LOCATION_META_TAG to location) + + @Test + fun `equals() should be the same as a standard library map`() { + assertEquals(generalMap, locationMap) + assertEquals(locationMap, generalMap) + assertEquals(locationMap, locationMap) + assertNotEquals(locationMap, mapOf(ION_LOCATION_META_TAG to IonBinaryLocation(1), "foo-bar" to IonBinaryLocation(99))) + assertNotEquals(locationMap, setOf(location)) + assertNotEquals(locationMap, mapOf(ION_LOCATION_META_TAG to IonBinaryLocation(99))) + assertNotEquals(locationMap, mapOf("foo-bar" to location)) + } + + @Test + fun `hashCode() should be the same as a standard library map`() = assertEquals(generalMap.hashCode(), locationMap.hashCode()) + + @Test + fun `toString() should be the same as a standard library map`() = assertEquals(generalMap.toString(), locationMap.toString()) + + @Test + fun `keys should be the same as an equivalent standard library map`() = assertEquals(generalMap.keys, locationMap.keys) + + @Test + fun `values should be the same as an equivalent standard library map`() = assertEquals(generalMap.values, locationMap.values) + + @Test + fun `entries should the same as an equivalent standard library map`() = assertEquals(generalMap.entries, locationMap.entries) + + @Test + fun `size should be 1`() = assertEquals(1, locationMap.size) + + @Test + fun `isEmpty() should return false`() = assertFalse(locationMap.isEmpty()) + + @Test + fun `get(ION_LOCATION_META_TAG) should return the expected value`() = assertEquals(location, locationMap[ION_LOCATION_META_TAG]) + + @Test + fun `get() should return null for any other key`() = assertNull(locationMap["foo-bar"]) + + @Test + fun `containsKey(ION_LOCATION_META_TAG) should return true`() = assertTrue(locationMap.containsKey(ION_LOCATION_META_TAG)) + + @Test + fun `containsKey() should return false for any other key`() = assertFalse(locationMap.containsKey("foo-bar")) + + @Test + fun `containsValue() should return true for the expected IonLocation`() = assertTrue(locationMap.containsValue(location)) + + @Test + fun `containsValue() should return false for any other value`() = assertFalse(locationMap.containsValue(ION_LOCATION_META_TAG)) + } +} From 06cb8034a7d525ece7afc42d5e2ad81ec6a8cb2b Mon Sep 17 00:00:00 2001 From: Matthew Pope Date: Fri, 2 Aug 2024 10:26:34 -0700 Subject: [PATCH 2/4] Clean up unused code --- .../com/amazon/ionelement/impl/IonElementLoaderImpl.kt | 10 ---------- src/main/kotlin/com/amazon/ionelement/impl/ListView.kt | 3 --- 2 files changed, 13 deletions(-) delete mode 100644 src/main/kotlin/com/amazon/ionelement/impl/ListView.kt diff --git a/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt b/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt index 1cc72cd..997e269 100644 --- a/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt +++ b/src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt @@ -45,16 +45,6 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions } } - private fun IonReader.getSpanProvider(): SpanProvider? = this.asFacet(SpanProvider::class.java) - - private fun SpanProvider.currentLocation(): IonLocation? { - return when (val currentSpan = currentSpan()) { - is TextSpan -> IonTextLocation(currentSpan.startLine, currentSpan.startColumn) - is OffsetSpan -> IonBinaryLocation(currentSpan.startOffset) - else -> null - } - } - private fun IonReader.currentLocation(): IonLocation? = when { // Can't attempt to get a SpanProvider unless we're on a value diff --git a/src/main/kotlin/com/amazon/ionelement/impl/ListView.kt b/src/main/kotlin/com/amazon/ionelement/impl/ListView.kt deleted file mode 100644 index 92d9c5c..0000000 --- a/src/main/kotlin/com/amazon/ionelement/impl/ListView.kt +++ /dev/null @@ -1,3 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 -package com.amazon.ionelement.impl From effed62098c8d69809f0a45e908d049feb369919 Mon Sep 17 00:00:00 2001 From: Matthew Pope Date: Sun, 4 Aug 2024 20:33:13 -0700 Subject: [PATCH 3/4] Use Immutable collection adapters from kotlinx.collections.immutables --- .../kotlin/com/amazon/ionelement/api/Ion.kt | 12 +- .../impl/collections/ImmutableList.kt | 92 --------- .../impl/collections/ImmutableMap.kt | 96 --------- .../IonLocationBackedImmutableMap.kt | 46 +++++ .../ionelement/impl/collections/extensions.kt | 87 ++++++++ .../impl/collections/ExtensionsTest.kt | 149 ++++++++++++++ .../impl/collections/ImmutableListTest.kt | 162 --------------- .../impl/collections/ImmutableMapTest.kt | 194 ------------------ .../IonLocationBackedImmutableMapTest.kt | 67 ++++++ 9 files changed, 355 insertions(+), 550 deletions(-) delete mode 100644 src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableList.kt delete mode 100644 src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableMap.kt create mode 100644 src/main/kotlin/com/amazon/ionelement/impl/collections/IonLocationBackedImmutableMap.kt create mode 100644 src/main/kotlin/com/amazon/ionelement/impl/collections/extensions.kt create mode 100644 src/test/kotlin/com/amazon/ionelement/impl/collections/ExtensionsTest.kt delete mode 100644 src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableListTest.kt delete mode 100644 src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableMapTest.kt create mode 100644 src/test/kotlin/com/amazon/ionelement/impl/collections/IonLocationBackedImmutableMapTest.kt diff --git a/src/main/kotlin/com/amazon/ionelement/api/Ion.kt b/src/main/kotlin/com/amazon/ionelement/api/Ion.kt index 27930f2..2ad84c3 100644 --- a/src/main/kotlin/com/amazon/ionelement/api/Ion.kt +++ b/src/main/kotlin/com/amazon/ionelement/api/Ion.kt @@ -520,13 +520,13 @@ public fun buildStruct( } // Memoized empty instances of our container types. -private val EMPTY_LIST = ListElementImpl(ImmutableList.EMPTY, ImmutableList.EMPTY, EMPTY_METAS) -private val EMPTY_SEXP = SexpElementImpl(ImmutableList.EMPTY, ImmutableList.EMPTY, EMPTY_METAS) -private val EMPTY_STRUCT = StructElementImpl(ImmutableList.EMPTY, ImmutableList.EMPTY, EMPTY_METAS) -private val EMPTY_BLOB = BlobElementImpl(ByteArray(0), ImmutableList.EMPTY, EMPTY_METAS) -private val EMPTY_CLOB = ClobElementImpl(ByteArray(0), ImmutableList.EMPTY, EMPTY_METAS) +private val EMPTY_LIST = ListElementImpl(EMPTY_IMMUTABLE_LIST, EMPTY_IMMUTABLE_LIST, EMPTY_METAS) +private val EMPTY_SEXP = SexpElementImpl(EMPTY_IMMUTABLE_LIST, EMPTY_IMMUTABLE_LIST, EMPTY_METAS) +private val EMPTY_STRUCT = StructElementImpl(EMPTY_IMMUTABLE_LIST, EMPTY_IMMUTABLE_LIST, EMPTY_METAS) +private val EMPTY_BLOB = BlobElementImpl(ByteArray(0), EMPTY_IMMUTABLE_LIST, EMPTY_METAS) +private val EMPTY_CLOB = ClobElementImpl(ByteArray(0), EMPTY_IMMUTABLE_LIST, EMPTY_METAS) // Memoized instances of all of our null values. private val ALL_NULLS = ElementType.values().map { - it to NullElementImpl(it, ImmutableList.EMPTY, EMPTY_METAS) as IonElement + it to NullElementImpl(it, EMPTY_IMMUTABLE_LIST, EMPTY_METAS) as IonElement }.toMap() diff --git a/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableList.kt b/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableList.kt deleted file mode 100644 index e1bc006..0000000 --- a/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableList.kt +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 -package com.amazon.ionelement.impl.collections - -import com.amazon.ionelement.impl.collections.ImmutableList.Companion.EMPTY - -/** - * A [List] that (unlike the standard library lists) cannot be modified from Java. - * - * We cannot use `List.of(...)` because those were introduced in JDK 9, and we still - * support JDK 8. - */ -// TODO: Mark as sealed once we're on a higher version of Kotlin. -internal interface ImmutableList : List { - companion object { - val EMPTY: ImmutableList = ListBackedImmutableList(emptyList()) - } -} - -/** - * Wraps any [List] as an [ImmutableList]. - */ -private class ListBackedImmutableList(private val list: List) : ImmutableList, List by list { - override fun toString(): String = list.toString() - override fun equals(other: Any?): Boolean = list == other - override fun hashCode(): Int = list.hashCode() -} - -/** - * Wraps an Array as an [ImmutableList]. - */ -private class ArrayBackedImmutableList(private val array: Array) : ImmutableList, AbstractList() { - override val size: Int get() = array.size - override fun get(index: Int): T = array[index] - - override fun toString(): String = array.contentDeepToString() - - override fun equals(other: Any?): Boolean { - if (this === other) return true - if (other !is List<*>) return false - if (other.size != array.size) return false - for (i in array.indices) { - if (array[i] != other[i]) return false - } - return true - } - - override fun hashCode(): Int = array.contentDeepHashCode() -} - -/** - * Creates a [ImmutableList] for [this]. - * This function creates a defensive copy of [this] unless [this] is already a [ImmutableList]. - */ -internal fun Iterable.toImmutableList(): ImmutableList { - if (this is ImmutableList) return this - val isEmpty = if (this is Collection<*>) { - this.isEmpty() - } else { - !this.iterator().hasNext() - } - return if (isEmpty) EMPTY else ListBackedImmutableList(this.toList()) -} - -/** - * Creates a [ImmutableList] for [this]. - * This function creates a defensive copy of [this] unless [this] is already a [ImmutableList]. - */ -internal fun List.toImmutableList(): ImmutableList { - if (this is ImmutableList) return this - if (isEmpty()) return EMPTY - return ListBackedImmutableList(this.toList()) -} - -/** - * Creates a [ImmutableList] for [this] without making a defensive copy. - * Only call this method if you are sure that [this] cannot leak anywhere it could be mutated. - */ -internal fun List.toImmutableListUnsafe(): ImmutableList { - if (this is ImmutableList) return this - if (isEmpty()) return EMPTY - return ListBackedImmutableList(this) -} - -/** - * Creates a [ImmutableList] for [this] without making a defensive copy. - * Only call this method if you are sure that [this] cannot leak anywhere it could be mutated. - */ -internal fun Array.toImmutableListUnsafe(): ImmutableList { - if (isEmpty()) return EMPTY - return ArrayBackedImmutableList(this) -} diff --git a/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableMap.kt b/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableMap.kt deleted file mode 100644 index 5f838ee..0000000 --- a/src/main/kotlin/com/amazon/ionelement/impl/collections/ImmutableMap.kt +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 -package com.amazon.ionelement.impl.collections - -import com.amazon.ionelement.api.* -import java.util.AbstractMap.SimpleImmutableEntry - -/** - * A [Map] that (unlike the standard library maps) cannot be modified from Java. - * - * We cannot use `Map.of(...)` because those were introduced in JDK 9, and we still - * support JDK 8. - */ -// TODO: Mark as sealed once we're on a higher version of Kotlin. -internal interface ImmutableMap : Map { - companion object { - val EMPTY: ImmutableMap = MapBackedImmutableMap(emptyMap()) - } -} - -/** - * Wraps any [Map] as an [ImmutableMap]. - */ -private class MapBackedImmutableMap(private val map: Map) : ImmutableMap, Map by map { - override fun toString(): String = map.toString() - override fun equals(other: Any?): Boolean = map == other - override fun hashCode(): Int = map.hashCode() -} - -/** - * Specialized implementation of [Map] that always has a size of 1 and contains only the key [ION_LOCATION_META_TAG]. - * - * This exists so that we can populate location metadata with as little overhead as possible. - * On 64-bit Hotspot JVM, this has an object size of only 16 bytes compared to [java.util.Collections.singletonMap] - * which creates a map with an object size of 40 bytes. - * - * We assume that by far the most common use case for this class is calling `get(ION_LOCATION_META_TAG)` - * rather than general `Map` operations. - */ -private class IonLocationBackedImmutableMap(private val value: IonLocation) : ImmutableMap { - override val size: Int get() = 1 - override fun isEmpty(): Boolean = false - - override fun get(key: String): IonLocation? = if (key == ION_LOCATION_META_TAG) value else null - override fun containsValue(value: IonLocation): Boolean = value == this.value - override fun containsKey(key: String): Boolean = key == ION_LOCATION_META_TAG - - override val keys: Set get() = KEY_SET - // We could memoize these values, but that would increase the memory footprint of this class. - override val values: Collection get() = setOf(value) - override val entries: Set> get() = setOf(SimpleImmutableEntry(ION_LOCATION_META_TAG, value)) - - override fun toString(): String = "{$ION_LOCATION_META_TAG=$value}" - override fun equals(other: Any?): Boolean { - if (this === other) return true - if (other !is Map<*, *>) return false - if (other.size != 1) return false - return other[ION_LOCATION_META_TAG] == value - } - override fun hashCode(): Int = ION_LOCATION_META_TAG.hashCode() xor value.hashCode() - - companion object { - private val KEY_SET = setOf(ION_LOCATION_META_TAG) - } -} - -/** - * Creates a [ImmutableMap] for [this] without making a defensive copy. - * Only call this method if you are sure that [this] cannot leak anywhere it could be mutated. - */ -internal fun Map.toImmutableMapUnsafe(): ImmutableMap { - if (this is ImmutableMap) return this - // Empty ImmutableMap can be safely cast to any `` because it is empty. - @Suppress("UNCHECKED_CAST") - if (isEmpty()) return (ImmutableMap.EMPTY as ImmutableMap) - return MapBackedImmutableMap(this) -} - -/** - * Creates a [ImmutableMap] for [this]. - * This function creates a defensive copy of [this] unless [this] is already a [ImmutableMap]. - */ -internal fun Map.toImmutableMap(): ImmutableMap { - if (this is ImmutableMap) return this - // Empty ImmutableMap can be safely cast to any `` because it is empty. - @Suppress("UNCHECKED_CAST") - if (isEmpty()) return (ImmutableMap.EMPTY as ImmutableMap) - return MapBackedImmutableMap(toMap()) -} - -/** - * Creates an [ImmutableMetaContainer] ([ImmutableMap]) that holds [this] [IonLocation] instance. - */ -internal fun IonLocation.toMetaContainer(): ImmutableMap { - return IonLocationBackedImmutableMap(this) -} diff --git a/src/main/kotlin/com/amazon/ionelement/impl/collections/IonLocationBackedImmutableMap.kt b/src/main/kotlin/com/amazon/ionelement/impl/collections/IonLocationBackedImmutableMap.kt new file mode 100644 index 0000000..7800779 --- /dev/null +++ b/src/main/kotlin/com/amazon/ionelement/impl/collections/IonLocationBackedImmutableMap.kt @@ -0,0 +1,46 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +package com.amazon.ionelement.impl.collections + +import com.amazon.ionelement.api.* +import java.util.AbstractMap.SimpleImmutableEntry +import kotlinx.collections.immutable.ImmutableCollection +import kotlinx.collections.immutable.ImmutableSet +import kotlinx.collections.immutable.adapters.ImmutableSetAdapter + +/** + * Specialized implementation of [Map] that always has a size of 1 and contains only the key [ION_LOCATION_META_TAG]. + * + * This exists so that we can populate location metadata with as little overhead as possible. + * On 64-bit Hotspot JVM, this has an object size of only 16 bytes compared to [java.util.Collections.singletonMap] + * which creates a map with an object size of 40 bytes. + * + * We assume that by far the most common use case for this class is calling `get(ION_LOCATION_META_TAG)` + * rather than general `Map` operations. + */ +internal class IonLocationBackedImmutableMap(private val value: IonLocation) : ImmutableMap { + override val size: Int get() = 1 + override fun isEmpty(): Boolean = false + + override fun get(key: String): IonLocation? = if (key == ION_LOCATION_META_TAG) value else null + override fun containsValue(value: IonLocation): Boolean = value == this.value + override fun containsKey(key: String): Boolean = key == ION_LOCATION_META_TAG + + override val keys: ImmutableSet get() = KEY_SET + // We could memoize these values, but that would increase the memory footprint of this class. + override val values: ImmutableCollection get() = ImmutableSetAdapter(setOf(value)) + override val entries: ImmutableSet> get() = ImmutableSetAdapter(setOf(SimpleImmutableEntry(ION_LOCATION_META_TAG, value))) + + override fun toString(): String = "{$ION_LOCATION_META_TAG=$value}" + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is Map<*, *>) return false + if (other.size != 1) return false + return other[ION_LOCATION_META_TAG] == value + } + override fun hashCode(): Int = ION_LOCATION_META_TAG.hashCode() xor value.hashCode() + + companion object { + private val KEY_SET = ImmutableSetAdapter(setOf(ION_LOCATION_META_TAG)) + } +} diff --git a/src/main/kotlin/com/amazon/ionelement/impl/collections/extensions.kt b/src/main/kotlin/com/amazon/ionelement/impl/collections/extensions.kt new file mode 100644 index 0000000..dc73c60 --- /dev/null +++ b/src/main/kotlin/com/amazon/ionelement/impl/collections/extensions.kt @@ -0,0 +1,87 @@ +package com.amazon.ionelement.impl.collections + +import com.amazon.ionelement.api.* +import kotlinx.collections.immutable.adapters.ImmutableListAdapter +import kotlinx.collections.immutable.adapters.ImmutableMapAdapter + +typealias ImmutableList = kotlinx.collections.immutable.ImmutableList +typealias ImmutableMap = kotlinx.collections.immutable.ImmutableMap + +internal val EMPTY_IMMUTABLE_MAP = ImmutableMapAdapter(emptyMap()) +internal val EMPTY_IMMUTABLE_LIST = ImmutableListAdapter(emptyList()) + +/** + * Creates a [ImmutableMap] for [this] without making a defensive copy. + * Only call this method if you are sure that [this] cannot leak anywhere it could be mutated. + */ +internal fun Map.toImmutableMapUnsafe(): ImmutableMap { + if (this is ImmutableMap) return this + // Empty ImmutableMap can be safely cast to any `` because it is empty. + @Suppress("UNCHECKED_CAST") + if (isEmpty()) return EMPTY_IMMUTABLE_MAP as ImmutableMap + return ImmutableMapAdapter(this) +} + +/** + * Creates a [ImmutableMap] for [this]. + * This function creates a defensive copy of [this] unless [this] is already a [ImmutableMap]. + */ +internal fun Map.toImmutableMap(): ImmutableMap { + if (this is ImmutableMap) return this + // Empty ImmutableMap can be safely cast to any `` because it is empty. + @Suppress("UNCHECKED_CAST") + if (isEmpty()) return (EMPTY_IMMUTABLE_MAP as ImmutableMap) + return ImmutableMapAdapter(toMap()) +} + +/** + * Creates an [ImmutableMetaContainer] ([ImmutableMap]) that holds [this] [IonLocation] instance. + */ +internal fun IonLocation.toMetaContainer(): ImmutableMap { + return IonLocationBackedImmutableMap(this) +} + +/** + * Creates a [ImmutableList] for [this]. + * This function creates a defensive copy of [this] unless [this] is already a [ImmutableList]. + */ +internal fun Iterable.toImmutableList(): ImmutableList { + if (this is ImmutableList) return this + val isEmpty = if (this is Collection<*>) { + this.isEmpty() + } else { + !this.iterator().hasNext() + } + return if (isEmpty) EMPTY_IMMUTABLE_LIST else ImmutableListAdapter(this.toList()) +} + +/** + * Creates a [ImmutableList] for [this]. + * This function creates a defensive copy of [this] unless [this] is already a [ImmutableList]. + */ +internal fun List.toImmutableList(): ImmutableList { + if (this is ImmutableList) return this + if (isEmpty()) return EMPTY_IMMUTABLE_LIST + return ImmutableListAdapter(this.toList()) +} + +/** + * Creates a [ImmutableList] for [this] without making a defensive copy. + * Only call this method if you are sure that [this] cannot leak anywhere it could be mutated. + */ +internal fun List.toImmutableListUnsafe(): ImmutableList { + if (this is ImmutableList) return this + if (isEmpty()) return EMPTY_IMMUTABLE_LIST + return ImmutableListAdapter(this) +} + +/** + * Creates a [ImmutableList] for [this] without making a defensive copy. + * Only call this method if you are sure that [this] cannot leak anywhere it could be mutated. + */ +internal fun Array.toImmutableListUnsafe(): ImmutableList { + if (isEmpty()) return EMPTY_IMMUTABLE_LIST + // This wraps the array in an ArrayList and then in an ImmutableListAdapter. In theory, we could reduce the overhead + // even further but using only a single wrapper layer, if we created such a thing. + return ImmutableListAdapter(asList()) +} diff --git a/src/test/kotlin/com/amazon/ionelement/impl/collections/ExtensionsTest.kt b/src/test/kotlin/com/amazon/ionelement/impl/collections/ExtensionsTest.kt new file mode 100644 index 0000000..e754486 --- /dev/null +++ b/src/test/kotlin/com/amazon/ionelement/impl/collections/ExtensionsTest.kt @@ -0,0 +1,149 @@ +package com.amazon.ionelement.impl.collections + +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Test + +class ExtensionsTest { + + @Test + fun `toImmutableMap() should return the singleton EMPTY when this is empty`() { + val immutableMap = mapOf().toImmutableMap() + Assertions.assertSame(EMPTY_IMMUTABLE_MAP, immutableMap) + } + + @Test + fun `toImmutableMapUnsafe() should return the singleton EMPTY when this is empty`() { + val immutableMap = mapOf().toImmutableMapUnsafe() + Assertions.assertSame(EMPTY_IMMUTABLE_MAP, immutableMap) + } + + @Test + fun `toImmutableMap() should return this when this is already a ImmutableMap`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ).toImmutableMap() + Assertions.assertSame(map, map.toImmutableMap()) + } + + @Test + fun `toImmutableMapUnsafe() should return this when this is already a ImmutableMap`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ).toImmutableMap() + Assertions.assertSame(map, map.toImmutableMapUnsafe()) + } + + @Test + fun `toImmutableMap() should create a defensive copy`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ) + val mutableCopy = map.toMutableMap() + val immutableMap = mutableCopy.toImmutableMap() + + // Add a value to `mutableCopy` and then make sure that ImmutableMap is still [1, 2, 3] + mutableCopy["d"] = 4 + assertEquals(map, immutableMap) + Assertions.assertNotEquals(mutableCopy, immutableMap) + } + + @Test + fun `toImmutableMapUnsafe() should not create a defensive copy`() { + val map = mapOf( + "a" to 1, + "b" to 2, + "c" to 3, + ) + val mutableCopy = map.toMutableMap() + val immutableMap = mutableCopy.toImmutableMapUnsafe() + + // Add a value to `mutableCopy` and then make sure that ImmutableMap now has "d" -> 4 + mutableCopy["d"] = 4 + Assertions.assertNotEquals(map, immutableMap) + assertEquals(mutableCopy, immutableMap) + } + + @Test + fun `List toImmutableList() should return the singleton EMPTY when this is empty`() { + assertSame(EMPTY_IMMUTABLE_LIST, listOf().toImmutableList()) + } + + @Test + fun `Iterable toImmutableList() should return the singleton EMPTY when this is empty`() { + assertSame(EMPTY_IMMUTABLE_LIST, sequenceOf().asIterable().toImmutableList()) + } + + @Test + fun `List toImmutableListUnsafe() should return the singleton EMPTY when this is empty`() { + val immutableList = listOf().toImmutableListUnsafe() + assertSame(EMPTY_IMMUTABLE_LIST, immutableList) + } + + @Test + fun `List toImmutableList() should return this when this is already a ImmutableList`() { + val list = listOf(1, 2, 3).toImmutableList() + assertSame(list, list.toImmutableList()) + } + + @Test + fun `Iterable toImmutableList() should return this when this is already a ImmutableList`() { + val list = listOf(1, 2, 3).toImmutableList() + assertSame(list, (list as Iterable).toImmutableList()) + } + + @Test + fun `List toImmutableListUnsafe() should return this when this is already a ImmutableList`() { + val list = listOf(1, 2, 3).toImmutableList() + assertSame(list, list.toImmutableListUnsafe()) + } + + @Test + fun `List toImmutableList() should create a defensive copy`() { + val list = listOf(1, 2, 3) + val mutableCopy = list.toMutableList() + val immutableList = mutableCopy.toImmutableList() + + // Add a value to `mutableCopy` and then make sure that ImmutableList is still [1, 2, 3] + mutableCopy.add(4) + assertEquals(list, immutableList) + Assertions.assertNotEquals(mutableCopy, immutableList) + } + + @Test + fun `List toImmutableListUnsafe() should not create a defensive copy`() { + val list = listOf(1, 2, 3) + val mutableCopy = list.toMutableList() + val immutableList = mutableCopy.toImmutableListUnsafe() + + // Add a value to `mutableCopy` and then make sure that ImmutableList is now [1, 2, 3, 4] + mutableCopy.add(4) + Assertions.assertNotEquals(list, immutableList) + assertEquals(mutableCopy, immutableList) + } + + @Test + fun `Array toImmutableListUnsafe() should return the singleton EMPTY when this is empty`() { + val immutableList = arrayOf().toImmutableListUnsafe() + assertSame(EMPTY_IMMUTABLE_LIST, immutableList) + } + + @Test + fun `Array toImmutableListUnsafe() should not create a defensive copy`() { + val array = arrayOf(1, 2, 3) + val list = listOf(*array) + val immutableList = array.toImmutableListUnsafe() + + // Change a value in `array` and then make sure that ImmutableList is now [4, 2, 3] + array[0] = 4 + Assertions.assertNotEquals(list, immutableList) + assertEquals(array.asList(), immutableList) + } +} diff --git a/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableListTest.kt b/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableListTest.kt deleted file mode 100644 index 584be27..0000000 --- a/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableListTest.kt +++ /dev/null @@ -1,162 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 -package com.amazon.ionelement.impl.collections - -import kotlin.test.assertNotEquals -import org.junit.jupiter.api.Assertions -import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertSame -import org.junit.jupiter.api.Nested -import org.junit.jupiter.api.Test - -class ImmutableListTest { - - @Nested - inner class ListBackedImmutableListTest { - @Test - fun `equals() should be the same as a standard library list`() { - val list = listOf(1, 2, 3) - - val immutableListUnsafe = list.toImmutableListUnsafe() - assertEquals(list, immutableListUnsafe) - assertEquals(immutableListUnsafe, list) - assertEquals(immutableListUnsafe, immutableListUnsafe) - - val immutableList = list.toImmutableList() - assertEquals(list, immutableList) - assertEquals(immutableList, list) - assertEquals(immutableList, immutableList) - - assertNotEquals(immutableList, listOf(1)) - assertNotEquals(immutableList, listOf(2, 3, 4)) - assertNotEquals(immutableList, "foo") - } - - @Test - fun `hashCode() should return the same as a standard library list`() { - val list = listOf(1, 2, 3) - - val immutableListUnsafe = list.toImmutableListUnsafe() - assertEquals(list.hashCode(), immutableListUnsafe.hashCode()) - - val immutableList = list.toImmutableList() - assertEquals(list.hashCode(), immutableList.hashCode()) - } - - @Test - fun `toString() should return the same as a standard library list`() { - val list = listOf(1, 2, 3) - - val immutableListUnsafe = list.toImmutableListUnsafe() - assertEquals(list.toString(), immutableListUnsafe.toString()) - - val immutableList = list.toImmutableList() - assertEquals(list.toString(), immutableList.toString()) - } - - @Test - fun `toImmutableList() should return the singleton EMPTY when this is empty`() { - assertSame(ImmutableList.EMPTY, listOf().toImmutableList()) - // Check again to cover the same case in the overload for Iterable - assertSame(ImmutableList.EMPTY, sequenceOf().asIterable().toImmutableList()) - } - - @Test - fun `toImmutableListUnsafe() should return the singleton EMPTY when this is empty`() { - val immutableList = listOf().toImmutableListUnsafe() - assertSame(ImmutableList.EMPTY, immutableList) - } - - @Test - fun `toImmutableList() should return this when this is already a ImmutableList`() { - val list = listOf(1, 2, 3).toImmutableList() - assertSame(list, list.toImmutableList()) - // Check again to cover the same case in the overload for Iterable - assertSame(list, (list as Iterable).toImmutableList()) - } - - @Test - fun `toImmutableListUnsafe() should return this when this is already a ImmutableList`() { - val list = listOf(1, 2, 3).toImmutableList() - assertSame(list, list.toImmutableListUnsafe()) - } - - @Test - fun `toImmutableList() should create a defensive copy`() { - val list = listOf(1, 2, 3) - val mutableCopy = list.toMutableList() - val immutableList = mutableCopy.toImmutableList() - - // Add a value to `mutableCopy` and then make sure that ImmutableList is still [1, 2, 3] - mutableCopy.add(4) - assertEquals(list, immutableList) - Assertions.assertNotEquals(mutableCopy, immutableList) - } - - @Test - fun `toImmutableListUnsafe() should not create a defensive copy`() { - val list = listOf(1, 2, 3) - val mutableCopy = list.toMutableList() - val immutableList = mutableCopy.toImmutableListUnsafe() - - // Add a value to `mutableCopy` and then make sure that ImmutableList is now [1, 2, 3, 4] - mutableCopy.add(4) - Assertions.assertNotEquals(list, immutableList) - assertEquals(mutableCopy, immutableList) - } - } - - @Nested - inner class ArrayBackedImmutableListTest { - @Test - fun `equals() should be the same as a standard library list`() { - val array = arrayOf(1, 2, 3) - val list = listOf(*array) - - val immutableListUnsafe = array.toImmutableListUnsafe() - assertEquals(list, immutableListUnsafe) - assertEquals(immutableListUnsafe, list) - assertEquals(immutableListUnsafe, immutableListUnsafe) - - assertNotEquals(immutableListUnsafe, listOf(1)) - assertNotEquals(immutableListUnsafe, listOf(2, 3, 4)) - assertNotEquals(immutableListUnsafe, "foo") - } - - @Test - fun `hashCode() should return the same as a standard library list`() { - val array = arrayOf(1, 2, 3) - val list = listOf(*array) - - val immutableListUnsafe = array.toImmutableListUnsafe() - assertEquals(list.hashCode(), immutableListUnsafe.hashCode()) - } - - @Test - fun `toString() should return the same as a standard library list`() { - val array = arrayOf(1, 2, 3) - val list = listOf(*array) - - val immutableListUnsafe = array.toImmutableListUnsafe() - assertEquals(list.toString(), immutableListUnsafe.toString()) - } - - @Test - fun `toImmutableListUnsafe() should return the singleton EMPTY when this is empty`() { - val immutableList = arrayOf().toImmutableListUnsafe() - assertSame(ImmutableList.EMPTY, immutableList) - } - - @Test - fun `toImmutableListUnsafe() should not create a defensive copy`() { - val array = arrayOf(1, 2, 3) - val list = listOf(*array) - val immutableList = array.toImmutableListUnsafe() - - // Change a value in `array` and then make sure that ImmutableList is now [4, 2, 3] - array[0] = 4 - Assertions.assertNotEquals(list, immutableList) - assertEquals(array.asList(), immutableList) - } - } -} diff --git a/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableMapTest.kt b/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableMapTest.kt deleted file mode 100644 index 3de2c6c..0000000 --- a/src/test/kotlin/com/amazon/ionelement/impl/collections/ImmutableMapTest.kt +++ /dev/null @@ -1,194 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 -package com.amazon.ionelement.impl.collections - -import com.amazon.ionelement.api.* -import kotlin.test.assertEquals -import kotlin.test.assertNotEquals -import kotlin.test.assertTrue -import org.junit.jupiter.api.Assertions -import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertFalse -import org.junit.jupiter.api.Assertions.assertNull -import org.junit.jupiter.api.Assertions.assertTrue -import org.junit.jupiter.api.Nested -import org.junit.jupiter.api.Test - -class ImmutableMapTest { - - @Nested - inner class MapBackedImmutableMapTest { - @Test - fun `equals() should be the same as a standard library map`() { - val map = mapOf( - 1 to "a", - 2 to "b", - 3 to "c", - ) - - val immutableMapUnsafe = map.toImmutableMapUnsafe() - assertEquals(map, immutableMapUnsafe) - assertEquals(immutableMapUnsafe, map) - assertEquals(immutableMapUnsafe, immutableMapUnsafe) - - val immutableMap = map.toImmutableMap() - assertEquals(map, immutableMap) - assertEquals(immutableMap, map) - assertEquals(immutableMap, immutableMap) - - assertNotEquals(immutableMap, mapOf(1 to "a")) - assertNotEquals(immutableMap, "foo") - } - - @Test - fun `hashCode() should be the same as a standard library map`() { - val map = mapOf( - "a" to 1, - "b" to 2, - "c" to 3, - ) - - val immutableMapUnsafe = map.toImmutableMapUnsafe() - assertEquals(map.hashCode(), immutableMapUnsafe.hashCode()) - - val immutableMap = map.toImmutableMap() - assertEquals(map.hashCode(), immutableMap.hashCode()) - } - - @Test - fun `toString() should be the same as a standard library map`() { - val map = mapOf( - "a" to 1, - "b" to 2, - "c" to 3, - ) - - val immutableMapUnsafe = map.toImmutableMapUnsafe() - assertEquals(map.toString(), immutableMapUnsafe.toString()) - - val immutableMap = map.toImmutableMap() - assertEquals(map.toString(), immutableMap.toString()) - } - - @Test - fun `toImmutableMap() should return the singleton EMPTY when this is empty`() { - val immutableMap = mapOf().toImmutableMap() - Assertions.assertSame(ImmutableMap.EMPTY, immutableMap) - } - - @Test - fun `toImmutableMapUnsafe() should return the singleton EMPTY when this is empty`() { - val immutableMap = mapOf().toImmutableMapUnsafe() - Assertions.assertSame(ImmutableMap.EMPTY, immutableMap) - } - - @Test - fun `toImmutableMap() should return this when this is already a ImmutableMap`() { - val map = mapOf( - "a" to 1, - "b" to 2, - "c" to 3, - ).toImmutableMap() - Assertions.assertSame(map, map.toImmutableMap()) - } - - @Test - fun `toImmutableMapUnsafe() should return this when this is already a ImmutableMap`() { - val map = mapOf( - "a" to 1, - "b" to 2, - "c" to 3, - ).toImmutableMap() - Assertions.assertSame(map, map.toImmutableMapUnsafe()) - } - - @Test - fun `toImmutableMap() should create a defensive copy`() { - val map = mapOf( - "a" to 1, - "b" to 2, - "c" to 3, - ) - val mutableCopy = map.toMutableMap() - val immutableMap = mutableCopy.toImmutableMap() - - // Add a value to `mutableCopy` and then make sure that ImmutableMap is still [1, 2, 3] - mutableCopy["d"] = 4 - assertEquals(map, immutableMap) - Assertions.assertNotEquals(mutableCopy, immutableMap) - } - - @Test - fun `toImmutableMapUnsafe() should not create a defensive copy`() { - val map = mapOf( - "a" to 1, - "b" to 2, - "c" to 3, - ) - val mutableCopy = map.toMutableMap() - val immutableMap = mutableCopy.toImmutableMapUnsafe() - - // Add a value to `mutableCopy` and then make sure that ImmutableMap now has "d" -> 4 - mutableCopy["d"] = 4 - Assertions.assertNotEquals(map, immutableMap) - assertEquals(mutableCopy, immutableMap) - } - } - - @Nested - inner class IonLocationBackedImmutableMapTest { - private val location = IonBinaryLocation(1) - private val locationMap: Map = location.toMetaContainer() - private val generalMap = mapOf(ION_LOCATION_META_TAG to location) - - @Test - fun `equals() should be the same as a standard library map`() { - assertEquals(generalMap, locationMap) - assertEquals(locationMap, generalMap) - assertEquals(locationMap, locationMap) - assertNotEquals(locationMap, mapOf(ION_LOCATION_META_TAG to IonBinaryLocation(1), "foo-bar" to IonBinaryLocation(99))) - assertNotEquals(locationMap, setOf(location)) - assertNotEquals(locationMap, mapOf(ION_LOCATION_META_TAG to IonBinaryLocation(99))) - assertNotEquals(locationMap, mapOf("foo-bar" to location)) - } - - @Test - fun `hashCode() should be the same as a standard library map`() = assertEquals(generalMap.hashCode(), locationMap.hashCode()) - - @Test - fun `toString() should be the same as a standard library map`() = assertEquals(generalMap.toString(), locationMap.toString()) - - @Test - fun `keys should be the same as an equivalent standard library map`() = assertEquals(generalMap.keys, locationMap.keys) - - @Test - fun `values should be the same as an equivalent standard library map`() = assertEquals(generalMap.values, locationMap.values) - - @Test - fun `entries should the same as an equivalent standard library map`() = assertEquals(generalMap.entries, locationMap.entries) - - @Test - fun `size should be 1`() = assertEquals(1, locationMap.size) - - @Test - fun `isEmpty() should return false`() = assertFalse(locationMap.isEmpty()) - - @Test - fun `get(ION_LOCATION_META_TAG) should return the expected value`() = assertEquals(location, locationMap[ION_LOCATION_META_TAG]) - - @Test - fun `get() should return null for any other key`() = assertNull(locationMap["foo-bar"]) - - @Test - fun `containsKey(ION_LOCATION_META_TAG) should return true`() = assertTrue(locationMap.containsKey(ION_LOCATION_META_TAG)) - - @Test - fun `containsKey() should return false for any other key`() = assertFalse(locationMap.containsKey("foo-bar")) - - @Test - fun `containsValue() should return true for the expected IonLocation`() = assertTrue(locationMap.containsValue(location)) - - @Test - fun `containsValue() should return false for any other value`() = assertFalse(locationMap.containsValue(ION_LOCATION_META_TAG)) - } -} diff --git a/src/test/kotlin/com/amazon/ionelement/impl/collections/IonLocationBackedImmutableMapTest.kt b/src/test/kotlin/com/amazon/ionelement/impl/collections/IonLocationBackedImmutableMapTest.kt new file mode 100644 index 0000000..a769b1a --- /dev/null +++ b/src/test/kotlin/com/amazon/ionelement/impl/collections/IonLocationBackedImmutableMapTest.kt @@ -0,0 +1,67 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +package com.amazon.ionelement.impl.collections + +import com.amazon.ionelement.api.* +import kotlin.test.assertNotEquals +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +class IonLocationBackedImmutableMapTest { + private val location = IonBinaryLocation(1) + private val locationMap: Map = location.toMetaContainer() + private val generalMap = mapOf(ION_LOCATION_META_TAG to location) + + @Test + fun `equals() should be the same as a standard library map`() { + assertEquals(generalMap, locationMap) + assertEquals(locationMap, generalMap) + assertEquals(locationMap, locationMap) + assertNotEquals(locationMap, mapOf(ION_LOCATION_META_TAG to IonBinaryLocation(1), "foo-bar" to IonBinaryLocation(99))) + assertNotEquals(locationMap, setOf(location)) + assertNotEquals(locationMap, mapOf(ION_LOCATION_META_TAG to IonBinaryLocation(99))) + assertNotEquals(locationMap, mapOf("foo-bar" to location)) + } + + @Test + fun `hashCode() should be the same as a standard library map`() = assertEquals(generalMap.hashCode(), locationMap.hashCode()) + + @Test + fun `toString() should be the same as a standard library map`() = assertEquals(generalMap.toString(), locationMap.toString()) + + @Test + fun `keys should be the same as an equivalent standard library map`() = assertEquals(generalMap.keys, locationMap.keys) + + @Test + fun `values should be the same as an equivalent standard library map`() = assertEquals(generalMap.values, locationMap.values) + + @Test + fun `entries should the same as an equivalent standard library map`() = assertEquals(generalMap.entries, locationMap.entries) + + @Test + fun `size should be 1`() = assertEquals(1, locationMap.size) + + @Test + fun `isEmpty() should return false`() = assertFalse(locationMap.isEmpty()) + + @Test + fun `get(ION_LOCATION_META_TAG) should return the expected value`() = assertEquals(location, locationMap[ION_LOCATION_META_TAG]) + + @Test + fun `get() should return null for any other key`() = assertNull(locationMap["foo-bar"]) + + @Test + fun `containsKey(ION_LOCATION_META_TAG) should return true`() = assertTrue(locationMap.containsKey(ION_LOCATION_META_TAG)) + + @Test + fun `containsKey() should return false for any other key`() = assertFalse(locationMap.containsKey("foo-bar")) + + @Test + fun `containsValue() should return true for the expected IonLocation`() = assertTrue(locationMap.containsValue(location)) + + @Test + fun `containsValue() should return false for any other value`() = assertFalse(locationMap.containsValue(ION_LOCATION_META_TAG)) +} From 7c22144fe83cba00515bbd80ccb231beef0da02d Mon Sep 17 00:00:00 2001 From: Matthew Pope Date: Sun, 4 Aug 2024 20:35:53 -0700 Subject: [PATCH 4/4] Add build.gradle.kts changes forgotten from last commit --- build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/build.gradle.kts b/build.gradle.kts index 61ddff9..87df654 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -30,6 +30,7 @@ val isReleaseVersion: Boolean = !version.toString().endsWith("SNAPSHOT") dependencies { api("com.amazon.ion:ion-java:[1.4.0,)") compileOnly("com.amazon.ion:ion-java:1.4.0") + implementation("org.jetbrains.kotlinx:kotlinx-collections-immutable:0.3.4") implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8") testImplementation("org.jetbrains.kotlin:kotlin-test") testImplementation("org.junit.jupiter:junit-jupiter-api:5.6.2")