From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id B9AFC1FF0C1 for ; Fri, 18 Sep 2026 18:09:32 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id B8D9221667; Fri, 18 Sep 2026 18:08:53 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Subject: [PATCH common v3 02/21] rest handler: handle: respect schema's 'type-property' when resolving type Date: Fri, 18 Sep 2026 18:08:08 +0200 Message-ID: <20260918160841.128088-3-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260918160841.128088-1-f.ebner@proxmox.com> References: <20260918160841.128088-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1789747729434 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.611 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust RCVD_IN_MSPIKE_H2 0.001 Average reputation (+2) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: AEC7XR2Z4ZL2PCYDUN2ADK2GISWEI25W X-Message-ID-Hash: AEC7XR2Z4ZL2PCYDUN2ADK2GISWEI25W X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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 [OPTIONS]\n" + . " --custom-type \n" + . "\t The type.\n" . "\n" + . " Conditional options:\n" . "\n" + . " [custom-type=one]\n" . "\n" + . " --implicit-type-discriminator \n" + . "\t A property from which the type can be deduced.\n" . "\n" + . " --prop-one \n" + . "\t a or b\n" . "\n" + . " [custom-type=two]\n" . "\n" + . " --implicit-type-discriminator \n" + . "\t A property from which the type can be deduced.\n" . "\n" + . " --prop-two (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