qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Victor Toso <victortoso@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Markus Armbruster" <armbru@redhat.com>,
	"John Snow" <jsnow@redhat.com>,
	"Daniel P . Berrangé" <berrange@redhat.com>,
	"Andrea Bolognani" <abologna@redhat.com>
Subject: [PATCH v2 06/11] qapi: golang: structs: Address 'null' members
Date: Mon, 16 Oct 2023 17:26:59 +0200	[thread overview]
Message-ID: <20231016152704.221611-7-victortoso@redhat.com> (raw)
In-Reply-To: <20231016152704.221611-1-victortoso@redhat.com>

Explaining why this is needed needs some context, so taking the
example of StrOrNull alternate type and considering a simplified
struct that has two fields:

qapi:
 | { 'struct': 'MigrationExample',
 |   'data': { '*label': 'StrOrNull',
 |             'target': 'StrOrNull' } }

We have a optional member 'label' which can have three JSON values:
 1. A string: { "target": "a.host.com", "label": "happy" }
 2. A null  : { "target": "a.host.com", "label": null }
 3. Absent  : { "target": null}

The member 'target' is not optional, hence it can't be absent.

A Go struct that contains a optional type that can be JSON Null like
'label' in the example above, will need extra care when Marshaling and
Unmarshaling from JSON.

This patch handles this very specific case:
 - It implements the Marshaler interface for these structs to properly
   handle these values.
 - It adds the interface AbsentAlternate() and implement it for any
   Alternate that can be JSON Null

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 scripts/qapi/golang.py | 243 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 228 insertions(+), 15 deletions(-)

diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
index 73d0804d0a..6a7e37dd90 100644
--- a/scripts/qapi/golang.py
+++ b/scripts/qapi/golang.py
@@ -110,6 +110,26 @@
 """
 
 
+TEMPLATE_STRUCT_WITH_NULLABLE_MARSHAL = """
+func (s {type_name}) MarshalJSON() ([]byte, error) {{
+\tm := make(map[string]any)
+{map_members}{map_special}
+\treturn json.Marshal(&m)
+}}
+
+func (s *{type_name}) UnmarshalJSON(data []byte) error {{
+\ttmp := {struct}{{}}
+
+\tif err := json.Unmarshal(data, &tmp); err != nil {{
+\t\treturn err
+\t}}
+
+{set_members}{set_special}
+\treturn nil
+}}
+"""
+
+
 def gen_golang(schema: QAPISchema, output_dir: str, prefix: str) -> None:
     vis = QAPISchemaGenGolangVisitor(prefix)
     schema.visit(vis)
@@ -256,21 +276,31 @@ def get_struct_field(
     self: QAPISchemaGenGolangVisitor,
     qapi_name: str,
     qapi_type_name: str,
+    within_nullable_struct: bool,
     is_optional: bool,
     is_variant: bool,
-) -> dict[str:str]:
+) -> Tuple[dict[str:str], bool]:
 
     field = qapi_to_field_name(qapi_name)
     member_type = qapi_schema_type_to_go_type(qapi_type_name)
+    is_nullable = False
 
     optional = ""
     if is_optional:
-        if member_type not in self.accept_null_types:
+        if member_type in self.accept_null_types:
+            is_nullable = True
+        else:
             optional = ",omitempty"
 
     # Use pointer to type when field is optional
     isptr = "*" if is_optional and member_type[0] not in "*[" else ""
 
+    if within_nullable_struct:
+        # Within a struct which has a field of type that can hold JSON NULL,
+        # we have to _not_ use a pointer, otherwise the Marshal methods are
+        # not called.
+        isptr = "" if member_type in self.accept_null_types else isptr
+
     fieldtag = (
         '`json:"-"`' if is_variant else f'`json:"{qapi_name}{optional}"`'
     )
@@ -279,32 +309,202 @@ def get_struct_field(
         "type": f"{isptr}{member_type}",
         "tag": f"{fieldtag}",
     }
-    return arg
+    return arg, is_nullable
+
+
+# This helper is used whithin a struct that has members that accept JSON NULL.
+def map_and_set(
+    is_nullable: bool, field: str, field_is_optional: bool, name: str
+) -> Tuple[str, str]:
+
+    mapstr = ""
+    setstr = ""
+    if is_nullable:
+        mapstr = f"""
+\tif val, absent := s.{field}.ToAnyOrAbsent(); !absent {{
+\t\tm["{name}"] = val
+\t}}
+"""
+        setstr += f"""
+\tif _, absent := (&tmp.{field}).ToAnyOrAbsent(); !absent {{
+\t\ts.{field} = &tmp.{field}
+\t}}
+"""
+    elif field_is_optional:
+        mapstr = f"""
+\tif s.{field} != nil {{
+\t\tm["{name}"] = s.{field}
+\t}}
+"""
+        setstr = f"""\ts.{field} = tmp.{field}\n"""
+    else:
+        mapstr = f"""\tm["{name}"] = s.{field}\n"""
+        setstr = f"""\ts.{field} = tmp.{field}\n"""
+
+    return mapstr, setstr
+
+
+def recursive_base_nullable(
+    self: QAPISchemaGenGolangVisitor, base: Optional[QAPISchemaObjectType]
+) -> Tuple[List[dict[str:str]], str, str, str, str]:
+    fields: List[dict[str:str]] = []
+    map_members = ""
+    set_members = ""
+    map_special = ""
+    set_special = ""
+
+    if not base:
+        return fields, map_members, set_members, map_special, set_special
+
+    if base.base is not None:
+        embed_base = self.schema.lookup_entity(base.base.name)
+        (
+            fields,
+            map_members,
+            set_members,
+            map_special,
+            set_special,
+        ) = recursive_base_nullable(self, embed_base)
+
+    for member in base.local_members:
+        field, _ = get_struct_field(
+            self, member.name, member.type.name, True, member.optional, False
+        )
+        fields.append(field)
+
+        member_type = qapi_schema_type_to_go_type(member.type.name)
+        nullable = member_type in self.accept_null_types
+        field_name = qapi_to_field_name(member.name)
+        tomap, toset = map_and_set(
+            nullable, field_name, member.optional, member.name
+        )
+        if nullable:
+            map_special += tomap
+            set_special += toset
+        else:
+            map_members += tomap
+            set_members += toset
+
+    return fields, map_members, set_members, map_special, set_special
+
+
+# Helper function. This is executed when the QAPI schema has members
+# that could accept JSON NULL (e.g: StrOrNull in QEMU"s QAPI schema).
+# This struct will need to be extended with Marshal/Unmarshal methods to
+# properly handle such atypical members.
+#
+# Only the Marshallaing methods are generated but we do need to iterate over
+# all the members to properly set/check them in those methods.
+def struct_with_nullable_generate_marshal(
+    self: QAPISchemaGenGolangVisitor,
+    name: str,
+    base: Optional[QAPISchemaObjectType],
+    members: List[QAPISchemaObjectTypeMember],
+    variants: Optional[QAPISchemaVariants],
+) -> str:
+
+    (
+        fields,
+        map_members,
+        set_members,
+        map_special,
+        set_special,
+    ) = recursive_base_nullable(self, base)
+
+    if members:
+        for member in members:
+            field, _ = get_struct_field(
+                self,
+                member.name,
+                member.type.name,
+                True,
+                member.optional,
+                False,
+            )
+            fields.append(field)
+
+            member_type = qapi_schema_type_to_go_type(member.type.name)
+            nullable = member_type in self.accept_null_types
+            tomap, toset = map_and_set(
+                nullable,
+                qapi_to_field_name(member.name),
+                member.optional,
+                member.name,
+            )
+            if nullable:
+                map_special += tomap
+                set_special += toset
+            else:
+                map_members += tomap
+                set_members += toset
+
+    if variants:
+        for variant in variants.variants:
+            if variant.type.is_implicit():
+                continue
+
+            field, _ = get_struct_field(
+                self,
+                variant.name,
+                variant.type.name,
+                True,
+                variant.optional,
+                True,
+            )
+            fields.append(field)
+
+            member_type = qapi_schema_type_to_go_type(variant.type.name)
+            nullable = member_type in self.accept_null_types
+            tomap, toset = map_and_set(
+                nullable,
+                qapi_to_field_name(variant.name),
+                variant.optional,
+                variant.name,
+            )
+            if nullable:
+                map_special += tomap
+                set_special += toset
+            else:
+                map_members += tomap
+                set_members += toset
+
+    type_name = qapi_to_go_type_name(name)
+    struct = generate_struct_type("", fields, ident=1)[:-1]
+    return TEMPLATE_STRUCT_WITH_NULLABLE_MARSHAL.format(
+        struct=struct[1:],
+        type_name=type_name,
+        map_members=map_members,
+        map_special=map_special,
+        set_members=set_members,
+        set_special=set_special,
+    )
 
 
 def recursive_base(
     self: QAPISchemaGenGolangVisitor,
     base: Optional[QAPISchemaObjectType],
     discriminator: Optional[str] = None,
-) -> List[dict[str:str]]:
+) -> Tuple[List[dict[str:str]], bool]:
     fields: List[dict[str:str]] = []
+    with_nullable = False
 
     if not base:
-        return fields
+        return fields, with_nullable
 
     if base.base is not None:
         embed_base = self.schema.lookup_entity(base.base.name)
-        fields = recursive_base(self, embed_base, discriminator)
+        fields, with_nullable = recursive_base(self, embed_base, discriminator)
 
     for member in base.local_members:
         if discriminator and member.name == discriminator:
             continue
-        field = get_struct_field(
-            self, member.name, member.type.name, member.optional, False
+        field, nullable = get_struct_field(
+            self, member.name, member.type.name, False, member.optional, False
         )
         fields.append(field)
+        with_nullable = True if nullable else with_nullable
 
-    return fields
+    return fields, with_nullable
 
 
 # Helper function that is used for most of QAPI types
@@ -319,14 +519,20 @@ def qapi_to_golang_struct(
     variants: Optional[QAPISchemaVariants],
 ) -> str:
 
-    fields = recursive_base(self, base)
+    fields, with_nullable = recursive_base(self, base)
 
     if members:
         for member in members:
-            field = get_struct_field(
-                self, member.name, member.type.name, member.optional, False
+            field, nullable = get_struct_field(
+                self,
+                member.name,
+                member.type.name,
+                False,
+                member.optional,
+                False,
             )
             fields.append(field)
+            with_nullable = True if nullable else with_nullable
 
     if variants:
         fields.append({"comment": "Variants fields"})
@@ -334,13 +540,18 @@ def qapi_to_golang_struct(
             if variant.type.is_implicit():
                 continue
 
-            field = get_struct_field(
-                self, variant.name, variant.type.name, True, True
+            field, nullable = get_struct_field(
+                self, variant.name, variant.type.name, False, True, True
             )
             fields.append(field)
+            with_nullable = True if nullable else with_nullable
 
     type_name = qapi_to_go_type_name(name)
     content = generate_struct_type(type_name, fields)
+    if with_nullable:
+        content += struct_with_nullable_generate_marshal(
+            self, name, base, members, variants
+        )
     return content
 
 
@@ -465,7 +676,9 @@ def visit_begin(self, schema: QAPISchema) -> None:
         for target in self.target:
             self.target[target] = f"package {self.golang_package_name}\n"
 
-            if target == "helper":
+            if target == "struct":
+                imports = '\nimport "encoding/json"\n'
+            elif target == "helper":
                 imports = """\nimport (
 \t"encoding/json"
 \t"strings"
-- 
2.41.0



  parent reply	other threads:[~2023-10-16 15:28 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-16 15:26 [PATCH v2 00/11] qapi-go: add generator for Golang interface Victor Toso
2023-10-16 15:26 ` [PATCH v2 01/11] qapi: re-establish linting baseline Victor Toso
2023-10-16 15:26 ` [PATCH v2 02/11] scripts: qapi: black format main.py Victor Toso
2023-10-18 11:00   ` Markus Armbruster
2023-10-18 11:13     ` Daniel P. Berrangé
2023-10-18 15:23     ` Victor Toso
2023-10-19  5:42       ` Markus Armbruster
2023-10-19  7:30         ` Daniel P. Berrangé
2023-10-16 15:26 ` [PATCH v2 03/11] qapi: golang: Generate qapi's enum types in Go Victor Toso
2023-10-16 15:26 ` [PATCH v2 04/11] qapi: golang: Generate qapi's alternate " Victor Toso
2023-11-06 15:28   ` Andrea Bolognani
2023-11-06 15:52     ` Victor Toso
2023-11-06 16:12       ` Andrea Bolognani
2023-11-09 17:34   ` Andrea Bolognani
2023-11-09 19:01     ` Victor Toso
2023-11-10  9:58       ` Andrea Bolognani
2023-11-10 15:43         ` Victor Toso
2023-10-16 15:26 ` [PATCH v2 05/11] qapi: golang: Generate qapi's struct " Victor Toso
2023-10-16 15:26 ` Victor Toso [this message]
2023-10-16 15:27 ` [PATCH v2 07/11] qapi: golang: Generate qapi's union " Victor Toso
2023-11-09 17:29   ` Andrea Bolognani
2023-11-09 18:35     ` Victor Toso
2023-11-10  9:54       ` Andrea Bolognani
2023-11-10 15:45         ` Victor Toso
2023-10-16 15:27 ` [PATCH v2 08/11] qapi: golang: Generate qapi's event " Victor Toso
2023-11-09 17:59   ` Andrea Bolognani
2023-11-09 19:13     ` Victor Toso
2023-11-10  9:52       ` Andrea Bolognani
2023-10-16 15:27 ` [PATCH v2 09/11] qapi: golang: Generate qapi's command " Victor Toso
2023-10-16 15:27 ` [PATCH v2 10/11] qapi: golang: Add CommandResult type to Go Victor Toso
2023-11-09 18:24   ` Andrea Bolognani
2023-11-09 19:31     ` Victor Toso
2023-11-10  9:46       ` Andrea Bolognani
2023-10-16 15:27 ` [PATCH v2 11/11] docs: add notes on Golang code generator Victor Toso
2023-10-18 11:47   ` Markus Armbruster
2023-10-18 16:21     ` Victor Toso
2023-10-19  6:56       ` Markus Armbruster
2023-10-27 17:33 ` [PATCH v2 00/11] qapi-go: add generator for Golang interface Victor Toso
2023-10-31 16:42   ` Andrea Bolognani
2023-11-03 18:34     ` Andrea Bolognani
2023-11-06 12:00       ` Victor Toso
2023-11-09 18:35 ` Andrea Bolognani
2023-11-09 19:03   ` Victor Toso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231016152704.221611-7-victortoso@redhat.com \
    --to=victortoso@redhat.com \
    --cc=abologna@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).