recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

differential.creatediff: More accurate parameter handling

Summary:
There are no checks against invalid or missing parameters.
In addition, despite the claims of the parameter documentation, many of these parameters are optional.
Add errors for required parameters, mark the rest as optional.

This also addresses deprecation warnings about null used as a string.

Closes T16428

Test Plan:
* Call http://phorge.localhost/api/differential.creatediff without parameters without this patch
* Call http://phorge.localhost/api/differential.creatediff with this patch, adding parameters one by one until the errors disappear and a wild diff appears

See also T16428 for deprecation warning related tests.

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T16428

Differential Revision: https://we.phorge.it/D26641

mainframe98 ba38757f 43c685be

+13 -7
+13 -7
src/applications/differential/conduit/DifferentialCreateDiffConduitAPIMethod.php
··· 30 30 31 31 return array( 32 32 'changes' => 'required list<dict>', 33 - 'sourceMachine' => 'required string', 33 + 'sourceMachine' => 'optional string', 34 34 'sourcePath' => 'required string', 35 - 'branch' => 'required string', 35 + 'branch' => 'optional string', 36 36 'bookmark' => 'optional string', 37 - 'sourceControlSystem' => 'required '.$vcs_const, 38 - 'sourceControlPath' => 'required string', 39 - 'sourceControlBaseRevision' => 'required string', 37 + 'sourceControlSystem' => 'optional '.$vcs_const, 38 + 'sourceControlPath' => 'optional string', 39 + 'sourceControlBaseRevision' => 'optional string', 40 40 'creationMethod' => 'optional string', 41 - 'lintStatus' => 'required '.$status_const, 42 - 'unitStatus' => 'required '.$status_const, 41 + 'lintStatus' => 'optional '.$status_const, 42 + 'unitStatus' => 'optional '.$status_const, 43 43 'repositoryPHID' => 'optional phid', 44 44 45 45 'parentRevisionID' => 'deprecated', ··· 55 55 protected function defineErrorTypes() { 56 56 return array( 57 57 'ERR-NO-CONTENT' => pht('Diff may not be empty.'), 58 + 'ERR-USAGE-NO-SOURCE-PATH' => pht('You must specify a source path.'), 58 59 ); 59 60 } 60 61 ··· 130 131 } 131 132 132 133 $source_path = $request->getValue('sourcePath'); 134 + 135 + if (!phutil_nonempty_string($source_path)) { 136 + throw new ConduitException('ERR-USAGE-NO-SOURCE-PATH'); 137 + } 138 + 133 139 $source_path = $this->normalizeSourcePath($source_path); 134 140 135 141 $diff_data_dict = array(