diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3453,6 +3453,7 @@ 'PhabricatorFileAES256StorageFormat' => 'applications/files/format/PhabricatorFileAES256StorageFormat.php', 'PhabricatorFileAltTextTransaction' => 'applications/files/xaction/PhabricatorFileAltTextTransaction.php', 'PhabricatorFileAttachment' => 'applications/files/storage/PhabricatorFileAttachment.php', + 'PhabricatorFileAttachmentQuery' => 'applications/files/query/PhabricatorFileAttachmentQuery.php', 'PhabricatorFileBundleLoader' => 'applications/files/query/PhabricatorFileBundleLoader.php', 'PhabricatorFileChunk' => 'applications/files/storage/PhabricatorFileChunk.php', 'PhabricatorFileChunkIterator' => 'applications/files/engine/PhabricatorFileChunkIterator.php', @@ -9892,7 +9893,12 @@ ), 'PhabricatorFileAES256StorageFormat' => 'PhabricatorFileStorageFormat', 'PhabricatorFileAltTextTransaction' => 'PhabricatorFileTransactionType', - 'PhabricatorFileAttachment' => 'PhabricatorFileDAO', + 'PhabricatorFileAttachment' => array( + 'PhabricatorFileDAO', + 'PhabricatorPolicyInterface', + 'PhabricatorExtendedPolicyInterface', + ), + 'PhabricatorFileAttachmentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorFileBundleLoader' => 'Phobject', 'PhabricatorFileChunk' => array( 'PhabricatorFileDAO', diff --git a/src/applications/files/query/PhabricatorFileAttachmentQuery.php b/src/applications/files/query/PhabricatorFileAttachmentQuery.php new file mode 100644 --- /dev/null +++ b/src/applications/files/query/PhabricatorFileAttachmentQuery.php @@ -0,0 +1,110 @@ +objectPHIDs = $object_phids; + return $this; + } + + public function needFiles($need) { + $this->needFiles = $need; + return $this; + } + + public function newResultObject() { + return new PhabricatorFileAttachment(); + } + + protected function loadPage() { + return $this->loadStandardPage($this->newResultObject()); + } + + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); + + if ($this->objectPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'attachments.objectPHID IN (%Ls)', + $this->objectPHIDs); + } + + return $where; + } + + protected function willFilterPage(array $attachments) { + $viewer = $this->getViewer(); + $object_phids = array(); + + foreach ($attachments as $attachment) { + $object_phid = $attachment->getObjectPHID(); + $object_phids[$object_phid] = $object_phid; + } + + if ($object_phids) { + $objects = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->setParentQuery($this) + ->withPHIDs($object_phids) + ->execute(); + $objects = mpull($objects, null, 'getPHID'); + } else { + $objects = array(); + } + + foreach ($attachments as $key => $attachment) { + $object_phid = $attachment->getObjectPHID(); + $object = idx($objects, $object_phid); + + if (!$object) { + $this->didRejectResult($attachment); + unset($attachments[$key]); + continue; + } + + $attachment->attachObject($object); + } + + if ($this->needFiles) { + $file_phids = array(); + foreach ($attachments as $attachment) { + $file_phid = $attachment->getFilePHID(); + $file_phids[$file_phid] = $file_phid; + } + + if ($file_phids) { + $files = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->setParentQuery($this) + ->withPHIDs($file_phids) + ->execute(); + $files = mpull($files, null, 'getPHID'); + } else { + $files = array(); + } + + foreach ($attachments as $key => $attachment) { + $file_phid = $attachment->getFilePHID(); + $file = idx($files, $file_phid); + + $attachment->attachFile($file); + } + } + + return $attachments; + } + + protected function getPrimaryTableAlias() { + return 'attachments'; + } + + public function getQueryApplicationClass() { + return 'PhabricatorFilesApplication'; + } + +} diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -309,9 +309,13 @@ $attachments = queryfx_all( $attachments_conn, - 'SELECT filePHID, objectPHID FROM %R WHERE filePHID IN (%Ls)', + 'SELECT filePHID, objectPHID FROM %R WHERE filePHID IN (%Ls) + AND attachmentMode IN (%Ls)', $attachments_table, - $file_phids); + $file_phids, + array( + PhabricatorFileAttachment::MODE_ATTACH, + )); $attachments_map = array_fill_keys($file_phids, array()); foreach ($attachments as $row) { @@ -374,8 +378,12 @@ if ($this->shouldJoinAttachmentsTable()) { $joins[] = qsprintf( $conn, - 'JOIN %R attachments ON attachments.filePHID = f.phid', - new PhabricatorFileAttachment()); + 'JOIN %R attachments ON attachments.filePHID = f.phid + AND attachmentMode IN (%Ls)', + new PhabricatorFileAttachment(), + array( + PhabricatorFileAttachment::MODE_ATTACH, + )); } return $joins; diff --git a/src/applications/files/storage/PhabricatorFileAttachment.php b/src/applications/files/storage/PhabricatorFileAttachment.php --- a/src/applications/files/storage/PhabricatorFileAttachment.php +++ b/src/applications/files/storage/PhabricatorFileAttachment.php @@ -1,13 +1,19 @@ object = $object; + return $this; + } + + public function getObject() { + return $this->assertAttached($this->object); + } + + public function attachFile(PhabricatorFile $file = null) { + $this->file = $file; + return $this; + } + + public function getFile() { + return $this->assertAttached($this->file); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + return PhabricatorPolicies::getMostOpenPolicy(); + } + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + + +/* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ + + + public function getExtendedPolicy($capability, PhabricatorUser $viewer) { + return array( + array($this->getObject(), $capability), + ); + } + } diff --git a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php --- a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php +++ b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php @@ -100,7 +100,14 @@ $xactions[] = id(new ManiphestTransaction()) ->setTransactionType( ManiphestTaskDescriptionTransaction::TRANSACTIONTYPE) - ->setNewValue('{'.$file->getMonogram().'}'); + ->setNewValue('{'.$file->getMonogram().'}') + ->setMetadataValue( + 'remarkup.control', + array( + 'attachedFilePHIDs' => array( + $file->getPHID(), + ), + )); id(new ManiphestTransactionEditor()) ->setActor($author) @@ -167,9 +174,10 @@ // Create an object and test object policies. - $object = ManiphestTask::initializeNewTask($author); - $object->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy()); - $object->save(); + $object = ManiphestTask::initializeNewTask($author) + ->setTitle(pht('File Visibility Test Task')) + ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy()) + ->save(); $this->assertTrue( $filter->hasCapability( @@ -185,10 +193,53 @@ PhabricatorPolicyCapability::CAN_VIEW), pht('Object Visible to Others')); + // Reference the file in a comment. This should not affect the file + // policy. + + $file_ref = '{F'.$file->getID().'}'; + + $xactions = array(); + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new ManiphestTransactionComment()) + ->setContent($file_ref)); + + id(new ManiphestTransactionEditor()) + ->setActor($author) + ->setContentSource($this->newContentSource()) + ->applyTransactions($object, $xactions); + + // Test the referenced file's visibility. + $this->assertEqual( + array( + true, + false, + ), + $this->canViewFile($users, $file), + pht('Referenced File Visibility')); + // Attach the file to the object and test that the association opens a // policy exception for the non-author viewer. - $file->attachToObject($object->getPHID()); + $xactions = array(); + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->setMetadataValue( + 'remarkup.control', + array( + 'attachedFilePHIDs' => array( + $file->getPHID(), + ), + )) + ->attachComment( + id(new ManiphestTransactionComment()) + ->setContent($file_ref)); + + id(new ManiphestTransactionEditor()) + ->setActor($author) + ->setContentSource($this->newContentSource()) + ->applyTransactions($object, $xactions); // Test the attached file's visibility. $this->assertEqual( diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -2080,26 +2080,6 @@ } } - public static function newTransactionsFromRemarkupMetadata( - PhabricatorApplicationTransaction $template, - array $metadata) { - - $xactions = array(); - - $attached_phids = idx($metadata, 'attachedFilePHIDs'); - if (is_array($attached_phids) && $attached_phids) { - $attachment_map = array_fill_keys( - $attached_phids, - PhabricatorFileAttachment::MODE_ATTACH); - - $xactions[] = id(clone $template) - ->setTransactionType(PhabricatorTransactions::TYPE_FILE) - ->setNewValue($attachment_map); - } - - return $xactions; - } - protected function newDraftEngine($object) { $viewer = $this->getViewer(); diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -2266,7 +2266,14 @@ $viewer = $this->getActor(); $old_blocks = mpull($remarkup_changes, 'getOldValue'); + foreach ($old_blocks as $key => $old_block) { + $old_blocks[$key] = phutil_string_cast($old_block); + } + $new_blocks = mpull($remarkup_changes, 'getNewValue'); + foreach ($new_blocks as $key => $new_block) { + $new_blocks[$key] = phutil_string_cast($new_block); + } $old_refs = PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles( $viewer,