Skip to content

Commit 5033ef6

Browse files
author
epriestley
committedMay 23, 2022
Give "FileAttachment" policy support and a query object
Summary: Ref T13682. This supports an "Attached Files" curtain UI element. Test Plan: See next change. Maniphest Tasks: T13682 Differential Revision: https://secure.phabricator.com/D21835
1 parent 631c36a commit 5033ef6

7 files changed

+248
-31
lines changed
 

‎src/__phutil_library_map__.php

+7-1
Original file line numberDiff line numberDiff line change
@@ -3453,6 +3453,7 @@
34533453
'PhabricatorFileAES256StorageFormat' => 'applications/files/format/PhabricatorFileAES256StorageFormat.php',
34543454
'PhabricatorFileAltTextTransaction' => 'applications/files/xaction/PhabricatorFileAltTextTransaction.php',
34553455
'PhabricatorFileAttachment' => 'applications/files/storage/PhabricatorFileAttachment.php',
3456+
'PhabricatorFileAttachmentQuery' => 'applications/files/query/PhabricatorFileAttachmentQuery.php',
34563457
'PhabricatorFileBundleLoader' => 'applications/files/query/PhabricatorFileBundleLoader.php',
34573458
'PhabricatorFileChunk' => 'applications/files/storage/PhabricatorFileChunk.php',
34583459
'PhabricatorFileChunkIterator' => 'applications/files/engine/PhabricatorFileChunkIterator.php',
@@ -9892,7 +9893,12 @@
98929893
),
98939894
'PhabricatorFileAES256StorageFormat' => 'PhabricatorFileStorageFormat',
98949895
'PhabricatorFileAltTextTransaction' => 'PhabricatorFileTransactionType',
9895-
'PhabricatorFileAttachment' => 'PhabricatorFileDAO',
9896+
'PhabricatorFileAttachment' => array(
9897+
'PhabricatorFileDAO',
9898+
'PhabricatorPolicyInterface',
9899+
'PhabricatorExtendedPolicyInterface',
9900+
),
9901+
'PhabricatorFileAttachmentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
98969902
'PhabricatorFileBundleLoader' => 'Phobject',
98979903
'PhabricatorFileChunk' => array(
98989904
'PhabricatorFileDAO',

‎src/applications/files/query/PhabricatorFileAttachmentQuery.php

+110
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
<?php
2+
3+
final class PhabricatorFileAttachmentQuery
4+
extends PhabricatorCursorPagedPolicyAwareQuery {
5+
6+
private $objectPHIDs;
7+
private $needFiles;
8+
9+
public function withObjectPHIDs(array $object_phids) {
10+
$this->objectPHIDs = $object_phids;
11+
return $this;
12+
}
13+
14+
public function needFiles($need) {
15+
$this->needFiles = $need;
16+
return $this;
17+
}
18+
19+
public function newResultObject() {
20+
return new PhabricatorFileAttachment();
21+
}
22+
23+
protected function loadPage() {
24+
return $this->loadStandardPage($this->newResultObject());
25+
}
26+
27+
protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
28+
$where = parent::buildWhereClauseParts($conn);
29+
30+
if ($this->objectPHIDs !== null) {
31+
$where[] = qsprintf(
32+
$conn,
33+
'attachments.objectPHID IN (%Ls)',
34+
$this->objectPHIDs);
35+
}
36+
37+
return $where;
38+
}
39+
40+
protected function willFilterPage(array $attachments) {
41+
$viewer = $this->getViewer();
42+
$object_phids = array();
43+
44+
foreach ($attachments as $attachment) {
45+
$object_phid = $attachment->getObjectPHID();
46+
$object_phids[$object_phid] = $object_phid;
47+
}
48+
49+
if ($object_phids) {
50+
$objects = id(new PhabricatorObjectQuery())
51+
->setViewer($viewer)
52+
->setParentQuery($this)
53+
->withPHIDs($object_phids)
54+
->execute();
55+
$objects = mpull($objects, null, 'getPHID');
56+
} else {
57+
$objects = array();
58+
}
59+
60+
foreach ($attachments as $key => $attachment) {
61+
$object_phid = $attachment->getObjectPHID();
62+
$object = idx($objects, $object_phid);
63+
64+
if (!$object) {
65+
$this->didRejectResult($attachment);
66+
unset($attachments[$key]);
67+
continue;
68+
}
69+
70+
$attachment->attachObject($object);
71+
}
72+
73+
if ($this->needFiles) {
74+
$file_phids = array();
75+
foreach ($attachments as $attachment) {
76+
$file_phid = $attachment->getFilePHID();
77+
$file_phids[$file_phid] = $file_phid;
78+
}
79+
80+
if ($file_phids) {
81+
$files = id(new PhabricatorFileQuery())
82+
->setViewer($viewer)
83+
->setParentQuery($this)
84+
->withPHIDs($file_phids)
85+
->execute();
86+
$files = mpull($files, null, 'getPHID');
87+
} else {
88+
$files = array();
89+
}
90+
91+
foreach ($attachments as $key => $attachment) {
92+
$file_phid = $attachment->getFilePHID();
93+
$file = idx($files, $file_phid);
94+
95+
$attachment->attachFile($file);
96+
}
97+
}
98+
99+
return $attachments;
100+
}
101+
102+
protected function getPrimaryTableAlias() {
103+
return 'attachments';
104+
}
105+
106+
public function getQueryApplicationClass() {
107+
return 'PhabricatorFilesApplication';
108+
}
109+
110+
}

