diff --git a/files/classes/conversion.php b/files/classes/conversion.php index 0a2e8610ed5..ffe01d1ce81 100644 --- a/files/classes/conversion.php +++ b/files/classes/conversion.php @@ -128,36 +128,13 @@ class conversion extends \core\persistent { // Fetch actual conversions which relate to the specified source file, and have a matching conversion record, // and either have a valid destination file which still exists, or do not have a destination file at all. - $dbfamily = $DB->get_dbfamily(); - switch ($dbfamily) { - // For certain DB engines, use a more optimised query. - case 'mysql': - case 'postgres': - $sql = "SELECT {$sqlfields} - FROM {" . self::TABLE . "} c - JOIN (SELECT id - FROM {files} - WHERE contenthash = :ccontenthash - LIMIT 1 - ) conversionsourcefile ON conversionsourcefile.id = c.sourcefileid - LEFT JOIN {files} conversiondestfile ON conversiondestfile.id = c.destfileid - WHERE c.targetformat = :cformat - AND (c.destfileid IS NULL - OR conversiondestfile.id IS NOT NULL)"; - break; - - // For everything else, use the standard cross-db compatible query. - default: - $sql = "SELECT {$sqlfields} - FROM {" . self::TABLE . "} c - INNER JOIN {files} conversionsourcefile ON conversionsourcefile.id = c.sourcefileid - LEFT JOIN {files} conversiondestfile ON conversiondestfile.id = c.destfileid - WHERE conversionsourcefile.contenthash = :ccontenthash - AND c.targetformat = :cformat - AND (c.destfileid IS NULL - OR conversiondestfile.id IS NOT NULL)"; - break; - } + $sql = "SELECT {$sqlfields} + FROM {" . self::TABLE . "} c + JOIN {files} conversionsourcefile ON conversionsourcefile.id = c.sourcefileid + LEFT JOIN {files} conversiondestfile ON conversiondestfile.id = c.destfileid + WHERE conversionsourcefile.contenthash = :ccontenthash + AND c.targetformat = :cformat + AND (c.destfileid IS NULL OR conversiondestfile.id IS NOT NULL)"; // Fetch a empty conversion record for each source/destination combination that we find to match where the // destination file is in the correct filearea/filepath/filename combination to meet the requirements. @@ -193,7 +170,6 @@ class conversion extends \core\persistent { $records = $DB->get_records_sql($sql, [ 'ccontenthash' => $file->get_contenthash(), 'osourcefileid' => $file->get_id(), - 'cfilepath' => "/{$format}/", 'ofilepath' => "/{$format}/", 'cformat' => $format, 'oformat' => $format, diff --git a/files/tests/conversion_test.php b/files/tests/conversion_test.php index 07b1f2669ca..1cacf28e0e2 100644 --- a/files/tests/conversion_test.php +++ b/files/tests/conversion_test.php @@ -75,6 +75,53 @@ class conversion_test extends \advanced_testcase { $this->assertFalse($conversion->get_destfile()); } + /** + * Ensure that get_conversions_for_file returns an existing conversion + * record with matching sourcefileid and targetformat when a file with the same + * contenthash is uploaded several times. + * + * @covers \core_files\conversion::get_conversions_for_file + */ + public function test_get_conversions_for_multiple_files_existing_conversion_incomplete() { + $this->resetAfterTest(); + + // Create a bunch of files with the same content. + for ($i = 0; $i < 5; $i++) { + $sourcefiles[] = $this->create_stored_file('test content', 'testfile' . $i . '.txt'); + } + + // Use only one file for the conversion. + // Pick some file in the middle. + $sourcefile = $sourcefiles[count($sourcefiles) - 2]; + + $existing = new conversion(0, (object) [ + 'sourcefileid' => $sourcefile->get_id(), + 'targetformat' => 'pdf', + ]); + $existing->create(); + + $conversions = conversion::get_conversions_for_file($sourcefile, 'pdf'); + $this->assertCount(1, $conversions); + + $conversion = array_shift($conversions); + $conversionfile = $conversion->get_sourcefile(); + + $this->assertEquals($sourcefile->get_id(), $conversionfile->get_id()); + $this->assertFalse($conversion->get_destfile()); + + // Check that getting the conversion for a different file record with the same contenthash + // returns the same conversion as above. + $conversions = conversion::get_conversions_for_file($sourcefiles[count($sourcefiles) - 1], 'pdf'); + $this->assertCount(1, $conversions); + + $conversion = array_shift($conversions); + $conversionfile = $conversion->get_sourcefile(); + + $this->assertEquals($existing->get('id'), $conversion->get('id')); + $this->assertEquals($sourcefile->get_id(), $conversionfile->get_id()); + $this->assertFalse($conversion->get_destfile()); + } + /** * Ensure that get_conversions_for_file returns an existing conversion * record with matching sourcefileid and targetformat when a second @@ -139,6 +186,7 @@ class conversion_test extends \advanced_testcase { $conversion = array_shift($conversions); + $this->assertEquals($existing->get('id'), $conversion->get('id')); $this->assertEquals($sourcefile->get_id(), $conversion->get_sourcefile()->get_id()); $this->assertEquals($destfile->get_id(), $conversion->get_destfile()->get_id()); }