"optional" flag in records makes validation to fail silently

[alessandrothea]

Hi, when preparing the first python-based configuration for the minidaq in dunedaq-v2.2.0 I stumbled on the following issue: a schema validation error can be hidden if it originates from an "optional" field.

Minimal example below based on the appfwk schema (runs off the appfwk/schema):

• Creating and validating a cmd.CmdObj() without the required modules filed correctly fails,
• The same encapsulated in cmd.Command(id=cmd.CmdId("aaa"), data=cmd.CmdObj()) doesn't. Note that data is optional

  command: s.record("Command", [
      s.field("id", self.cmdid,
              doc="Identify the type of command"),
      s.field("data", self.data, optional=true,
              doc="Command data object with type-specific structure"),
  ], doc="Top-level command object structure"),

• maybe not surprisingly, the resulting command object present not data field at all.

Minimal example

import json

# Load configuration types
import moo.otypes
moo.otypes.load_types('appfwk-cmd-schema.jsonnet')

import dunedaq.appfwk.cmd as cmd 

print("Building cmd.CmdObj")
try:
    cmd.CmdObj().pod()
    print("CmdObj build")
except AttributeError:
    print ("Failed to build CmdObj")

print()
print("Building cmd.CmdObj in cmd.Command")
try:
    c = cmd.Command(
            id=cmd.CmdId("aaa"),
            data=cmd.CmdObj()
        ).pod()
    print("CmdObj in Command build\n", json.dumps(c, indent=4, sort_keys=True))
except AttributeError:
    print ("Failed to build CmdObj in cmd.Command")

Results in

Building cmd.CmdObj
Failed to build CmdObj

Building cmd.CmdObj in cmd.Command
CmdObj in Command build
{
    "id": "aaa"
}

[brettviren]

A case of unintentional exception hiding.

The cmd.CmdObj() produces an incomplete record, which is okay up until a finalizing .pod() is called.

The Command.pod() requires (required) that the CmdObj is already complete (its own .pod() is called). Since it isn't complete an AttributeError is raised which is then internally caught and wrongly interpreted in the outer Command.pod() as data attribute missing. Since data is optional, the code happily continues on its merry way emitting the object the test shows.

The fix is to probe that fields exist without triggering their value's .pod() call so any field values that happen to be incomplete records will have their AttributeError raised later.

Commit inbound with test_issue13.py holding test_hidden_attrerr() to reproduce this issue and show the code now correctly throws AttributeError to the outer caller.

[brettviren]

About the test code itself. It's never worthwhile passing an incomplete record R as a field value to a parent record P as this is a pass-by-value. Subsequent completion of the R -> R' will not be reflected in the passed copy. The field of P will need to be explicitly set to the completed R'.

The new test exercises this completion.

Of course, moo should properly complain if this isn't done correctly, which it should now!