diff mbox series

[RFC,v2,7/8] qapi: golang: Add CommandResult type to Go

Message ID 20220617121932.249381-8-victortoso@redhat.com
State New
Headers show
Series qapi: add generator for Golang interface | expand

Commit Message

Victor Toso June 17, 2022, 12:19 p.m. UTC
This patch adds a struct type in Go that will handle return values for
QAPI's command types.

The return value of a Command is, encouraged to be, QAPI's complex
types or an Array of those.

Every Command has a underlying CommandResult. The EmptyCommandReturn
is for those that don't expect any data e.g: `{ "return": {} }`.

All CommandReturn types implement the CommandResult interface.

Example:
qapi:
  | { 'command': 'query-sev', 'returns': 'SevInfo',
  |   'if': 'TARGET_I386' }

go:
  | type QuerySevCommandReturn struct {
  |         CommandId string     `json:"id,omitempty"`
  |         Result    *SevInfo   `json:"return"`
  |         Error     *QapiError `json:"error,omitempty"`
  | }

usage:
  | // One can use QuerySevCommandReturn directly or
  | // command's interface GetReturnType() instead.
  |
  | input := `{ "return": { "enabled": true, "api-major" : 0,` +
  |                        `"api-minor" : 0, "build-id" : 0,` +
  |                        `"policy" : 0, "state" : "running",` +
  |                        `"handle" : 1 } } `
  | ret := QuerySevCommandReturn{}
  | err := json.Unmarshal([]byte(input), &ret)
  | if ret.Error != nil {
  |     // Handle command failure {"error": { ...}}
  | } else if ret.Result != nil {
  |     // ret.Result.Enable == true
  | }

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

Comments

Andrea Bolognani July 5, 2022, 3:46 p.m. UTC | #1
On Fri, Jun 17, 2022 at 02:19:31PM +0200, Victor Toso wrote:
> +type EmptyCommandReturn struct {
> +    CommandId string          `json:"id,omitempty"`
> +    Error     *QapiError      `json:"error,omitempty"`
> +    Name      string          `json:"-"`
> +}

Do we need a specific type for this? Can't we just generate an
appropriately-named type for each of the commands that don't return
any data? It's not like we would have to write that code manually :)

> +func (r *EmptyCommandReturn) GetCommandName() string {
> +    return r.Name
> +}

Just like Event.GetName() and Command.GetName(), I'm not convinced we
should have this.


Of course, all the comments about how marshalling and unmarshalling
are handled made for events also apply here.
Daniel P. Berrangé July 5, 2022, 4:49 p.m. UTC | #2
On Tue, Jul 05, 2022 at 08:46:21AM -0700, Andrea Bolognani wrote:
> On Fri, Jun 17, 2022 at 02:19:31PM +0200, Victor Toso wrote:
> > +type EmptyCommandReturn struct {
> > +    CommandId string          `json:"id,omitempty"`
> > +    Error     *QapiError      `json:"error,omitempty"`
> > +    Name      string          `json:"-"`
> > +}
> 
> Do we need a specific type for this? Can't we just generate an
> appropriately-named type for each of the commands that don't return
> any data? It's not like we would have to write that code manually :)

Yes, I think having an explicit named return struct even for commands
not /currently/ returning data is good, and anticipates future changes
that might add extra return data fields to the QAPI schema.

> 
> > +func (r *EmptyCommandReturn) GetCommandName() string {
> > +    return r.Name
> > +}
> 
> Just like Event.GetName() and Command.GetName(), I'm not convinced we
> should have this.
> 

With regards,
Daniel
Victor Toso Aug. 17, 2022, 3 p.m. UTC | #3
Hi,

On Tue, Jul 05, 2022 at 05:49:22PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 05, 2022 at 08:46:21AM -0700, Andrea Bolognani wrote:
> > On Fri, Jun 17, 2022 at 02:19:31PM +0200, Victor Toso wrote:
> > > +type EmptyCommandReturn struct {
> > > +    CommandId string          `json:"id,omitempty"`
> > > +    Error     *QapiError      `json:"error,omitempty"`
> > > +    Name      string          `json:"-"`
> > > +}
> >
> > Do we need a specific type for this? Can't we just generate an
> > appropriately-named type for each of the commands that don't return
> > any data? It's not like we would have to write that code manually :)
>
> Yes, I think having an explicit named return struct even for commands
> not /currently/ returning data is good, and anticipates future changes
> that might add extra return data fields to the QAPI schema.

