diff --git a/perl-xCAT/xCAT/TableUtils.pm b/perl-xCAT/xCAT/TableUtils.pm index dc18128da..04d45e691 100644 --- a/perl-xCAT/xCAT/TableUtils.pm +++ b/perl-xCAT/xCAT/TableUtils.pm @@ -2171,4 +2171,37 @@ sub rmnodegroups { $tabhd->setNodeAttribs($node, { groups => join(",", @newgroups) }); } +#----------------------------------------------------------------------------- + + +=head3 delete_rows + delete the rows that match the keys from each named table + + Arguments: + keys: a hash reference of key column values + names: the table names + Returns: + undef, or the name of the first table that does not open + Example: + my $missing = xCAT::TableUtils->delete_rows({ node => 'n1' }, 'nodehm', 'noderes'); + +=cut + +#----------------------------------------------------------------------------- + +sub delete_rows { + my ($class, $keys, @names) = @_; + + # Every table opens before the first delete, so a table that does not open leaves all of them unchanged. + my %tables; + foreach my $name (@names) { + $tables{$name} = xCAT::Table->new($name, -create => 1, -autocommit => 0) or return $name; + } + foreach my $name (@names) { + $tables{$name}->delEntries($keys); + $tables{$name}->commit; + } + return; +} + 1; diff --git a/xCAT-server/lib/xcat/plugins/tabutils.pm b/xCAT-server/lib/xcat/plugins/tabutils.pm index b77c6c464..cc0aab444 100644 --- a/xCAT-server/lib/xcat/plugins/tabutils.pm +++ b/xCAT-server/lib/xcat/plugins/tabutils.pm @@ -430,6 +430,7 @@ sub tabrestore my $origline = $line; #save for error reporting my %record; my $col; + my $separated; #a comma after the field of the last column starts one more field foreach $col (@colns) { if ($line =~ /^,/ or $line eq "") @@ -438,7 +439,7 @@ sub tabrestore if (!exists($auto_cols{$col})) { $record{$col} = undef; } - $line =~ s/^,//; + $separated = ($line =~ s/^,//); } elsif ($line =~ /^[^,]*"/) { # We have stuff in quotes... pain... @@ -501,7 +502,7 @@ sub tabrestore elsif ($offset eq length($line) or $nextchar eq ',') { #hit the end of the line or at least the end of the column $ent = substr($line, 0, $offset, ''); - $line =~ s/^,//; + $separated = ($line =~ s/^,//); chop $ent; $ent = substr($ent, 1); $ent =~ s/""/"/g; @@ -531,10 +532,10 @@ sub tabrestore { $record{$col} = $1; } - $line =~ s/^([^,]+)(,|$)//; + $separated = ($line =~ s/^([^,]+)(,|$)//) && $2 eq ','; } } - if ($line) + if ($line ne '' or $separated) { $rollback = 1; $cb->({ error => "Too many fields on line $linenumber: $origline | $line", errorcode => 4 }); diff --git a/xCAT-server/sbin/chtab b/xCAT-server/sbin/chtab index 18287fd35..56835c5a8 100755 --- a/xCAT-server/sbin/chtab +++ b/xCAT-server/sbin/chtab @@ -6,6 +6,7 @@ BEGIN } use lib "$::XCATROOT/lib/perl"; use xCAT::Table; +use xCAT::TableUtils; use xCAT::NodeRange; use Getopt::Long; @@ -97,10 +98,10 @@ if ($::DELETE) { usage; exit(1); } - for (@tables_to_del) { - $tables{$_} = xCAT::Table->new($_, -create => 1, -autocommit => 0); - $tables{$_}->delEntries(\%keyhash); - $tables{$_}->commit; + my $missing = xCAT::TableUtils->delete_rows(\%keyhash, @tables_to_del); + if (defined($missing)) { + print "Table $missing does not exist.\n"; + exit(1); } } else { diff --git a/xCAT-test/unit/tableutils_delete_rows.t b/xCAT-test/unit/tableutils_delete_rows.t new file mode 100644 index 000000000..2260b3fc3 --- /dev/null +++ b/xCAT-test/unit/tableutils_delete_rows.t @@ -0,0 +1,47 @@ +#!/usr/bin/env perl +use strict; +use warnings; +use FindBin; +use lib "$FindBin::Bin/../../perl-xCAT"; +use File::Path qw(make_path); +use File::Temp qw(tempdir); +use Test::More; + +my $fixture = tempdir(CLEANUP => 1); +make_path("$fixture/db"); +local $ENV{XCATCFG} = "SQLite:$fixture/db"; +require xCAT::Table; +require xCAT::TableUtils; + +for my $table (qw(nodehm noderes)) { + xCAT::Table->new($table, -create => 1)->setAttribs({ node => $_ }, { comments => "row of $_" }) for qw(n1 n2 n3); +} + +sub nodes { + my ($table) = @_; + return join(',', sort map { $_->{node} } xCAT::Table->new($table)->getAllAttribs('node')); +} + +sub delete_rows { + my @args = @_; + my $missing = eval { xCAT::TableUtils->delete_rows(@args) }; + return ($missing, $@); +} + +my ($missing, $error) = delete_rows({ node => 'n1' }, 'nodehm', 'noderes'); +is($error, '', 'rows are deleted from tables that open'); +is($missing, undef, 'no table is reported when every table opens'); +is(nodes('nodehm'), 'n2,n3', 'the row of the key is deleted from the first table'); +is(nodes('noderes'), 'n2,n3', 'the row of the key is deleted from the last table'); + +for my $case ([ 'between two tables', 'nodehm', 'notatable', 'noderes' ], [ 'first', 'notatable', 'nodehm', 'noderes' ], + [ 'last', 'nodehm', 'noderes', 'notatable' ]) { + my ($label, @tables) = @$case; + ($missing, $error) = delete_rows({ node => 'n2' }, @tables); + is($error, '', "a missing table named $label does not die"); + is($missing, 'notatable', "a missing table named $label is returned"); + is(nodes('nodehm'), 'n2,n3', "nodehm keeps its rows when a missing table is named $label"); + is(nodes('noderes'), 'n2,n3', "noderes keeps its rows when a missing table is named $label"); +} + +done_testing(); diff --git a/xCAT-test/unit/tabrestore_field_count.t b/xCAT-test/unit/tabrestore_field_count.t new file mode 100644 index 000000000..ac9d13080 --- /dev/null +++ b/xCAT-test/unit/tabrestore_field_count.t @@ -0,0 +1,55 @@ +#!/usr/bin/env perl +use strict; +use warnings; +use FindBin; +use lib "$FindBin::Bin/../lib", "$FindBin::Bin/../../perl-xCAT", "$FindBin::Bin/../../xCAT-server/lib/perl"; +use File::Path qw(make_path); +use File::Temp qw(tempdir); +use Test::More; +use XCAT::Test::File qw(repo_path); + +my $fixture = tempdir(CLEANUP => 1); +make_path("$fixture/db"); +local $ENV{XCATCFG} = "SQLite:$fixture/db"; +require xCAT::Table; +require(repo_path('xCAT-server/lib/xcat/plugins/tabutils.pm')); + +# Restores the rows into the site table, which has 4 columns, over a row that was there before. +sub restore { + my @rows = @_; + my $site = xCAT::Table->new('site', -create => 1); + $site->delEntries(); + $site->setAttribs({ key => 'before' }, { value => 'kept' }); + my @errors; + xCAT_plugin::tabutils::process_request({ command => ['tabrestore'], table => ['site'], data => [ '#key,value,comments,disable', @rows ] }, + sub { push @errors, map { ref($_) ? @$_ : $_ } grep { defined } $_[0]{error} }); + my %rows = map { ($_->{key} => join('|', map { $_ // '' } @{$_}{qw(value comments)})) } xCAT::Table->new('site')->getAllAttribs('key', 'value', 'comments'); + return (join("\n", @errors), \%rows); +} + +for my $case ( + [ 'one field for each column', '"k1","v1",,' ], + [ 'fewer fields than columns', '"k1","v1"' ], + [ 'a quoted field that holds a comma', '"k1","v1","a,b",', 'v1|a,b' ], + [ 'a quoted comment before the last field', '"k1","v1","note",', 'v1|note' ]) +{ + my ($label, $row, $want) = @$case; + my ($errors, $rows) = restore($row); + is($errors, '', "$label: the row is restored without an error"); + is_deeply($rows, { k1 => $want // 'v1|' }, "$label: the restore replaces the table with the row"); +} + +for my $case ( + [ 'an empty field after the last column', '"k1","v1",,,' ], + [ 'a field of 0 after the last column', '"k1","v1",,,0' ], + [ 'an empty field after a quoted last column', '"k1","v1",,"yes",' ], + [ 'an empty field after an unquoted last column', '"k1","v1",,yes,' ], + [ 'a text field after the last column', '"k1","v1",,,extra' ]) +{ + my ($label, $row) = @$case; + my ($errors, $rows) = restore('"k0","v0",,', $row); + like($errors, qr/Too many fields on line \d+: \Q$row\E/, "$label: the row is refused"); + is_deeply($rows, { before => 'kept|' }, "$label: the restore is rolled back"); +} + +done_testing();