Give Pholio Images an authorPHID and use ExtendedPolicies to implement policy behavior

Summary:
Depends on D19912. Ref T11351. Images currently use `getMock()->getPolicy()` stuff to define policies. This causes bugs with object policies like "Subscribers", since the policy engine tries to evaluate the subscribers //for the image// when the intent is to evaluate the subscribers for the mock.

Move this to ExtendedPolicies to fix the behavior, and give Images sensible policy behavior when they aren't attached to a mock (specifically: only the user who created the image can see it).

Test Plan: Applied migrations, created and edited mocks and images without anything blowing up. Set mock visibility to "Subscribers", everything worked great.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T11351

Differential Revision: https://secure.phabricator.com/D19913

resources/sql/autopatches/20181218.pholio.01.imageauthor.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_pholio.pholio_image
+  ADD authorPHID VARBINARY(64) NOT NULL;

src/__phutil_library_map__.php

@@ -10947,6 +10947,7 @@ phutil_register_library_map(array(
     'PholioImage' => array(
       'PholioDAO',
       'PhabricatorPolicyInterface',
+      'PhabricatorExtendedPolicyInterface',
     ),
     'PholioImageDescriptionTransaction' => 'PholioImageTransactionType',
     'PholioImageFileTransaction' => 'PholioImageTransactionType',

src/applications/pholio/controller/PholioImageUploadController.php

@@ -23,6 +23,7 @@ final class PholioImageUploadController extends PholioController {
     }
 
     $image = PholioImage::initializeNewImage()
+      ->setAuthorPHID($viewer->getPHID())
       ->attachFile($file)
       ->setName($title)
       ->setDescription($description)

src/applications/pholio/controller/PholioMockEditController.php

@@ -141,6 +141,7 @@ final class PholioMockEditController extends PholioController {
 
         if ($replaces_image_phid) {
           $replace_image = PholioImage::initializeNewImage()
+            ->setAuthorPHID($viewer->getPHID())
             ->setReplacesImagePHID($replaces_image_phid)
             ->setFilePhid($file_phid)
             ->attachFile($file)
@@ -154,6 +155,7 @@ final class PholioMockEditController extends PholioController {
           $posted_mock_images[] = $replace_image;
         } else if (!$existing_image) { // this is an add
           $add_image = PholioImage::initializeNewImage()
+            ->setAuthorPHID($viewer->getPHID())
             ->setFilePhid($file_phid)
             ->attachFile($file)
             ->setName(strlen($title) ? $title : $file->getName())

src/applications/pholio/controller/PholioMockViewController.php

@@ -82,7 +82,7 @@ final class PholioMockViewController extends PholioController {
     $add_comment = $this->buildAddCommentView($mock, $comment_form_id);
 
     $crumbs = $this->buildApplicationCrumbs();
-    $crumbs->addTextCrumb('M'.$mock->getID(), '/M'.$mock->getID());
+    $crumbs->addTextCrumb($mock->getMonogram(), $mock->getURI());
     $crumbs->setBorder(true);
 
     $thumb_grid = id(new PholioMockThumbGridView())
@@ -92,16 +92,17 @@ final class PholioMockViewController extends PholioController {
     $view = id(new PHUITwoColumnView())
       ->setHeader($header)
       ->setCurtain($curtain)
-      ->setMainColumn(array(
+      ->setMainColumn(
-        $output,
+        array(
-        $thumb_grid,
+          $output,
-        $details,
+          $thumb_grid,
-        $timeline,
+          $details,
-        $add_comment,
+          $timeline,
-      ));
+          $add_comment,
+        ));
 
     return $this->newPage()
-      ->setTitle('M'.$mock->getID().' '.$title)
+      ->setTitle(pht('%s %s', $mock->getMonogram(), $title))
       ->setCrumbs($crumbs)
       ->setPageObjectPHIDs(array($mock->getPHID()))
       ->addQuicksandConfig(

src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php

@@ -42,6 +42,7 @@ final class PhabricatorPholioMockTestDataGenerator
     $images = array();
     foreach ($files as $file) {
       $image = PholioImage::initializeNewImage()
+        ->setAuthorPHID($author_phid)
         ->setFilePHID($file->getPHID())
         ->setSequence($sequence++)
         ->attachMock($mock);

src/applications/pholio/storage/PholioImage.php

@@ -2,8 +2,10 @@
 
 final class PholioImage extends PholioDAO
   implements
-    PhabricatorPolicyInterface {
+    PhabricatorPolicyInterface,
+    PhabricatorExtendedPolicyInterface {
 
+  protected $authorPHID;
   protected $mockID;
   protected $filePHID;
   protected $name;
@@ -57,8 +59,7 @@ final class PholioImage extends PholioDAO
   }
 
   public function getFile() {
-    $this->assertAttached($this->file);
+    return $this->assertAttached($this->file);
-    return $this->file;
   }
 
   public function attachMock(PholioMock $mock) {
@@ -67,8 +68,7 @@ final class PholioImage extends PholioDAO
   }
 
   public function getMock() {
-    $this->assertAttached($this->mock);
+    return $this->assertAttached($this->mock);
-    return $this->mock;
   }
 
   public function attachInlineComments(array $inline_comments) {
@@ -83,20 +83,46 @@ final class PholioImage extends PholioDAO
   }
 
 
-/* -(  PhabricatorPolicyInterface Implementation  )-------------------------- */
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
 
   public function getCapabilities() {
-    return $this->getMock()->getCapabilities();
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+    );
   }
 
   public function getPolicy($capability) {
-    return $this->getMock()->getPolicy($capability);
+    // If the image is attached to a mock, we use an extended policy to match
+    // the mock's permissions.
+    if ($this->getMockID()) {
+      return PhabricatorPolicies::getMostOpenPolicy();
+    }
+
+    // If the image is not attached to a mock, only the author can see it.
+    return $this->getAuthorPHID();
   }
 
-  // really the *mock* controls who can see an image
   public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
-    return $this->getMock()->hasAutomaticCapability($capability, $viewer);
+    return false;
+  }
+
+
+/* -(  PhabricatorExtendedPolicyInterface  )--------------------------------- */
+
+
+  public function getExtendedPolicy($capability, PhabricatorUser $viewer) {
+    if ($this->getMockID()) {
+      return array(
+        array(
+          $this->getMock(),
+          $capability,
+        ),
+      );
+    }
+
+    return array();
   }
 
 }