‎src/applications/files/query/PhabricatorFileQuery.php

+12-4
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,13 @@ private function newAttachmentsMap(array $files) {
309309

310310
$attachments = queryfx_all(
311311
$attachments_conn,
312-
'SELECT filePHID, objectPHID FROM %R WHERE filePHID IN (%Ls)',
312+
'SELECT filePHID, objectPHID FROM %R WHERE filePHID IN (%Ls)
313+
AND attachmentMode IN (%Ls)',
313314
$attachments_table,
314-
$file_phids);
315+
$file_phids,
316+
array(
317+
PhabricatorFileAttachment::MODE_ATTACH,
318+
));
315319

316320
$attachments_map = array_fill_keys($file_phids, array());
317321
foreach ($attachments as $row) {
@@ -374,8 +378,12 @@ protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
374378
if ($this->shouldJoinAttachmentsTable()) {
375379
$joins[] = qsprintf(
376380
$conn,
377-
'JOIN %R attachments ON attachments.filePHID = f.phid',
378-
new PhabricatorFileAttachment());
381+
'JOIN %R attachments ON attachments.filePHID = f.phid
382+
AND attachmentMode IN (%Ls)',
383+
new PhabricatorFileAttachment(),
384+
array(
385+
PhabricatorFileAttachment::MODE_ATTACH,
386+
));
379387
}
380388

381389
return $joins;

‎src/applications/files/storage/PhabricatorFileAttachment.php

+56-1
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
<?php
22

33
final class PhabricatorFileAttachment
4-
extends PhabricatorFileDAO {
4+
extends PhabricatorFileDAO
5+
implements
6+
PhabricatorPolicyInterface,
7+
PhabricatorExtendedPolicyInterface {
58

69
protected $objectPHID;
710
protected $filePHID;
811
protected $attacherPHID;
912
protected $attachmentMode;
1013

14+
private $object = self::ATTACHABLE;
15+
private $file = self::ATTACHABLE;
16+
1117
const MODE_ATTACH = 'attach';
1218
const MODE_REFERENCE = 'reference';
1319
const MODE_DETACH = 'detach';
@@ -40,4 +46,53 @@ public static function getModeList() {
4046
);
4147
}
4248

49+
public function attachObject($object) {
50+
$this->object = $object;
51+
return $this;
52+
}
53+
54+
public function getObject() {
55+
return $this->assertAttached($this->object);
56+
}
57+
58+
public function attachFile(PhabricatorFile $file = null) {
59+
$this->file = $file;
60+
return $this;
61+
}
62+
63+
public function getFile() {
64+
return $this->assertAttached($this->file);
65+
}
66+
67+
68+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
69+
70+
71+
public function getCapabilities() {
72+
return array(
73+
PhabricatorPolicyCapability::CAN_VIEW,
74+
);
75+
}
76+
77+
public function getPolicy($capability) {
78+
switch ($capability) {
79+
case PhabricatorPolicyCapability::CAN_VIEW:
80+
return PhabricatorPolicies::getMostOpenPolicy();
81+
}
82+
}
83+
84+
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
85+
return false;
86+
}
87+
88+
89+
/* -( PhabricatorExtendedPolicyInterface )--------------------------------- */
90+
91+
92+
public function getExtendedPolicy($capability, PhabricatorUser $viewer) {
93+
return array(
94+
array($this->getObject(), $capability),
95+
);
96+
}
97+
4398
}

‎src/applications/files/storage/__tests__/PhabricatorFileTestCase.php

