Skip to content

Commit

Permalink
Fix over-consuming XML de-serialization in Golang (#489)
Browse files Browse the repository at this point in the history
We read a token too much in `unmarshalXxx` functions. The bug went
unnoticed as all our examples used indented XML, which eclipsed the
over-consumption.

In this patch, we do not generate the `unmarshalXxx`, as they turn out
to be redundant, and use `readXxxWithLookahead` to appropriately read
the tokens without over-consumption.
  • Loading branch information
mristin authored May 16, 2024
1 parent 6d7fd9f commit 3b20999
Show file tree
Hide file tree
Showing 2 changed files with 148 additions and 1,437 deletions.
61 changes: 10 additions & 51 deletions aas_core_codegen/golang/xmlization/_generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -818,18 +818,19 @@ def _generate_snippet_to_switch_on_property_deserialization(
)

else:
unmarshal_function = golang_naming.private_function_name(
Identifier(f"unmarshal_{type_anno.our_type.name}")
read_with_lookahead_function = golang_naming.private_function_name(
Identifier(f"read_{type_anno.our_type.name}_with_lookahead")
)

case_body_blocks.append(
Stripped(
f"""\
{prop_var}, valueErr = {unmarshal_function}(
{prop_var}, valueErr = {read_with_lookahead_function}(
{I}decoder,
{I}current,
)
// {unmarshal_function} stops at the end element,
// so we look ahead to the next element.
// {read_with_lookahead_function} stops at the end element,
// so we look ahead to the next element, just after the end element.
if valueErr == nil {{
{I}current, valueErr = readNext(decoder, current)
}}"""
Expand Down Expand Up @@ -1154,7 +1155,8 @@ def _generate_read_with_lookahead_without_dispatch(
// as an XML element where the start element is expected to have been already
// read as `current` token.
//
// The de-serialization stops by consuming the final end element.
// The de-serialization stops by consuming the final end element. The next call to
// the `decoder.Token()` will return the element just after the end element.
func {function_name}(
{I}decoder *xml.Decoder,
{I}current xml.Token,
Expand Down Expand Up @@ -1283,7 +1285,8 @@ def _generate_read_with_lookahead_with_dispatch(
// as an XML element where the start element is expected to have been already read
// as `current` token.
//
// The de-serialization stops by consuming the final end element.
// The de-serialization stops by consuming the final end element. The next call to
// the `decoder.Token()` will return the element just after the end element.
func {function_name}(
{I}decoder *xml.Decoder,
{I}current xml.Token,
Expand Down Expand Up @@ -1320,46 +1323,6 @@ def _generate_read_with_lookahead_with_dispatch(
)


def _generate_unmarshal_for(cls: intermediate.ClassUnion) -> Stripped:
interface_name = golang_naming.interface_name(cls.name)
function_name = golang_naming.private_function_name(
Identifier(f"unmarshal_{cls.name}")
)

read_with_lookahead = golang_naming.private_function_name(
Identifier(f"read_{cls.name}_with_lookahead")
)

return Stripped(
f"""\
// Unmarshal an instance of [aastypes.{interface_name}]
// serialized as an XML element.
//
// The XML element must live in the [Namespace] space.
func {function_name}(
{I}decoder *xml.Decoder,
) (instance aastypes.{interface_name},
{I}err error,
) {{
{I}var current xml.Token
{I}current, err = readNext(decoder, nil)
{I}if _, isEOF := current.(eof); isEOF {{
{II}err = newDeserializationError(
{III}"Expected an instance of {interface_name} "+
{IIII}"serialized as an XML element, but reached the end of file.",
{II})
{II}return
{I}}}
{I}instance, err = {read_with_lookahead}(
{II}decoder,
{II}current,
{I})
{I}return
}}"""
)


def _generate_unmarshal(symbol_table: intermediate.SymbolTable) -> Stripped:
case_blocks = [] # type: List[Stripped]
for cls in symbol_table.concrete_classes:
Expand Down Expand Up @@ -2434,8 +2397,6 @@ def generate(
elif isinstance(our_type, intermediate.AbstractClass):
blocks.append(_generate_read_with_lookahead_with_dispatch(cls=our_type))

blocks.append(_generate_unmarshal_for(cls=our_type))

elif isinstance(our_type, intermediate.ConcreteClass):
if our_type.is_implementation_specific:
implementation_key = specific_implementations.ImplementationKey(
Expand All @@ -2462,8 +2423,6 @@ def generate(
blocks.append(
_generate_read_with_lookahead_without_dispatch(cls=our_type)
)

blocks.append(_generate_unmarshal_for(cls=our_type))
else:
assert_never(our_type)

Expand Down
Loading

0 comments on commit 3b20999

Please sign in to comment.