Mark questionable column nullability for later

Summary:
Ref T1191. Ref T6203. While generating expected schemata, I ran into these columns which seem to have sketchy nullability.

  - Mark most of them for later resolution (T6203). They work fine today and don't need to block T1191. Changing them can break the application, so we can't autofix them.
  - Forgive a couple of them that are sort-of reasonable or going to get wiped out.

Test Plan: Saw 94 remaining warnings.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: hach-que, epriestley

Maniphest Tasks: T1191, T6203

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

src/applications/config/schema/PhabricatorConfigStorageSchema.php

@@ -171,10 +171,10 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
       case self::ISSUE_MISSING:
       case self::ISSUE_SURPLUS:
       case self::ISSUE_SUBFAIL:
+      case self::ISSUE_NULLABLE:
         return self::STATUS_FAIL;
       case self::ISSUE_SUBWARN:
       case self::ISSUE_COLUMNTYPE:
-      case self::ISSUE_NULLABLE:
         return self::STATUS_WARN;
       case self::ISSUE_SUBNOTE:
       case self::ISSUE_CHARSET:

src/applications/differential/storage/DifferentialChangeset.php

@@ -36,6 +36,14 @@ final class DifferentialChangeset extends DifferentialDAO
         'fileType' => 'uint32',
         'addLines' => 'uint32',
         'delLines' => 'uint32',
+
+        // T6203/NULLABILITY
+        // These should all be non-nullable, and store reasonable default
+        // JSON values if empty.
+        'awayPaths' => 'text?',
+        'metadata' => 'text?',
+        'oldProperties' => 'text?',
+        'newProperties' => 'text?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'diffID' => array(

src/applications/differential/storage/DifferentialDiff.php

@@ -57,9 +57,13 @@ final class DifferentialDiff
         'branch' => 'text255?',
         'bookmark' => 'text255?',
         'arcanistProjectPHID' => 'phid?',
-        'creationMethod' => 'text255',
-        'description' => 'text255',
         'repositoryUUID' => 'text64?',
+
+        // T6203/NULLABILITY
+        // These should be non-null; all diffs should have a creation method
+        // and the description should just be empty.
+        'creationMethod' => 'text255?',
+        'description' => 'text255?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'revisionID' => array(

src/applications/differential/storage/DifferentialRevision.php

@@ -84,7 +84,7 @@ final class DifferentialRevision extends DifferentialDAO
         'lastReviewerPHID' => 'phid?',
         'lineCount' => 'uint32?',
         'mailKey' => 'bytes40',
-        'branchName' => 'text255',
+        'branchName' => 'text255?',
         'arcanistProjectPHID' => 'phid?',
         'repositoryPHID' => 'phid?',
       ),

src/applications/harbormaster/storage/HarbormasterObject.php

@@ -8,7 +8,7 @@ final class HarbormasterObject extends HarbormasterDAO {
     return array(
       self::CONFIG_AUX_PHID => true,
       self::CONFIG_COLUMN_SCHEMA => array(
-        'name' => 'text255',
+        'name' => 'text255?',
       ),
     ) + parent::getConfiguration();
   }

src/applications/harbormaster/storage/HarbormasterSchemaSpec.php

