Fabio/ddclient
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

read_recap: Fix copying of recap values into %config

Browse source
This commit is contained in:
Richard Hansen 2024-08-22 03:16:32 -04:00
parent fbd7167b94
commit c9cdb96086
2 changed files with 7 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
ddclient.in
View file

@ -1641,17 +1641,13 @@ sub read_recap {
# different pieces of code need to know the list of status variables, and keeping them all # 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 # in sync is error-prone (DRY). Also, different protocols might need different status
# variables. # 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 for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6
warned-min-interval warned-min-error-interval)) { warned-min-interval warned-min-error-interval)) {
# TODO: This is a no-op. I believe the original intent was to copy values that convey $config{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v});
# 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});
} }
} }
} }

3
t/read_recap.pl
View file

@ -108,7 +108,6 @@ my @test_cases = (
'warned-min-interval' => 1234567893, 'warned-min-interval' => 1234567893,
'warned-min-error-interval' => 1234567894, 'warned-min-error-interval' => 1234567894,
}}, }},
want_config_changes_TODO => "longstanding bug",
}, },
{ {
desc => "unset status var clears config", desc => "unset status var clears config",
@ -145,6 +144,7 @@ my @test_cases = (
cachefile_lines => ["mtime=1234567890 host_b"], cachefile_lines => ["mtime=1234567890 host_b"],
want => {host_b => {host => 'host_b'}}, want => {host_b => {host => 'host_b'}},
want_TODO => "longstanding minor issue, doesn't affect functionality", want_TODO => "longstanding minor issue, doesn't affect functionality",
want_config_changes_TODO => "longstanding bug",
}, },
{ {
desc => "non-recap vars are scrubbed from %recap", desc => "non-recap vars are scrubbed from %recap",
@ -152,6 +152,7 @@ my @test_cases = (
recap => {host_b => {host => 'host_b', mtime => 1234567891}}, recap => {host_b => {host => 'host_b', mtime => 1234567891}},
want => {host_b => {host => 'host_b'}}, want => {host_b => {host => 'host_b'}},
want_TODO => "longstanding minor issue, doesn't affect functionality", 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", desc => "unknown hosts are scrubbed from %recap",