[pve-devel] [PATCH v2 common 1/2] JSONSchema: add format validator support and cleanup check_format

[Stefan Reiter]

Adds a third, optional parameter to register_format that allows specifying
a function that will be called after parsing and can validate the parsed
data. A validator should die on failed validation, and can also change the
parsed object by returning a modified version of it.

This is useful so one can register a format with its hash, thus allowing
documentation to be generated automatically, while still enforcing certain
validation rules.

The validator only needs to be called in parse_property_string, since
check_format always calls parse_property_string if there is a
possibility of a validator existing at all. parse_property_string should
then be called with named formats for best effect, as only then can
validators be used.

Clean up 'check_format' as well (which pretty much amounts to a rewrite).
No existing functionality is intentionally changed.

Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---
 src/PVE/JSONSchema.pm | 87 +++++++++++++++++++++++++++----------------
 1 file changed, 55 insertions(+), 32 deletions(-)

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 84fb694..f987006 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -121,19 +121,26 @@ register_standard_option('pve-snapshot-name', {
 });
 
 my $format_list = {};
+my $format_validators = {};
 
 sub register_format {
-    my ($format, $code) = @_;
+    my ($name, $format, $validator) = @_;
 
-    die "JSON schema format '$format' already registered\n"
-	if $format_list->{$format};
+    die "JSON schema format '$name' already registered\n"
+	if $format_list->{$name};
 
-    $format_list->{$format} = $code;
+    if ($validator) {
+	die "A \$validator function can only be specified for hash-based formats\n"
+	    if ref($format) ne 'HASH';
+	$format_validators->{$name} = $validator;
+    }
+
+    $format_list->{$name} = $format;
 }
 
 sub get_format {
-    my ($format) = @_;
-    return $format_list->{$format};
+    my ($name) = @_;
+    return $format_list->{$name};
 }
 
 my $renderer_hash = {};
@@ -647,39 +654,47 @@ sub pve_verify_tfa_secret {
 sub check_format {
     my ($format, $value, $path) = @_;
 
-    return parse_property_string($format, $value, $path) if ref($format) eq 'HASH';
-    return if $format eq 'regex';
+    if (ref($format) eq 'HASH') {
+	# hash ref cannot have validator/list/opt handling attached
+	return parse_property_string($format, $value, $path);
+    }
 
-    if ($format =~ m/^(.*)-a?list$/) {
+    if (ref($format) eq 'CODE') {
+	# we are the (sole, old-style) validator
+	return $format->($value);
+    }
+
+    return if $format eq 'regex';
 
-	my $code = $format_list->{$1};
+    my $parsed;
+    $format =~ m/^(.*?)(?:-a?(list|opt))?$/;
+    my ($format_name, $format_type) = ($1, $2 // 'none');
+    my $registered = get_format($format_name);
+    die "undefined format '$format'\n" if !$registered;
 
-	die "undefined format '$format'\n" if !$code;
+    die "'-$format_type' format must have code ref, not hash\n"
+	if $format_type ne 'none' && ref($registered) ne 'CODE';
 
+    if ($format_type eq 'list') {
 	# Note: we allow empty lists
 	foreach my $v (split_list($value)) {
-	    &$code($v);
+	    $parsed = $registered->($v);
 	}
-
-    } elsif ($format =~ m/^(.*)-opt$/) {
-
-	my $code = $format_list->{$1};
-
-	die "undefined format '$format'\n" if !$code;
-
-	return if !$value; # allow empty string
-
- 	&$code($value);
-
+    } elsif ($format_type eq 'opt') {
+	$parsed = $registered->($value) if $value;
    } else {
-
-	my $code = $format_list->{$format};
-
-	die "undefined format '$format'\n" if !$code;
-
-	return parse_property_string($code, $value, $path) if ref($code) eq 'HASH';
-	&$code($value);
+	if (ref($registered) eq 'HASH') {
+	    # Note: this is the only case where a validator function could be
+	    # attached, hence it's safe to handle that in parse_property_string.
+	    # We do however have to call it with $format_name instead of
+	    # $registered, so it knows about the name (and thus any validators).
+	    $parsed = parse_property_string($format, $value, $path);
+	} else {
+	    $parsed = $registered->($value);
+	}
     }
+
+    return $parsed;
 }
 
 sub parse_size {
@@ -735,9 +750,16 @@ sub parse_property_string {
     $additional_properties = 0 if !defined($additional_properties);
 
     # Support named formats here, too:
+    my $validator;
     if (!ref($format)) {
-	if (my $desc = $format_list->{$format}) {
-	    $format = $desc;
+	if (my $reg = get_format($format)) {
+	    die "parse_property_string only accepts hash based named formats\n"
+		if ref($reg) ne 'HASH';
+
+	    # named formats can have validators attached
+	    $validator = $format_validators->{$format};
+
+	    $format = $reg;
 	} else {
 	    die "unknown format: $format\n";
 	}
@@ -793,6 +815,7 @@ sub parse_property_string {
 	raise "format error\n", errors => $errors;
     }
 
+    return $validator->($res) if $validator;
     return $res;
 }
 
-- 
2.20.1