On 6/23/20 3:39 PM, Fabian Grünbichler wrote:
LGTM, what do you think about the following follow-up:
------8<-----
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index b2ba9f7..d28143d 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -1878,9 +1878,12 @@ sub generate_typetext {
sub print_property_string {
my ($data, $format, $skip, $path) = @_;
+ my $validator;
if (ref($format) ne 'HASH') {
my $schema = get_format($format);
die "not a valid format: $format\n" if !$schema;
+ # named formats can have validators attached
+ $validator = $format_validators->{$format};
$format = $schema;
}
@@ -1890,6 +1893,8 @@ sub print_property_string {
raise "format error", errors => $errors;
}
+ $res = $validator->($res) if $validator;
This would have to be $data, I suppose?
+
my ($default_key, $keyAliasProps) = &$find_schema_default_key($format);
my $res = '';
----->8------
to ensure our code calling 'print_property_string' also adheres to the
format the validator enforces?
Fine by me, though I'm not sure how necessary. It implies that
validators always validate positively on values they have already
accepted (and potentially modified) once before though. Also I'm not
sure I would use the validators result for print_property_string, since
that means that the value the user passes in might not be the one
printed (if a validator modifies it) - so maybe call the validator but
throw away it's result?
it might also be nice to mark formats with a validator (at least for the
API dump) to make it obvious that the displayed format might be further
restricted.
Sounds good to me, I'll look into it.
I went through the code paths (again) and still think it might be
worthwhile to extend named formats to check_object as well, instead of
just scalar property string values. but that is a bigger follow-up, for
now limiting the scope of these validators to just property strings
seems sensible.
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
