Skip to content

Commit

Permalink
Issue #109: Make replacements idempotent and improve feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
petersistrom committed Dec 17, 2024
1 parent 12a7258 commit 885e6aa
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 32 deletions.
95 changes: 65 additions & 30 deletions classes/helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,11 @@ public static function find_link_function($table, $column) {
* @param string $search The text to search for.
* @param string $replace The text to replace with.
* @param int $id The id of the record to restrict the search.
* @param array $rowcounts The count/outcome for each row.
*
*/
public static function replace_text_in_a_record(string $table, string $columnname,
string $search, string $replace, int $id) : bool {
string $search, string $replace, int $id, &$rowcounts) {
global $DB;

$column = self::get_column_info($table, $columnname);
Expand All @@ -670,21 +672,24 @@ public static function replace_text_in_a_record(string $table, string $columnnam
if (!$record) {
mtrace(get_string('errorreplacingstringnorecord', 'tool_advancedreplace',
['id' => $id, 'table' => $table, 'column' => $columnname]));
return false;
$rowcounts['error']++;
return;
}

$escapedsearchstring = str_replace("\n", "\r\n", $search);

if (str_contains($record->$columnname, $search)) {
$newstring = str_replace($search, $replace, $record->$columnname);
return $DB->set_field($table, $columnname, $newstring, array('id' => $id));
$DB->set_field($table, $columnname, $newstring, array('id' => $id)) ? $rowcounts['success']++ : $rowcounts['error']++;
} else if (str_contains($record->$columnname, $escapedsearchstring)) {
$newstring = str_replace($escapedsearchstring, $replace, $record->$columnname);
return $DB->set_field($table, $columnname, $newstring, array('id' => $id));
$DB->set_field($table, $columnname, $newstring, array('id' => $id)) ? $rowcounts['success']++ : $rowcounts['error']++;
} else if (str_contains($record->$columnname, $replace)) {
$rowcounts['replacematch']++;
} else {
mtrace(get_string('errorreplacingstring', 'tool_advancedreplace',
['id' => $id, 'table' => $table, 'column' => $columnname]));
return false;
$rowcounts['error']++;
}
}

Expand Down Expand Up @@ -781,19 +786,21 @@ public static function handle_replace_csv(string $data, string $type = 'db') {

// Read the data and replace the strings.
$csvimport->init();
$rowcount = 0;
$rowskip = 0;
$rowerror = 0;
$rowcounts = [
'success' => 0,
'skipped' => 0,
'error' => 0,
'replacematch' => 0,
];
$totalrows = 0;
while ($record = $csvimport->next()) {
if (empty($record[$replaceindex])) {
// Skip if 'replace' is empty.
$rowskip++;
$rowcounts['skipped']++;
} else if ($type == 'db') {
// Replace the string.
if (!self::replace_text_in_a_record($record[$tableindex], $record[$columnindex],
$record[$matchindex], $record[$replaceindex], $record[$idindex])) {
$rowerror++;
}
self::replace_text_in_a_record($record[$tableindex], $record[$columnindex],
$record[$matchindex], $record[$replaceindex], $record[$idindex], $rowcounts);
} else if ($type == 'files') {
$filerecord = [
'contextid' => $record[$contextidindex],
Expand All @@ -805,20 +812,28 @@ public static function handle_replace_csv(string $data, string $type = 'db') {
'mimetype' => $record[$mimeindex],
];

if (!self::replace_text_in_file($filerecord, $record[$matchindex], $record[$replaceindex],
$record[$internalindex])) {
$rowskip++;
}
self::replace_text_in_file($filerecord, $record[$matchindex], $record[$replaceindex],
$record[$internalindex], $rowcounts);
}

$totalrows++;
// Update the progress bar.
$rowcount++;
$progress->update_full(100 * $rowcount / $contentcount,
"Replaced $rowcount rows. Skipped $rowskip rows. Error replacing $rowerror rows.");
$progress->update_full(
100 * $totalrows / $contentcount,
"Replaced " . $rowcounts['success'] . " rows. " .
"Skipped " . $rowcounts['skipped'] . " rows, replacement string empty in CSV. " .
"Skipped " . $rowcounts['replacematch'] . " rows, replacement string already exists in match. " .
"Error replacing " . $rowcounts['error'] . " rows."
);
}

// Show progress.
$progress->update_full('100', "Replaced $rowcount rows. Skipped $rowskip rows. Error replacing $rowerror rows.");
$progress->update_full('100',
"Replaced " . $rowcounts['success'] . " rows. " .
"Skipped " . $rowcounts['skipped'] . " rows, replacement string empty in CSV. " .
"Skipped " . $rowcounts['replacematch'] . " rows, replacement string already exists in match. " .
"Error replacing " . $rowcounts['error'] . " rows."
);
;

$csvimport->cleanup();
$csvimport->close();
Expand Down Expand Up @@ -849,10 +864,10 @@ public static function get_replace_csv_content(int $draftid): string {
* @param string $match The string to search for in the file's contents.
* @param string $replace The string to replace the matched string with.
* @param string $internal The name of the internal file to modify (only used for zip files).
* @param array $rowcounts The count/outcome for each row.
*
* @return bool Returns true if the string was replaced and the file updated successfully, false otherwise.
*/
public static function replace_text_in_file(array $filerecord, string $match, string $replace, string $internal): bool {
public static function replace_text_in_file(array $filerecord, string $match, string $replace, string $internal, array &$rowcounts) {
$fs = get_file_storage();
$file = $fs->get_file(
$filerecord['contextid'],
Expand All @@ -866,30 +881,41 @@ public static function replace_text_in_file(array $filerecord, string $match, st
if (!$file) {
mtrace(get_string('errorreplacingfilenotfound', 'tool_advancedreplace',
['filename' => $filerecord['filename']]));
return false;
$rowcounts['error']++;
return;
}

// Specify tmp filename to avoid unique constraint conflict.
$filerecord['filename'] = time();

if ($filerecord['mimetype'] == 'application/zip' || $filerecord['mimetype'] == 'application/zip.h5p') {
$newzip = self::replace_text_in_zip($file, $match, $replace, $internal);
$newfile = $fs->create_file_from_pathname($filerecord, $newzip);
if ($newzip = self::replace_text_in_zip($file, $match, $replace, $internal, $rowcounts)) {
$newfile = $fs->create_file_from_pathname($filerecord, $newzip);
} else {
return;
}

unlink($newzip);
} else {

$content = $file->get_content();
$newcontent = str_replace($match, $replace, $content);

// New and old content are the same, we assume this replace has already been run.
if ($newcontent == $content) {
$rowcounts['replacematch']++;
return;
}
$newfile = $fs->create_file_from_string($filerecord, $newcontent);
}
if ($newfile) {
$file->replace_file_with($newfile);
$newfile->delete();
return true;
$rowcounts['success']++;
} else {
mtrace(get_string('errorreplacingfile', 'tool_advancedreplace',
['replace' => $match, 'filename' => $filerecord['filepath']]));
return false;
$rowcounts['error']++;
}
}

Expand All @@ -902,7 +928,7 @@ public static function replace_text_in_file(array $filerecord, string $match, st
* @param string $internalfilename The file name of the internal file to be modified.
*/
public static function replace_text_in_zip(\stored_file $zipfile, string $searchstring,
string $replacestring, string $internalfilename) {
string $replacestring, string $internalfilename, array &$rowcounts) {

// Create a temporary file path for working with the ZIP file.
$tempzip = make_request_directory() . '/' . $zipfile->get_filename();
Expand All @@ -911,26 +937,35 @@ public static function replace_text_in_zip(\stored_file $zipfile, string $search
// Open and modify the ZIP file.
$zip = new \ZipArchive();
if ($zip->open($tempzip) !== true) {
$rowcounts['error']++;
return false;
}

// Check if the target file exists in the zip.
$fileindex = $zip->locateName($internalfilename);
if ($fileindex === false) {
$zip->close();
$rowcounts['error']++;
return false;
}

// Extract the target file's content.
$filecontent = $zip->getFromIndex($fileindex);
if ($filecontent === false) {
$zip->close();
$rowcounts['error']++;
return false;
}

// Replace the string in the file's contents.
$modifiedcontents = str_replace($searchstring, $replacestring, $filecontent);

if ($modifiedcontents == $filecontent) {
$zip->close();
$rowcounts['replacematch']++;
return false;
}

// Delete the old file and add the modified file back to the ZIP.
$zip->deleteName($internalfilename);
$zip->addFromString($internalfilename, $modifiedcontents);
Expand Down
9 changes: 7 additions & 2 deletions tests/helper_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -393,10 +393,15 @@ public function test_replace_text_in_a_record(): void {
'content' => 'This is a page content with a link to https://example.com.au/1234',
'contentformat' => FORMAT_HTML,
]);

$rowcounts = [
'success' => 0,
'skipped' => 0,
'error' => 0,
'replacematch' => 0,
];
// Replace the text in the page content.
helper::replace_text_in_a_record('page', 'content', 'https://example.com.au/1234',
'https://example.com.au/5678', $page->id);
'https://example.com.au/5678', $page->id, $rowcounts);

// Get the updated page content.
$updatedpage = $DB->get_record('page', ['id' => $page->id]);
Expand Down

0 comments on commit 885e6aa

Please sign in to comment.