Skip to content

Commit

Permalink
Item14237: Extended tests & fixed some bugs.
Browse files Browse the repository at this point in the history 
- Don't add source information from specs if key definition has 'source' option.

- Tidied check for key type change with proper handling of enhancing
definition.

- Fixed key type defined in header (construct
  Key => TYPE => [...definitions...]) being ignored.
  • Loading branch information
@vrurg
vrurg committed Mar 21, 2017
1 parent d61e8b8 commit 34082d8
Show file tree
Hide file tree
Showing 3 changed files with 175 additions and 31 deletions.
142 changes: 132 additions & 10 deletions UnitTestContrib/test/unit/ConfigTests.pm
View file
Expand Up @@ -122,11 +122,8 @@ sub test_specRegister {
-type => 'BOOLEAN',
-default => 0,
],
Setting => [
-type => 'SELECT',

#-variants => [qw(one two three)],
],
Setting => SELECT =>
[ -expert, -select_from => [qw(one two three)], ],
'Sub.Setting.Deep' => [
'Opt.K1' => [ -type => 'STRING', ],
'Opt.K2' => [ -type => 'NUMBER', ],
Expand Down Expand Up @@ -191,17 +188,23 @@ sub test_specRegister {
$this->assert_equals( "STRING", $strKey->getOpt('type') );
$this->assert_equals( 32, $strKey->getOpt('size') );

my $expertKey = $cfg->getKeyNode('Extensions.SampleExt.Setting');

$this->assert( $expertKey->getOpt('expert'),
"Extensions.SampleExt.Setting must have 'expert' option set" );
$this->assert_equals( "SELECT", $expertKey->getOpt('type') );
$this->assert_deep_equals( [qw(one two three)],
$expertKey->getOpt('select_from') );

return;
}

sub test_specOnLocalData {
my $this = shift;

Foswiki::load_class('Foswiki::Config::DataHash');

my %data;
my $data = $this->app->cfg->makeSpecsHash;

my $dataObj = tie %data, 'Foswiki::Config::DataHash', app => $this->app;
my $dataObj = tied %$data;

my $cfg = $this->app->cfg;

Expand Down Expand Up @@ -230,7 +233,7 @@ sub test_specOnLocalData {
Test1 => { Key1 => 'Default Key1', },
Test2 => { Key2 => 3.1415926, },
},
\%data
$data
);
}

Expand Down Expand Up @@ -466,4 +469,123 @@ sub test_legacyParse {
return;
}

sub test_enhancing {
my $this = shift;

my $cfg = $this->app->cfg;

my $defText = "This is base text.";
my $addedText = "And some more text\nto be added.";

$cfg->spec(
source => __FILE__,
specs => [
-section => "ConfigTests test section" => [
Test => [
KeyForEnhance => STRING =>
[ -text => $defText, -default => "default value", ],
],
],
],
);

$cfg->spec(
source => __FILE__,
specs => [
-section => "ConfigTests test section 2" => [
Test =>
[ KeyForEnhance => [ -enhance, -text => $addedText, ], ],
],
],
);

my $keyNode = $cfg->getKeyNode("Test.KeyForEnhance");

$this->assert_equals( "$defText\n\n$addedText", $keyNode->getOpt('text') );

try {
$cfg->spec(
source => __FILE__,
specs => [
-section => "Completely different section" => [
Test => [
KeyForEnhance => NUMBER => [
-enhance,
-text => "And changing the type to NUMBER",
],
],
],
],
);
$this->assert( 0, "Change of a key type must have failed." );
}
catch {
my $e = Foswiki::Exception::Fatal->transmute( $_, 0 );

if ( $e->isa('Foswiki::Exception::Config::BadSpecData') ) {
$this->assert_matches(
"Key type 'NUMBER' is different from the previously declared 'STRING'",
$e->text
);
}
else {
$e->rethrow;
}
};

}

sub test_enhanceNonExisting {
my $this = shift;

my $cfg = $this->app->cfg;

try {
Foswiki::Exception::Fatal->throw(
text => "Unexpectedly found a key ThisKey.DoesNot.Exists." )
if exists $cfg->data->{ThisKey}{DoesNot}{Exists};

$cfg->spec(
source => __FILE__,
specs => [
-section => "Test enhancing" => [
"ThisKey.DoesNot.Exists" =>
[ -enhance, -text => "Irrelevant", ],
],
],
);

$this->assert( 0,
"Must have failed while enhancing a key which doesn't exists" );
}
catch {
my $e = Foswiki::Exception::Fatal->transmute( $_, 0 );

if ( $e->isa('Foswiki::Exception::Config::BadSpecData') ) {
$this->assert_matches( "Cannot enhance key, no original found",
$e->text );
}
else {
$e->rethrow;
}
};

my $data = $cfg->makeSpecsHash;
my $dataObj = tied %$data;
my $section = $this->create( 'Foswiki::Config::Section', name => 'Root', );

# This call must just pass.
$cfg->spec(
source => __FILE__,
data => $dataObj,
section => $section,
specs => [
-section => "Test enhancing" => [
"ThisKey.DoesNot.Exists" =>
[ -enhance, -text => "Irrelevant", ],
],
],
);
}

