[pve-devel] [RFC v1 pve-storage 5/6] introduce check for `type` method and duplicate types

[Max Carrara <m.carrara@proxmox.com>]

Check whether a plugin defines a `type` method (as required by
PVE::SectionConfig). Throw and exception if it doesn't.

Also check for the same type being declared more than once.

All this is added to make implementing a plugin a little more
developer-friendly through more expressive error messages. Both cases
are otherwise also handled by the `register` method of
PVE::SectionConfig.

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/Storage.pm | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index a9568b1..4cf591f 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -312,6 +312,7 @@ my sub do_pre_register_check_plugin : prototype(%) {
     my (%params) = @_;
 
     my $plugin_name = $params{name};
+    my $plugin_types = $params{plugin_types};
 
     my $plugin_index = $plugin_register_state->{index};
 
@@ -345,6 +346,26 @@ my sub do_pre_register_check_plugin : prototype(%) {
 	    if $version < $min_version;
     };
 
+    my $check_plugin_type = sub {
+	# so that we may call methods on $plugin_name
+	no strict 'refs'; ## no critic
+
+	# Method is inherited, no need to check for its existence.
+	# However, if not overridden, it will `die`
+	my $type = eval {
+	    $plugin_name->type()
+	};
+
+	die "plugin does not override \"type()\" method\n"
+	    if $@;
+
+	my $source_plugin = $plugin_types->{$type};
+	die "plugin with type \"$type\" already exists: $source_plugin\n"
+	    if defined($source_plugin);
+
+	$plugin_types->{$type} = $plugin_name;
+    };
+
     my $check_symbols = sub {
 	my $prohibited_symbols = PVE::Storage::Plugin::get_prohibited_symbols();
 
@@ -364,6 +385,7 @@ my sub do_pre_register_check_plugin : prototype(%) {
     eval {
 	$check_derives_base->();
 	$check_plugin_api->() if $is_custom;
+	$check_plugin_type->();
 	$check_symbols->();
     };
 
@@ -389,10 +411,13 @@ C<L<< do_pre_register_check_plugin()|/"do_pre_register_check_plugin(%params)" >>
 my sub pre_register_check_plugins : prototype() {
     my @errs = ();
 
+    # { type() => plugin_name }
+    my $plugin_types = {};
+
     my @standard_plugins = get_indexed_plugins(is_custom => 0);
     for my $plugin (@standard_plugins) {
 	eval {
-	    do_pre_register_check_plugin(name => $plugin);
+	    do_pre_register_check_plugin(name => $plugin, plugin_types => $plugin_types);
 	};
 
 	push(@errs, $@) if $@;
@@ -408,7 +433,7 @@ my sub pre_register_check_plugins : prototype() {
     my @custom_plugins = get_indexed_plugins(is_custom => 1);
     for my $plugin (@custom_plugins) {
 	eval {
-	    do_pre_register_check_plugin(name => $plugin);
+	    do_pre_register_check_plugin(name => $plugin, plugin_types => $plugin_types);
 	};
 
 	if ($@) {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel