Re: [pve-devel] [PATCH common v3 1/1] SectionConfig: add helper to delete keys from a section config entry

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 17/01/2023 um 12:46 schrieb Dominik Csapak:
> this is a pattern we have very often, so we can implement that here and
> reuse it
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/SectionConfig.pm | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm
> index 0a9f1cd..e354655 100644
> --- a/src/PVE/SectionConfig.pm
> +++ b/src/PVE/SectionConfig.pm
> @@ -543,4 +543,19 @@ sub assert_if_modified {
>      PVE::Tools::assert_if_modified($cfg->{digest}, $digest);
>  }
>  
> +sub delete_from_config {
> +    my ($config, $option_schema, $new_options, $to_delete) = @_;
> +
> +    for my $k ($to_delete->@*) {
> +	my $d = $option_schema->{$k} || die "no such option '$k'\n";
> +	die "unable to delete required option '$k'\n" if !$d->{optional};

Should we use the (here already imported but not used) raise_param_exc ?

As that would then also make the magic "mark-field-as-invalid" work if
used in API calls?

FWIW, we could use a cummulative approach, e.g. something like like:

my $errors = {};

for ... {
   eval {
       die "unable to delete fixed option\n" if $d->{fixed};
       ...
   };
   if (my $err = $@) {
       $errors->{$k} = $err;
   } else {
       delete ...
   }
}

raise_param_exc($errors) if scalar($errors->%*);


eval + die could be replaced by $err string in the loop for accumulating even
more errors, not sure how relevant in practice though.


Anyhow, method signature wouldn't change, so we could always do this as
followup too.

> +	die "unable to delete fixed option '$k'\n" if $d->{fixed};
> +	die "cannot set and delete property '$k' at the same time!\n"
> +	    if defined($new_options->{$k});
> +	delete $config->{$k};
> +    }
> +
> +    return $config;
> +}
> +
>  1;