diff --git a/api/IonElement.api b/api/IonElement.api index 233cbb4..bec2bf5 100644 --- a/api/IonElement.api +++ b/api/IonElement.api @@ -577,19 +577,19 @@ public abstract interface class com/amazon/ionelement/api/LobElement : com/amazo public abstract fun withoutMetas ()Lcom/amazon/ionelement/api/LobElement; } -public abstract interface class com/amazon/ionelement/api/MutableStructFields : java/lang/Iterable, kotlin/jvm/internal/markers/KMappedMarker { - public abstract fun add (Lcom/amazon/ionelement/api/StructField;)Lcom/amazon/ionelement/api/MutableStructFields; - public abstract fun add (Ljava/lang/String;Lcom/amazon/ionelement/api/IonElement;)Lcom/amazon/ionelement/api/MutableStructFields; +public abstract interface class com/amazon/ionelement/api/MutableStructFields : java/util/Collection, kotlin/jvm/internal/markers/KMutableCollection { + public abstract fun add (Lcom/amazon/ionelement/api/StructField;)Z + public abstract fun add (Ljava/lang/String;Lcom/amazon/ionelement/api/IonElement;)Z + public abstract fun clearField (Ljava/lang/String;)Z public abstract fun containsField (Ljava/lang/String;)Z public abstract fun get (Ljava/lang/String;)Lcom/amazon/ionelement/api/AnyElement; public abstract fun getAll (Ljava/lang/String;)Ljava/util/Collection; public abstract fun getOptional (Ljava/lang/String;)Lcom/amazon/ionelement/api/AnyElement; - public abstract fun plusAssign (Lcom/amazon/ionelement/api/StructField;)V - public abstract fun plusAssign (Ljava/lang/Iterable;)V - public abstract fun remove (Lcom/amazon/ionelement/api/StructField;)Lcom/amazon/ionelement/api/MutableStructFields; - public abstract fun removeAll (Ljava/lang/String;)Lcom/amazon/ionelement/api/MutableStructFields; - public abstract fun set (Ljava/lang/String;Lcom/amazon/ionelement/api/IonElement;)Lcom/amazon/ionelement/api/MutableStructFields; - public abstract fun setAll (Ljava/lang/Iterable;)Lcom/amazon/ionelement/api/MutableStructFields; + public abstract fun plusAssign (Ljava/util/Collection;)V + public abstract fun remove (Lcom/amazon/ionelement/api/StructField;)Z + public abstract fun removeAll (Ljava/util/Collection;)Z + public abstract fun set (Ljava/lang/String;Lcom/amazon/ionelement/api/IonElement;)V + public abstract fun setAll (Ljava/lang/Iterable;)V } public abstract interface class com/amazon/ionelement/api/SeqElement : com/amazon/ionelement/api/ContainerElement { diff --git a/src/com/amazon/ionelement/api/MutableStructFields.kt b/src/com/amazon/ionelement/api/MutableStructFields.kt index b12a173..984d73e 100644 --- a/src/com/amazon/ionelement/api/MutableStructFields.kt +++ b/src/com/amazon/ionelement/api/MutableStructFields.kt @@ -3,7 +3,7 @@ package com.amazon.ionelement.api /** * Represents a mutable view of a collection of struct fields. */ -public interface MutableStructFields : Iterable { +public interface MutableStructFields : MutableCollection { /** * Retrieves the value of the first field found with the specified name. @@ -26,34 +26,44 @@ public interface MutableStructFields : Iterable { /** * If one or more fields with the specified name already exists, this replaces all of them with the value provided. * - * Otherwise, a new field with the given name and value is added to the collection + * Otherwise, a new field with the given name and value is added to the collection. + * + * Returns true if a field was replaced. */ - public operator fun set(fieldName: String, value: IonElement): MutableStructFields + public operator fun set(fieldName: String, value: IonElement) /** * Adds all the given fields to the collection. For existing fields with the same names as the fields provided, all * instances of those fields will be removed. */ - public fun setAll(fields: Iterable): MutableStructFields + public fun setAll(fields: Iterable) /** * Adds a new field to the collection with the given name and value. The collection may have multiple fields with * the same name. */ - public fun add(fieldName: String, value: IonElement): MutableStructFields + public fun add(fieldName: String, value: IonElement): Boolean - /** Adds the given field to the collection. The collection may have multiple fields with the same name. */ - public fun add(field: StructField): MutableStructFields + /** Adds the given field to the collection. The collection may have multiple fields with the same name. + * + * Repeated fields are allowed, so this will always return true. */ + public override fun add(element: StructField): Boolean - /** Removes a random occurrence of a field the matches the given field, or does nothing if no field exists */ - public fun remove(field: StructField): MutableStructFields + /** Removes a random occurrence of a field the matches the given field, or does nothing if no field exists. + * + * Returns true is a field was removed. */ + public override fun remove(element: StructField): Boolean - /** Removes all fields found with the given name or does nothing if no fields exist */ - public fun removeAll(fieldName: String): MutableStructFields + /** Removes all occurrence of a field the matches the given field name, or does nothing if no field exists. + * + * Returns true if a field was removed. */ + public fun clearField(fieldName: String): Boolean - /** Adds the given field to the collection */ - public operator fun plusAssign(field: StructField) + /** Removes all of this collection's elements that are also contained in the specified collection. + * + * At most one field per element in the give collection is removed. */ + public override fun removeAll(elements: Collection): Boolean /** Adds all the given fields to the collection */ - public operator fun plusAssign(fields: Iterable) + public operator fun plusAssign(fields: Collection) } diff --git a/src/com/amazon/ionelement/impl/MutableStructFieldsImpl.kt b/src/com/amazon/ionelement/impl/MutableStructFieldsImpl.kt index b82696a..38d8f33 100644 --- a/src/com/amazon/ionelement/impl/MutableStructFieldsImpl.kt +++ b/src/com/amazon/ionelement/impl/MutableStructFieldsImpl.kt @@ -5,82 +5,173 @@ import com.amazon.ionelement.api.IonElement import com.amazon.ionelement.api.MutableStructFields import com.amazon.ionelement.api.StructField import com.amazon.ionelement.api.field +import java.util.NoSuchElementException -internal class MutableStructFieldsImpl(private val fields: MutableMap>) : +internal class MutableStructFieldsImpl(private val fields: MutableMap>) : MutableStructFields { + override val size: Int + get() = fields.values.sumBy { it.size } override fun get(fieldName: String): AnyElement { - return requireNotNull(fields[fieldName]?.firstOrNull()) { + return requireNotNull(fields[fieldName]?.firstOrNull()?.value) { "Required struct field '$fieldName' missing" } } override fun getOptional(fieldName: String): AnyElement? { - return fields[fieldName]?.firstOrNull() + return fields[fieldName]?.firstOrNull()?.value } override fun getAll(fieldName: String): Collection { - return fields[fieldName] ?: mutableListOf() + return fields[fieldName]?.map { it.value } ?: mutableListOf() } override fun containsField(fieldName: String): Boolean { return fieldName in fields } - override fun set(fieldName: String, value: IonElement): MutableStructFields { + override fun set(fieldName: String, value: IonElement) { val values = fields.getOrPut(fieldName, ::mutableListOf) values.clear() - values.add(value as AnyElement) - return this + values.add(field(fieldName, value)) } - override fun setAll(fields: Iterable): MutableStructFields { + override fun setAll(fields: Iterable) { fields.groupByTo(mutableMapOf(), { it.name }, { it.value }).forEach { (name, values) -> - this.fields[name] = values + this.fields[name] = values.map { field(name, it) }.toMutableList() } - return this } - override fun add(fieldName: String, value: IonElement): MutableStructFields { + override fun add(fieldName: String, value: IonElement): Boolean { value as AnyElement val values = fields[fieldName] if (values == null) { - fields[fieldName] = mutableListOf(value) + fields[fieldName] = mutableListOf(field(fieldName, value)) } else { - values.add(value) + values.add(field(fieldName, value)) } + return true + } + + override fun add(element: StructField): Boolean { + return add(element.name, element.value) + } + + override fun remove(element: StructField): Boolean { + return fields[element.name]?.remove(element) ?: false + } + + override fun clearField(fieldName: String): Boolean { + fields[fieldName]?.clear() ?: return false + return true + } + + override fun removeAll(elements: Collection): Boolean { + var modified = false + for (element in elements) { + modified = remove(element) || modified + } + return modified + } + + override fun plusAssign(fields: Collection) { + addAll(fields) + } + + override fun addAll(elements: Collection): Boolean { + elements.forEach { add(it) } + return true + } + + override fun clear() { + fields.clear() + } + + override fun contains(element: StructField): Boolean { + return fields[element.name]?.contains(element) ?: false + } + + override fun containsAll(elements: Collection): Boolean { + return elements.all { contains(it) } + } - return this + override fun isEmpty(): Boolean { + return fields.all { it.value.isEmpty() } } - override fun add(field: StructField): MutableStructFields { - add(field.name, field.value) - return this + override fun iterator(): MutableIterator { + return MutableStructFieldsIterator(fields) } - override fun remove(field: StructField): MutableStructFields { - fields[field.name]?.remove(field.value) - return this + override fun retainAll(elements: Collection): Boolean { + var modified = false + val it = iterator() + while (it.hasNext()) { + val field = it.next() + if (field !in elements) { + it.remove() + modified = true + } + } + return modified + } +} + +internal class MutableStructFieldsIterator(fieldsMap: MutableMap>) : + MutableIterator { + + private val mapIterator = fieldsMap.iterator() + private var listIterator: MutableIterator = + if (mapIterator.hasNext()) { + mapIterator.next().value.iterator() + } else { + EMPTY_ITERATOR + } + + override fun hasNext(): Boolean { + if (listIterator.hasNext()) { + return true + } + + nextFieldList() + + return listIterator.hasNext() } - override fun removeAll(fieldName: String): MutableStructFields { - fields[fieldName]?.clear() - return this + override fun next(): StructField { + nextFieldList() + + return listIterator.next() } - override fun plusAssign(field: StructField) { - add(field.name, field.value) + override fun remove() { + listIterator.remove() } - override fun plusAssign(fields: Iterable) { - fields.forEach { add(it) } + private fun nextFieldList() { + while (!listIterator.hasNext()) { + if (mapIterator.hasNext()) { + listIterator = mapIterator.next().value.iterator() + } else { + listIterator = EMPTY_ITERATOR + break + } + } } - override fun iterator(): Iterator { - return fields.flatMap { (fieldName, values) -> - values.map { value -> - field(fieldName, value) + companion object { + private val EMPTY_ITERATOR = object : MutableIterator { + override fun hasNext(): Boolean { + return false } - }.iterator() + + override fun next(): StructField { + throw NoSuchElementException() + } + + override fun remove() { + throw IllegalStateException() + } + } } } diff --git a/src/com/amazon/ionelement/impl/StructElementImpl.kt b/src/com/amazon/ionelement/impl/StructElementImpl.kt index ce75551..7d9eb3e 100644 --- a/src/com/amazon/ionelement/impl/StructElementImpl.kt +++ b/src/com/amazon/ionelement/impl/StructElementImpl.kt @@ -69,8 +69,12 @@ internal class StructElementImpl( } override fun mutableFields(): MutableStructFields { - val internalMap = mutableMapOf>() - return MutableStructFieldsImpl(fieldsByName.mapValuesTo(internalMap) { it.value.toMutableList() }) + val internalMap = mutableMapOf>() + return MutableStructFieldsImpl( + fieldsByName.mapValuesTo(internalMap) { (name, values) -> + values.map { field(name, it) }.toMutableList() + } + ) } override fun update(body: MutableStructFields.() -> Unit): StructElement { @@ -92,9 +96,11 @@ internal class StructElementImpl( override fun copy(annotations: List, metas: MetaContainer): StructElementImpl = StructElementImpl(allFields, annotations.toPersistentList(), metas.toPersistentMap()) - override fun withAnnotations(vararg additionalAnnotations: String): StructElementImpl = _withAnnotations(*additionalAnnotations) + override fun withAnnotations(vararg additionalAnnotations: String): StructElementImpl = + _withAnnotations(*additionalAnnotations) - override fun withAnnotations(additionalAnnotations: Iterable): StructElementImpl = _withAnnotations(additionalAnnotations) + override fun withAnnotations(additionalAnnotations: Iterable): StructElementImpl = + _withAnnotations(additionalAnnotations) override fun withoutAnnotations(): StructElementImpl = _withoutAnnotations() override fun withMetas(additionalMetas: MetaContainer): StructElementImpl = _withMetas(additionalMetas) diff --git a/test/com/amazon/ionelement/MutableStructFieldsTests.kt b/test/com/amazon/ionelement/MutableStructFieldsTests.kt index 820d94e..b9c6ace 100644 --- a/test/com/amazon/ionelement/MutableStructFieldsTests.kt +++ b/test/com/amazon/ionelement/MutableStructFieldsTests.kt @@ -1,6 +1,7 @@ package com.amazon.ionelement import com.amazon.ionelement.api.buildStruct +import com.amazon.ionelement.api.emptyIonStruct import com.amazon.ionelement.api.field import com.amazon.ionelement.api.ionInt import com.amazon.ionelement.api.ionListOf @@ -8,7 +9,6 @@ import com.amazon.ionelement.api.ionString import com.amazon.ionelement.api.ionStructOf import com.amazon.ionelement.api.ionSymbol import com.amazon.ionelement.api.loadSingleElement -import java.lang.IllegalArgumentException import kotlin.test.assertEquals import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertNull @@ -19,6 +19,7 @@ import org.junit.jupiter.api.assertThrows class MutableStructFieldsTests { @Test fun `StructElement to mutable fields back to StructElement`() { + testStruct.mutableFields().size assertEquals(testStruct, ionStructOf(testStruct.mutableFields())) } @@ -152,9 +153,160 @@ class MutableStructFieldsTests { @Test fun removeAll() { val mutableFields = testStruct.mutableFields() - mutableFields.removeAll("author") - assertNull(mutableFields.getOptional("author")) + mutableFields.removeAll( + listOf( + field( + "author", + buildStruct { + add("lastname", ionString("Doe")) + add("firstname", ionString("Jane")) + } + ), + field( + "author", + buildStruct { + add("lastname", ionString("Smith")) + add("firstname", ionString("Jane")) + } + ) + ) + ) + + val expected = buildStruct { + add("isbn", ionString("123-456-789")) + add("title", ionString("AWS User Guide")) + add("category", ionListOf(ionSymbol("Non-Fiction"), ionSymbol("Technology"))) + } + + assertEquals(expected, ionStructOf(mutableFields)) + } + + @Test + fun retainAll() { + val mutableFields = testStruct.mutableFields() + + mutableFields.retainAll( + listOf( + field( + "author", + buildStruct { + add("lastname", ionString("Doe")) + add("firstname", ionString("Jane")) + } + ), + field( + "author", + buildStruct { + add("lastname", ionString("Smith")) + add("firstname", ionString("Jane")) + } + ) + ).toSet() + ) + + val updated = ionStructOf(mutableFields) + + val expected = testStruct.update { + this.removeIf { + it.name in setOf("isbn", "title", "category") + } + } + + assertEquals(expected, updated) + } + + @Test + fun testIterator() { + val mutableFields = testStruct.mutableFields() + + // Ensure the iterator works properly when fields have been removed + mutableFields.add("foo", ionString("999-999")) + mutableFields.add("foo", ionString("999-999")) + mutableFields.add("bar", ionString("999-999")) + mutableFields.remove(field("foo", ionString("999-999"))) + mutableFields.remove(field("foo", ionString("999-999"))) + mutableFields.remove(field("bar", ionString("999-999"))) + + val output = buildStruct { + for (field in testStruct.mutableFields()) { + add(field) + } + } + + assertEquals(testStruct, output) + } + + @Test + fun testMutableIterator() { + val mutableFields = testStruct.mutableFields() + + val it = mutableFields.iterator() + while (it.hasNext()) { + val field = it.next() + if (field.name in setOf("title", "author", "category")) { + it.remove() + } + } + + val updated = ionStructOf(mutableFields) + + assertEquals(ionStructOf(field("isbn", ionString("123-456-789"))), updated) + } + + @Test + fun testMutableIteratorAfterRemovingAll() { + val mutableFields = testStruct.mutableFields() + + val it = mutableFields.iterator() + while (it.hasNext()) { + it.next() + it.remove() + } + + val updated = ionStructOf(mutableFields) + + assertEquals(emptyIonStruct(), updated) + assertFalse(it.hasNext()) + assertThrows { + it.next() + } + } + + @Test + fun testIteratorFailureCases() { + val it = testStruct.mutableFields().iterator() + + assertThrows { + it.remove() + } + + while (it.hasNext()) { + it.next() + } + + assertThrows { + it.next() + } + + assertThrows { + it.remove() + } + } + + @Test + fun testEmptyIterator() { + val it = emptyIonStruct().mutableFields().iterator() + + assertFalse(it.hasNext()) + + assertThrows { + it.next() + } + + assertThrows { + it.remove() + } } @Test diff --git a/test/com/amazon/ionelement/demos/java/MutableStructFields.java b/test/com/amazon/ionelement/demos/java/MutableStructFieldsDemo.java similarity index 62% rename from test/com/amazon/ionelement/demos/java/MutableStructFields.java rename to test/com/amazon/ionelement/demos/java/MutableStructFieldsDemo.java index 45d0298..6036982 100644 --- a/test/com/amazon/ionelement/demos/java/MutableStructFields.java +++ b/test/com/amazon/ionelement/demos/java/MutableStructFieldsDemo.java @@ -3,28 +3,30 @@ import com.amazon.ionelement.api.Ion; import com.amazon.ionelement.api.StructElement; import kotlin.Unit; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static com.amazon.ionelement.api.ElementLoader.loadSingleElement; import static org.junit.jupiter.api.Assertions.assertEquals; -public class MutableStructFields { +@Disabled +public class MutableStructFieldsDemo { @Test - void CreateUpdatedStructFromExistingStruct() { + void createUpdatedStructFromExistingStruct() { StructElement original = loadSingleElement( "{name:\"Alice\",scores:{darts:100,billiards:15,}}").asStruct(); StructElement expected = loadSingleElement( "{name:\"Alice\",scores:{darts:100,billiards:30,pingPong:200,}}").asStruct(); - StructElement updatedScores = Ion.ionStructOf( - original.get("scores").asStruct().mutableFields() - .add("pingPong", Ion.ionInt(200)) - .set("billiards", Ion.ionInt(30))); - StructElement updated = original.update(fields -> { - fields.set("scores", updatedScores); + fields.set("scores", + original.get("scores").asStruct().update(scoreFields -> { + scoreFields.add("pingPong", Ion.ionInt(200)); + scoreFields.set("billiards", Ion.ionInt(30)); + return Unit.INSTANCE; + })); return Unit.INSTANCE; }); diff --git a/test/com/amazon/ionelement/demos/kotlin/MutableStructFields.kt b/test/com/amazon/ionelement/demos/kotlin/MutableStructFieldsDemo.kt similarity index 52% rename from test/com/amazon/ionelement/demos/kotlin/MutableStructFields.kt rename to test/com/amazon/ionelement/demos/kotlin/MutableStructFieldsDemo.kt index 164d5f2..8624ec0 100644 --- a/test/com/amazon/ionelement/demos/kotlin/MutableStructFields.kt +++ b/test/com/amazon/ionelement/demos/kotlin/MutableStructFieldsDemo.kt @@ -1,12 +1,11 @@ package com.amazon.ionelement.demos.kotlin import com.amazon.ionelement.api.ionInt -import com.amazon.ionelement.api.ionStructOf import com.amazon.ionelement.api.loadSingleElement import kotlin.test.assertEquals import org.junit.jupiter.api.Test -class MutableStructFields { +class MutableStructFieldsDemo { @Test fun `create updated struct from existing struct`() { val original = loadSingleElement( @@ -23,27 +22,26 @@ class MutableStructFields { val expected = loadSingleElement( """ - { - name: "Alice", - scores: { - darts: 100, - billiards: 30, - pingPong: 200, - } - } + { + name: "Alice", + scores: { + darts: 100, + billiards: 30, + pingPong: 200, + } + } """.trimIndent() ).asStruct() - val updated = ionStructOf( - original.mutableFields().set( + val updated = original.update { + set( "scores", - ionStructOf( - original["scores"].asStruct().mutableFields() - .set("pingPong", ionInt(200)) - .set("billiards", ionInt(30)) - ) + original["scores"].asStruct().update { + set("pingPong", ionInt(200)) + set("billiards", ionInt(30)) + } ) - ) + } assertEquals(expected, updated) }