Sure.

Cheers,
Victor
diff mbox series

Patch

diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
index 123179cced..ab91cf124f 100644
--- a/scripts/qapi/golang.py
+++ b/scripts/qapi/golang.py
@@ -89,7 +89,8 @@ 
 }}
 '''
 
-# Only variable is @unm_cases to handle all command's names and associated types.
+# Only variable is @unm_cases to handle
+# all command's names and associated types.
 TEMPLATE_COMMAND = '''
 type Command interface {{
     GetId()         string
@@ -145,10 +146,49 @@ 
 }}
 '''
 
+TEMPLATE_COMMAND_RETURN = '''
+type CommandReturn interface {
+    GetId()          string
+    GetCommandName() string
+    GetError()       error
+}
+
+type EmptyCommandReturn struct {
+    CommandId string          `json:"id,omitempty"`
+    Error     *QapiError      `json:"error,omitempty"`
+    Name      string          `json:"-"`
+}
+
+func (r EmptyCommandReturn) MarshalJSON() ([]byte, error) {
+    return []byte(`{"return":{}}`), nil
+}
+
+func (r *EmptyCommandReturn) GetId() string {
+    return r.CommandId
+}
+
+func (r *EmptyCommandReturn) GetCommandName() string {
+    return r.Name
+}
+
+func (r *EmptyCommandReturn) GetError() error {
+    return r.Error
+}
+'''
+
 TEMPLATE_HELPER = '''
 // Alias for go version lower than 1.18
 type Any = interface{}
 
+type QapiError struct {
+    Class       string `json:"class"`
+    Description string `json:"desc"`
+}
+
+func (err *QapiError) Error() string {
+    return fmt.Sprintf("%s: %s", err.Class, err.Description)
+}
+
 // Creates a decoder that errors on unknown Fields
 // Returns true if successfully decoded @from string @into type
 // Returns false without error is failed with "unknown field"
@@ -176,6 +216,7 @@  def __init__(self, prefix: str):
         self.schema = None
         self.events = {}
         self.commands = {}
+        self.command_results = {}
         self.golang_package_name = "qapi"
 
     def visit_begin(self, schema):
@@ -224,6 +265,7 @@  def visit_end(self):
 '''
         self.target["command"] += TEMPLATE_COMMAND.format(unm_cases=unm_cases)
 
+        self.target["command"] += TEMPLATE_COMMAND_RETURN
 
     def visit_object_type(self: QAPISchemaGenGolangVisitor,
                           name: str,
@@ -390,6 +432,31 @@  def visit_command(self,
         self.commands[name] = type_name
         command_ret = ""
         init_ret_type_name = f'''EmptyCommandReturn {{ Name: "{name}" }}'''
+        if ret_type:
+            cmd_ret_name = qapi_to_go_type_name(name, "command return")
+            ret_type_name = qapi_schema_type_to_go_type(ret_type.name)
+            init_ret_type_name = f'''{cmd_ret_name}{{}}'''
+            isptr = "*" if ret_type_name[0] not in "*[" else ""
+            self.command_results[name] = ret_type_name
+            command_ret = f'''
+type {cmd_ret_name} struct {{
+    CommandId  string                `json:"id,omitempty"`
+    Result    {isptr}{ret_type_name} `json:"return"`
+    Error     *QapiError             `json:"error,omitempty"`
+}}
+
+func (r *{cmd_ret_name}) GetCommandName() string {{
+    return "{name}"
+}}
+
+func (r *{cmd_ret_name}) GetId() string {{
+    return r.CommandId
+}}
+
+func (r *{cmd_ret_name}) GetError() error {{
+    return r.Error
+}}
+'''
 
         self_contained = True
         if arg_type and arg_type.name.startswith("q_obj"):
@@ -423,7 +490,7 @@  def visit_command(self,
     return &{init_ret_type_name}
 }}
 '''
-        self.target["command"] += content + methods
+        self.target["command"] += content + methods + command_ret
 
     def visit_event(self, name, info, ifcond, features, arg_type, boxed):
         assert name == info.defn_name
@@ -686,7 +753,7 @@  def qapi_to_go_type_name(name: str, meta: str) -> str:
 
     name += ''.join(word.title() for word in words[1:])
 
-    if meta in ["event", "command"]:
+    if meta in ["event", "command", "command return"]:
         name = name[:-3] if name.endswith("Arg") else name
         name += meta.title().replace(" ", "")