From e60e6e804b164d850c1ef1781c03577fbe95d05c Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Tue, 28 May 2024 02:58:32 -0400
Subject: [PATCH] group_hosts_by: Use `Data::Dumper` to make the group
 signature

This is a bit more robust than manually making the group signature
because it gracefully handles corner cases such as `undef`.
---
 configure.ac        | 2 +-
 ddclient.in         | 5 ++++-
 t/group_hosts_by.pl | 9 ++-------
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index f65b8b3..d1fc02c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -49,6 +49,7 @@ AC_SUBST([PERL])
 # package doesn't depend on all of them, so their availability can't
 # be assumed.
 m4_foreach_w([_m], [
+    Data::Dumper
     File::Basename
     File::Path
     File::Temp
@@ -63,7 +64,6 @@ m4_foreach_w([_m], [
 # then some tests will fail. Only prints a warning if not installed.
 m4_foreach_w([_m], [
     B
-    Data::Dumper
     File::Spec::Functions
     File::Temp
   ], [AX_PROG_PERL_MODULES([_m], [],
diff --git a/ddclient.in b/ddclient.in
index 24cae4f..2bf68e8 100755
--- a/ddclient.in
+++ b/ddclient.in
@@ -15,6 +15,7 @@ package ddclient;
 require v5.10.1;
 use strict;
 use warnings;
+use Data::Dumper;
 use File::Basename;
 use File::Path qw(make_path);
 use File::Temp;
@@ -3425,8 +3426,10 @@ sub group_hosts_by {
     my %attrs = map({ ($_ => 1) } @$attributes);
     my @attrs = sort(keys(%attrs));
     my %groups = ();
+    my $d = Data::Dumper->new([])->Indent(0)->Sortkeys(1)->Terse(1)->Useqq(1);
     for my $h (@$hosts) {
-        my $sig = join(',', map({ sprintf("%s=%s", $_, $config{$h}{$_} // '') } @attrs));
+        my %cfg = map({ ($_ => $config{$h}{$_}); } grep(exists($config{$h}{$_}), @attrs));
+        my $sig = $d->Reset()->Values([\%cfg])->Dump();
         push @{$groups{$sig}}, $h;
     }
     return %groups;
diff --git a/t/group_hosts_by.pl b/t/group_hosts_by.pl
index cc2cc09..61acd0f 100644
--- a/t/group_hosts_by.pl
+++ b/t/group_hosts_by.pl
@@ -60,13 +60,11 @@ my @test_cases = (
         desc => 'falsy values',
         groupby => [qw(falsy)],
         want => [[$h1], [$h2], [$h3]],
-        todo => 'support for undef not yet added',
     },
     {
         desc => 'set, unset, undef',
         groupby => [qw(maybeunset)],
         want => [[$h1], [$h2], [$h3]],
-        todo => 'support for unset and undef not yet added',
     },
     {
         desc => 'missing attribute',
@@ -85,11 +83,8 @@ for my $tc (@test_cases) {
         }
         return @$a <=> @$b;
     } map({ [sort(@$_)]; } values(%got)));
-    TODO: {
-        local $TODO = $tc->{todo};
-        is_deeply(\@got, $tc->{want}, $tc->{desc})
-            or diag(Data::Dumper->Dump([\@got, $tc->{want}], [qw(got want)]));
-    }
+    is_deeply(\@got, $tc->{want}, $tc->{desc})
+        or diag(Data::Dumper->Dump([\@got, $tc->{want}], [qw(got want)]));
 }
 
 done_testing();