diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php index 5a110afaf9..cd6ba68641 100644 --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -24,6 +24,38 @@ final class PhabricatorFileQuery return $this; } + /** + * Select files which are transformations of some other file. For example, + * you can use this query to find previously generated thumbnails of an image + * file. + * + * As a parameter, provide a list of transformation specifications. Each + * specification is a dictionary with the keys `originalPHID` and `transform`. + * The `originalPHID` is the PHID of the original file (the file which was + * transformed) and the `transform` is the name of the transform to query + * for. If you pass `true` as the `transform`, all transformations of the + * file will be selected. + * + * For example: + * + * array( + * array( + * 'originalPHID' => 'PHID-FILE-aaaa', + * 'transform' => 'sepia', + * ), + * array( + * 'originalPHID' => 'PHID-FILE-bbbb', + * 'transform' => true, + * ), + * ) + * + * This selects the `"sepia"` transformation of the file with PHID + * `PHID-FILE-aaaa` and all transformations of the file with PHID + * `PHID-FILE-bbbb`. + * + * @param list List of transform specifications, described above. + * @return this + */ public function withTransforms(array $specs) { foreach ($specs as $spec) { if (!is_array($spec) || @@ -50,7 +82,7 @@ final class PhabricatorFileQuery $data = queryfx_all( $conn_r, - 'SELECT * FROM %T f %Q %Q %Q %Q', + 'SELECT f.* FROM %T f %Q %Q %Q %Q', $table->getTableName(), $this->buildJoinClause($conn_r), $this->buildWhereClause($conn_r), @@ -108,11 +140,18 @@ final class PhabricatorFileQuery if ($this->transforms) { $clauses = array(); foreach ($this->transforms as $transform) { - $clauses[] = qsprintf( - $conn_r, - '(t.originalPHID = %s AND t.transform = %s)', - $transform['originalPHID'], - $transform['transform']); + if ($transform['transform'] === true) { + $clauses[] = qsprintf( + $conn_r, + '(t.originalPHID = %s)', + $transform['originalPHID']); + } else { + $clauses[] = qsprintf( + $conn_r, + '(t.originalPHID = %s AND t.transform = %s)', + $transform['originalPHID'], + $transform['transform']); + } } $where[] = qsprintf($conn_r, '(%Q)', implode(') OR (', $clauses)); } diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 732ede3de2..1ef5ae5aed 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -384,29 +384,54 @@ final class PhabricatorFile extends PhabricatorFileDAO } public function delete() { - // delete all records of this file in transformedfile - $trans_files = id(new PhabricatorTransformedFile())->loadAllWhere( - 'TransformedPHID = %s', $this->getPHID()); + + // We want to delete all the rows which mark this file as the transformation + // of some other file (since we're getting rid of it). We also delete all + // the transformations of this file, so that a user who deletes an image + // doesn't need to separately hunt down and delete a bunch of thumbnails and + // resizes of it. + + $outbound_xforms = id(new PhabricatorFileQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withTransforms( + array( + array( + 'originalPHID' => $this->getPHID(), + 'transform' => true, + ), + )) + ->execute(); + + foreach ($outbound_xforms as $outbound_xform) { + $outbound_xform->delete(); + } + + $inbound_xforms = id(new PhabricatorTransformedFile())->loadAllWhere( + 'transformedPHID = %s', + $this->getPHID()); $this->openTransaction(); - foreach ($trans_files as $trans_file) { - $trans_file->delete(); - } - $ret = parent::delete(); + foreach ($inbound_xforms as $inbound_xform) { + $inbound_xform->delete(); + } + $ret = parent::delete(); $this->saveTransaction(); // Check to see if other files are using storage $other_file = id(new PhabricatorFile())->loadAllWhere( 'storageEngine = %s AND storageHandle = %s AND - storageFormat = %s AND id != %d LIMIT 1', $this->getStorageEngine(), - $this->getStorageHandle(), $this->getStorageFormat(), + storageFormat = %s AND id != %d LIMIT 1', + $this->getStorageEngine(), + $this->getStorageHandle(), + $this->getStorageFormat(), $this->getID()); // If this is the only file using the storage, delete storage - if (count($other_file) == 0) { + if (!$other_file) { $engine = $this->instantiateStorageEngine(); $engine->deleteFile($this->getStorageHandle()); } + return $ret; } diff --git a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php index d2b2112c0f..ff0068bacd 100644 --- a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php +++ b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php @@ -145,4 +145,107 @@ final class PhabricatorFileTestCase extends PhabricatorTestCase { $this->assertEqual($ttl, $file->getTTL()); } + public function testFileTransformDelete() { + // We want to test that a file deletes all its inbound transformation + // records and outbound transformed derivatives when it is deleted. + + // First, we create a chain of transforms, A -> B -> C. + + $engine = new PhabricatorTestStorageEngine(); + + $params = array( + 'name' => 'test.txt', + 'storageEngines' => array( + $engine, + ), + ); + + $a = PhabricatorFile::newFromFileData('a', $params); + $b = PhabricatorFile::newFromFileData('b', $params); + $c = PhabricatorFile::newFromFileData('c', $params); + + id(new PhabricatorTransformedFile()) + ->setOriginalPHID($a->getPHID()) + ->setTransform('test:a->b') + ->setTransformedPHID($b->getPHID()) + ->save(); + + id(new PhabricatorTransformedFile()) + ->setOriginalPHID($b->getPHID()) + ->setTransform('test:b->c') + ->setTransformedPHID($c->getPHID()) + ->save(); + + // Now, verify that A -> B and B -> C exist. + + $xform_a = id(new PhabricatorFileQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withTransforms( + array( + array( + 'originalPHID' => $a->getPHID(), + 'transform' => true, + ), + )) + ->execute(); + + $this->assertEqual(1, count($xform_a)); + $this->assertEqual($b->getPHID(), head($xform_a)->getPHID()); + + $xform_b = id(new PhabricatorFileQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withTransforms( + array( + array( + 'originalPHID' => $b->getPHID(), + 'transform' => true, + ), + )) + ->execute(); + + $this->assertEqual(1, count($xform_b)); + $this->assertEqual($c->getPHID(), head($xform_b)->getPHID()); + + // Delete "B". + + $b->delete(); + + // Now, verify that the A -> B and B -> C links are gone. + + $xform_a = id(new PhabricatorFileQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withTransforms( + array( + array( + 'originalPHID' => $a->getPHID(), + 'transform' => true, + ), + )) + ->execute(); + + $this->assertEqual(0, count($xform_a)); + + $xform_b = id(new PhabricatorFileQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withTransforms( + array( + array( + 'originalPHID' => $b->getPHID(), + 'transform' => true, + ), + )) + ->execute(); + + $this->assertEqual(0, count($xform_b)); + + // Also verify that C has been deleted. + + $alternate_c = id(new PhabricatorFileQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs(array($c->getPHID())) + ->execute(); + + $this->assertEqual(array(), $alternate_c); + } + }