Skip to content

Commit

Permalink
Item13077: typos in regex fields are now ok - for me, anyway
Browse files Browse the repository at this point in the history
  • Loading branch information
Comment committed Dec 7, 2014
1 parent 267d4f7 commit 65cb864
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 119 deletions.
3 changes: 2 additions & 1 deletion UnitTestContrib/test/unit/ConfigureQueryTests.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ sub test_getcfg {
my $reporter = Foswiki::Configure::Reporter->new();
my $result = Foswiki::Configure::Query::getcfg( $params, $reporter );
$this->assert( !$reporter->has_level('errors') );

#print STDERR Data::Dumper->Dump([$result]);
$this->assert_deep_equals(
{
UnitTestContrib => {
Expand Down Expand Up @@ -211,7 +213,6 @@ sub test_getspec_REGEX {
$spec = $spec->[0];
$this->assert_str_equals( 'REGEX', $spec->{typename} );
$this->assert_str_equals( '^regex$', $spec->{default} );
$this->assert_str_equals( '^regex$', $spec->{current_value} );
$this->assert_str_equals( 'Default: \'^regex$\'', $spec->{desc} );
$this->assert_str_equals( '{UnitTestContrib}{Configure}{REGEX}',
$spec->{keys} );
Expand Down
2 changes: 1 addition & 1 deletion UnitTestContrib/test/unit/ConfigureSaveTests.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ sub test_changecfg {
{
level => 'notes',
text =>
q<| {UnitTestContrib}{Configure}{REGEX} | (^regex$) | qr/(black&#124;white)+/ |>,
q<| {UnitTestContrib}{Configure}{REGEX} | (^regex$) | '(black&#124;white)+' |>,
}
];
my $ms = $reporter->messages();
Expand Down
14 changes: 7 additions & 7 deletions core/lib/Foswiki/Configure/Checkers/PERL.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,6 @@ sub check_current_value {
my ( $this, $reporter ) = @_;

my $value = $this->{item}->getRawValue();

eval $value;
if ($@) {
$reporter->ERROR( 'Not a valid PERL value: '
. Foswiki::Configure::Reporter::stripStacktrace($@) );
return;
}
if ( !defined $value ) {
my $check = $this->{item}->{CHECK}->[0];
unless ( $check && $check->{nullok}[0] ) {
Expand All @@ -26,6 +19,13 @@ sub check_current_value {
return;
}

eval $value;
if ($@) {
$reporter->ERROR( 'Not a valid PERL value: '
. Foswiki::Configure::Reporter::stripStacktrace($@) );
return;
}

_check_for_null( $value, $reporter );
}

Expand Down
223 changes: 114 additions & 109 deletions core/lib/Foswiki/Configure/Query.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use Foswiki::Configure::Reporter ();
use Foswiki::Configure::Checker ();
use Foswiki::Configure::Wizard ();

use constant TRACE_CHECK => 0;

=begin TML
---+ package Foswiki::Configure::Query
Expand Down Expand Up @@ -83,9 +85,6 @@ sub _getSetParams {
---++ StaticMethod getcfg(\%params, $reporter) -> \%response
Retrieve for the value of one or more keys. \%params may include
* =set= - hash of key-value pairs that maps the names of keys
to the value to be set. Strings in the values are assumed to be
unexpanded (i.e. with =$Foswiki::cfg= references intact).
* =keys= - array of key names to recover values for.
If there isn't at least one key parameter, returns the
entire configuration hash. Values are returned unexpanded
Expand Down Expand Up @@ -303,91 +302,55 @@ sub check_current_value {

my $reporter = Foswiki::Configure::Reporter->new();

# Apply "set" values to $Foswiki::cfg
eval { _getSetParams( $params, $root, $frep ); };
if ( $frep->has_level('errors') ) {
return undef;
}

# Because we're running in a plugin, we already have LocalSite.cfg
# loaded. It's in $Foswiki::cfg! Of course if we're bootstrapping,
# that config is wishful thinking, but hey, can't have everything.

# Determine the set of value keys being checked

my $keys = $params->{keys};
if ( !$keys || scalar @$keys == 0 ) {
push( @$keys, '' );
}
my %check;
my @checko;
foreach my $k (@$keys) {
# Determine the set of value keys being checked. We start with
# the keys passed in as parameters.

# $k='' is the root section
my $v = $root->getValueObject($k);
if ($v) {
push( @checko, $k ) unless $check{$k};
$check{$k} = 1;
my @keys;
foreach my $k ( @{ $params->{keys} } ) {
if ( $root->getValueObject($k) || $root->getSectionObject($k) ) {
push( @keys, $k );
}
else {
$v = $root->getSectionObject($k);
if ($v) {
foreach my $kk ( $v->getAllValueKeys() ) {
push( @checko, $k ) unless $check{$kk};
$check{$kk} = 1;
$k = "'$k'" unless $k =~ /^\{.*\}$/;
push(
@report,
{
keys => $k,
path => [],
reports => [
{
text => "$k is not part of this .spec",
level => 'errors'
}
]
}
}
else {
$k = "'$k'" unless $k =~ /^\{.*\}$/;
push(
@report,
{
keys => $k,
path => [],
reports => [
{
text => "$k is not part of this .spec",
level => 'errors'
}
]
}
);
}
);
}
}

# Are we to follow dependencies?
my $dependants = 0;

# Apply "set" values to $Foswiki::cfg
eval { _getSetParams( $params, $root, $frep ); };
if ( $frep->has_level('errors') ) {
return undef;
if ( scalar @keys == 0 ) {
push( @keys, '' );
}

my $deps; # forward and reverse dependencies computed from values
if ( $params->{check_dependencies} ) {

# First get reverse dependencies expressed in CHECK_ON_CHANGE
# Get reverse dependencies expressed in CHECK_ON_CHANGE
# and add them as CHECK="also: forward dependencies to the
# item they depend on. We only do this now, as it is quite
# demanding.
# item they depend on. We only do this if check_dependencies
# is set, as it is quite demanding.
$root->find_also_dependencies($root);

# Now look at the CHECK="also: for the items we've been asked
# to check.
my $changed;
do {
$changed = 0;
foreach my $k (@checko) {
next unless $k;
my $spec = $root->getValueObject($k);
next unless $spec;
foreach my $ch ( @{ $spec->{CHECK} } ) {
next unless $ch->{also};
foreach my $dep ( @{ $ch->{also} } ) {
next if $check{$dep};
push( @checko, $dep );
$check{$dep} = 1;
$changed = 1;
}
}
}
} while ($changed);

# Reload Foswiki::cfg without expansions so we can find
# string-embedded dependencies
local %Foswiki::cfg = ();
Expand All @@ -397,54 +360,96 @@ sub check_current_value {
eval "\$Foswiki::cfg$k=$v";
}
}
my $deps = Foswiki::Configure::Load::findDependencies();

# Extend the list of requested keys with the keys that depend
# on their values.
my @dep_keys = @checko;
my %done;
while ( my $dep = shift @dep_keys ) {
next if $done{$dep};
$done{$dep} = 1;

# Find the closest enclosing key that has a spec (we only
# check things with specs) and add it to the check set
my $cd = $dep;
while ( $cd && !$root->getValueObject($cd) ) {
$cd =~ s/(.*){.*?}$/$1/;
$deps = Foswiki::Configure::Load::findDependencies();
}

#print STDERR Data::Dumper->Dump([$deps]);

my %check; # set of keys to be checked
my @checko; # list of Value objects for keys to be checked
while ( defined( my $k = shift(@keys) ) ) {
print STDERR "Find dependencies for $k\n" if TRACE_CHECK;
next if $check{$k}; # already done?
$check{$k} = 1;
my $v = _getValueSpec( $root, $k );
if ($v) {
print STDERR "\t'$k' is a key\n" if TRACE_CHECK;
push( @checko, $v );
if ( $params->{check_dependencies} ) {

# Look at the CHECK="also:" explicit dependencies
foreach my $ch ( @{ $v->{CHECK} } ) {
next unless $ch->{also};
foreach my $dep ( @{ $ch->{also} } ) {
next if $check{$dep};
print STDERR "\t... has a check:also for $dep\n"
if TRACE_CHECK;
push( @keys, $dep ) unless $check{$dep};
}
}
}
if ($cd) {
push( @checko, $cd ) unless $check{$cd};
$check{$cd} = 1 if $cd;
}
else {
$v = $root->getSectionObject($k);
if ($v) {
print STDERR "\n'$k' is a section\n" if TRACE_CHECK;
foreach my $kk ( $v->getAllValueKeys() ) {
unless ( $check{$kk} ) {
print STDERR "\tcontains key '$kk'\n" if TRACE_CHECK;
push( @keys, $kk );
}
}
}
else {
print STDERR "\t'$k' is not a key or a section\n"
if TRACE_CHECK;
if ( $k =~ s/{[^{}]+}$// && !$check{$k} ) {
print STDERR "\tcheck parent '$k' instead\n" if TRACE_CHECK;
push( @keys, $k );
}
}
push( @dep_keys, @{ $deps->{forward}->{$dep} } )
if $deps->{forward}->{$dep};
}

# Look at forward dependencies i.e. the keys that depend
# on the value of this key
if ( $deps && $deps->{forward}->{$k} ) {
my @more = grep { !$check{$_} } @{ $deps->{forward}->{$k} };
map { print STDERR "\t$_ depends on $k\n"; $_ } @more
if TRACE_CHECK;
push( @keys, @more );
}
}
foreach my $k (@checko) {
next unless $k;
my $spec = $root->getValueObject($k);
ASSERT( $spec, $k ) if DEBUG;

foreach my $spec (@checko) {
my $checker = Foswiki::Configure::Checker::loadChecker($spec);
if ($checker) {
$reporter->clear();
$reporter->NOTE("Checking $k") if $params->{trace};
$checker->check_current_value($reporter);
my @path = $spec->getPath();
pop(@path); # remove keys
push(
@report,
{
keys => $k,
path => [@path],
reports => $reporter->messages()
}
);
}
next unless $checker;
ASSERT( $spec->{keys} ) if DEBUG;
$reporter->clear();
$reporter->NOTE("Checking $spec-.{keys}") if $params->{trace};
$checker->check_current_value($reporter);
my @path = $spec->getPath();
pop(@path); # remove keys
push(
@report,
{
keys => $spec->{keys},
path => [@path],
reports => $reporter->messages()
}
);
}
return \@report;
}

sub _getValueSpec {
my ( $root, $k ) = @_;
return undef unless $k;
my $v = $root->getValueObject($k);
return $v if $v;
return undef unless $k =~ s/{[^{}]+}$//;
return _getValueSpec($k);
}

=begin TML
---++ StaticMethod wizard(\%params, $reporter) -> \%response
Expand Down
5 changes: 4 additions & 1 deletion core/lib/Foswiki/Configure/Value.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,10 @@ sub decodeValue {
return undef unless defined $value;

if ( $this->{typename} eq 'REGEX' ) {
return qr/$value/;

# Regexes are stored as strings, because the conversion back
# and forth to regex objects is a PITA, and really a bit pointless.
return $value;
}
elsif ( $this->{typename} eq 'PERL' ) {
my $value = eval $value;
Expand Down

0 comments on commit 65cb864

Please sign in to comment.