clarify corner case on yaml structure

engine-api/src/main/java/io/nosqlbench/engine/api/activityconfig/rawyaml/RawStmtDef.java

@@ -21,11 +21,14 @@ import io.nosqlbench.nb.api.errors.BasicError;
 
 import java.util.*;
 
+/**
+ * See specification for what this should do in UniformWorkloadSpecificationTest
+ */
 public class RawStmtDef extends RawStmtFields {
 
     private Object op;
 
-    private final static List<String> opNames = List.of("stmt","statement","op","operation");
+    private final static List<String> opFieldSynonyms = List.of("stmt", "statement", "op", "operation");
 
     public RawStmtDef() {
     }
@@ -37,64 +40,43 @@ public class RawStmtDef extends RawStmtFields {
 
     @SuppressWarnings("unchecked")
     public RawStmtDef(String defaultName, Map<String, Object> map) {
+        setFieldsByReflection(map);
+    }
+
+    public void setFieldsByReflection(Map<String, Object> map) {
+        super.setFieldsByReflection(map);
+
+
         HashSet<String> found = new HashSet<>();
-        for (String opName : opNames) {
+        for (String opName : opFieldSynonyms) {
             if (map.containsKey(opName)) {
                 found.add(opName);
             }
         }
-        if (found.size()>1) {
+        if (found.size() == 1) {
-            throw new BasicError("You used " + found + " as an op name, but only one of these is allowed.");
-        }
-        if (found.size()==1) {
             Object op = map.remove(found.iterator().next());
-            this.setOp(op);
+            setOp(op);
+        } else if (found.size() > 1) {
+            throw new BasicError("You used " + found + " as an op name, but only one of these is allowed at a time.");
+        } else if ((getName() == null || getName().isEmpty()) && op == null && map.size() > 0) {
+            System.out.println("here");
+            Map.Entry<String, Object> first = map.entrySet().iterator().next();
+            setName(first.getKey());
+            setOp(first.getValue());
+            map.remove(first.getKey());
         }
+        boolean _params = !getParams().isEmpty();
+        boolean _op = op != null;
 
-        Optional.ofNullable((String) map.remove("name")).ifPresent(this::setName);
+        if (!_op && !_params) {
-        Optional.ofNullable((String) map.remove("desc")).ifPresent(this::setDesc);
+            LinkedHashMap<String, Object> newop = new LinkedHashMap<>();
-        Optional.ofNullable((String) map.remove("description")).ifPresent(this::setDesc);
+            newop.putAll(map);
-
+            setOp(newop);
-        Optional.ofNullable((Map<String, String>) map.remove("tags")).ifPresent(this::setTags);
+            map.clear();
-        Optional.ofNullable((Map<String, String>) map.remove("bindings")).ifPresent(this::setBindings);
+        } else if (_op) {
-        Optional.ofNullable((Map<String, Object>) map.remove("params")).ifPresent(this::setParams);
+            getParams().putAll(map);
-
+            map.clear();
-
-        // Depends on order stability, relying on LinkedHashMap -- Needs stability unit tests
-        if (this.op == null) {
-            Iterator<Map.Entry<String, Object>> iterator = map.entrySet().iterator();
-            if (!iterator.hasNext()) {
-                throw new RuntimeException("undefined-name-statement-tuple:" +
-                        " The statement is not set, and no statements remain to pull 'name: statement' values from." +
-                        " For more details on this error see " +
-                        "the troubleshooting section of the YAML format" +
-                        " docs for undefined-name-statement-tuple");
-            }
-            Map.Entry<String, Object> firstEntry = iterator.next();
-            if (firstEntry.getValue() instanceof Map && map.size()==1) {
-                Map values = (Map) firstEntry.getValue();
-                setFieldsByReflection(values);
-                map = values;
-            } else if (firstEntry.getValue() instanceof CharSequence) {
-                setStmt(((CharSequence) firstEntry.getValue()).toString());
-            }
-            if (getName().isEmpty()) {
-                map.remove(firstEntry.getKey());
-                setName(firstEntry.getKey());
-            }
-            // TODO: Add explicit check condition for this error
-//            else {
-//                throw new RuntimeException("redefined-name-in-statement-tuple: Statement name has already been set by name parameter. Remove the name parameter for a statement definition map." +
-//                        " For more details on this error see " +
-//                        "the troubleshooting section in the " +
-//                        "YAML format docs for redefined-name-statement-tuple");
-//            }
         }
-        if (getName().isEmpty()) {
-            setName(defaultName);
-        }
-
-        map.forEach((key, value) -> getParams().put(key, value));
     }
 
     private void setOp(Object op) {
@@ -119,7 +101,7 @@ public class RawStmtDef extends RawStmtFields {
 
     public String getName() {
         Object name = getParams().get("name");
-        if (name!=null) {
+        if (name != null) {
             return name.toString();
         }
         return super.getName();

engine-api/src/main/java/io/nosqlbench/engine/api/activityconfig/rawyaml/StatementsOwner.java

@@ -78,7 +78,8 @@ public class StatementsOwner extends RawStmtFields {
                 if (o instanceof String) {
                     defs.add(new RawStmtDef(defaultName, (String) o));
                 } else if (o instanceof Map) {
-                    defs.add(new RawStmtDef(defaultName, (Map<String, Object>) o));
+                    RawStmtDef def = new RawStmtDef(defaultName, (Map<String, Object>) o);
+                    defs.add(def);
                 } else {
                     throw new RuntimeException("Can not construct stmt def from object type:" + o.getClass());
                 }

engine-api/src/main/resources/workload_definition/templated_operation_variations.md

@@ -2,7 +2,12 @@
 
 These examples are here to illustrate and test specific variations of op templates.
 
-## keyed name statement form
+## Op Naming
+
+### map of op templates with explicit name
+
+If you use a map of op templates, they can still override the name of the op simply by adding
+the `name` key.
 
 *yaml:*
 
@@ -11,8 +16,6 @@ ops:
   op1:
     name: special-op-name
     op: select * from ks1.tb1;
-    params:
-      prepared: false
 ```
 
 *json:*
@@ -22,10 +25,7 @@ ops:
   "ops": {
     "op1": {
       "name": "special-op-name",
-      "op": "select * from ks1.tb1;",
+      "op": "select * from ks1.tb1;"
-      "params": {
-        "prepared": false
-      }
     }
   }
 }
@@ -40,9 +40,6 @@ ops:
     "op": {
       "stmt": "select * from ks1.tb1;"
     },
-    "params": {
-      "prepared": false
-    },
     "tags": {
       "block": "block0",
       "name": "block0--special-op-name"
@@ -51,54 +48,12 @@ ops:
 ]
 ```
 
-## keyed name statement-map form with name field
+### map of op templates without explicit name
 
-*yaml:*
+This yaml document contains a single named op template `op1` which contains a scoped op `op` with
+two op fields `field1` and `field2`.
 
-```yaml
+The op template takes its name `op1` from the map key under the ops property.
-ops:
-  op1:
-    name: special-op-name
-    op:
-      field1: select * from ks1.tb1;
-      field2: field 2 value
-```
-
-*json:*
-
-```json5
-{
-  "ops": {
-    "op1": {
-      "name": "special-op-name",
-      "op": {
-        "field1": "select * from ks1.tb1;",
-        "field2": "field 2 value"
-      }
-    }
-  }
-}
-```
-
-*ops:*
-
-```json5
-[
-  {
-    "name": "block0--special-op-name",
-    "op": {
-      "field1": "select * from ks1.tb1;",
-      "field2": "field 2 value"
-    },
-    "tags": {
-      "block": "block0",
-      "name": "block0--special-op-name"
-    }
-  }
-]
-```
-
-## keyed name statement-map form WITHOUT name field WITH op key
 
 *yaml:*
 
@@ -143,7 +98,159 @@ ops:
 ]
 ```
 
-## keyed name statement-map form WITHOUT name field WITHOUT op key
+## Op Fields
+
+### Anonymous fields go to op by default
+
+*yaml:*
+
+```yaml
+ops:
+  op1:
+    field1: select * from ks1.tb1;
+    field2: field 2 value
+```
+
+*json:*
+
+```json5
+{
+  "ops": {
+    "op1": {
+      "field1": "select * from ks1.tb1;",
+      "field2": "field 2 value"
+    }
+  }
+}
+```
+
+*ops:*
+
+```json5
+[
+  {
+    "name": "block0--op1",
+    "op": {
+      "field1": "select * from ks1.tb1;",
+      "field2": "field 2 value"
+    },
+    "tags": {
+      "block": "block0",
+      "name": "block0--op1"
+    }
+  }
+]
+```
+
+### Scoped op fields allow dangling param values
+
+*yaml:*
+
+```yaml
+ops:
+  op1:
+    op:
+      field1: select * from ks1.tb1;
+      field2: field 2 value
+    paramname1: paramval1
+```
+
+*json:*
+
+```json5
+{
+  "ops": {
+    "op1": {
+      "op": {
+        "field1": "select * from ks1.tb1;",
+        "field2": "field 2 value"
+      },
+      "paramname1": "paramval1"
+    }
+  }
+}
+```
+
+*ops:*
+
+```json5
+[
+  {
+    "name": "block0--op1",
+    "op": {
+      "field1": "select * from ks1.tb1;",
+      "field2": "field 2 value"
+    },
+    "params": {
+      "paramname1": "paramval1"
+    },
+    "tags": {
+      "block": "block0",
+      "name": "block0--op1"
+    }
+  }
+]
+```
+
+### Scoped op and param fields disallow dangling fields
+
+*yaml:*
+
+```yaml
+ops:
+  op1:
+    op:
+      field1: select * from ks1.tb1;
+      field2: field 2 value
+    params:
+      paramname1: paramval1
+#   dangling1: value
+#   ^ NOT ALLOWED HERE
+```
+
+*json:*
+
+```json5
+{
+  "ops": {
+    "op1": {
+      "op": {
+        "field1": "select * from ks1.tb1;",
+        "field2": "field 2 value"
+      },
+      "params": {
+        "paramname1": "paramval1"
+      }
+    }
+  }
+}
+```
+
+*ops:*
+
+```json5
+[
+  {
+    "name": "block0--op1",
+    "op": {
+      "field1": "select * from ks1.tb1;",
+      "field2": "field 2 value"
+    },
+    "params": {
+      "paramname1": "paramval1"
+    },
+    "tags": {
+      "block": "block0",
+      "name": "block0--op1"
+    }
+  }
+]
+```
+
+## keyed name statement-map form WITHOUT name field WITHOUT op key
+
+When statements are named by key, and you need to specify a query string of some type, then it must
+be explicitly part of the naming structure, as with a field name like `stmt` or `op`.
 
 *yaml:*
 
@@ -174,9 +281,6 @@ ops:
   {
     "name": "block0--op1",
     "op": {
-      "stmt": "select * from ks1.tb1;"
-    },
-    "params": {
       "field1": "select * from ks1.tb1;",
       "field2": "field 2 value"
     },

engine-api/src/test/java/io/nosqlbench/engine/api/activityconfig/rawyaml/OpDefTest.java

@@ -22,9 +22,9 @@ import io.nosqlbench.engine.api.activityconfig.yaml.OpTemplate;
 import io.nosqlbench.engine.api.activityconfig.yaml.StmtsBlock;
 import io.nosqlbench.engine.api.activityconfig.yaml.StmtsDoc;
 import io.nosqlbench.engine.api.activityconfig.yaml.StmtsDocList;
-import org.junit.jupiter.api.Test;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.junit.jupiter.api.Test;
 
 import java.util.List;
 import java.util.Map;
@@ -105,8 +105,9 @@ public class OpDefTest {
         assertThat(block1.getOps()).hasSize(2);
         OpTemplate op0 = block1.getOps().get(0);
         assertThat(op0.getName()).isEqualTo("map-of-maps--block0--s3");
-        assertThat(op0.getParams()).hasSize(2);
+        assertThat(op0.getOp()).contains(Map.of("p1","v7","p2","v8"));
-        assertThat(op0.getParams()).containsAllEntriesOf(Map.of("p1", "v7", "p2", "v8"));
+        assertThat(op0.getParams()).hasSize(0);
+        assertThat(op0.getParams()).hasSize(0);
         OpTemplate op1 = block1.getOps().get(1);
         assertThat(op1.getParams()).containsAllEntriesOf(Map.of());
         assertThat(op1.getName()).isEqualTo("map-of-maps--block0--s2");
@@ -141,7 +142,7 @@ public class OpDefTest {
         System.out.println(op0.getParams());
 
         assertThat(op0.getName()).isEqualTo("list-of-named-map--block1--s1");
-        assertThat(op0.getOp()).isNull();
+        assertThat(op0.getOp()).contains(Map.of("p1","v1","p2","v2"));
 //        System.out.println("here");
         // TODO: This needs to be clarified and the logic made less ambiguous
 //        assertThat(op0.getParams()).hasSize(1);