json schema: implement 'oneOf' schema

a schema can now have the 'oneOf' property which is an array of regular
schemas. In the default case any of that has to match. If the
'type-property'/'instance-types' are given, only the schema for the specific
type will be checked (and handles as 'additionalProperties' if there is
no matching type)

the field found in 'type-property' has to be on the same level
(so for oneOf the nested schemas should not include that).

Documentation is adapted so that options are grouped per `type-property=value`
after the regular options (with their individual descriptions/types/etc.)

oneOfs without 'type-property'/'instance-tyeps' simply show up twice for
now with an 'or' line in between.

command line parsing is a bit weird for now since Getopt::Long
can't have multiple variants for the same property (but works fine with
pvesh for our current use cases). it gets shown as '--foo <multiple' if
they are not optional.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
index 5c7863a886abb4eb406e4780b926900f6302151c..bb97a7d7d2a0866b8dbb30aa1096f2a92543e155 100644 (file)
--- a/src/PVE/CLIHandler.pm
+++ b/src/PVE/CLIHandler.pm
@@ -433,7 +433,7 @@ my $print_bash_completion = sub {
                my $res = $d->{completion}->($cmd, $pname, $cur, $args);
                &$print_result(@$res);
            }
-       } elsif ($d->{type} eq 'boolean') {
+       } elsif ($d->{type} && $d->{type} eq 'boolean') {
            &$print_result('0', '1');
        } elsif ($d->{enum}) {
            &$print_result(@{$d->{enum}});

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 49e0d7a679762ffd5f8daf57ab95cc1fa91ccfdd..6e1cd11384e1e3964a9b5712368be949cccb5eaa 100644 (file)
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -1087,6 +1087,16 @@ sub check_type {
     return undef;
 }
 
+my sub get_instance_type {
+    my ($schema, $key, $value) = @_;
+
+    if (my $type_property = $schema->{$key}->{'type-property'}) {
+       return $value->{$type_property};
+    }
+
+    return undef;
+}
+
 sub check_object {
     my ($path, $schema, $value, $additional_properties, $errors) = @_;
 
@@ -1105,7 +1115,8 @@ sub check_object {
     }
 
     foreach my $k (keys %$schema) {
-       check_prop($value->{$k}, $schema->{$k}, $path ? "$path.$k" : $k, $errors);
+       my $instance_type = get_instance_type($schema, $k, $value);
+       check_prop($value->{$k}, $schema->{$k}, $path ? "$path.$k" : $k, $errors, $instance_type);
     }
 
     foreach my $k (keys %$value) {
@@ -1123,7 +1134,23 @@ sub check_object {
                }
            }
 
-           next; # value is already checked above
+           # if it's a oneOf, check if there is a matching type
+           my $matched_type = 1;
+           if ($subschema->{oneOf}) {
+               my $instance_type = get_instance_type($schema, $k, $value);
+               $matched_type = 0;
+               for my $alternative ($subschema->{oneOf}->@*) {
+                   if (my $instance_types = $alternative->{'instance-types'}) {
+                       if (!grep { $instance_type eq $_ } $instance_types->@*) {
+                           next;
+                       }
+                   }
+                   $matched_type = 1;
+                   last;
+               }
+           }
+
+           next if $matched_type; # value is already checked above
        }
 
        if (defined ($additional_properties) && !$additional_properties) {
@@ -1150,7 +1177,7 @@ sub check_object_warn {
 }
 
 sub check_prop {
-    my ($value, $schema, $path, $errors) = @_;
+    my ($value, $schema, $path, $errors, $instance_type) = @_;
 
     die "internal error - no schema" if !$schema;
     die "internal error" if !$errors;
@@ -1163,6 +1190,58 @@ sub check_prop {
        return;
     }
 
+    # must pass any of the given schemas
+    my $optional_for_type = 0;
+    if ($schema->{oneOf}) {
+       # in case we have an instance_type given, just check for that variant
+       if ($schema->{'type-property'}) {
+           $optional_for_type = 1;
+           for (my $i = 0; $i < scalar($schema->{oneOf}->@*); $i++) {
+               last if !$instance_type; # treat as optional if we don't have a type
+               my $inner_schema = $schema->{oneOf}->[$i];
+
+               if (!defined($inner_schema->{'instance-types'})) {
+                   add_error($errors, $path, "missing 'instance-types' in oneOf alternative");
+                   return;
+               }
+
+               next if !grep { $_ eq $instance_type } $inner_schema->{'instance-types'}->@*;
+               $optional_for_type = $inner_schema->{optional} // 0;
+               check_prop($value, $inner_schema, $path, $errors);
+           }
+       } else {
+           my $is_valid = 0;
+           my $collected_errors = {};
+           for (my $i = 0; $i < scalar($schema->{oneOf}->@*); $i++) {
+               my $inner_schema = $schema->{oneOf}->[$i];
+               my $inner_errors = {};
+               check_prop($value, $inner_schema, "$path.oneOf[$i]", $inner_errors);
+               if (!$inner_errors->%*) {
+                   $is_valid = 1;
+                   last;
+               }
+
+               for my $inner_path (keys $inner_errors->%*) {
+                   add_error($collected_errors, $inner_path, $inner_errors->{$path});
+               }
+           }
+
+           if (!$is_valid) {
+               for my $inner_path (keys $collected_errors->%*) {
+                   add_error($errors, $inner_path, $collected_errors->{$path});
+               }
+           }
+       }
+    } elsif ($instance_type) {
+       if (!defined($schema->{'instance-types'})) {
+           add_error($errors, $path, "missing 'instance-types'");
+           return;
+       }
+       if (grep { $_ eq $instance_type} $schema->{'instance_types'}->@*) {
+           $optional_for_type = 1;
+       }
+    }
+
     # if it extends another schema, it must pass that schema as well
     if($schema->{extends}) {
        check_prop($value, $schema->{extends}, $path, $errors);
@@ -1170,7 +1249,7 @@ sub check_prop {
 
     if (!defined ($value)) {
        return if $schema->{type} && $schema->{type} eq 'null';
-       if (!$schema->{optional} && !$schema->{alias} && !$schema->{group}) {
+       if (!$schema->{optional} && !$schema->{alias} && !$schema->{group} && !$optional_for_type) {
            add_error($errors, $path, "property is missing and it is not optional");
        }
        return;
@@ -1317,6 +1396,28 @@ my $default_schema_noref = {
            },
            enum => $schema_valid_types,
        },
+       oneOf => {
+           type => 'array',
+           description => "This represents the alternative options for this Schema instance.",
+           optional => 1,
+           items => {
+               type => 'object',
+               description => "A valid option of the properties",
+           },
+       },
+       'instance-types' => {
+           type => 'array',
+           description => "Indicate to which type the parameter (or variant if inside a oneOf) belongs.",
+           optional => 1,
+           items => {
+               type => 'string',
+           },
+       },
+       'type-property' => {
+           type => 'string',
+           description => "The property to check for instance types.",
+           optional => 1,
+       },
        optional => {
            type => "boolean",
            description => "This indicates that the instance property in the instance object is not required.",
@@ -1491,6 +1592,7 @@ my $default_schema = Storable::dclone($default_schema_noref);
 
 $default_schema->{properties}->{properties}->{additionalProperties} = $default_schema;
 $default_schema->{properties}->{additionalProperties}->{properties} = $default_schema->{properties};
+$default_schema->{properties}->{oneOf}->{items}->{properties} = $default_schema->{properties};
 
 $default_schema->{properties}->{items}->{properties} = $default_schema->{properties};
 $default_schema->{properties}->{items}->{additionalProperties} = 0;
@@ -1713,12 +1815,12 @@ sub get_options {
            # optional and call the mapping function afterwards.
            push @getopt, "$prop:s";
            push @interactive, [$prop, $mapping->{func}];
-       } elsif ($pd->{type} eq 'boolean') {
+       } elsif ($pd->{type} && $pd->{type} eq 'boolean') {
            push @getopt, "$prop:s";
        } else {
            if ($pd->{format} && $pd->{format} =~ m/-list/) {
                push @getopt, "$prop=s@";
-           } elsif ($pd->{type} eq 'array') {
+           } elsif ($pd->{type} && $pd->{type} eq 'array') {
                push @getopt, "$prop=s@";
            } else {
                push @getopt, "$prop=s";
@@ -1807,7 +1909,7 @@ sub get_options {
 
     foreach my $p (keys %$opts) {
        if (my $pd = $schema->{properties}->{$p}) {
-           if ($pd->{type} eq 'boolean') {
+           if ($pd->{type} && $pd->{type} eq 'boolean') {
                if ($opts->{$p} eq '') {
                    $opts->{$p} = 1;
                } elsif (defined(my $bool = parse_boolean($opts->{$p}))) {

diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm
index b99eb15b245e9c60eb5544197e08401158a98452..7bf6b7450a2e7ab6a414f4e5be19cafa384a1fd0 100644 (file)
--- a/src/PVE/RESTHandler.pm
+++ b/src/PVE/RESTHandler.pm
@@ -725,12 +725,19 @@ sub getopt_usage {
     my $idx_param = {}; # -vlan\d+ -scsi\d+
 
     my $opts = '';
+
+    my $type_specific_opts = {};
+
     foreach my $k (sort keys %$prop) {
        next if $arg_hash->{$k};
        next if defined($fixed_param->{$k});
 
        my $type_text = $prop->{$k}->{type} || 'string';
 
+       if ($prop->{$k}->{oneOf}) {
+           $type_text = 'multiple';
+       }
+
        my $param_map = {};
 
        if (defined($param_cb)) {
@@ -749,10 +756,51 @@ sub getopt_usage {
            }
        }
 
+       my $is_optional = $prop->{$k}->{optional} // 0;
+
+       if (my $type_property = $prop->{$k}->{'type-property'}) {
+           # save type specific descriptions for later
+           my $type_schema = $prop->{$type_property};
+           if ($prop->{$k}->{oneOf}) {
+               # it's optional if there are less options than types
+               $is_optional = 1 if scalar($type_schema->{enum}->@*) > scalar($prop->{$k}->{oneOf}->@*);
+               for my $alternative ($prop->{$k}->{oneOf}->@*) {
+                   # it's optional if at least one variant is optional
+                   $is_optional = 1 if $alternative->{optional};
+                   for my $type ($alternative->{'instance-types'}->@*) {
+                       my $key = "${type_property}=${type}";
+                       $type_specific_opts->{$key} //= "";
+                       $type_specific_opts->{$key}
+                           .= $get_property_description->($base, 'arg', $alternative, $format, $param_map->{$k});
+                   }
+               }
+           } elsif (my $types = $prop->{$k}->{'instance-types'}) {
+               # it's optional if not all types has that option
+               $is_optional = 1 if scalar($type_schema->{enum}->@*) > scalar($types->@*);
+               for my $type ($types->@*) {
+                   my $key = "${type_property}=${type}";
+                   $type_specific_opts->{$key} //= "";
+                   $type_specific_opts->{$key}
+                       .= $get_property_description->($base, 'arg', $prop->{$k}, $format, $param_map->{$k});
+               }
+           }
+       } elsif ($prop->{$k}->{oneOf}) {
+           my $res = [];
+           for my $alternative ($prop->{$k}->{oneOf}->@*) {
+               # it's optional if at least one variant is optional
+               $is_optional = 1 if $alternative->{optional};
+               push $res->@*, $get_property_description->($base, 'arg', $alternative, $format, $param_map->{$k});
+           }
+           if ($format eq 'asciidoc') {
+               $opts .= join("\n\nor\n\n", $res->@*);
+           } else {
+               $opts .= join("  or\n\n", $res->@*);
+           }
+       } else {
+           $opts .= $get_property_description->($base, 'arg', $prop->{$k}, $format, $param_map->{$k});
+       }
 
-       $opts .= $get_property_description->($base, 'arg', $prop->{$k}, $format, $param_map->{$k});
-
-       if (!$prop->{$k}->{optional}) {
+       if (!$is_optional) {
            $args .= " " if $args;
            $args .= "--$base <$type_text>"
        }
@@ -780,7 +828,7 @@ sub getopt_usage {
            $out .= "\n$desc\n\n";
        } elsif ($format eq 'full') {
            my $desc = Text::Wrap::wrap('  ', '  ', ($info->{description}));
-           $out .= "\n$desc\n";
+           $out .= "\n$desc\n\n";
        }
     }
 
@@ -788,6 +836,23 @@ sub getopt_usage {
 
     $out .= $opts if $opts;
 
+    if (scalar(keys $type_specific_opts->%*)) {
+       if ($format eq 'asciidoc') {
+           $out .= "\n\n\n`Conditional options:`\n\n";
+       } else {
+           $out .= " Conditional options:\n\n";
+       }
+    }
+
+    for my $type_opts (sort keys $type_specific_opts->%*) {
+       if ($format eq 'asciidoc') {
+           $out .= "`[$type_opts]` ;;\n\n";
+       } else {
+           $out .= " [$type_opts]\n\n";
+       }
+       $out .= $type_specific_opts->{$type_opts};
+    }
+
     return $out;
 }
 
@@ -825,7 +890,14 @@ sub dump_properties {
            }
        }
 
-       $raw .= $get_property_description->($base, $style, $phash, $format);
+       if ($phash->{oneOf}) {
+           for my $alternative ($phash->{oneOf}->@*) {
+               $raw .= $get_property_description->($base, $style, $alternative, $format);
+           }
+       } else {
+           $raw .= $get_property_description->($base, $style, $phash, $format);
+       }
+
 
        next if $style ne 'config';