openconfig / ygot

A YANG-centric Go toolkit - Go/Protobuf Code Generation; Validation; Marshaling/Unmarshaling
Apache License 2.0
286 stars 107 forks source link

Preserve YANG elements order when generating structs #942

Open hellt opened 11 months ago

hellt commented 11 months ago

Hi all, I wonder which code path is responsible to sort alphabetically generated struct's fields?

consider the following module:

module test {
  yang-version 1.1;
  namespace "urn:srlinux:ndk:test";
  prefix srl-labs;

  container app {
    leaf name {
      type string;
    }
    leaf address {
      type string;
    }
  }
}

The generated struct will have fields sorted as

type Foo struct {
  Address string
  Name string
}

and I would like to maintain the order of the struct fields (Name first, Address second). Would you hint me where the relevant code path is so that we might implement a preserve-yang-elements-order flag?

wenovus commented 11 months ago

Hi, the fields are ordered here: https://github.com/openconfig/ygot/blob/8efc81471e0fe679c453aa0e8c03d752721733bc/gogen/gogen.go#L1032, and appended to an ordered slice here: https://github.com/openconfig/ygot/blob/8efc81471e0fe679c453aa0e8c03d752721733bc/gogen/gogen.go#L1259

and the writing to the template is done here by looping through the slice: https://github.com/openconfig/ygot/blob/8efc81471e0fe679c453aa0e8c03d752721733bc/gogen/gogen.go#L1325

If you trace back the dataflow from ParsedDirectory.Fields, then you'll eventually reach goyang's Entry type with its Dir map[string]*Entry field, whereupon you'll end up at https://github.com/openconfig/goyang/blob/5ad0d2feb9ce655fb39e414bd4e3696356780cdb/pkg/yang/entry.go#L410-L418, which is invoked in ToEntry: https://github.com/openconfig/goyang/blob/5ad0d2feb9ce655fb39e414bd4e3696356780cdb/pkg/yang/entry.go#L712-L720, and here you'll finally find some of the original ordering, but even then they're grouped by node type (e.g. leaf vs. leaf-list): https://github.com/openconfig/goyang/blob/5ad0d2feb9ce655fb39e414bd4e3696356780cdb/pkg/yang/ast.go#L454

So fundamentally the issue is that the order of fields are not respected when the Entry representation is converted from the original parsed AST (i.e. []*Statement).


TLDR: The easiest way I can see is to loop through the set of Entry.Node.Statement().SubStatements() to find the ordering, and use that order at https://github.com/openconfig/ygot/blob/8efc81471e0fe679c453aa0e8c03d752721733bc/gogen/gogen.go#L1032, unfortunately Entry is not available in ParsedDirectory, so you would need to populate this information into ParsedDirectory from https://github.com/openconfig/ygot/blob/8efc81471e0fe679c453aa0e8c03d752721733bc/ygen/directory.go#L321, by accessing Entry from https://github.com/openconfig/ygot/blob/8efc81471e0fe679c453aa0e8c03d752721733bc/ygen/directory.go#L223C4-L223C9

Any other solution would involve adding a new field to either the Entry AST or other Node types, and that would involve some more effort.

nleiva commented 1 month ago

@hellt Did you make any progress on this? I think we may also need it, so we could collaborate on this preserve-yang-elements-order flag.

hellt commented 1 month ago

@nleiva unfortunately I haven't...

nleiva commented 1 month ago

No worries. We are exploring avenues to address non-RFC7951 vendor implementations 1 when serializing YANG to JSON.

However, even if the generated struct fields follow a specific order, ygot doesn't serialize them as such when marshaling to JSON. I might be off here but this could be due to storing struct data in a map(structJSON). encoding/json: no way to preserve the order of map keys

// structJSON marshals a GoStruct to a map[string]any which can be
// handed to JSON marshal. parentMod specifies the module that the supplied
// GoStruct is defined within such that RFC7951 format JSON is able to consider
// whether to prepend the name of the module to an element. The format of JSON to
// be produced and whether such module names are prepended is controlled through the
// supplied jsonOutputConfig. Returns an error if the GoStruct cannot be rendered
// to JSON.
func structJSON(s GoStruct, parentMod string, args jsonOutputConfig) (map[string]any, error) {
// ...
}

The Junos schema defines certain configuration objects as lists. In JSON configuration data, a list instance is encoded as a name/array pair, and the array elements are JSON objects. Generally, the order of members in a JSON-encoded list entry is arbitrary because JSON objects are fundamentally unordered collections of members. However, the Junos schema requires that list keys precede any other siblings within a list entry and appear in the order specified by the schema. source