From 0dd15dd216e5084b7594dba7a23e61d65c07d73d Mon Sep 17 00:00:00 2001 From: David Steele Date: Sat, 21 Jun 2014 20:08:49 -0400 Subject: [PATCH] Cleanup. --- bin/pg_backrest_remote.pl | 14 ++--- lib/BackRest/File.pm | 89 ++++++++++++----------------- lib/BackRest/Remote.pm | 14 ++--- test/lib/BackRestTest/FileTest.pm | 95 +++++++++++++++---------------- 4 files changed, 96 insertions(+), 116 deletions(-) diff --git a/bin/pg_backrest_remote.pl b/bin/pg_backrest_remote.pl index a06d49016..f299749fd 100755 --- a/bin/pg_backrest_remote.pl +++ b/bin/pg_backrest_remote.pl @@ -60,14 +60,14 @@ sub param_get my $oParamHashRef = shift; my $strParam = shift; my $bRequired = shift; - + my $strValue = ${$oParamHashRef}{$strParam}; - + if (!defined($strValue) && (!defined($bRequired) || $bRequired)) { confess "${strParam} must be defined"; } - + return $strValue; } @@ -106,7 +106,7 @@ while ($strCommand ne OP_EXIT) $oFile->copy(PATH_ABSOLUTE, param_get(\%oParamHash, 'source_file'), PIPE_STDOUT, undef, param_get(\%oParamHash, 'source_compressed'), undef); - + $oRemote->output_write(); } elsif ($strCommand eq OP_FILE_COPY_IN) @@ -114,13 +114,13 @@ while ($strCommand ne OP_EXIT) $oFile->copy(PIPE_STDIN, undef, PATH_ABSOLUTE, param_get(\%oParamHash, 'destination_file'), undef, param_get(\%oParamHash, 'destination_compress')); - + $oRemote->output_write(); } elsif ($strCommand eq OP_FILE_LIST) { my $strOutput; - + foreach my $strFile ($oFile->list(PATH_ABSOLUTE, param_get(\%oParamHash, 'path'), param_get(\%oParamHash, 'expression', false), param_get(\%oParamHash, 'sort_order'))) @@ -132,7 +132,7 @@ while ($strCommand ne OP_EXIT) $strOutput .= $strFile; } - + $oRemote->output_write($strOutput); } elsif ($strCommand eq OP_FILE_PATH_CREATE) diff --git a/lib/BackRest/File.pm b/lib/BackRest/File.pm index 6d091cfc5..ba1d2c4ef 100644 --- a/lib/BackRest/File.pm +++ b/lib/BackRest/File.pm @@ -25,9 +25,10 @@ use BackRest::Exception; use BackRest::Utility; use BackRest::Remote; +# Exports use Exporter qw(import); our @EXPORT = qw(PATH_ABSOLUTE PATH_DB PATH_DB_ABSOLUTE PATH_BACKUP PATH_BACKUP_ABSOLUTE - PATH_BACKUP_CLUSTERPATH_BACKUP_TMP PATH_BACKUP_ARCHIVE + PATH_BACKUP_CLUSTER PATH_BACKUP_TMP PATH_BACKUP_ARCHIVE COMMAND_ERR_FILE_MISSING COMMAND_ERR_FILE_READ COMMAND_ERR_FILE_MOVE COMMAND_ERR_FILE_TYPE COMMAND_ERR_LINK_READ COMMAND_ERR_PATH_MISSING COMMAND_ERR_PATH_CREATE COMMAND_ERR_PARAM @@ -37,7 +38,6 @@ our @EXPORT = qw(PATH_ABSOLUTE PATH_DB PATH_DB_ABSOLUTE PATH_BACKUP PATH_BACKUP_ OP_FILE_LIST OP_FILE_EXISTS OP_FILE_HASH OP_FILE_REMOVE OP_FILE_MANIFEST OP_FILE_COMPRESS OP_FILE_MOVE OP_FILE_COPY OP_FILE_COPY_OUT OP_FILE_COPY_IN OP_FILE_PATH_CREATE); - # Extension and permissions has strCompressExtension => (is => 'ro', default => 'gz'); has strDefaultPathPermission => (is => 'bare', default => '0750'); @@ -51,7 +51,6 @@ has strRemote => (is => 'bare'); # Remote type (db or backup) has oRemote => (is => 'bare'); # Remote object has strBackupPath => (is => 'bare'); # Backup base path -has strBackupClusterPath => (is => 'bare'); # Backup cluster path # Process flags has strStanza => (is => 'bare'); @@ -139,13 +138,6 @@ sub BUILD { my $self = shift; - # Make sure the backup path is defined - if (defined($self->{strBackupPath})) - { - # Create the backup cluster path - $self->{strBackupClusterPath} = $self->{strBackupPath} . "/" . $self->{strStanza}; - } - # If remote is defined check parameters and open session if (defined($self->{strRemote})) { @@ -171,18 +163,12 @@ sub clone my $self = shift; my $iThreadIdx = shift; - return pg_backrest_file->new + return BackRest::File->new ( - strCompressExtension => $self->{strCompressExtension}, - strDefaultPathPermission => $self->{strDefaultPathPermission}, - strDefaultFilePermission => $self->{strDefaultFilePermission}, strCommand => $self->{strCommand}, - strDbUser => $self->{strDbUser}, - strDbHost => $self->{strDbHost}, - strBackupUser => $self->{strBackupUser}, - strBackupHost => $self->{strBackupHost}, + strRemote => $self->{strRemote}, + oRemote => defined($self->{oRemote}) ? $self->{oRemote}->clone() : undef, strBackupPath => $self->{strBackupPath}, - strBackupClusterPath => $self->{strBackupClusterPath}, strStanza => $self->{strStanza}, iThreadIdx => $iThreadIdx ); @@ -546,7 +532,7 @@ sub compress confess &log(ERROR, "${strDebug}: " . $strError); } - + unlink($strPathOp) or die &log(ERROR, "${strDebug}: unable to remove ${strPathOp}"); } @@ -569,7 +555,7 @@ sub path_create # Set operation and debug strings my $strOperation = OP_FILE_PATH_CREATE; - my $strDebug = " ${strPathType}:${strPath}, permission " . (defined($strPermission) ? $strPermission : "[undef]"); + my $strDebug = " ${strPathType}:${strPathOp}, permission " . (defined($strPermission) ? $strPermission : "[undef]"); &log(DEBUG, "${strOperation}: ${strDebug}"); if ($self->is_remote($strPathType)) @@ -596,7 +582,7 @@ sub path_create { # Attempt the create the directory my $bResult; - + if (defined($strPermission)) { $bResult = mkdir($strPathOp, oct($strPermission)); @@ -605,11 +591,11 @@ sub path_create { $bResult = mkdir($strPathOp); } - + if (!$bResult) { # Capture the error - my $strError = "${strPath} could not be created: " . $!; + my $strError = "${strPathOp} could not be created: " . $!; # If running on command line the return directly if ($strPathType eq PATH_ABSOLUTE) @@ -704,7 +690,7 @@ sub remove # Set operation variables my $strPathOp = $self->path_get($strPathType, $strPath, $bTemp); my $bRemoved = true; - + # Set operation and debug strings my $strOperation = OP_FILE_EXISTS; my $strDebug = "${strPathType}:${strPathOp}"; @@ -850,7 +836,7 @@ sub list # Execute the command my $strOutput = $self->{oRemote}->command_execute($strOperation, \%oParamHash, false, $strDebug); - + if (defined($strOutput)) { @stryFileList = split(/\n/, $strOutput); @@ -934,12 +920,13 @@ sub manifest # Run locally else { - manifest_recurse($strPathType, $strPathOp, undef, 0, $oManifestHashRef); + $self->manifest_recurse($strPathType, $strPathOp, undef, 0, $oManifestHashRef); } } sub manifest_recurse { + my $self = shift; my $strPathType = shift; my $strPathOp = shift; my $strPathFileOp = shift; @@ -953,18 +940,17 @@ sub manifest_recurse if (!opendir($hPath, $strPathRead)) { my $strError = "${strPathRead} could not be read: " . $!; - my $iErrorCode = 2; + my $iErrorCode = COMMAND_ERR_PATH_READ; - unless (-e $strPathRead) + if (!$self->exists($strPathType, $strPathOp)) { $strError = "${strPathRead} does not exist"; - $iErrorCode = 1; + $iErrorCode = COMMAND_ERR_PATH_MISSING; } if ($strPathType eq PATH_ABSOLUTE) { - print $strError; - exit ($iErrorCode); + confess &log(ERROR, $strError, $iErrorCode); } confess &log(ERROR, "${strErrorPrefix}: " . $strError); @@ -996,20 +982,21 @@ sub manifest_recurse if (!defined($oStat)) { - if (-e $strPathFile) + my $strError = "${strPathFile} could not be read: " . $!; + my $iErrorCode = COMMAND_ERR_FILE_READ; + + if (!$self->exists($strPathType, $strPathOp)) { - my $strError = "${strPathFile} could not be read: " . $!; - - if ($strPathType eq PATH_ABSOLUTE) - { - print $strError; - exit COMMAND_ERR_FILE_READ; - } - - confess &log(ERROR, "${strErrorPrefix}: " . $strError); + $strError = "${strPathRead} does not exist"; + $iErrorCode = COMMAND_ERR_FILE_MISSING; } - next; + if ($strPathType eq PATH_ABSOLUTE) + { + confess &log(ERROR, $strError, COMMAND_ERR_FILE_READ); + } + + confess &log(ERROR, "${strErrorPrefix}: " . $strError, COMMAND_ERR_FILE_READ); } # Check for regular file @@ -1083,9 +1070,7 @@ sub manifest_recurse # Recurse into directories if (${$oManifestHashRef}{name}{"${strFile}"}{type} eq "d" && !$bCurrentDir) { - manifest_recurse($strPathType, $strPathOp, - $strFile, - $iDepth + 1, $oManifestHashRef); + $self->manifest_recurse($strPathType, $strPathOp, $strFile, $iDepth + 1, $oManifestHashRef); } } } @@ -1132,9 +1117,9 @@ sub copy # Set debug string and log my $strDebug = ($bSourceRemote ? " remote" : " local") . " ${strSourcePathType}" . - (defined($strSourceFile) ? ":${strSourceFile}" : "") . + (defined($strSourceOp) ? ":${strSourceFile}" : "") . " to" . ($bDestinationRemote ? " remote" : " local") . " ${strDestinationPathType}" . - (defined($strDestinationFile) ? ":${strDestinationFile}" : "") . + (defined($strDestinationOp) ? ":${strDestinationFile}" : "") . ", source_compressed = " . ($bSourceCompressed ? "true" : "false") . ", destination_compress = " . ($bDestinationCompress ? "true" : "false") . ", ignore_missing_source = " . ($bIgnoreMissingSource ? "true" : "false"); @@ -1170,7 +1155,7 @@ sub copy { $self->{oRemote}->write_line(*STDOUT, "block 0"); } - + confess &log(ERROR, $strError, $iErrorCode); } @@ -1275,18 +1260,18 @@ sub copy { # Test for an error when reading output my $strOutput; - + eval { $strOutput = $self->{oRemote}->output_read($strOperation eq OP_FILE_COPY, $strDebug); }; - + # If there is an error then evaluate if ($@) { my $oMessage = $@; - + # We'll ignore this error if the source file was missing and missing file exception was returned # and bIgnoreMissingSource is set if ($bIgnoreMissingSource && $strRemote eq "in" && $oMessage->isa("BackRest::Exception") && diff --git a/lib/BackRest/Remote.pm b/lib/BackRest/Remote.pm index 3c6aba296..3d16b8532 100644 --- a/lib/BackRest/Remote.pm +++ b/lib/BackRest/Remote.pm @@ -90,10 +90,6 @@ sub BUILD $self->greeting_read(); } - else - { - - } } #################################################################################################################################### @@ -103,11 +99,11 @@ sub clone { my $self = shift; - return pg_backrest_remote->new + return BackRest::Remote->new ( strCommand => $self->{strCommand}, - strHost => $self->{strUser}, - strUser => $self->{strHost}, + strHost => $self->{strHost}, + strUser => $self->{strUser}, iBlockSize => $self->{iBlockSize} ); } @@ -266,7 +262,7 @@ sub read_line #################################################################################################################################### # WRITE_LINE -# +# # Write a line data #################################################################################################################################### sub write_line @@ -287,7 +283,7 @@ sub write_line #################################################################################################################################### # WAIT_PID -# +# # See if the remote process has terminated unexpectedly. #################################################################################################################################### sub wait_pid diff --git a/test/lib/BackRestTest/FileTest.pm b/test/lib/BackRestTest/FileTest.pm index 76f141389..796259852 100755 --- a/test/lib/BackRestTest/FileTest.pm +++ b/test/lib/BackRestTest/FileTest.pm @@ -36,21 +36,21 @@ sub BackRestFileTestSetup { my $bPrivate = shift; my $bDropOnly = shift; - + # Remove the backrest private directory if (-e "${strTestPath}/private") { system("ssh ${strUserBackRest}\@${strHost} 'rm -rf ${strTestPath}/private'"); } - + # Remove the test directory system("rm -rf ${strTestPath}") == 0 or die 'unable to drop test path'; - + if (!defined($bDropOnly) || !$bDropOnly) { # Create the test directory mkdir($strTestPath, oct("0770")) or confess "Unable to create test directory"; - + # Create the backrest private directory if (defined($bPrivate) && $bPrivate) { @@ -113,14 +113,13 @@ sub BackRestFileTest for (my $bRemote = 0; $bRemote <= 1; $bRemote++) { # Create the file object - my $oFile = BackRest::File->new + my $oFile = (BackRest::File->new ( strStanza => "db", - strBackupClusterPath => undef, strBackupPath => ${strTestPath}, strRemote => $bRemote ? 'backup' : undef, oRemote => $bRemote ? $oRemote : undef - ); + ))->clone(); # Loop through exists (does the paren path exist?) for (my $bExists = 0; $bExists <= 1; $bExists++) @@ -131,6 +130,8 @@ sub BackRestFileTest # Loop through permission (permission will be set on true) for (my $bPermission = 0; $bPermission <= $bExists; $bPermission++) { + my $strPathType = PATH_BACKUP_CLUSTER; + $iRun++; &log(INFO, "run ${iRun} - " . @@ -139,7 +140,10 @@ sub BackRestFileTest # Setup test directory BackRestFileTestSetup($bError); - my $strPath = "${strTestPath}/path"; + mkdir("$strTestPath/backup") or confess "Unable to create test/backup directory"; + mkdir("$strTestPath/backup/db") or confess "Unable to create test/backup/db directory"; + + my $strPath = "path"; my $strPermission; # If permission then set one (other than the default) @@ -159,10 +163,11 @@ sub BackRestFileTest if ($bError) { $strPath = "${strTestPath}/private/path"; + $strPathType = PATH_BACKUP_ABSOLUTE; } elsif (!$bExists) { - $strPath = "${strTestPath}/error/path"; + $strPath = "error/path"; } # Execute in eval to catch errors @@ -170,7 +175,7 @@ sub BackRestFileTest eval { - $oFile->path_create(PATH_BACKUP_ABSOLUTE, $strPath, $strPermission); + $oFile->path_create($strPathType, $strPath, $strPermission); }; # Check for errors @@ -191,17 +196,19 @@ sub BackRestFileTest } # Make sure the path was actually created - unless (-e $strPath) + my $strPathCheck = $oFile->path_get($strPathType, $strPath); + + unless (-e $strPathCheck) { confess "path was not created"; } # Check that the permissions were set correctly - my $oStat = lstat($strPath); + my $oStat = lstat($strPathCheck); if (!defined($oStat)) { - confess "unable to stat ${strPath}"; + confess "unable to stat ${strPathCheck}"; } if ($bPermission) @@ -233,7 +240,6 @@ sub BackRestFileTest my $oFile = BackRest::File->new ( strStanza => "db", - strBackupClusterPath => undef, strBackupPath => ${strTestPath}, strRemote => $bRemote ? 'backup' : undef, oRemote => $bRemote ? $oRemote : undef @@ -334,7 +340,6 @@ sub BackRestFileTest my $oFile = BackRest::File->new ( strStanza => "db", - strBackupClusterPath => undef, strBackupPath => ${strTestPath}, strRemote => $bRemote ? 'backup' : undef, oRemote => $bRemote ? $oRemote : undef @@ -378,10 +383,10 @@ sub BackRestFileTest { next; } - + confess "error raised: " . $@ . "\n"; } - + if (!$bExists || $bError) { confess "expected error"; @@ -398,11 +403,11 @@ sub BackRestFileTest { confess "file was not compressed"; } - + system("gzip -d ${strDestinationFile}") == 0 or die "could not decompress ${strDestinationFile}"; - + my $strDestinationHash = $oFile->hash(PATH_BACKUP_ABSOLUTE, $strFile); - + if ($strSourceHash ne $strDestinationHash) { confess "source ${strSourceHash} and destination ${strDestinationHash} file hashes do not match"; @@ -440,7 +445,6 @@ sub BackRestFileTest my $oFile = BackRest::File->new ( strStanza => "db", - strBackupClusterPath => undef, strBackupPath => ${strTestPath}, strRemote => $bRemote ? 'backup' : undef, oRemote => $bRemote ? $oRemote : undef @@ -512,10 +516,10 @@ sub BackRestFileTest { next; } - + confess "error raised: " . $@ . "\n"; } - + # Check for an expected error if ($bErrorExpected) { @@ -585,7 +589,6 @@ sub BackRestFileTest my $oFile = BackRest::File->new ( strStanza => "db", - strBackupClusterPath => undef, strBackupPath => ${strTestPath}, strRemote => $bRemote ? 'backup' : undef, oRemote => $bRemote ? $oRemote : undef @@ -615,7 +618,7 @@ sub BackRestFileTest # Loop through exists for (my $bExists = 0; $bExists <= 1; $bExists++) { - + # Loop through error for (my $bError = 0; $bError <= 1; $bError++) { @@ -665,7 +668,7 @@ sub BackRestFileTest confess "error raised: " . $@ . "\n"; } - + if ($bErrorExpected) { confess 'error was expected'; @@ -712,7 +715,6 @@ sub BackRestFileTest my $oFile = BackRest::File->new ( strStanza => $strStanza, - strBackupClusterPath => ${strTestPath}, strBackupPath => ${strTestPath}, strRemote => $bRemote ? 'backup' : undef, oRemote => $bRemote ? $oRemote : undef @@ -756,7 +758,7 @@ sub BackRestFileTest # Execute in eval in case of error my $bRemoved; - + eval { $bRemoved = $oFile->remove(PATH_BACKUP_ABSOLUTE, $strFile, $bTemp, $bIgnoreMissing); @@ -768,7 +770,7 @@ sub BackRestFileTest { next; } - + if (!$bExists && !$bIgnoreMissing) { next; @@ -776,19 +778,19 @@ sub BackRestFileTest confess "unexpected error raised: " . $@; } - + if ($bError || $bRemote) { confess 'error should have been returned'; } - + if (!$bRemoved) { if (!$bExists && $bIgnoreMissing) { next; } - + confess 'remove returned false, but something should have been removed'; } @@ -818,7 +820,6 @@ sub BackRestFileTest my $oFile = BackRest::File->new ( strStanza => $strStanza, - strBackupClusterPath => ${strTestPath}, strBackupPath => ${strTestPath}, strRemote => $bRemote ? 'backup' : undef, oRemote => $bRemote ? $oRemote : undef @@ -856,7 +857,7 @@ sub BackRestFileTest # Execute in eval in case of error my $strHash; my $bErrorExpected = !$bExists || $bError || $bRemote; - + eval { $strHash = $oFile->hash(PATH_BACKUP_ABSOLUTE, $strFile) @@ -876,7 +877,7 @@ sub BackRestFileTest { confess "error was expected"; } - + if ($strHash ne '06364afe79d801433188262478a76d19777ef351') { confess 'hashes do not match'; @@ -901,7 +902,6 @@ sub BackRestFileTest my $oFile = BackRest::File->new ( strStanza => $strStanza, - strBackupClusterPath => ${strTestPath}, strBackupPath => ${strTestPath}, strRemote => $bRemote ? 'backup' : undef, oRemote => $bRemote ? $oRemote : undef @@ -1002,7 +1002,6 @@ sub BackRestFileTest my $oFile = BackRest::File->new ( strStanza => "db", - strBackupClusterPath => undef, strBackupPath => ${strTestPath}, strRemote => $strRemote, oRemote => $bBackupRemote || $bDbRemote ? $oRemote : undef @@ -1033,7 +1032,7 @@ sub BackRestFileTest my $strDestinationPath = $bDestinationPathType ? "db" : "backup"; $iRun++; - + &log(INFO, "run ${iRun} - " . "srcpth " . (defined($strRemote) && $strRemote eq $strSourcePath ? "remote" : "local") . ":${strSourcePath}, srccmp $bSourceCompressed, srcmiss ${bSourceMissing}, " . @@ -1051,7 +1050,7 @@ sub BackRestFileTest # Create the compressed or uncompressed test file my $strSourceHash; - + if (!$bSourceMissing) { system("echo 'TESTDATA' > ${strSourceFile}"); @@ -1084,7 +1083,7 @@ sub BackRestFileTest if ($@) { my $oMessage = $@; - + if (blessed($oMessage)) { if ($oMessage->isa("BackRest::Exception")) @@ -1093,7 +1092,7 @@ sub BackRestFileTest { next; } - + confess $oMessage->message(); } else @@ -1115,13 +1114,13 @@ sub BackRestFileTest { confess 'copy() returned ' . $bReturn . ' when ignore missing set'; } - + next; } - + confess "expected source file missing error"; } - + unless (-e $strDestinationFile) { confess "could not find destination file ${strDestinationFile}"; @@ -1132,9 +1131,9 @@ sub BackRestFileTest system("gzip -d ${strDestinationFile}") == 0 or die "could not decompress ${strDestinationFile}"; $strDestinationFile = substr($strDestinationFile, 0, length($strDestinationFile) - 3); } - + my $strDestinationHash = $oFile->hash(PATH_ABSOLUTE, $strDestinationFile); - + if ($strSourceHash ne $strDestinationHash) { confess "source ${strSourceHash} and destination ${strDestinationHash} file hashes do not match"; @@ -1148,8 +1147,8 @@ sub BackRestFileTest } } } - + BackRestFileTestSetup(false, true); } -1; \ No newline at end of file +1;