[PATCH common v3 02/21] rest handler: handle: respect schema's 'type-property' when resolving type

[Fiona Ebner <f.ebner@proxmox.com>]

For the HA resources 'migrate' endpoint, the plan is to use a oneOf
schema with the type parameter being 'resource-type' and a
resolve_type() function resolving the resource type based on the
service ID, which consists of the resource type and the numerical ID
of the resource. Resolve the type property instead of hard-coding
'type' and add a test case modeled after the above use case.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v3.

 src/PVE/JSONSchema.pm    |  10 +++
 src/PVE/RESTHandler.pm   |  21 ++++--
 test/get-options-test.pl | 150 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 7 deletions(-)

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 34f2949..4b4842d 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -2254,6 +2254,16 @@ sub get_object_property_schema($schema, $key, $object_data = undef) {
     return;
 }
 
+# Get the type property of an object-like schema. Returns undef if there is no type property.
+sub get_object_type_property($schema) {
+    if (my $all_of = $schema->{allOf}) {
+        for my $subschema ($all_of->@*) {
+            return $subschema->{'type-property'} if defined($subschema->{'type-property'});
+        }
+    }
+    return $schema->{'type-property'};
+}
+
 # $schema: an object-like schema
 # $sub: sub($key, $schema, $one_of_instance_info) ->
 #    ()     empty list -> continue
diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm
index dc6f329..316ce3a 100644
--- a/src/PVE/RESTHandler.pm
+++ b/src/PVE/RESTHandler.pm
@@ -522,14 +522,21 @@ sub handle {
         # Type resolution must happen before normalization, since normalization needs to know
         # the schema of values, for which the type must already be known, otherwise the oneOf
         # variants will be ignored and normalization silently skips over parameters.
-        # The property name is fixed, matching the 'type-property' SectionConfig generates.
         my $resolve_type_hook = $info->{resolve_type};
-        if ($resolve_type_hook && ref($param) eq 'HASH' && !defined($param->{type})) {
-            # The callback only inspects the parameters, so hand it a clone.
-            my $resolved_type = $resolve_type_hook->(clone($param));
-            # NOTE: the resolved type is passed on to the method's code, which must tolerate it.
-            if (defined($resolved_type)) {
-                $param->{type} = $resolved_type;
+        if ($resolve_type_hook && ref($param) eq 'HASH') {
+            my $type_property;
+            if (PVE::JSONSchema::is_object_like_schema($schema)) {
+                $type_property = PVE::JSONSchema::get_object_type_property($schema);
+            }
+            # TODO enforce that 'type-property' is defined instead of using a default?
+            $type_property //= 'type';
+            if (!defined($param->{$type_property})) {
+                # The callback only inspects the parameters, so hand it a clone.
+                my $resolved_type = $resolve_type_hook->(clone($param));
+                # NOTE: the resolved type is passed on to the method's code, which must tolerate it.
+                if (defined($resolved_type)) {
+                    $param->{$type_property} = $resolved_type;
+                }
             }
         }
 
diff --git a/test/get-options-test.pl b/test/get-options-test.pl
index 6eeee37..154cd11 100755
--- a/test/get-options-test.pl
+++ b/test/get-options-test.pl
@@ -753,6 +753,156 @@ package DirectOneOf {
     }
 }
 
+package OneOfWithCustomResolvedTypeProperty {
+    usebase;
+
+    sub desc($class) {
+        "oneOf as top-level with a custom 'type-property' that is resolved via 'resolve_type'";
+    }
+
+    sub schema($class) {
+        my $implicit_type_discriminator = {
+            type => 'string',
+            description => 'A property from which the type can be deduced.',
+            enum => ['deduce-type-one', 'deduce-type-two'],
+        };
+        return {
+            'type-property' => 'custom-type',
+            'type-property-schema' => {
+                type => 'string',
+                description => 'The type.',
+                enum => ['one', 'two'],
+            },
+            oneOf => [
+                {
+                    'instance-type' => 'one',
+                    additionalProperties => 0,
+                    properties => {
+                        'implicit-type-discriminator' => $implicit_type_discriminator,
+                        'prop-one' => {
+                            optional => 1,
+                            type => 'string',
+                            description => 'a or b',
+                            enum => ['a', 'b'],
+                        },
+                    },
+                },
+                {
+                    'instance-type' => 'two',
+                    additionalProperties => 0,
+                    properties => {
+                        'implicit-type-discriminator' => $implicit_type_discriminator,
+                        'prop-two' => {
+                            type => 'number',
+                            description => 'number',
+                            minimum => 3,
+                            maximum => 100,
+                        },
+                    },
+                },
+            ],
+        };
+    }
+
+    sub additional_method_info($class) {
+        return (
+            resolve_type => sub {
+                my ($param) = @_;
+                return 'one' if $param->{'implicit-type-discriminator'} eq 'deduce-type-one';
+                return 'two' if $param->{'implicit-type-discriminator'} eq 'deduce-type-two';
+                die "unable to resolve type\n";
+            },
+        );
+    }
+
+    sub long_usage_str($class, $prefix) {
+        "USAGE: $prefix --custom-type <string> [OPTIONS]\n"
+            . "  --custom-type <one | two>\n"
+            . "\t     The type.\n" . "\n"
+            . " Conditional options:\n" . "\n"
+            . " [custom-type=one]\n" . "\n"
+            . "  --implicit-type-discriminator <deduce-type-one | deduce-type-two>\n"
+            . "\t     A property from which the type can be deduced.\n" . "\n"
+            . "  --prop-one <a | b>\n"
+            . "\t     a or b\n" . "\n"
+            . " [custom-type=two]\n" . "\n"
+            . "  --implicit-type-discriminator <deduce-type-one | deduce-type-two>\n"
+            . "\t     A property from which the type can be deduced.\n" . "\n"
+            . "  --prop-two <number> (3 - 100)\n"
+            . "\t     number\n" . "\n";
+    }
+
+    sub invocations($class) {
+        return (
+            {
+                desc => "implicit-type-discriminator parameter works",
+                args => [qw(--implicit-type-discriminator deduce-type-one)],
+                expected => {
+                    'custom-type' => 'one',
+                    'implicit-type-discriminator' => 'deduce-type-one',
+                },
+            },
+            {
+                desc => "valid options parse",
+                args => [qw(--implicit-type-discriminator deduce-type-one --prop-one a)],
+                expected => {
+                    'custom-type' => 'one',
+                    'implicit-type-discriminator' => 'deduce-type-one',
+                    'prop-one' => 'a',
+                },
+            },
+            {
+                desc => "invalid options are rejected",
+                args => [qw(--implicit-type-discriminator deduce-type-one --prop-invalid a)],
+                error => "400 unable to parse option\n",
+            },
+            {
+                desc => "optional arg_param is optional",
+                args => [qw(--implicit-type-discriminator deduce-type-one)],
+                arg_param => [qw(prop-one)],
+                expected => {
+                    'custom-type' => 'one',
+                    'implicit-type-discriminator' => 'deduce-type-one',
+                },
+            },
+            {
+                desc => "optional arg_param is functional",
+                args => [qw(--implicit-type-discriminator deduce-type-one b)],
+                arg_param => [qw(prop-one)],
+                expected => {
+                    'custom-type' => 'one',
+                    'implicit-type-discriminator' => 'deduce-type-one',
+                    'prop-one' => 'b',
+                },
+            },
+            {
+                desc => "mandatory arg_param is mandatory",
+                args => [qw(--implicit-type-discriminator deduce-type-two)],
+                arg_param => [qw(prop-two)],
+                error => "400 not enough arguments\n",
+            },
+            {
+                desc => "mandatory arg_param is is functional",
+                args => [qw(--implicit-type-discriminator deduce-type-two 33)],
+                arg_param => [qw(prop-two)],
+                expected => {
+                    'custom-type' => 'two',
+                    'implicit-type-discriminator' => 'deduce-type-two',
+                    'prop-two' => 33,
+                },
+            },
+            {
+                desc => "explicit type takes precedence",
+                args => [qw(--implicit-type-discriminator deduce-type-two --custom-type one)],
+                expected => {
+                    'custom-type' => 'one',
+                    'implicit-type-discriminator' => 'deduce-type-two',
+                },
+            },
+        );
+    }
+}
+
 package OneOfArrayVsScalar {
     usebase;
 
-- 
2.47.3