+56-5
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,14 @@ public function testFileIndirectScramble() {
100100
$xactions[] = id(new ManiphestTransaction())
101101
->setTransactionType(
102102
ManiphestTaskDescriptionTransaction::TRANSACTIONTYPE)
103-
->setNewValue('{'.$file->getMonogram().'}');
103+
->setNewValue('{'.$file->getMonogram().'}')
104+
->setMetadataValue(
105+
'remarkup.control',
106+
array(
107+
'attachedFilePHIDs' => array(
108+
$file->getPHID(),
109+
),
110+
));
104111

105112
id(new ManiphestTransactionEditor())
106113
->setActor($author)
@@ -167,9 +174,10 @@ public function testFileVisibility() {
167174

168175
// Create an object and test object policies.
169176

170-
$object = ManiphestTask::initializeNewTask($author);
171-
$object->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy());
172-
$object->save();
177+
$object = ManiphestTask::initializeNewTask($author)
178+
->setTitle(pht('File Visibility Test Task'))
179+
->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy())
180+
->save();
173181

174182
$this->assertTrue(
175183
$filter->hasCapability(
@@ -185,10 +193,53 @@ public function testFileVisibility() {
185193
PhabricatorPolicyCapability::CAN_VIEW),
186194
pht('Object Visible to Others'));
187195

196+
// Reference the file in a comment. This should not affect the file
197+
// policy.
198+
199+
$file_ref = '{F'.$file->getID().'}';
200+
201+
$xactions = array();
202+
$xactions[] = id(new ManiphestTransaction())
203+
->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
204+
->attachComment(
205+
id(new ManiphestTransactionComment())
206+
->setContent($file_ref));
207+
208+
id(new ManiphestTransactionEditor())
209+
->setActor($author)
210+
->setContentSource($this->newContentSource())
211+
->applyTransactions($object, $xactions);
212+
213+
// Test the referenced file's visibility.
214+
$this->assertEqual(
215+
array(
216+
true,
217+
false,
218+
),
219+
$this->canViewFile($users, $file),
220+
pht('Referenced File Visibility'));
221+
188222
// Attach the file to the object and test that the association opens a
189223
// policy exception for the non-author viewer.
190224

191-
$file->attachToObject($object->getPHID());
225+
$xactions = array();
226+
$xactions[] = id(new ManiphestTransaction())
227+
->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
228+
->setMetadataValue(
229+
'remarkup.control',
230+
array(
231+
'attachedFilePHIDs' => array(
232+
$file->getPHID(),
233+
),
234+
))
235+
->attachComment(
236+
id(new ManiphestTransactionComment())
237+
->setContent($file_ref));
238+
239+
id(new ManiphestTransactionEditor())
240+
->setActor($author)
241+
->setContentSource($this->newContentSource())
242+
->applyTransactions($object, $xactions);
192243

193244
// Test the attached file's visibility.
194245
$this->assertEqual(

‎src/applications/transactions/editengine/PhabricatorEditEngine.php

-20
Original file line numberDiff line numberDiff line change
@@ -2080,26 +2080,6 @@ private function buildCommentResponse($object) {
20802080
}
20812081
}
20822082

2083-
public static function newTransactionsFromRemarkupMetadata(
2084-
PhabricatorApplicationTransaction $template,
2085-
array $metadata) {
2086-
2087-
$xactions = array();
2088-
2089-
$attached_phids = idx($metadata, 'attachedFilePHIDs');
2090-
if (is_array($attached_phids) && $attached_phids) {
2091-
$attachment_map = array_fill_keys(
2092-
$attached_phids,
2093-
PhabricatorFileAttachment::MODE_ATTACH);
2094-
2095-
$xactions[] = id(clone $template)
2096-
->setTransactionType(PhabricatorTransactions::TYPE_FILE)
2097-
->setNewValue($attachment_map);
2098-
}
2099-
2100-
return $xactions;
2101-
}
2102-
21032083
protected function newDraftEngine($object) {
21042084
$viewer = $this->getViewer();
21052085

‎src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

+7
Original file line numberDiff line numberDiff line change
@@ -2266,7 +2266,14 @@ private function newFileTransaction(
22662266
$viewer = $this->getActor();
22672267

22682268
$old_blocks = mpull($remarkup_changes, 'getOldValue');
2269+
foreach ($old_blocks as $key => $old_block) {
2270+
$old_blocks[$key] = phutil_string_cast($old_block);
2271+
}
2272+
22692273
$new_blocks = mpull($remarkup_changes, 'getNewValue');
2274+
foreach ($new_blocks as $key => $new_block) {
2275+
$new_blocks[$key] = phutil_string_cast($new_block);
2276+
}
22702277

22712278
$old_refs = PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles(
22722279
$viewer,

0 commit comments

Comments
 (0)
Failed to load comments.