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 @@
+<?php
+
+final class PhabricatorFileAttachmentQuery
+ extends PhabricatorCursorPagedPolicyAwareQuery {
+
+ private $objectPHIDs;
+ private $needFiles;
+
+ public function withObjectPHIDs(array $object_phids) {
+ $this->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 @@
<?php
final class PhabricatorFileAttachment
- extends PhabricatorFileDAO {
+ extends PhabricatorFileDAO
+ implements
+ PhabricatorPolicyInterface,
+ PhabricatorExtendedPolicyInterface {
protected $objectPHID;
protected $filePHID;
protected $attacherPHID;
protected $attachmentMode;
+ private $object = self::ATTACHABLE;
+ private $file = self::ATTACHABLE;
+
const MODE_ATTACH = 'attach';
const MODE_REFERENCE = 'reference';
const MODE_DETACH = 'detach';
@@ -40,4 +46,53 @@
);
}
+ public function attachObject($object) {
+ $this->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,