simplify the NamedValueContainers structure (#1414)

Co-authored-by: Tatu Saloranta <tatu.saloranta@datastax.com>

src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/WriteableTableRowBuilder.java

@@ -73,11 +73,11 @@ public WriteableTableRow build(JsonNamedValueContainer source) {
     // the validation steps
     checkAllPrimaryKeys(cqlIdentifierToJsonValue.keySet());
     checkUnknownColumns(cqlIdentifierToJsonValue.keySet());
-    var decoded = decodeJsonToCQL(cqlIdentifierToJsonValue);
+    var decoded = encodeJsonToCql(cqlIdentifierToJsonValue);
 
     // now need to split the columns into key and non key columns
-    var keyColumns = new OrderedCqlNamedValueContainer();
-    var nonKeyColumns = new UnorderedCqlNamedValueContainer();
+    var keyColumns = new CqlNamedValueContainer();
+    var nonKeyColumns = new CqlNamedValueContainer();
     for (var cqlNamedValue : decoded.values()) {
       if (tableMetadata.getPrimaryKey().contains(cqlNamedValue.name())) {
         keyColumns.put(cqlNamedValue);
@@ -162,10 +162,10 @@ private void checkUnknownColumns(Collection<CqlIdentifier> suppliedColumns) {
    * DocumentException.Code#INVALID_COLUMN_VALUES} if any of the values failed to convert to CQL
    * value.
    */
-  private UnorderedCqlNamedValueContainer decodeJsonToCQL(
+  private CqlNamedValueContainer encodeJsonToCql(
       Map<CqlIdentifier, JsonNamedValue> cqlIdentifierToRaw) {
 
-    UnorderedCqlNamedValueContainer decoded = new UnorderedCqlNamedValueContainer();
+    var cqlNamedValues = new CqlNamedValueContainer();
     Map<ColumnMetadata, MissingJSONCodecException> unsupportedErrors = new HashMap<>();
     Map<ColumnMetadata, ToCQLCodecException> codecErrors = new HashMap<>();
 
@@ -189,7 +189,7 @@ private UnorderedCqlNamedValueContainer decodeJsonToCQL(
       try {
         var codec = codecRegistry.codecToCQL(tableMetadata, identifier, rawJsonValue);
         var cqlNamedValue = new CqlNamedValue(metadata, codec.toCQL(rawJsonValue));
-        decoded.put(cqlNamedValue);
+        cqlNamedValues.put(cqlNamedValue);
       } catch (UnknownColumnException e) {
         // this should not happen, we checked above but the codecs are written to be very safe and
         // will check and throw
@@ -232,12 +232,12 @@ private UnorderedCqlNamedValueContainer decodeJsonToCQL(
               }));
     }
 
-    if (decoded.size() != cqlIdentifierToRaw.size()) {
+    if (cqlNamedValues.size() != cqlIdentifierToRaw.size()) {
       throw new IllegalStateException(
           String.format(
               "decodeJsonToCQL: decoded size does not match raw size, decoded=%d, raw=%d",
-              decoded.size(), cqlIdentifierToRaw.size()));
+              cqlNamedValues.size(), cqlIdentifierToRaw.size()));
     }
-    return decoded;
+    return cqlNamedValues;
   }
 }

src/main/java/io/stargate/sgv2/jsonapi/service/shredding/CqlNamedValue.java

@@ -15,4 +15,15 @@ public class CqlNamedValue extends NamedValue<ColumnMetadata, Object> {
   public CqlNamedValue(ColumnMetadata name, Object value) {
     super(name, value);
   }
+
+  @Override
+  public String toString() {
+    return new StringBuilder(getClass().getSimpleName())
+        .append("{columnName=")
+        .append(name().getName())
+        .append(", value=")
+        .append(value())
+        .append("}")
+        .toString();
+  }
 }

src/main/java/io/stargate/sgv2/jsonapi/service/shredding/CqlNamedValueContainer.java

@@ -1,10 +1,26 @@
 package io.stargate.sgv2.jsonapi.service.shredding;
 
 import com.datastax.oss.driver.api.core.metadata.schema.ColumnMetadata;
+import java.util.Collection;
 
-/**
- * Base interface for {@link CqlNamedValue} containers, so they can be used in a generic way,
- * regardless of order.
- */
-public interface CqlNamedValueContainer
-    extends NamedValueContainer<ColumnMetadata, Object, CqlNamedValue> {}
+/** A container for {@link CqlNamedValue}'s that maintains the order the named values were added. */
+public class CqlNamedValueContainer
+    extends NamedValueContainer<ColumnMetadata, Object, CqlNamedValue> {
+
+  public CqlNamedValueContainer() {
+    super();
+  }
+
+  public CqlNamedValueContainer(int initialCapacity) {
+    super(initialCapacity);
+  }
+
+  public CqlNamedValueContainer(CqlNamedValueContainer container) {
+    super(container);
+  }
+
+  public CqlNamedValueContainer(Collection<CqlNamedValue> values) {
+    super();
+    putAll(values);
+  }
+}

src/main/java/io/stargate/sgv2/jsonapi/service/shredding/JsonNamedValueContainer.java

@@ -2,10 +2,26 @@
 
 import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonLiteral;
 import io.stargate.sgv2.jsonapi.service.shredding.collections.JsonPath;
+import java.util.Collection;
 
-/**
- * Base interface for {@link JsonNamedValue} containers, so they can be used in a generic way,
- * regardless of order.
- */
-public interface JsonNamedValueContainer
-    extends NamedValueContainer<JsonPath, JsonLiteral<?>, JsonNamedValue> {}
+/** A container for {@link JsonNamedValue} that maintains the order the named values were added. */
+public class JsonNamedValueContainer
+    extends NamedValueContainer<JsonPath, JsonLiteral<?>, JsonNamedValue> {
+
+  public JsonNamedValueContainer() {
+    super();
+  }
+
+  public JsonNamedValueContainer(int initialCapacity) {
+    super(initialCapacity);
+  }
+
+  public JsonNamedValueContainer(JsonNamedValueContainer container) {
+    super(container);
+  }
+
+  public JsonNamedValueContainer(Collection<JsonNamedValue> values) {
+    super();
+    putAll(values);
+  }
+}

src/main/java/io/stargate/sgv2/jsonapi/service/shredding/NamedValue.java

@@ -16,7 +16,7 @@
  * </ul>
  *
  * @param <NameT> The type of the name of the value
- * @param <ValueT> The value
+ * @param <ValueT> The type of the value
  */
 public abstract class NamedValue<NameT, ValueT> implements PrettyPrintable {
 

src/main/java/io/stargate/sgv2/jsonapi/service/shredding/NamedValueContainer.java

@@ -3,21 +3,41 @@
 import io.stargate.sgv2.jsonapi.util.PrettyPrintable;
 import io.stargate.sgv2.jsonapi.util.PrettyToStringBuilder;
 import java.util.Collection;
-import java.util.Map;
+import java.util.LinkedHashMap;
 import java.util.Objects;
 
 /**
- * Base for all containers that hold {@link NamedValue}s.
+ * Base implementation for a {@link NamedValueContainer} that maintains the order the named values
+ * were added.
  *
- * <p>Useful so we can deal with a container of {@link NamedValue}s in a generic way, provides some
- * helper methods, and implements {@link PrettyPrintable}.
+ * <p>Marked abstract to force the instantiation of a concrete types the define the {@link
+ * NamedValue} type.
+ *
+ * <p>Provides some helper methods and supports {@link PrettyPrintable} to make it easier to debug.
  *
  * @param <NameT> The type of the name, this is the key in the map
  * @param <ValueT> The type of the value stored in the {@link NamedValue}
  * @param <NvT> The type of the {@link NamedValue} stored in the map
  */
-public interface NamedValueContainer<NameT, ValueT, NvT extends NamedValue<NameT, ValueT>>
-    extends Map<NameT, NvT>, PrettyPrintable {
+public abstract class NamedValueContainer<NameT, ValueT, NvT extends NamedValue<NameT, ValueT>>
+    extends LinkedHashMap<NameT, NvT> implements PrettyPrintable {
+
+  public NamedValueContainer() {
+    super();
+  }
+
+  public NamedValueContainer(int initialCapacity) {
+    super(initialCapacity);
+  }
+
+  public NamedValueContainer(NamedValueContainer<NameT, ValueT, NvT> container) {
+    super(container);
+  }
+
+  public NamedValueContainer(Collection<NvT> values) {
+    super();
+    putAll(values);
+  }
 
   /**
    * Helper to add a {@link NamedValue} to the container, keyed on the {@link NamedValue#name()}.
@@ -26,30 +46,36 @@ public interface NamedValueContainer<NameT, ValueT, NvT extends NamedValue<NameT
    * @return The previous value associated with the name, or null if there was no mapping for the
    *     name
    */
-  default NamedValue<NameT, ValueT> put(NvT namedValue) {
+  public NamedValue<NameT, ValueT> put(NvT namedValue) {
     return put(namedValue.name(), namedValue);
   }
 
-  default void putAll(Collection<NvT> namedValues) {
+  public void putAll(Collection<NvT> namedValues) {
     Objects.requireNonNull(namedValues, "namedValues must not be null");
     namedValues.forEach(this::put);
   }
 
   /** Helper that returns an immutable list of the {@link NamedValue#value()}s in the container. */
-  default Collection<ValueT> valuesValue() {
+  public Collection<ValueT> valuesValue() {
     return values().stream().map(NamedValue::value).toList();
   }
 
-  default String toString(boolean pretty) {
+  @Override
+  public String toString() {
+    return toString(false);
+  }
+
+  public String toString(boolean pretty) {
     return toString(new PrettyToStringBuilder(getClass(), pretty)).toString();
   }
 
-  default PrettyToStringBuilder appendTo(PrettyToStringBuilder prettyToStringBuilder) {
+  @Override
+  public PrettyToStringBuilder appendTo(PrettyToStringBuilder prettyToStringBuilder) {
     var sb = prettyToStringBuilder.beginSubBuilder(getClass());
     return toString(sb).endSubBuilder();
   }
 
-  default PrettyToStringBuilder toString(PrettyToStringBuilder prettyToStringBuilder) {
+  public PrettyToStringBuilder toString(PrettyToStringBuilder prettyToStringBuilder) {
     forEach((key, value) -> prettyToStringBuilder.append(key.toString(), value));
     return prettyToStringBuilder;
   }