@@ -27,7 +27,11 @@ final class HarbormasterSchemaSpec extends PhabricatorConfigSchemaSpec {
         'id' => 'id',
         'logID' => 'id',
         'encoding' => 'text32',
-        'size' => 'uint32',
+
+        // T6203/NULLABILITY
+        // Both the type and nullability of this column are crazily wrong.
+        'size' => 'uint32?',
+
         'chunk' => 'bytes',
       ),
       array(

src/applications/harbormaster/storage/build/HarbormasterBuildLog.php

@@ -31,9 +31,13 @@ final class HarbormasterBuildLog extends HarbormasterDAO
     return array(
       self::CONFIG_AUX_PHID => true,
       self::CONFIG_COLUMN_SCHEMA => array(
-        'logSource' => 'text255',
+        // T6203/NULLABILITY
-        'logType' => 'text255',
+        // It seems like these should be non-nullable? All logs should have a
-        'duration' => 'uint32',
+        // source, etc.
+        'logSource' => 'text255?',
+        'logType' => 'text255?',
+        'duration' => 'uint32?',
+
         'live' => 'bool',
       ),
       self::CONFIG_KEY_SCHEMA => array(

src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php

@@ -103,10 +103,13 @@ final class HarbormasterBuildTarget extends HarbormasterDAO
       self::CONFIG_COLUMN_SCHEMA => array(
         'className' => 'text255',
         'targetStatus' => 'text64',
-        'name' => 'text255',
         'dateStarted' => 'epoch?',
         'dateCompleted' => 'epoch?',
         'buildGeneration' => 'uint32',
+
+        // T6203/NULLABILITY
+        // This should not be nullable.
+        'name' => 'text255?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_build' => array(

src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php

@@ -29,8 +29,13 @@ final class HarbormasterBuildStep extends HarbormasterDAO
       self::CONFIG_COLUMN_SCHEMA => array(
         'className' => 'text255',
         'sequence' => 'uint32',
-        'name' => 'text255',
         'description' => 'text',
+
+        // T6203/NULLABILITY
+        // This should not be nullable. Current `null` values indicate steps
+        // which predated editable names. These should be backfilled with
+        // default names, then the code for handling `null` shoudl be removed.
+        'name' => 'text255?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_plan' => array(

src/applications/herald/storage/HeraldRule.php

@@ -36,10 +36,13 @@ final class HeraldRule extends HeraldDAO
         'contentType' => 'text255',
         'mustMatchAll' => 'bool',
         'configVersion' => 'uint32',
-        'repetitionPolicy' => 'uint32',
         'ruleType' => 'text255',
         'isDisabled' => 'uint32',
         'triggerObjectPHID' => 'phid?',
+
+        // T6203/NULLABILITY
+        // This should not be nullable.
+        'repetitionPolicy' => 'uint32?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_phid' => null,

src/applications/maniphest/storage/ManiphestTask.php

@@ -78,6 +78,11 @@ final class ManiphestTask extends ManiphestDAO
         'ownerOrdering' => 'text64?',
         'originalEmailSource' => 'text255?',
         'subpriority' => 'double',
+
+        // T6203/NULLABILITY
+        // This should not be nullable. It's going away soon anyway.
+        'ccPHIDs' => 'text?',
+
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_phid' => null,

src/applications/metamta/storage/PhabricatorMetaMTAMail.php

@@ -34,8 +34,11 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
       ),
       self::CONFIG_COLUMN_SCHEMA => array(
         'status' => 'text255',
-        'message' => 'text',
         'relatedPHID' => 'phid?',
+
+        // T6203/NULLABILITY
+        // This should just be empty if there's no body.
+        'message' => 'text?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'status' => array(

src/applications/paste/storage/PhabricatorPaste.php

@@ -48,6 +48,10 @@ final class PhabricatorPaste extends PhabricatorPasteDAO
         'language' => 'text64',
         'mailKey' => 'bytes20',
         'parentPHID' => 'phid?',
+
+        // T6203/NULLABILITY
+        // Pastes should always have a view policy.
+        'viewPolicy' => 'policy?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'parentPHID' => array(

src/applications/phame/storage/PhameBlog.php

@@ -31,7 +31,12 @@ final class PhameBlog extends PhameDAO
         'name' => 'text64',
         'description' => 'text',
         'domain' => 'text255?',
-        'joinPolicy' => 'policy',
+
+        // T6203/NULLABILITY
+        // These policies should always be non-null.
+        'joinPolicy' => 'policy?',
+        'editPolicy' => 'policy?',
+        'viewPolicy' => 'policy?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_phid' => null,

src/applications/phame/storage/PhamePost.php

@@ -91,9 +91,17 @@ final class PhamePost extends PhameDAO
       self::CONFIG_COLUMN_SCHEMA => array(
         'title' => 'text255',
         'phameTitle' => 'text64',
-        'body' => 'text',
         'visibility' => 'uint32',
-        'datePublished' => 'epoch?',
+
+        // T6203/NULLABILITY
+        // These seem like they should always be non-null?
+        'blogPHID' => 'phid?',
+        'body' => 'text?',
+        'configData' => 'text?',
+
+        // T6203/NULLABILITY
+        // This one probably should be nullable?
+        'datePublished' => 'epoch',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_phid' => null,

src/applications/phriction/storage/PhrictionContent.php

@@ -35,9 +35,12 @@ final class PhrictionContent extends PhrictionDAO
         'title' => 'text',
         'slug' => 'text128',
         'content' => 'text',
-        'description' => 'text',
         'changeType' => 'uint32',
         'changeRef' => 'uint32?',
+
+        // T6203/NULLABILITY
+        // This should just be empty if not provided?
+        'description' => 'text?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'documentID' => array(

src/applications/ponder/storage/PonderAnswer.php

@@ -70,7 +70,10 @@ final class PonderAnswer extends PonderDAO
       self::CONFIG_COLUMN_SCHEMA => array(
         'voteCount' => 'sint32',
         'content' => 'text',
-        'contentSource' => 'text',
+
+        // T6203/NULLABILITY
+        // This should always exist.
+        'contentSource' => 'text?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_phid' => null,

src/applications/ponder/storage/PonderQuestion.php

@@ -38,10 +38,13 @@ final class PonderQuestion extends PonderDAO
         'voteCount' => 'sint32',
         'status' => 'uint32',
         'content' => 'text',
-        'contentSource' => 'text',
         'heat' => 'double',
         'answerCount' => 'uint32',
         'mailKey' => 'bytes20',
+
+        // T6203/NULLABILITY
+        // This should always exist.
+        'contentSource' => 'text?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_phid' => null,

src/applications/project/storage/PhabricatorProject.php

@@ -125,11 +125,16 @@ final class PhabricatorProject extends PhabricatorProjectDAO
         'name' => 'text255',
         'status' => 'text32',
         'phrictionSlug' => 'text128?',
-        'joinPolicy' => 'policy',
         'isMembershipLocked' => 'bool',
         'profileImagePHID' => 'phid?',
         'icon' => 'text32',
         'color' => 'text32',
+
+        // T6203/NULLABILITY
+        // These are definitely wrong and should always exist.
+        'editPolicy' => 'policy?',
+        'viewPolicy' => 'policy?',
+        'joinPolicy' => 'policy?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_phid' => null,

src/applications/releeph/storage/ReleephRequest.php

@@ -155,10 +155,12 @@ final class ReleephRequest extends ReleephDAO
       ),
       self::CONFIG_COLUMN_SCHEMA => array(
         'requestCommitPHID' => 'phid?',
-        'commitIdentifier' => 'text40',
+        'commitIdentifier' => 'text40?',
-        'pickStatus' => 'uint32',
+        'commitPHID' => 'phid?',
+        'pickStatus' => 'uint32?',
         'inBranch' => 'bool',
         'mailKey' => 'bytes20',
+        'userIntents' => 'text?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_phid' => null,

src/applications/repository/storage/PhabricatorRepositoryRefCursor.php

@@ -30,8 +30,11 @@ final class PhabricatorRepositoryRefCursor extends PhabricatorRepositoryDAO
       self::CONFIG_COLUMN_SCHEMA => array(
         'refType' => 'text32',
         'refNameHash' => 'bytes12',
-        'refNameEncoding' => 'text16',
         'commitIdentifier' => 'text40',
+
+        // T6203/NULLABILITY
+        // This probably should not be nullable; refNameRaw is not nullable.
+        'refNameEncoding' => 'text16?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_cursor' => array(

src/applications/settings/storage/PhabricatorUserSSHKey.php

@@ -12,11 +12,14 @@ final class PhabricatorUserSSHKey extends PhabricatorUserDAO {
   public function getConfiguration() {
     return array(
       self::CONFIG_COLUMN_SCHEMA => array(
-        'name' => 'text255',
-        'keyType' => 'text255',
-        'keyBody' => 'text',
         'keyHash' => 'bytes32',
         'keyComment' => 'text255?',
+
+        // T6203/NULLABILITY
+        // These seem like they should not be nullable.
+        'name' => 'text255?',
+        'keyType' => 'text255?',
+        'keyBody' => 'text?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'userPHID' => array(

src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php

@@ -36,6 +36,13 @@ final class PhabricatorWorkerActiveTask extends PhabricatorWorkerTask {
       ),
     );
 
+    $config[self::CONFIG_COLUMN_SCHEMA] = array(
+      // T6203/NULLABILITY
+      // This isn't nullable in the archive table, so at a minimum these
+      // should be the same.
+      'dataID' => 'uint32?',
+    ) + $parent[self::CONFIG_COLUMN_SCHEMA];
+
     return $config + $parent;
   }