1;
59 changes: 42 additions & 17 deletions core/lib/Foswiki/Config.pm
View file
Expand Up @@ -2277,7 +2277,7 @@ sub spec {
# the attribute.
my $globData = tied %{ $this->data };
$localData = $params{localData}
// ( defined($globData) && $globData == $data );
// ( !defined($globData) || $globData != $data );
}
else {
$this->specsMode;
Expand Down Expand Up @@ -2593,12 +2593,13 @@ sub _specCfgKey {
# – i.e. defines a key storing value, not other keys.
# The node is non-leaf if it hold a hash ref. For a newly created node
# its value is undefined and thus
my ( $isLeafKey, $isEnhancing, $keyType, $keySize, $keyText );
my ( $isLeafKey, $isEnhancing, $keyType, $prevKeyType, $keySize, $keyText );
my $noSourceFromSpec = 0;

unless ( ref($value) ) {
my @ktype = $this->_parseSpecKeyType($value);
$keyType = $ktype[0];
push @keyOptions, size => $ktype[1] if @ktype > 1;
$keySize = $ktype[1] if @ktype > 1;
$isLeafKey = 1; # Type could be assigned to a leaf node only.
$this->_isCompleteKeyDef( $specs, $section, $keyFullName );
$value = $specs->fetch;
Expand All @@ -2619,7 +2620,7 @@ sub _specCfgKey {
$isLeafKey //= $keyNode->isVague ? undef : $keyNode->isLeaf;

# Type could only be valid for a leaf node.
$keyType //= $keyNode->getOpt('type') if $isLeafKey;
$prevKeyType = $keyNode->getOpt('type') if $isLeafKey;
}

while ( $keySpecs->hasNext ) {
Expand All @@ -2637,24 +2638,27 @@ sub _specCfgKey {
$this->_fetchOptVal( $+{option}, $keySpecs, $arity,
"of key $keyFullName" );

if ( $option eq 'source' ) {
$noSourceFromSpec = 1;
}

if ( $option eq 'type' ) {
my @ktype = $this->_parseSpecKeyType( $values[0] );
if ( defined $keyType ) {
unless ( defined $keyType ) {
$keyType = $ktype[0];
push @keyOptions, size => $ktype[1] if @ktype > 1;
push @keyOptions, $option, $keyType;
}
elsif ( $keyType ne $ktype[0] ) {
Foswiki::Exception::Config::BadSpecData->throw(
text =>
"Another key type found which differs from the earlier one: '"
. $values[0]
. "' vs '"
text => "Conflicting types in key definition: '"
. $keyType
. "' respectively",
. "' vs. '"
. $ktype[0] . "'",
section => $section,
key => $keyFullName,
) if $ktype[0] ne $keyType;
}
else {
$keyType = $ktype[0];
push @keyOptions, size => $ktype[1] if @ktype > 1;
push @keyOptions, $option, $keyType;
srcFile => $specs->source,
);
}
}
elsif ( $arity->{$option} < 2 ) {
Expand Down Expand Up @@ -2727,6 +2731,27 @@ sub _specCfgKey {
) unless defined $keyNode || $specs->localData;
}

if ( defined $keyNode ) {
if ( $keyNode->isLeaf && defined $keyType ) {
Foswiki::Exception::Config::BadSpecData->throw(
text => "Key type '"
. $keyType
. "' is different from the previously declared '"
. $prevKeyType . "'",
section => $section,
key => $keyFullName,
)
if ( $prevKeyType ne $keyType )
&& !( $isEnhancing && $keyType eq 'VOID' );
}
}
else {
if ( defined $keyType ) {
push @keyOptions, type => $keyType;
push @keyOptions, size => $keySize if defined $keySize;
}
}

push @keyProfile, leafState => $isLeafKey if defined $isLeafKey;
$keyNode //= $keyObject->makeNode(
key => $keyName,
Expand All @@ -2742,7 +2767,7 @@ sub _specCfgKey {
$keyNode->setOpt( text => $keyText );
}
}
$keyNode->addSource( $keySpecs->source );
$keyNode->addSource( $keySpecs->source ) unless $noSourceFromSpec;

if ( $keyNode->isBranch ) {
foreach my $subKeySpec (@subKeySpecs) {
Expand Down
5 changes: 1 addition & 4 deletions core/lib/Foswiki/Config/Node.pm
View file
Expand Up @@ -554,6 +554,7 @@ around optionDefinitions => sub {
default => { arity => 1, leaf => 1, },
display_if => { arity => 1, leaf => 1, },
enhance => { arity => 0, leaf => 1, },
expert => { arity => 0, leaf => 1, },
feedback => { arity => 1, leaf => 1, },
hidden => { arity => 0, leaf => 1, },
label => { arity => 1, dual => 1, },
Expand All @@ -564,10 +565,6 @@ around optionDefinitions => sub {
spellcheck => { arity => 0, dual => 1, },
type => { arity => 1, leaf => 1, },
wizard => { arity => 1, leaf => 1, },

# Even though expert is a boolean option we cannot make it's arity 0
# because of legacy format partser limitation.
expert => { arity => 1, leaf => 1, },
);
};

Expand Down

0 comments on commit 34082d8

Please sign in to comment.