From ee6c24b98d1a8adbc86228da6979326e6326afeb Mon Sep 17 00:00:00 2001 From: mgummelt Date: Tue, 23 Aug 2011 17:16:43 -0700 Subject: [PATCH] Add two new conduit methods: createlintresults and getdiffproperty Summary: We need createlintresults because we are doing extended static analysis offline, and thus we need to be able to update the lint results associated with a diff. This is similar to updateunitresults, but "create" is more accurate than "update" since we never need to change existing lint results. getdiffproperty is used by the client to ensure it isn't creating any duplicates lint results. It's the symmetric operation to setdiffproperty, which already exists. Test Plan: We have a new offline linter that I used to test. This linter calls getdiffproperty on every run. 1. Tested updating an existing set of lint results by first running "arc diff" with lint errors caught by the local linter, then later running offline analysis which catches one other error and updates via createlintresults. Ensured the differential lint results were as expected. 2. Tested the creation of an entirely new diff property through createlintresults. I first ran "arc diff --nolint" to skip all lint results, then ran offline analysis which caught an error and updated through createlintresults. Ensured differential lint results were as expected. Reviewers: epriestley Reviewed By: epriestley CC: dpepper, aran, mgummelt, jungejason, epriestley Differential Revision: 868 --- src/__phutil_library_map__.php | 4 ++ ...ConduitAPI_differential_getdiff_Method.php | 14 ++++- .../method/differential/getdiff/__init__.php | 1 + ...PI_differential_setdiffproperty_Method.php | 58 +++++++++++++++++-- .../differential/setdiffproperty/__init__.php | 7 +++ 5 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 36f9964370..4599c55c72 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -92,12 +92,14 @@ phutil_register_library_map(array( 'ConduitAPI_daemon_launched_Method' => 'applications/conduit/method/daemon/launched', 'ConduitAPI_daemon_log_Method' => 'applications/conduit/method/daemon/log', 'ConduitAPI_differential_creatediff_Method' => 'applications/conduit/method/differential/creatediff', + 'ConduitAPI_differential_createlintresults_Method' => 'applications/conduit/method/differential/createlintresults', 'ConduitAPI_differential_createrevision_Method' => 'applications/conduit/method/differential/createrevision', 'ConduitAPI_differential_find_Method' => 'applications/conduit/method/differential/find', 'ConduitAPI_differential_getalldiffs_Method' => 'applications/conduit/method/differential/getalldiffs', 'ConduitAPI_differential_getcommitmessage_Method' => 'applications/conduit/method/differential/getcommitmessage', 'ConduitAPI_differential_getcommitpaths_Method' => 'applications/conduit/method/differential/getcommitpaths', 'ConduitAPI_differential_getdiff_Method' => 'applications/conduit/method/differential/getdiff', + 'ConduitAPI_differential_getdiffproperty_Method' => 'applications/conduit/method/differential/getdiffproperty', 'ConduitAPI_differential_getrevision_Method' => 'applications/conduit/method/differential/getrevision', 'ConduitAPI_differential_getrevisionfeedback_Method' => 'applications/conduit/method/differential/getrevisionfeedback', 'ConduitAPI_differential_markcommitted_Method' => 'applications/conduit/method/differential/markcommitted', @@ -771,12 +773,14 @@ phutil_register_library_map(array( 'ConduitAPI_daemon_launched_Method' => 'ConduitAPIMethod', 'ConduitAPI_daemon_log_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_creatediff_Method' => 'ConduitAPIMethod', + 'ConduitAPI_differential_createlintresults_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_createrevision_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_find_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getalldiffs_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getcommitmessage_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getcommitpaths_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getdiff_Method' => 'ConduitAPIMethod', + 'ConduitAPI_differential_getdiffproperty_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getrevision_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getrevisionfeedback_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_markcommitted_Method' => 'ConduitAPIMethod', diff --git a/src/applications/conduit/method/differential/getdiff/ConduitAPI_differential_getdiff_Method.php b/src/applications/conduit/method/differential/getdiff/ConduitAPI_differential_getdiff_Method.php index 2e090a74e5..0fa0ea340f 100644 --- a/src/applications/conduit/method/differential/getdiff/ConduitAPI_differential_getdiff_Method.php +++ b/src/applications/conduit/method/differential/getdiff/ConduitAPI_differential_getdiff_Method.php @@ -72,10 +72,15 @@ class ConduitAPI_differential_getdiff_Method extends ConduitAPIMethod { $changeset->attachHunks($changeset->loadHunks()); } - return $this->createDiffDict($diff); + $properties = id(new DifferentialDiffProperty())->loadAllWhere( + 'diffID = %d', + $diff_id); + + return $this->createDiffDict($diff, $properties); } - public static function createDiffDict(DifferentialDiff $diff) { + public static function createDiffDict(DifferentialDiff $diff, + array $properties) { $dict = array( 'id' => $diff->getID(), 'parent' => $diff->getParentRevisionID(), @@ -83,6 +88,7 @@ class ConduitAPI_differential_getdiff_Method extends ConduitAPIMethod { 'sourceControlBaseRevision' => $diff->getSourceControlBaseRevision(), 'sourceControlPath' => $diff->getSourceControlPath(), 'changes' => array(), + 'properties' => array(), ); foreach ($diff->getChangesets() as $changeset) { @@ -115,6 +121,10 @@ class ConduitAPI_differential_getdiff_Method extends ConduitAPIMethod { $dict['changes'][] = $change; } + foreach ($properties as $property) { + $dict['properties'][$property->getName()] = $property->getData(); + } + return $dict; } diff --git a/src/applications/conduit/method/differential/getdiff/__init__.php b/src/applications/conduit/method/differential/getdiff/__init__.php index 15399190d3..5ad4547ee8 100644 --- a/src/applications/conduit/method/differential/getdiff/__init__.php +++ b/src/applications/conduit/method/differential/getdiff/__init__.php @@ -9,6 +9,7 @@ phutil_require_module('phabricator', 'applications/conduit/method/base'); phutil_require_module('phabricator', 'applications/conduit/protocol/exception'); phutil_require_module('phabricator', 'applications/differential/storage/diff'); +phutil_require_module('phabricator', 'applications/differential/storage/diffproperty'); phutil_require_module('phabricator', 'applications/differential/storage/revision'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/conduit/method/differential/setdiffproperty/ConduitAPI_differential_setdiffproperty_Method.php b/src/applications/conduit/method/differential/setdiffproperty/ConduitAPI_differential_setdiffproperty_Method.php index 44e1840227..2a521c5437 100644 --- a/src/applications/conduit/method/differential/setdiffproperty/ConduitAPI_differential_setdiffproperty_Method.php +++ b/src/applications/conduit/method/differential/setdiffproperty/ConduitAPI_differential_setdiffproperty_Method.php @@ -43,13 +43,61 @@ class ConduitAPI_differential_setdiffproperty_Method extends ConduitAPIMethod { ); } + private static function updateLintStatus($diff_id, array $results) { + $diff = id(new DifferentialDiff())->load($diff_id); + if (!$diff) { + throw new ConduitException('ERR_BAD_DIFF'); + } + + $has_error = false; + $has_warning = false; + foreach ($results as $result) { + if ($result['severity'] === ArcanistLintSeverity::SEVERITY_ERROR) { + $has_error = true; + break; + } else if ($result['severity'] === + ArcanistLintSeverity::SEVERITY_WARNING) { + $has_warning = true; + } + } + + if ($has_error) { + $diff->setLintStatus(DifferentialLintStatus::LINT_FAIL); + } else if ($has_warning) { + $diff->setLintStatus(DifferentialLintStatus::LINT_WARN); + } else if ($results && + $diff->getLintStatus() === DifferentialLintStatus::LINT_NONE) { + $diff->setLintStatus(DifferentialLintStatus::LINT_OKAY); + } + $diff->save(); + } + protected function execute(ConduitAPIRequest $request) { - $property = new DifferentialDiffProperty(); - $property->setDiffID($request->getValue('diff_id')); - $property->setName($request->getValue('name')); - $property->setData(json_decode($request->getValue('data'), true)); - $property->save(); + $diff_id = $request->getValue('diff_id'); + $name = $request->getValue('name'); + $data = json_decode($request->getValue('data'), true); + + self::updateDiffProperty($diff_id, $name, $data); + + if ($name === 'arc:lint') { + self::updateLintStatus($diff_id, $data); + } + return; } + private static function updateDiffProperty($diff_id, $name, $data) { + $property = id(new DifferentialDiffProperty())->loadOneWhere( + 'diffID = %d AND name = %s', + $diff_id, + $name); + if (!$property) { + $property = new DifferentialDiffProperty(); + $property->setDiffID($diff_id); + $property->setName($name); + } + $property->setData($data); + $property->save(); + return $property; + } } diff --git a/src/applications/conduit/method/differential/setdiffproperty/__init__.php b/src/applications/conduit/method/differential/setdiffproperty/__init__.php index cb78726783..67c376c57b 100644 --- a/src/applications/conduit/method/differential/setdiffproperty/__init__.php +++ b/src/applications/conduit/method/differential/setdiffproperty/__init__.php @@ -6,8 +6,15 @@ +phutil_require_module('arcanist', 'lint/severity'); + phutil_require_module('phabricator', 'applications/conduit/method/base'); +phutil_require_module('phabricator', 'applications/conduit/protocol/exception'); +phutil_require_module('phabricator', 'applications/differential/constants/lintstatus'); +phutil_require_module('phabricator', 'applications/differential/storage/diff'); phutil_require_module('phabricator', 'applications/differential/storage/diffproperty'); +phutil_require_module('phutil', 'utils'); + phutil_require_source('ConduitAPI_differential_setdiffproperty_Method.php');