From c9cdb9608666cdcf044dfa3bf3a80f68bd12b991 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 22 Aug 2024 03:16:32 -0400 Subject: [PATCH] read_recap: Fix copying of recap values into `%config` --- ddclient.in | 14 +++++--------- t/read_recap.pl | 3 ++- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/ddclient.in b/ddclient.in index 3bf4a7a..743993f 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1641,17 +1641,13 @@ sub read_recap { # different pieces of code need to know the list of status variables, and keeping them all # in sync is error-prone (DRY). Also, different protocols might need different status # variables. + # TODO: None of the recap variables should be copied into `%config` because `%config` is + # not the semantically appropriate place to record update status. Code that currently + # reads/sets `$config{$h}{'status-ipv4'}` (and friends) should instead access + # `$recap{$h}{'status-ipv4'}` directly. for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 warned-min-interval warned-min-error-interval)) { - # TODO: This is a no-op. I believe the original intent was to copy values that convey - # update status from `%recap` into `%config` so that ddclient can be restarted and - # resume where it left off and not violate `min-interval`, `min-error-interval`, etc. - # For the short term, this should copy the values into `%config`, not `%recap`. In the - # long term, nothing should be copied from `%recap` into `%config` because `%config` is - # not the semantically appropriate place to record update status. Code that currently - # reads/sets `$config{$h}{'status-ipv4'}` (and friends) should instead access - # `$recap{$h}{'status-ipv4'}` directly. - $recap{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v}); + $config{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v}); } } } diff --git a/t/read_recap.pl b/t/read_recap.pl index 5577738..ebec050 100644 --- a/t/read_recap.pl +++ b/t/read_recap.pl @@ -108,7 +108,6 @@ my @test_cases = ( 'warned-min-interval' => 1234567893, 'warned-min-error-interval' => 1234567894, }}, - want_config_changes_TODO => "longstanding bug", }, { desc => "unset status var clears config", @@ -145,6 +144,7 @@ my @test_cases = ( cachefile_lines => ["mtime=1234567890 host_b"], want => {host_b => {host => 'host_b'}}, want_TODO => "longstanding minor issue, doesn't affect functionality", + want_config_changes_TODO => "longstanding bug", }, { desc => "non-recap vars are scrubbed from %recap", @@ -152,6 +152,7 @@ my @test_cases = ( recap => {host_b => {host => 'host_b', mtime => 1234567891}}, want => {host_b => {host => 'host_b'}}, want_TODO => "longstanding minor issue, doesn't affect functionality", + want_config_changes_TODO => "longstanding minor issue, doesn't affect functionality", }, { desc => "unknown hosts are scrubbed from %recap",