Page MenuHomePhabricator
Files F15428484

D13404.id32463.diff
No OneTemporary
Actions

D13404.id32463.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -376,6 +376,7 @@
'DifferentialGetWorkingCopy' => 'applications/differential/DifferentialGetWorkingCopy.php',
'DifferentialGitHubLandingStrategy' => 'applications/differential/landing/DifferentialGitHubLandingStrategy.php',
'DifferentialGitSVNIDField' => 'applications/differential/customfield/DifferentialGitSVNIDField.php',
+ 'DifferentialHarbormasterField' => 'applications/differential/customfield/DifferentialHarbormasterField.php',
'DifferentialHiddenComment' => 'applications/differential/storage/DifferentialHiddenComment.php',
'DifferentialHostField' => 'applications/differential/customfield/DifferentialHostField.php',
'DifferentialHostedGitLandingStrategy' => 'applications/differential/landing/DifferentialHostedGitLandingStrategy.php',
@@ -3739,6 +3740,7 @@
'DifferentialGetWorkingCopy' => 'Phobject',
'DifferentialGitHubLandingStrategy' => 'DifferentialLandingStrategy',
'DifferentialGitSVNIDField' => 'DifferentialCustomField',
+ 'DifferentialHarbormasterField' => 'DifferentialCustomField',
'DifferentialHiddenComment' => 'DifferentialDAO',
'DifferentialHostField' => 'DifferentialCustomField',
'DifferentialHostedGitLandingStrategy' => 'DifferentialLandingStrategy',
@@ -3764,7 +3766,7 @@
'DifferentialLandingStrategy' => 'Phobject',
'DifferentialLegacyHunk' => 'DifferentialHunk',
'DifferentialLineAdjustmentMap' => 'Phobject',
- 'DifferentialLintField' => 'DifferentialCustomField',
+ 'DifferentialLintField' => 'DifferentialHarbormasterField',
'DifferentialLintStatus' => 'Phobject',
'DifferentialLocalCommitsView' => 'AphrontView',
'DifferentialManiphestTasksField' => 'DifferentialCoreCustomField',
@@ -3840,7 +3842,7 @@
'DifferentialTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView',
- 'DifferentialUnitField' => 'DifferentialCustomField',
+ 'DifferentialUnitField' => 'DifferentialHarbormasterField',
'DifferentialUnitStatus' => 'Phobject',
'DifferentialUnitTestResult' => 'Phobject',
'DifferentialUpdateRevisionConduitAPIMethod' => 'DifferentialConduitAPIMethod',
diff --git a/src/applications/differential/customfield/DifferentialHarbormasterField.php b/src/applications/differential/customfield/DifferentialHarbormasterField.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/customfield/DifferentialHarbormasterField.php
@@ -0,0 +1,96 @@
+<?php
+
+abstract class DifferentialHarbormasterField
+ extends DifferentialCustomField {
+
+ abstract protected function getDiffPropertyKeys();
+ abstract protected function loadHarbormasterTargetMessages(
+ array $target_phids);
+ abstract protected function getLegacyProperty();
+ abstract protected function newModernMessage(array $message);
+ abstract protected function renderHarbormasterStatus(
+ DifferentialDiff $diff,
+ array $messages);
+ abstract protected function newHarbormasterMessageView(array $messages);
+
+ public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+ // TODO: This load is slightly inefficient, but most of this is moving
+ // to Harbormaster and this simplifies the transition. Eat 1-2 extra
+ // queries for now.
+ $keys = $this->getDiffPropertyKeys();
+
+ $properties = id(new DifferentialDiffProperty())->loadAllWhere(
+ 'diffID = %d AND name IN (%Ls)',
+ $diff->getID(),
+ $keys);
+ $properties = mpull($properties, 'getData', 'getName');
+
+ foreach ($keys as $key) {
+ $diff->attachProperty($key, idx($properties, $key));
+ }
+
+ $messages = array();
+
+ $buildable = $diff->getBuildable();
+ if ($buildable) {
+ $target_phids = array();
+ foreach ($buildable->getBuilds() as $build) {
+ foreach ($build->getBuildTargets() as $target) {
+ $target_phids[] = $target->getPHID();
+ }
+ }
+
+ if ($target_phids) {
+ $messages = $this->loadHarbormasterTargetMessages($target_phids);
+ }
+ }
+
+ if (!$messages) {
+ // No Harbormaster messages, so look for legacy messages and make them
+ // look like modern messages.
+ $legacy_messages = $diff->getProperty($this->getLegacyProperty());
+ if ($legacy_messages) {
+ // Show the top 100 legacy lint messages. Previously, we showed some
+ // by default and let the user toggle the rest. With modern messages,
+ // we can send the user to the Harbormaster detail page. Just show
+ // "a lot" of messages in legacy cases to try to strike a balance
+ // between implementation simplicitly and compatibility.
+ $legacy_messages = array_slice($legacy_messages, 0, 100);
+
+ foreach ($legacy_messages as $message) {
+ try {
+ $modern = $this->newModernMessage($message);
+ $messages[] = $modern;
+ } catch (Exception $ex) {
+ // Ignore any poorly formatted messages.
+ }
+ }
+ }
+ }
+
+ $status = $this->renderHarbormasterStatus($diff, $messages);
+
+ if ($messages) {
+ $path_map = mpull($diff->loadChangesets(), 'getID', 'getFilename');
+ foreach ($path_map as $path => $id) {
+ $href = '#C'.$id.'NL';
+
+ // TODO: When the diff is not the right-hand-size diff, we should
+ // ideally adjust this URI to be absolute.
+
+ $path_map[$path] = $href;
+ }
+
+ $view = $this->newHarbormasterMessageView($messages)
+ ->setPathURIMap($path_map);
+ } else {
+ $view = null;
+ }
+
+ return array(
+ $status,
+ $view,
+ );
+ }
+
+}
diff --git a/src/applications/differential/customfield/DifferentialLintField.php b/src/applications/differential/customfield/DifferentialLintField.php
--- a/src/applications/differential/customfield/DifferentialLintField.php
+++ b/src/applications/differential/customfield/DifferentialLintField.php
@@ -1,7 +1,7 @@
<?php
final class DifferentialLintField
- extends DifferentialCustomField {
+ extends DifferentialHarbormasterField {
public function getFieldKey() {
return 'differential:lint';
@@ -31,91 +31,32 @@
return $this->getFieldName();
}
- public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
- // TODO: This load is slightly inefficient, but most of this is moving
- // to Harbormaster and this simplifies the transition. Eat 1-2 extra
- // queries for now.
- $keys = array(
+ protected function getLegacyProperty() {
+ return 'arc:lint';
+ }
+
+ protected function getDiffPropertyKeys() {
+ return array(
'arc:lint',
'arc:lint-excuse',
);
+ }
- $properties = id(new DifferentialDiffProperty())->loadAllWhere(
- 'diffID = %d AND name IN (%Ls)',
- $diff->getID(),
- $keys);
- $properties = mpull($properties, 'getData', 'getName');
-
- foreach ($keys as $key) {
- $diff->attachProperty($key, idx($properties, $key));
- }
-
- $status = $this->renderLintStatus($diff);
-
- $lint = array();
-
- $buildable = $diff->getBuildable();
- if ($buildable) {
- $target_phids = array();
- foreach ($buildable->getBuilds() as $build) {
- foreach ($build->getBuildTargets() as $target) {
- $target_phids[] = $target->getPHID();
- }
- }
-
- $lint = id(new HarbormasterBuildLintMessage())->loadAllWhere(
- 'buildTargetPHID IN (%Ls) LIMIT 25',
- $target_phids);
- }
-
- if (!$lint) {
- // No Harbormaster messages, so look for legacy messages and make them
- // look like modern messages.
- $legacy_lint = $diff->getProperty('arc:lint');
- if ($legacy_lint) {
- // Show the top 100 legacy lint messages. Previously, we showed some
- // by default and let the user toggle the rest. With modern messages,
- // we can send the user to the Harbormaster detail page. Just show
- // "a lot" of messages in legacy cases to try to strike a balance
- // between implementation simplicitly and compatibility.
- $legacy_lint = array_slice($legacy_lint, 0, 100);
-
- $target = new HarbormasterBuildTarget();
- foreach ($legacy_lint as $message) {
- try {
- $modern = HarbormasterBuildLintMessage::newFromDictionary(
- $target,
- $this->getModernLintMessageDictionary($message));
- $lint[] = $modern;
- } catch (Exception $ex) {
- // Ignore any poorly formatted messages.
- }
- }
- }
- }
-
- if ($lint) {
- $path_map = mpull($diff->loadChangesets(), 'getID', 'getFilename');
- foreach ($path_map as $path => $id) {
- $href = '#C'.$id.'NL';
-
- // TODO: When the diff is not the right-hand-size diff, we should
- // ideally adjust this URI to be absolute.
-
- $path_map[$path] = $href;
- }
+ protected function loadHarbormasterTargetMessages(array $target_phids) {
+ return id(new HarbormasterBuildLintMessage())->loadAllWhere(
+ 'buildTargetPHID IN (%Ls) LIMIT 25',
+ $target_phids);
+ }
- $view = id(new HarbormasterLintPropertyView())
- ->setPathURIMap($path_map)
- ->setLintMessages($lint);
- } else {
- $view = null;
- }
+ protected function newHarbormasterMessageView(array $messages) {
+ return id(new HarbormasterLintPropertyView())
+ ->setLintMessages($messages);
+ }
- return array(
- $status,
- $view,
- );
+ protected function newModernMessage(array $message) {
+ return HarbormasterBuildLintMessage::newFromDictionary(
+ new HarbormasterBuildTarget(),
+ $this->getModernLintMessageDictionary($message));
}
public function getWarningsForDetailView() {
@@ -141,7 +82,10 @@
return $warnings;
}
- private function renderLintStatus(DifferentialDiff $diff) {
+ protected function renderHarbormasterStatus(
+ DifferentialDiff $diff,
+ array $messages) {
+
$colors = array(
DifferentialLintStatus::LINT_NONE => 'grey',
DifferentialLintStatus::LINT_OKAY => 'green',
diff --git a/src/applications/differential/customfield/DifferentialUnitField.php b/src/applications/differential/customfield/DifferentialUnitField.php
--- a/src/applications/differential/customfield/DifferentialUnitField.php
+++ b/src/applications/differential/customfield/DifferentialUnitField.php
@@ -1,7 +1,7 @@
<?php
final class DifferentialUnitField
- extends DifferentialCustomField {
+ extends DifferentialHarbormasterField {
public function getFieldKey() {
return 'differential:unit';
@@ -31,84 +31,32 @@
return $this->getFieldName();
}
- public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
- // TODO: See DifferentialLintField.
- $keys = array(
+ protected function getLegacyProperty() {
+ return 'arc:unit';
+ }
+
+ protected function getDiffPropertyKeys() {
+ return array(
'arc:unit',
'arc:unit-excuse',
);
+ }
- $properties = id(new DifferentialDiffProperty())->loadAllWhere(
- 'diffID = %d AND name IN (%Ls)',
- $diff->getID(),
- $keys);
- $properties = mpull($properties, 'getData', 'getName');
-
- foreach ($keys as $key) {
- $diff->attachProperty($key, idx($properties, $key));
- }
-
- $status = $this->renderUnitStatus($diff);
-
- $unit = array();
-
- $buildable = $diff->getBuildable();
- if ($buildable) {
- $target_phids = array();
- foreach ($buildable->getBuilds() as $build) {
- foreach ($build->getBuildTargets() as $target) {
- $target_phids[] = $target->getPHID();
- }
- }
-
- $unit = id(new HarbormasterBuildUnitMessage())->loadAllWhere(
- 'buildTargetPHID IN (%Ls) LIMIT 25',
- $target_phids);
- }
-
- if (!$unit) {
- $legacy_unit = $diff->getProperty('arc:unit');
- if ($legacy_unit) {
- // Show the top 100 legacy unit messages.
- $legacy_unit = array_slice($legacy_unit, 0, 100);
-
- $target = new HarbormasterBuildTarget();
- foreach ($legacy_unit as $message) {
- try {
- $modern = HarbormasterBuildUnitMessage::newFromDictionary(
- $target,
- $this->getModernUnitMessageDictionary($message));
- $unit[] = $modern;
- } catch (Exception $ex) {
- // Just ignore it if legacy messages aren't formatted like
- // we expect.
- }
- }
- }
- }
-
- if ($unit) {
- $path_map = mpull($diff->loadChangesets(), 'getID', 'getFilename');
- foreach ($path_map as $path => $id) {
- $href = '#C'.$id.'NL';
-
- // TODO: When the diff is not the right-hand-size diff, we should
- // ideally adjust this URI to be absolute.
-
- $path_map[$path] = $href;
- }
+ protected function loadHarbormasterTargetMessages(array $target_phids) {
+ return id(new HarbormasterBuildUnitMessage())->loadAllWhere(
+ 'buildTargetPHID IN (%Ls) LIMIT 25',
+ $target_phids);
+ }
- $view = id(new HarbormasterUnitPropertyView())
- ->setPathURIMap($path_map)
- ->setUnitMessages($unit);
- } else {
- $view = null;
- }
+ protected function newModernMessage(array $message) {
+ return HarbormasterBuildUnitMessage::newFromDictionary(
+ new HarbormasterBuildTarget(),
+ $this->getModernUnitMessageDictionary($message));
+ }
- return array(
- $status,
- $view,
- );
+ protected function newHarbormasterMessageView(array $messages) {
+ return id(new HarbormasterUnitPropertyView())
+ ->setUnitMessages($messages);
}
public function getWarningsForDetailView() {
@@ -132,8 +80,10 @@
return $warnings;
}
+ protected function renderHarbormasterStatus(
+ DifferentialDiff $diff,
+ array $messages) {
- private function renderUnitStatus(DifferentialDiff $diff) {
$colors = array(
DifferentialUnitStatus::UNIT_NONE => 'grey',
DifferentialUnitStatus::UNIT_OKAY => 'green',

File Metadata

Mime Type
text/plain
Expires
Mon, Mar 24, 8:39 PM (21 h, 27 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7712655
Default Alt Text
D13404.id32463.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility