Allow functions to accept optional parameters as a hash.

Refactor File->list() and fileList() to accept optional parameters.

doc/xml/release.xml

@@ -172,6 +172,16 @@
                         <p>Added validation of <setting>pgbackrest.conf</setting> to display warnings if options are not valid or are not in the correct section.</p>
                     </release-item>
                 </release-feature-list>
+
+                <release-refactor-list>
+                    <release-item>
+                        <p>Allow functions to accept optional parameters as a hash.</p>
+                    </release-item>
+
+                    <release-item>
+                        <p>Refactor <code>File->list()</code> and <code>fileList()</code> to accept optional parameters.</p>
+                    </release-item>
+                </release-refactor-list>
             </release-core-list>
 
             <release-test-list>

lib/pgBackRest/Archive/ArchiveCommon.pm

@@ -299,7 +299,8 @@ sub walSegmentFind
         else
         {
             @stryTimelineMajor = $oFile->list(
-                PATH_BACKUP_ARCHIVE, $strArchiveId, '[0-F]{8}' . substr($strWalSegment, 0, 8), undef, true);
+                PATH_BACKUP_ARCHIVE, $strArchiveId,
+                {strExpression => '[0-F]{8}' . substr($strWalSegment, 0, 8), bIgnoreMissing => true});
         }
 
         # Search each timelin/major path
@@ -311,9 +312,10 @@ sub walSegmentFind
             # Get the name of the requested WAL segment (may have hash info and compression extension)
             push(@stryWalFileName, $oFile->list(
                 PATH_BACKUP_ARCHIVE, "${strArchiveId}/${strTimelineMajor}",
+                {strExpression =>
                     "^${strWalSegmentFind}" . (walIsPartial($strWalSegment) ? "\\.partial" : '') .
                     "-[0-f]{40}(\\." . COMPRESS_EXT . "){0,1}\$",
-                undef, true));
+                    bIgnoreMissing => true}));
         }
     }
     while (@stryWalFileName == 0 && waitMore($oWait));

lib/pgBackRest/Archive/ArchiveInfo.pm

@@ -250,7 +250,8 @@ sub reconstruct
 
     my $strInvalidFileStructure = undef;
 
-    my @stryArchiveId = fileList($self->{strArchiveClusterPath}, REGEX_ARCHIVE_DIR_DB_VERSION, 'forward', true);
+    my @stryArchiveId = fileList(
+        $self->{strArchiveClusterPath}, {strExpression => REGEX_ARCHIVE_DIR_DB_VERSION, bIgnoreMissing => true});
     my %hDbHistoryVersion;
 
     # Get the db-version and db-id (history id) from the upper level directory names, e.g. 9.4-1
@@ -267,8 +268,9 @@ sub reconstruct
         my $strVersionDir = $strDbVersion . "-" . $iDbHistoryId;
 
         # Get the name of the first archive directory
-        my $strArchiveDir =
+        my $strArchiveDir = (fileList(
-            (fileList($self->{strArchiveClusterPath} . "/${strVersionDir}", REGEX_ARCHIVE_DIR_WAL, 'forward', true))[0];
+            $self->{strArchiveClusterPath} . "/${strVersionDir}",
+            {strExpression => REGEX_ARCHIVE_DIR_WAL, bIgnoreMissing => true}))[0];
 
         # Continue if any file structure or missing files info
         if (!defined($strArchiveDir))
@@ -278,9 +280,10 @@ sub reconstruct
         }
 
         # ??? Should probably make a function in ArchiveCommon
-        my $strArchiveFile =
+        my $strArchiveFile = (fileList(
-            (fileList($self->{strArchiveClusterPath} . "/${strVersionDir}/${strArchiveDir}",
+            $self->{strArchiveClusterPath} . "/${strVersionDir}/${strArchiveDir}",
-            "^[0-F]{24}(\\.partial){0,1}(-[0-f]+){0,1}(\\.$oFile->{strCompressExtension}){0,1}\$", 'forward', true))[0];
+            {strExpression => "^[0-F]{24}(\\.partial){0,1}(-[0-f]+){0,1}(\\.$oFile->{strCompressExtension}){0,1}\$",
+                bIgnoreMissing => true}))[0];
 
         # Continue if any file structure or missing files info
         if (!defined($strArchiveFile))

lib/pgBackRest/Archive/ArchivePush.pm

@@ -185,7 +185,7 @@ sub walStatus
     my $bResult = false;
 
     # Find matching status files
