StringValue#isNull() behavior directly contradicts specification

[ZackPierce]

uk.ac.ed.ph.jqtiplus.value.StringValue#isNull() should return true when the wrapped string contents are empty (non-null, but length of 0).

The specification states that "empty containers and empty strings are both treated as NULL."

Presently StringValue#isNull() always returns false, because it does not override the base class SingleValue's default behavior.

The isNull method is widely used throughout the processing portions of JQTI+, and thus this issue impacts the correct behavior of several operators, including core expressions like "And", "Equal", "Or", and naturally, "IsNull".

[davemckain]

Hi Zack,

Thanks for the bug report and sorry for the slow fix. This bug should now be fixed. Analysis of the problem identified a few related issues in the handling of whitespace when building the JQTI+ object model from the DOM, so I've had to fix those too, and there are some minor internal API changes as a result.

Main changes in this fix are:

• SingleValue is no longer hard-coded to return true for isNull()
• StringValue now returns true for isNull() if the value is an empty string
• Whitespace handling changes:
  • Attribute value parsing no longer trims the string.
  • Parsing of baseValue content no longer trims the string by default.
  • Instead of these, there's a new BaseType.parseSingleValueLax() that trims leading/trailing whitespace from the input string only if it is safe to do so, i.e. for non-STRING baseTypes. Calls to BaseType.parseSingleValue() have been refactored to use this where appropriate.
• Attribute loading is now done inside AttributeList
• Changes to Attribute interface:
  • parseQtiString() has been renamed as parseDomAttributeValue() to reflect its actual usage
  • valueToQtiString() renamed as toDomAttributeValue()
  • defaultValueToQtiString() was never used internally so has been removed
• (Moved DateFormat stuff into DataTypeBinder for consistency)

[ZackPierce]

Dave, thank you for the thorough and thoughtful fix. I'll be updating openoperator to make use of this quite soon.