Make PhabricatorPolicyInterface require a getPHID() method

Summary:
Ref T603. This cleans up an existing callsite in the policy filter, and opens up some stuff in the future.

Some policy objects don't have real PHIDs:

  PhabricatorTokenGiven
  PhabricatorSavedQuery
  PhabricatorNamedQuery
  PhrequentUserTime
  PhabricatorFlag
  PhabricatorDaemonLog
  PhabricatorConduitMethodCallLog
  ConduitAPIMethod
  PhabricatorChatLogEvent
  PhabricatorChatLogChannel

Although it would be reasonable to add real PHIDs to some of these (like `ChatLogChannel`), it probably doesn't make much sense for others (`DaemonLog`, `MethodCallLog`). Just let them return `null`.

Also remove some duplicate `$id` and `$phid` properties. These are declared on `PhabricatorLiskDAO` and do not need to be redeclared.

Test Plan: Ran the `testEverythingImplemented` unit test, which verifies that all classes conform to the interface.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

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

src/applications/conduit/method/ConduitAPIMethod.php

@@ -25,7 +25,7 @@ abstract class ConduitAPIMethod
 
   /**
    * This is mostly for compatibility with
-   * @{class:AphrontCursorPagedPolicyAwareQuery}.
+   * @{class:PhabricatorCursorPagedPolicyAwareQuery}.
    */
   public function getID() {
     return $this->getAPIMethodName();
@@ -166,6 +166,10 @@ abstract class ConduitAPIMethod
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
 
+  public function getPHID() {
+    return null;
+  }
+
   public function getCapabilities() {
     return array(
       PhabricatorPolicyCapability::CAN_VIEW,

src/applications/config/storage/PhabricatorConfigEntry.php

@@ -3,8 +3,6 @@
 final class PhabricatorConfigEntry extends PhabricatorConfigEntryDAO
   implements PhabricatorPolicyInterface {
 
-  protected $id;
-  protected $phid;
   protected $namespace;
   protected $configKey;
   protected $value;

src/applications/conpherence/storage/ConpherenceThread.php

@@ -6,8 +6,6 @@
 final class ConpherenceThread extends ConpherenceDAO
   implements PhabricatorPolicyInterface {
 
-  protected $id;
-  protected $phid;
   protected $title;
   protected $messageCount;
   protected $recentParticipantPHIDs = array();

src/applications/countdown/storage/PhabricatorCountdown.php

@@ -7,8 +7,6 @@ final class PhabricatorCountdown
   extends PhabricatorCountdownDAO
   implements PhabricatorPolicyInterface {
 
-  protected $id;
-  protected $phid;
   protected $title;
   protected $authorPHID;
   protected $epoch;

src/applications/daemon/storage/PhabricatorDaemonLog.php

@@ -26,6 +26,9 @@ final class PhabricatorDaemonLog extends PhabricatorDaemonDAO
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
+  public function getPHID() {
+    return null;
+  }
 
   public function getCapabilities() {
     return array(

src/applications/differential/storage/DifferentialRevision.php

@@ -13,7 +13,6 @@ final class DifferentialRevision extends DifferentialDAO
   protected $summary = '';
   protected $testPlan = '';
 
-  protected $phid;
   protected $authorPHID;
   protected $lastReviewerPHID;
 

src/applications/diviner/storage/DivinerLiveBook.php

@@ -3,7 +3,6 @@
 final class DivinerLiveBook extends DivinerDAO
   implements PhabricatorPolicyInterface {
 
-  protected $phid;
   protected $name;
   protected $viewPolicy;
   protected $configurationData = array();

src/applications/diviner/storage/DivinerLiveSymbol.php

@@ -3,7 +3,6 @@
 final class DivinerLiveSymbol extends DivinerDAO
   implements PhabricatorPolicyInterface, PhabricatorMarkupInterface {
 
-  protected $phid;
   protected $bookPHID;
   protected $context;
   protected $type;

src/applications/feed/story/PhabricatorFeedStory.php

@@ -283,6 +283,9 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface {
 
 /* -(  PhabricatorPolicyInterface Implementation  )-------------------------- */
 
+  public function getPHID() {
+    return null;
+  }
 
   /**
    * @task policy

src/applications/files/storage/PhabricatorFile.php

@@ -14,7 +14,6 @@ final class PhabricatorFile extends PhabricatorFileDAO
   const METADATA_IMAGE_WIDTH  = 'width';
   const METADATA_IMAGE_HEIGHT = 'height';
 
-  protected $phid;
   protected $name;
   protected $mimeType;
   protected $byteSize;

src/applications/herald/storage/transcript/HeraldTranscript.php

@@ -3,9 +3,6 @@
 final class HeraldTranscript extends HeraldDAO
   implements PhabricatorPolicyInterface {
 
-  protected $id;
-  protected $phid;
-
   protected $objectTranscript;
   protected $ruleTranscripts = array();
   protected $conditionTranscripts = array();

src/applications/legalpad/storage/LegalpadDocument.php

@@ -9,7 +9,6 @@ final class LegalpadDocument extends LegalpadDAO
     PhabricatorSubscribableInterface,
     PhabricatorApplicationTransactionInterface {
 
-  protected $phid;
   protected $title;
   protected $contributorCount;
   protected $recentContributorPHIDs = array();

src/applications/macro/storage/PhabricatorFileImageMacro.php

@@ -8,7 +8,6 @@ final class PhabricatorFileImageMacro extends PhabricatorFileDAO
 
   protected $authorPHID;
   protected $filePHID;
-  protected $phid;
   protected $name;
   protected $isDisabled = 0;
   protected $audioPHID;

src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php

@@ -4,7 +4,6 @@ final class PhabricatorMetaMTAMailingList extends PhabricatorMetaMTADAO
   implements PhabricatorPolicyInterface {
 
   protected $name;
-  protected $phid;
   protected $email;
   protected $uri;
 

src/applications/maniphest/storage/ManiphestTask.php

@@ -10,7 +10,6 @@ final class ManiphestTask extends ManiphestDAO
 
   const MARKUP_FIELD_DESCRIPTION = 'markup:desc';
 
-  protected $phid;
   protected $authorPHID;
   protected $ownerPHID;
   protected $ccPHIDs = array();

src/applications/owners/storage/PhabricatorOwnersPackage.php

@@ -3,7 +3,6 @@
 final class PhabricatorOwnersPackage extends PhabricatorOwnersDAO
   implements PhabricatorPolicyInterface {
 
-  protected $phid;
   protected $name;
   protected $originalName;
   protected $auditingEnabled;

src/applications/paste/storage/PhabricatorPaste.php

@@ -9,7 +9,6 @@ final class PhabricatorPaste extends PhabricatorPasteDAO
     PhabricatorTokenReceiverInterface,
     PhabricatorPolicyInterface {
 
-  protected $phid;
   protected $title;
   protected $authorPHID;
   protected $filePHID;

src/applications/people/storage/PhabricatorUser.php

@@ -11,7 +11,6 @@ final class PhabricatorUser
   const NAMETOKEN_TABLE = 'user_nametoken';
   const MAXIMUM_USERNAME_LENGTH = 64;
 
-  protected $phid;
   protected $userName;
   protected $realName;
   protected $sex;

src/applications/phame/storage/PhameBlog.php

@@ -10,8 +10,6 @@ final class PhameBlog extends PhameDAO
 
   const SKIN_DEFAULT = 'oblivious';
 
-  protected $id;
-  protected $phid;
   protected $name;
   protected $description;
   protected $domain;

src/applications/phame/storage/PhamePost.php

@@ -15,8 +15,6 @@ final class PhamePost extends PhameDAO
   const VISIBILITY_DRAFT     = 0;
   const VISIBILITY_PUBLISHED = 1;
 
-  protected $id;
-  protected $phid;
   protected $bloggerPHID;
   protected $title;
   protected $phameTitle;

src/applications/phriction/storage/PhrictionDocument.php

@@ -9,8 +9,6 @@ final class PhrictionDocument extends PhrictionDAO
     PhabricatorSubscribableInterface,
     PhabricatorTokenReceiverInterface {
 
-  protected $id;
-  protected $phid;
   protected $slug;
   protected $depth;
   protected $contentID;

src/applications/policy/__tests__/PhabricatorPolicyTestObject.php

@@ -10,6 +10,10 @@ final class PhabricatorPolicyTestObject
   private $policies               = array();
   private $automaticCapabilities  = array();
 
+  public function getPHID() {
+    return null;
+  }
+
   public function getCapabilities() {
     return $this->capabilities;
   }

src/applications/policy/filter/PhabricatorPolicyFilter.php

@@ -282,19 +282,9 @@ final class PhabricatorPolicyFilter {
 
     $details = array_filter(array_merge(array($more), (array)$exceptions));
 
-    // NOTE: Not every policy object has a PHID, just pull an arbitrary
-    // "unknown object" handle if this fails. We're just using this to provide
-    // a better error message if we can.
-
-    $phid = '?';
-    if (($object instanceof PhabricatorLiskDAO) ||
-        (method_exists($object, 'getPHID'))) {
-      try {
-        $phid = $object->getPHID();
-      } catch (Exception $ignored) {
-        // Ignore.
-      }
-    }
+    // NOTE: Not every type of policy object has a real PHID; just load an
+    // empty handle if a real PHID isn't available.
+    $phid = nonempty($object->getPHID(), PhabricatorPHIDConstants::PHID_VOID);
 
     $handle = id(new PhabricatorHandleQuery())
       ->setViewer($this->viewer)

src/applications/policy/interface/PhabricatorPolicyInterface.php

@@ -2,6 +2,7 @@
 
 interface PhabricatorPolicyInterface {
 
+  public function getPHID();
   public function getCapabilities();
   public function getPolicy($capability);
   public function hasAutomaticCapability($capability, PhabricatorUser $viewer);

src/applications/project/storage/PhabricatorProject.php

@@ -4,7 +4,6 @@ final class PhabricatorProject extends PhabricatorProjectDAO
   implements PhabricatorPolicyInterface {
 
   protected $name;
-  protected $phid;
   protected $status = PhabricatorProjectStatus::STATUS_ACTIVE;
   protected $authorPHID;
   protected $subprojectPHIDs = array();

src/applications/releeph/storage/ReleephBranch.php

@@ -3,7 +3,6 @@
 final class ReleephBranch extends ReleephDAO
   implements PhabricatorPolicyInterface {
 
-  protected $phid;
   protected $releephProjectID;
   protected $isActive;
   protected $createdByUserPHID;

src/applications/releeph/storage/ReleephProject.php

@@ -10,7 +10,6 @@ final class ReleephProject extends ReleephDAO
   const COMMIT_AUTHOR_FROM_DIFF = 'commit-author-is-from-diff';
   const COMMIT_AUTHOR_REQUESTOR = 'commit-author-is-requestor';
 
-  protected $phid;
   protected $name;
 
   // Specifying the place to pick from is a requirement for svn, though not

src/applications/releeph/storage/ReleephRequest.php

@@ -5,7 +5,6 @@ final class ReleephRequest extends ReleephDAO
     PhabricatorPolicyInterface,
     PhabricatorCustomFieldInterface {
 
-  protected $phid;
   protected $branchID;
   protected $requestUserPHID;
   protected $details = array();

src/applications/repository/storage/PhabricatorRepository.php

@@ -25,7 +25,6 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
   const TABLE_BADCOMMIT = 'repository_badcommit';
   const TABLE_LINTMESSAGE = 'repository_lintmessage';
 
-  protected $phid;
   protected $name;
   protected $callsign;
   protected $uuid;

src/applications/repository/storage/PhabricatorRepositoryArcanistProject.php

@@ -8,7 +8,6 @@ final class PhabricatorRepositoryArcanistProject
   implements PhabricatorPolicyInterface {
 
   protected $name;
-  protected $phid;
   protected $repositoryID;
 
   protected $symbolIndexLanguages = array();

src/infrastructure/storage/lisk/LiskDAO.php

@@ -892,6 +892,11 @@ abstract class LiskDAO {
   }
 
 
+  public function getPHID() {
+    return $this->phid;
+  }
+
+
   /**
    * Test if a property exists.
    *