Dynamically compute default for use based on usev4, usev6

This is mostly to simplify tests, but it also improves readability.

The infrastructure changes in this commit also make it possible to
introduce a new `url` variable that defaults to `opt('server', $h)`
concatenated with `opt('script', $h)` so that we can start migrating
away from those user-unfriendly variables.
This commit is contained in:
Richard Hansen 2024-08-18 03:10:07 -04:00
parent 46bd2f1771
commit f024bcce34
4 changed files with 84 additions and 19 deletions

View file

@ -78,6 +78,7 @@ m4_foreach_w([_m], [
File::Spec::Functions File::Spec::Functions
File::Temp File::Temp
List::Util List::Util
re
], [AX_PROG_PERL_MODULES([_m], [], ], [AX_PROG_PERL_MODULES([_m], [],
[AC_MSG_WARN([some tests will fail due to missing module _m])])]) [AC_MSG_WARN([some tests will fail due to missing module _m])])])

View file

@ -597,7 +597,13 @@ our %variables = (
'password' => setv(T_PASSWD,1, 0, undef, undef), 'password' => setv(T_PASSWD,1, 0, undef, undef),
'host' => setv(T_STRING,1, 1, undef, undef), 'host' => setv(T_STRING,1, 1, undef, undef),
'use' => setv(T_USE, 0, 0, 'ip', undef), 'use' => setv(T_USE, 0, 0, sub {
my ($h) = @_;
return "'disabled' if '--usev4' or '--usev6' is enabled, otherwise 'ip'"
if ($h // '') eq '<usage>';
return 'disabled' if opt('usev4', $h) ne 'disabled' || opt('usev6', $h) ne 'disabled';
return 'ip';
}, undef),
'usev4' => setv(T_USEV4, 0, 0, 'disabled', undef), 'usev4' => setv(T_USEV4, 0, 0, 'disabled', undef),
'usev6' => setv(T_USEV6, 0, 0, 'disabled', undef), 'usev6' => setv(T_USEV6, 0, 0, 'disabled', undef),
'if' => setv(T_IF, 0, 0, 'ppp0', undef), 'if' => setv(T_IF, 0, 0, 'ppp0', undef),
@ -1092,7 +1098,7 @@ $variables{'merged'} = {
}; };
# This will hold the processed args. # This will hold the processed args.
my %opt = (); our %opt;
my $deprecated_handler = sub { warning("'-$_[0]' is deprecated and does nothing"); }; my $deprecated_handler = sub { warning("'-$_[0]' is deprecated and does nothing"); };
$opt{'fw-banlocal'} = $deprecated_handler; $opt{'fw-banlocal'} = $deprecated_handler;
$opt{'if-skip'} = $deprecated_handler; $opt{'if-skip'} = $deprecated_handler;
@ -1937,7 +1943,7 @@ sub usage {
for (@_) { for (@_) {
if (ref $_) { if (ref $_) {
my ($key, $specifier, $arg_usage) = @$_; my ($key, $specifier, $arg_usage) = @$_;
my $value = default($key); my $value = default($key, '<usage>');
next unless $arg_usage; next unless $arg_usage;
$usage .= " $arg_usage"; $usage .= " $arg_usage";
if (defined($value) && $value ne '') { if (defined($value) && $value ne '') {
@ -2220,15 +2226,17 @@ sub split_by_comma {
} }
sub default { sub default {
my ($v, $h) = @_; my ($v, $h) = @_;
my $var;
if (defined($h) && $config{$h}) { if (defined($h) && $config{$h}) {
my $proto = $protocols{opt('protocol', $v eq 'protocol' ? undef : $h)}; my $proto = $protocols{opt('protocol', $v eq 'protocol' ? undef : $h)};
my $var = $proto->{variables}{$v} if $proto; $var = $proto->{variables}{$v} if $proto;
return $var->{default} if $var;
} }
return undef if !defined($variables{'merged'}{$v});
# TODO: This might grab an arbitrary protocol-specific variable definition, which could cause # TODO: This might grab an arbitrary protocol-specific variable definition, which could cause
# surprising behavior. # surprising behavior.
return $variables{'merged'}{$v}{'default'}; $var //= $variables{'merged'}{$v};
return undef if !defined($var);
return $var->{'default'}($h) if ref($var->{default}) eq 'CODE';
return $var->{'default'};
} }
sub opt { sub opt {
my $v = shift; my $v = shift;

View file

@ -70,7 +70,6 @@ my @test_cases = (
desc => "legacy, fresh, $desc", desc => "legacy, fresh, $desc",
cfg => { cfg => {
'protocol' => 'legacy', 'protocol' => 'legacy',
'use' => 'disabled',
%cfg, %cfg,
}, },
want_update => 1, want_update => 1,
@ -136,7 +135,6 @@ my @test_cases = (
ipv6 => 1, ipv6 => 1,
cfg => { cfg => {
'protocol' => 'legacy', 'protocol' => 'legacy',
'use' => 'disabled',
'usev6' => 'webv6', 'usev6' => 'webv6',
}, },
want_update => 1, want_update => 1,
@ -168,7 +166,6 @@ my @test_cases = (
ipv6 => 1, ipv6 => 1,
cfg => { cfg => {
'protocol' => 'legacy', 'protocol' => 'legacy',
'use' => 'disabled',
'usev4' => 'webv4', 'usev4' => 'webv4',
'usev6' => 'webv6', 'usev6' => 'webv6',
}, },
@ -209,7 +206,6 @@ my @test_cases = (
}, },
cfg => { cfg => {
'protocol' => 'legacy', 'protocol' => 'legacy',
'use' => 'disabled',
%cfg, %cfg,
}, },
want_cfg_changes => { want_cfg_changes => {
@ -238,7 +234,6 @@ my @test_cases = (
}, },
cfg => { cfg => {
'protocol' => 'legacy', 'protocol' => 'legacy',
'use' => 'disabled',
%cfg, %cfg,
}, },
want_cfg_changes => { want_cfg_changes => {
@ -267,7 +262,6 @@ my @test_cases = (
}, },
cfg => { cfg => {
'protocol' => 'legacy', 'protocol' => 'legacy',
'use' => 'disabled',
%cfg, %cfg,
}, },
want_recap_changes => { want_recap_changes => {
@ -299,7 +293,6 @@ my @test_cases = (
}, },
cfg => { cfg => {
'protocol' => 'legacy', 'protocol' => 'legacy',
'use' => 'disabled',
%cfg, %cfg,
}, },
want_update => 1, want_update => 1,
@ -345,7 +338,6 @@ my @test_cases = (
}, },
cfg => { cfg => {
'protocol' => 'legacy', 'protocol' => 'legacy',
'use' => 'disabled',
%cfg, %cfg,
}, },
want_recap_changes => { want_recap_changes => {
@ -374,7 +366,6 @@ my @test_cases = (
}, },
cfg => { cfg => {
'protocol' => 'legacy', 'protocol' => 'legacy',
'use' => 'disabled',
%cfg, %cfg,
}, },
want_update => 1, want_update => 1,

View file

@ -1,4 +1,5 @@
use Test::More; use Test::More;
use re qw(is_regexp);
SKIP: { eval { require Test::Warnings; } or skip($@, 1); } SKIP: { eval { require Test::Warnings; } or skip($@, 1); }
eval { require 'ddclient'; } or BAIL_OUT($@); eval { require 'ddclient'; } or BAIL_OUT($@);
@ -23,14 +24,78 @@ for my $tc (@test_cases) {
if ($tc->{def}{required}) { if ($tc->{def}{required}) {
is($tc->{def}{default}, undef, "'$tc->{desc}' (required) has no default"); is($tc->{def}{default}, undef, "'$tc->{desc}' (required) has no default");
} else { } else {
local %ddclient::variables = (merged => {var => $tc->{def}}); # Preserve all existing variables in $variables{merged} so that variables with dynamic
# defaults can reference them.
local %ddclient::variables = (merged => {
%{$ddclient::variables{merged}},
'var for test' => $tc->{def},
});
# Variables with dynamic defaults will need their own unit tests, but we can still check the
# clean-slate hostless default.
local %ddclient::config;
local %ddclient::opt;
local %ddclient::globals;
my $norm; my $norm;
my $default = ddclient::default('var'); my $default = ddclient::default('var for test');
diag("'$tc->{desc}' default: " . ($default // '<undefined>')); diag("'$tc->{desc}' default: " . ($default // '<undefined>'));
is($default, $tc->{def}{default}, "'$tc->{desc}' default() return value matches default"); is($default, $tc->{def}{default}, "'$tc->{desc}' default() return value matches default")
if ref($tc->{def}{default}) ne 'CODE';
my $valid = eval { $norm = ddclient::check_value($default, $tc->{def}); 1; } or diag($@); my $valid = eval { $norm = ddclient::check_value($default, $tc->{def}); 1; } or diag($@);
ok($valid, "'$tc->{desc}' (optional) has a valid default"); ok($valid, "'$tc->{desc}' (optional) has a valid default");
is($norm, $default, "'$tc->{desc}' default normalizes to itself") if $valid; is($norm, $default, "'$tc->{desc}' default normalizes to itself") if $valid;
} }
} }
my @use_test_cases = (
{
desc => 'clean slate hostless default',
want => 'ip',
},
{
desc => 'usage string',
host => '<usage>',
want => qr/disabled.*ip|ip.*disabled/,
},
{
desc => 'usev4 disables use by default',
host => 'host',
cfg => {usev4 => 'webv4'},
want => 'disabled',
},
{
desc => 'usev6 disables use by default',
host => 'host',
cfg => {usev4 => 'webv4'},
want => 'disabled',
},
{
desc => 'explicitly setting use re-enables it',
host => 'host',
cfg => {use => 'web', usev4 => 'webv4'},
want => 'web',
},
);
for my $tc (@use_test_cases) {
my $desc = "'use' dynamic default: $tc->{desc}";
local %ddclient::protocols =
(protocol => {variables => $ddclient::variables{'protocol-common-defaults'}});
local %ddclient::variables = (merged => {
'protocol' => $ddclient::variables{'merged'}{'protocol'},
'use' => $ddclient::variables{'protocol-common-defaults'}{'use'},
'usev4' => $ddclient::variables{'merged'}{'usev4'},
'usev6' => $ddclient::variables{'merged'}{'usev6'},
});
local %ddclient::config = (host => {protocol => 'protocol', %{$tc->{cfg} // {}}});
local %ddclient::opt;
local %ddclient::globals;
my $got = ddclient::opt('use', $tc->{host});
if (is_regexp($tc->{want})) {
like($got, $tc->{want}, $desc);
} else {
is($got, $tc->{want}, $desc);
}
}
done_testing(); done_testing();