-    my @stryStatusFile = fileList($strSpoolPath, '^' . $strWalFile . '\.(ok|error)$', undef, true);
+    my @stryStatusFile = fileList($strSpoolPath, {strExpression => '^' . $strWalFile . '\.(ok|error)$', bIgnoreMissing => true});
 
     if (@stryStatusFile > 0)
     {
@@ -272,7 +272,7 @@ sub readyList
 
     if (defined($self->{strSpoolPath}))
     {
-        foreach my $strOkFile (fileList($self->{strSpoolPath}, '\.ok$', undef, true))
+        foreach my $strOkFile (fileList($self->{strSpoolPath}, {strExpression => '\.ok$', bIgnoreMissing => true}))
         {
             $strOkFile = substr($strOkFile, 0, length($strOkFile) - length('.ok'));
             $hOkFile->{$strOkFile} = true;
@@ -281,7 +281,7 @@ sub readyList
 
     # Read the .ready files
     my $strWalStatusPath = "$self->{strWalPath}/archive_status";
-    my @stryReadyFile = fileList($strWalStatusPath, '\.ready$');
+    my @stryReadyFile = fileList($strWalStatusPath, {strExpression => '\.ready$'});
 
     # Generate a list of new files
     my @stryNewReadyFile;

lib/pgBackRest/Backup.pm

@@ -950,13 +950,14 @@ sub process
     # clocks.  In practice this is most useful for making offline testing faster since it allows the wait after manifest build to
     # be skipped by dealing with any backup label collisions here.
     if (fileList($oFileLocal->pathGet(PATH_BACKUP_CLUSTER),
-                 ($strType eq BACKUP_TYPE_FULL ? '^' : '_') .
+                 {strExpression =>
-                 timestampFileFormat(undef, $lTimestampStop) .
+                    ($strType eq BACKUP_TYPE_FULL ? '^' : '_') . timestampFileFormat(undef, $lTimestampStop) .
-                 ($strType eq BACKUP_TYPE_FULL ? 'F' : '(D|I)$')) ||
+                    ($strType eq BACKUP_TYPE_FULL ? 'F' : '(D|I)$')}) ||
         fileList($oFileLocal->pathGet(PATH_BACKUP_CLUSTER, PATH_BACKUP_HISTORY . '/' . timestampFormat('%4d', $lTimestampStop)),
-                 ($strType eq BACKUP_TYPE_FULL ? '^' : '_') .
+                 {strExpression =>
-                 timestampFileFormat(undef, $lTimestampStop) .
+                    ($strType eq BACKUP_TYPE_FULL ? '^' : '_') . timestampFileFormat(undef, $lTimestampStop) .
-                 ($strType eq BACKUP_TYPE_FULL ? 'F' : '(D|I)\.manifest\.' . $oFileLocal->{strCompressExtension}), undef, true))
+                    ($strType eq BACKUP_TYPE_FULL ? 'F' : '(D|I)\.manifest\.' . $oFileLocal->{strCompressExtension}),
+                    bIgnoreMissing => true}))
     {
         waitRemainder();
         $strBackupLabel = backupLabelFormat($strType, $strBackupLastPath, time());

lib/pgBackRest/BackupInfo.pm

@@ -209,7 +209,7 @@ sub reconstruct
     );
 
     # Check for backups that are not in FILE_BACKUP_INFO
-    foreach my $strBackup (fileList($self->{strBackupClusterPath}, backupRegExpGet(true, true, true)))
+    foreach my $strBackup (fileList($self->{strBackupClusterPath}, {strExpression => backupRegExpGet(true, true, true)}))
     {
         my $strManifestFile = "$self->{strBackupClusterPath}/${strBackup}/" . FILE_MANIFEST;
 

lib/pgBackRest/Common/Log.pm

@@ -124,7 +124,7 @@ sub logFileSet
         $bLogFileExists = -e $strFile ? true : false;
         $bLogFileFirst = true;
 
-        $hLogFile = fileOpen($strFile, O_WRONLY | O_CREAT | O_APPEND, '0660');
+        $hLogFile = fileOpen($strFile, O_WRONLY | O_CREAT | O_APPEND);
 
         # Write out anything that was cached before the file was opened
         if (defined($strLogFileCache))
@@ -303,6 +303,7 @@ sub logDebugProcess
 
     # Process each parameter hash
     my $oParam = shift;
+    my $bOptionalBlock = false;
 
     # Strip the package name off strFunction if it's pgBackRest
     $strFunction =~ s/^pgBackRest[^\:]*\:\://;
@@ -310,14 +311,43 @@ sub logDebugProcess
     while (defined($oParam))
     {
         my $strParamName = $$oParam{name};
+        my $bParamOptional = defined($oParam->{optional}) && $oParam->{optional};
+        my $bParamRequired = !defined($oParam->{required}) || $oParam->{required};
         my $oValue;
 
+        # If param is optional then the optional block has been entered
+        if ($bParamOptional)
+        {
+            if (defined($oParam->{required}))
+            {
+                confess &log(ASSERT, "cannot define 'required' for optional parameter '${strParamName}'");
+            }
+
+            $bParamRequired = false;
+            $bOptionalBlock = true;
+        }
+
+        # Don't allow non-optional parameters once optional block has started
+        if ($bParamOptional != $bOptionalBlock)
+        {
+            confess &log(ASSERT, "non-optional parameter '${strParamName}' invalid after optional parameters");
+        }
+
         # Push the return value into the return value array
         if ($strType eq DEBUG_PARAM)
         {
-            if (defined($$oyParamRef[$iIndex]))
+            if ($bParamOptional)
             {
-                push(@oyResult, $$oyParamRef[$iIndex]);
+                $oValue = $$oyParamRef[$iIndex]->{$strParamName};
+            }
+            else
+            {
+                $oValue = $$oyParamRef[$iIndex];
+            }
+
+            if (defined($oValue))
+            {
+                push(@oyResult, $oValue);
             }
             else
             {
@@ -327,7 +357,7 @@ sub logDebugProcess
 
             $oValue = $oyResult[@oyResult - 1];
 
-            if (!defined($oValue) && (!defined($${oParam}{required}) || $${oParam}{required}))
+            if (!defined($oValue) && $bParamRequired)
             {
                 confess &log(ASSERT, "${strParamName} is required in ${strFunction}");
             }
@@ -355,7 +385,6 @@ sub logDebugProcess
 
         if (!defined($$oParam{log}) || $$oParam{log})
         {
-
             # If the parameter is a hash but not blessed then represent it as a string
             # ??? This should go away once the inputs to logDebug can be changed
             if (ref($oValue) eq 'HASH' && !blessed($oValue))
@@ -378,8 +407,12 @@ sub logDebugProcess
 
         # Get the next parameter hash
         $oParam = shift;
+
+        if (!$bParamOptional)
+        {
             $iIndex++;
         }
+    }
 
     if (defined($strDetail) && $iIndex == 0)
     {

lib/pgBackRest/Expire.pm

@@ -222,7 +222,8 @@ sub process
     $oBackupInfo->save();
 
     # Remove backups from disk
-    foreach my $strBackup ($oFile->list(PATH_BACKUP_CLUSTER, undef, backupRegExpGet(true, true, true), 'reverse'))
+    foreach my $strBackup ($oFile->list(
+        PATH_BACKUP_CLUSTER, undef, {strExpression => backupRegExpGet(true, true, true), strSortOrder => 'reverse'}))
     {
         if (!$oBackupInfo->current($strBackup))
         {
@@ -263,7 +264,8 @@ sub process
         if ($iBackupTotal > 0)
         {
             my $oArchiveInfo = new pgBackRest::Archive::ArchiveInfo($oFile->pathGet(PATH_BACKUP_ARCHIVE), true);
-            my @stryListArchiveDisk = fileList($oFile->pathGet(PATH_BACKUP_ARCHIVE), REGEX_ARCHIVE_DIR_DB_VERSION, 'forward', true);
+            my @stryListArchiveDisk = fileList(
+                $oFile->pathGet(PATH_BACKUP_ARCHIVE), {strExpression => REGEX_ARCHIVE_DIR_DB_VERSION, bIgnoreMissing => true});
 
             # Make sure the current database versions match between the two files
             if (!($oArchiveInfo->test(INFO_ARCHIVE_SECTION_DB, INFO_ARCHIVE_KEY_DB_VERSION, undef,
@@ -397,7 +399,8 @@ sub process
                         }
 
                         # Get all major archive paths (timeline and first 32 bits of LSN)
-                        foreach my $strPath ($oFile->list(PATH_BACKUP_ARCHIVE, $strArchiveId, REGEX_ARCHIVE_DIR_WAL))
+                        foreach my $strPath ($oFile->list(
+                            PATH_BACKUP_ARCHIVE, $strArchiveId, {strExpression => REGEX_ARCHIVE_DIR_WAL}))
                         {
                             logDebugMisc($strOperation, "found major WAL path: ${strPath}");
                             $bRemove = true;
@@ -431,8 +434,8 @@ sub process
                             elsif ($strPath le substr($strArchiveExpireMax, 0, 16))
                             {
                                 # Look for files in the archive directory
-                                foreach my $strSubPath ($oFile->list(PATH_BACKUP_ARCHIVE,
+                                foreach my $strSubPath ($oFile->list(
-                                                                     "${strArchiveId}/${strPath}", "^[0-F]{24}.*\$"))
+                                    PATH_BACKUP_ARCHIVE, "${strArchiveId}/${strPath}", {strExpression => "^[0-F]{24}.*\$"}))
                                 {
                                     $bRemove = true;
 

lib/pgBackRest/File.pm

@@ -966,9 +966,9 @@ sub list
             __PACKAGE__ . '->list', \@_,
             {name => 'strPathType'},
             {name => 'strPath', required => false},
-            {name => 'strExpression', required => false},
+            {name => 'strExpression', optional => true},
-            {name => 'strSortOrder', default => 'forward'},
+            {name => 'strSortOrder', optional => true, default => 'forward'},
-            {name => 'bIgnoreMissing', default => false}
+            {name => 'bIgnoreMissing', optional => true, default => false}
         );
 
     # Set operation variables
@@ -979,12 +979,14 @@ sub list
     if ($self->isRemote($strPathType))
     {
         @stryFileList = $self->{oProtocol}->cmdExecute(
-            OP_FILE_LIST, [$strPathOp, $strExpression, $strSortOrder, $bIgnoreMissing]);
+            OP_FILE_LIST,
+            [$strPathOp, {strExpression => $strExpression, strSortOrder => $strSortOrder, bIgnoreMissing => $bIgnoreMissing}]);
     }
     # Run locally
     else
     {
-        @stryFileList = fileList($strPathOp, $strExpression, $strSortOrder, $bIgnoreMissing);
+        @stryFileList = fileList(
+            $strPathOp, {strExpression => $strExpression, strSortOrder => $strSortOrder, bIgnoreMissing => $bIgnoreMissing});
     }
 
     # Return from function and log return values if any

lib/pgBackRest/FileCommon.pm

@@ -264,15 +264,15 @@ sub fileList
         $strPath,
         $strExpression,
         $strSortOrder,
-        $bIgnoreMissing
+        $bIgnoreMissing,
     ) =
         logDebugParam
         (
             __PACKAGE__ . '::fileList', \@_,
             {name => 'strPath', trace => true},
-            {name => 'strExpression', required => false, trace => true},
+            {name => 'strExpression', optional => true, trace => true},
-            {name => 'strSortOrder', default => 'forward', trace => true},
+            {name => 'strSortOrder', optional => true, default => 'forward', trace => true},
-            {name => 'bIgnoreMissing', default => false, trace => true}
+            {name => 'bIgnoreMissing', optional => true, default => false, trace => true},
         );
 
     # Working variables
@@ -390,7 +390,7 @@ sub fileManifestRecurse
     }
 
     # Get a list of all files in the path (including .)
-    my @stryFileList = fileList($strPathRead, undef, undef, $iDepth != 0);
+    my @stryFileList = fileList($strPathRead, {bIgnoreMissing => $iDepth != 0});
     unshift(@stryFileList, '.');
     my $hFileStat = fileManifestList($strPathRead, \@stryFileList);
 

lib/pgBackRest/Info.pm

@@ -345,7 +345,7 @@ sub stanzaList
     # Run locally
     else
     {
-        my @stryStanza = $oFile->list(PATH_BACKUP, CMD_BACKUP, undef, undef, true);
+        my @stryStanza = $oFile->list(PATH_BACKUP, CMD_BACKUP, {bIgnoreMissing => true});
 
         foreach my $strStanzaFound (@stryStanza)
         {
@@ -389,14 +389,14 @@ sub stanzaList
 
                 if ($oFile->exists(PATH_BACKUP, $strArchivePath))
                 {
-                    my @stryWalMajor = $oFile->list(PATH_BACKUP, $strArchivePath, '^[0-F]{16}$');
+                    my @stryWalMajor = $oFile->list(PATH_BACKUP, $strArchivePath, {strExpression => '^[0-F]{16}$'});
 
                     # Get first WAL segment
                     foreach my $strWalMajor (@stryWalMajor)
                     {
                         my @stryWalFile = $oFile->list(
                             PATH_BACKUP, "${strArchivePath}/${strWalMajor}",
-                            "^[0-F]{24}-[0-f]{40}(\\." . COMPRESS_EXT . "){0,1}\$");
+                            {strExpression => "^[0-F]{24}-[0-f]{40}(\\." . COMPRESS_EXT . "){0,1}\$"});
 
                         if (@stryWalFile > 0)
                         {
@@ -410,7 +410,7 @@ sub stanzaList
                     {
                         my @stryWalFile = $oFile->list(
                             PATH_BACKUP, "${strArchivePath}/${strWalMajor}",
-                            "^[0-F]{24}-[0-f]{40}(\\." . COMPRESS_EXT . "){0,1}\$", 'reverse');
+                            {strExpression => "^[0-F]{24}-[0-f]{40}(\\." . COMPRESS_EXT . "){0,1}\$", strSortOrder => 'reverse'});
 
                         if (@stryWalFile > 0)
                         {

lib/pgBackRest/Stanza.pm

@@ -119,8 +119,8 @@ sub stanzaCreate
     my $strParentPathBackup = $self->parentPathGet($oFile, PATH_BACKUP_CLUSTER);
 
     # Get a listing of files in the directory, ignoring if any are missing
-    my @stryFileListArchive = fileList($strParentPathArchive, undef, 'forward', true);
+    my @stryFileListArchive = fileList($strParentPathArchive, {bIgnoreMissing => true});
-    my @stryFileListBackup = fileList($strParentPathBackup, undef, 'forward', true);
+    my @stryFileListBackup = fileList($strParentPathBackup, {bIgnoreMissing => true});
 
     # If force not used and at least one directory is not empty, then check to see if the info files exist
     if (!optionGet(OPTION_FORCE) && (@stryFileListArchive || @stryFileListBackup))

test/lib/pgBackRestTest/Archive/ArchiveStopTest.pm

@@ -96,7 +96,8 @@ sub run
 
         #---------------------------------------------------------------------------------------------------------------------------
         $self->testResult(
-            sub {$oFile->list(PATH_BACKUP_ARCHIVE, PG_VERSION_94 . '-1/0000000100000001', '^(?!000000010000000100000002).+')},
+            sub {$oFile->list(
+                PATH_BACKUP_ARCHIVE, PG_VERSION_94 . '-1/0000000100000001', {strExpression => '^(?!000000010000000100000002).+'})},
             "000000010000000100000001-72b9da071c13957fb4ca31f05dbd5c644297c2f7${strCompressExt}",
             'segment 2-4 not pushed (2 is pushed sometimes when remote but ignore)', 5);
 
@@ -104,7 +105,8 @@ sub run
         $oHostDbMaster->archivePush($strXlogPath, $strArchiveTestFile, 5);
 
         $self->testResult(
-            sub {$oFile->list(PATH_BACKUP_ARCHIVE, PG_VERSION_94 . '-1/0000000100000001', '^(?!000000010000000100000002).+')},
+            sub {$oFile->list(
+                PATH_BACKUP_ARCHIVE, PG_VERSION_94 . '-1/0000000100000001', {strExpression => '^(?!000000010000000100000002).+'})},
             "(000000010000000100000001-72b9da071c13957fb4ca31f05dbd5c644297c2f7${strCompressExt}, " .
                 "000000010000000100000005-72b9da071c13957fb4ca31f05dbd5c644297c2f7${strCompressExt})",
             'segment 5 is pushed', 5);

test/lib/pgBackRestTest/Common/Host/HostBackupTest.pm

@@ -587,7 +587,8 @@ sub backupLast
     my $self = shift;
 
     my @stryBackup = $self->{oFile}->list(
-        PATH_BACKUP_CLUSTER, undef, '[0-9]{8}-[0-9]{6}F(_[0-9]{8}-[0-9]{6}(D|I)){0,1}', 'reverse');
+        PATH_BACKUP_CLUSTER, undef,
+        {strExpression => '[0-9]{8}-[0-9]{6}F(_[0-9]{8}-[0-9]{6}(D|I)){0,1}', strSortOrder => 'reverse'});
 
     if (!defined($stryBackup[0]))
     {

test/lib/pgBackRestTest/File/FileListTest.pm

@@ -74,7 +74,9 @@ sub run
 
         eval
         {
-            @stryFileList = $oFile->list(PATH_BACKUP_ABSOLUTE, $strPath, $strExpression, $strSort, $bIgnoreMissing);
+            @stryFileList = $oFile->list(
+                PATH_BACKUP_ABSOLUTE, $strPath,
+                {strExpression => $strExpression, strSortOrder => $strSort, bIgnoreMissing => $bIgnoreMissing});
             return true;
         }
         or do