Refactor read_products and its callers

- Promote to public function since it's used in other plugins anyway
- Move resolving of addons to read_products (print errors immediately)
- Cache product list based on mtime; mostly to reduce the amount of spam
  from errors as performance was never an issue.
- Cache product object in cart entry, so statiegeld_tokens plugin
  doesn't have to do the lookup all over again.

lib/RevBank/Plugins.pm

@@ -53,7 +53,7 @@ sub load($class) {
         RevBank::Eval::clean_eval(qq[
             use strict;
             use warnings;
-            use feature qw(signatures);
+            use feature qw(signatures state);
             no warnings 'experimental::signatures';
             package $package;
             BEGIN { RevBank::Global->import; }

plugins/products

@@ -5,9 +5,15 @@ HELP1 "<productID>" => "Add a product to pending transaction";
 my $filename = 'revbank.products';
 my $default_contra = '+sales/products';
 
-sub _read_products() {
-    my %products;
+sub read_products() {
+    state %products;
+    state $mtime;
+
+    return \%products if $mtime and $mtime == -M $filename;
+    $mtime = -M $filename;
+
     my $line = 0;
+
     for (slurp $filename) {
         $line++;
 
@@ -40,8 +46,8 @@ sub _read_products() {
             }
         }
 
-        my @addons;
-        unshift @addons, $1 while $desc =~ s/\s+ \+ (\S+)$//x;
+        my @addon_ids;
+        unshift @addon_ids, $1 while $desc =~ s/\s+ \+ (\S+)$//x;
 
         $products{$_} = {
             id          => $ids[0],
@@ -49,11 +55,34 @@ sub _read_products() {
             percent     => $percent,
             description => $desc,
             contra      => $contra || $default_contra,
-            addons      => \@addons,
+            _addon_ids  => \@addon_ids,
             line        => $line,
         } for @ids;
     }
 
+    PRODUCT: for my $product (values %products) {
+        my %ids_seen = ($product->{id} => 1);
+        my @addon_ids = @{ $product->{_addon_ids} };
+
+        while (my $addon_id = shift @addon_ids) {
+            $addon_id = "+$addon_id" if exists $products{"+$addon_id"};
+
+            if ($ids_seen{$addon_id}++) {
+                warn "Infinite addon loop for '$product->{id}' at $filename line $product->{line}.\n";
+                next PRODUCT;
+            }
+
+            my $addon = $products{$addon_id};
+            if (not $addon) {
+                warn "Addon '$addon_id' does not exist for '$product->{id}' at $filename line $product->{line}.\n";
+                next PRODUCT;
+            }
+
+            push @{ $product->{addons} }, $addon;
+            push @addon_ids, @{ $addon->{_addon_ids} };
+        }
+    }
+
     return \%products;
 }
 
@@ -61,7 +90,7 @@ sub command :Tab(&tab) ($self, $cart, $command, @) {
     $command =~ /\S/ or return NEXT;
     $command =~ /^\+/ and return NEXT;
 
-    my $products = _read_products;
+    my $products = read_products;
     my $product = $products->{ $command } or return NEXT;
     my $price = $product->{price};
 
@@ -77,7 +106,7 @@ sub command :Tab(&tab) ($self, $cart, $command, @) {
 
     my $contra_desc = "\$you bought $product->{description}";
 
-    my @addons = @{ $product->{addons} };
+    my @addons = @{ $product->{addons} // [] };
 
     my $display = undef;
     $display = "Product"       if @addons and $price->cents > 0;
@@ -86,7 +115,7 @@ sub command :Tab(&tab) ($self, $cart, $command, @) {
     my $entry = $cart->add(
         -$price,
         $product->{description},
-        { product_id => $product->{id}, plugin => $self->id, addons => $product->{addons} }
+        { product_id => $product->{id}, plugin => $self->id, product => $product }
     );
     $entry->add_contra(
         $product->{contra},
@@ -95,18 +124,7 @@ sub command :Tab(&tab) ($self, $cart, $command, @) {
         $display
     );
 
-    my %ids_seen = ($product->{id} => 1);
-
-    while (my $addon_id = shift @addons) {
-        $addon_id = "+$addon_id" if exists $products->{"+$addon_id"};
-
-        if ($ids_seen{$addon_id}++) {
-            return REJECT, "Infinite addons are not supported.";
-        }
-
-        my $addon = $products->{$addon_id}
-            or return REJECT, "Addon '$addon_id' does not exist.";
-
+    for my $addon (@addons) {
         my $addon_price = $addon->{price};
         if ($addon->{percent}) {
             my $sum = List::Util::sum map {
@@ -126,13 +144,11 @@ sub command :Tab(&tab) ($self, $cart, $command, @) {
             "$addon->{description} ($contra_desc)",
             $addon->{description}
         );
-
-        push @addons, @{ $addon->{addons} };
     }
 
     return ACCEPT;
 }
 
 sub tab {
-    return grep /\D/, keys %{ _read_products() };
+    return grep /\D/, keys %{ read_products() };
 }

plugins/statiegeld

@@ -23,24 +23,20 @@ my $nope = "Sorry, no deposit on that product.\n";
 
 our $S = ($ENV{REVBANK_STATIEGELD} // 0) == 1;
 
-sub statiegeld_product($product_id, $products = undef) {
-	$products ||= RevBank::Plugin::products::_read_products();
+sub statiegeld_product($product) {
+	if (not ref $product) {
+		# $product is a product id string; look up in product list
+		my $products = RevBank::Plugin::products::read_products();
+		$product = $products->{$product} or return;
+	}
 
-	my $product = $products->{$product_id} or return;
+	my @relevant_addons = grep {
+		my $addon = $_;
 
-	my @addons = @{ $product->{addons} };
-	my @relevant_addons;
-
-	while (my $product_id = shift @addons) {
-		my $addon = $products->{"+$product_id"} // $products->{$product_id};
-		
-		push @relevant_addons, $addon
-			if !$addon->{percent}
-			and (List::Util::any { $addon->{contra} eq $_ } @addon_accounts)
-			and $addon->{price} > 0;
-
-		push @addons, @{ $addon->{addons} };
-	};
+		!$addon->{percent}
+		and (List::Util::any { $addon->{contra} eq $_ } @addon_accounts)
+		and $addon->{price} > 0;
+	} @{ $product->{addons} // [] };
 
 	return 0 if not @relevant_addons;
 	return { product => $product, statiegeld_addons => \@relevant_addons };
@@ -65,7 +61,7 @@ sub hook_deposit_command($class, $prompt, $array, @) {
 sub command ($invocant, $cart, $command, @) {
 	$S or return NEXT;
 
-	defined &RevBank::Plugin::products::_read_products
+	defined &RevBank::Plugin::products::read_products
 		or die "statiegeld plugin requires products plugin";
 
 	my $sg = statiegeld_product($command) // return NEXT;

plugins/statiegeld_tokens

@@ -100,13 +100,11 @@ sub hook_checkout_prepare($class, $cart, $username, $transaction_id, @) {
 	my $is_new = !defined $tokens_by_id;
 	$tokens_by_id = {} if $is_new;
 
-	my $products = RevBank::Plugin::products::_read_products();
 	my $tokens_changed = 0;
 
 	# Products bought: add tokens
-	for my $entry ($cart->entries('product_id')) {
-		my $id = $entry->attribute('product_id');
-		my $sg = RevBank::Plugin::statiegeld::statiegeld_product($id, $products)
+	for my $entry ($cart->entries('product')) {
+		my $sg = RevBank::Plugin::statiegeld::statiegeld_product($entry->attribute('product'))
 			or next;
 
 		for my $addon (@{ $sg->{statiegeld_addons} }) {
@@ -133,7 +131,6 @@ sub hook_checkout_prepare($class, $cart, $username, $transaction_id, @) {
 		my $available = @{ $tokens_by_id->{$id} // [] };
 
 		if ($available < $entry->quantity) {
-
 			if ($available == 0) {
 				$cart->delete($entry);
 				$warnings_by_id{$id}++;
@@ -147,6 +144,7 @@ sub hook_checkout_prepare($class, $cart, $username, $transaction_id, @) {
 		$tokens_changed++;
 	}
 	for my $id (keys %warnings_by_id) {
+		my $products = RevBank::Plugin::products::read_products();
 		my $addon = $products->{"+$id"} // $products->{$id};
 		my $avail = $had_num_tokens_by_id{$id};
 		my $only = $avail ? "only $avail" : "0";