From 9d1af762d5182723282f62c8fb5c1bb69b7bf96d Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 5 Feb 2020 07:36:31 -0800
Subject: [PATCH] In summary interfaces, don't render very large inline
 remarkup details for unit test messages

Summary: Ref T10635. An install with large blocks of remarkup (4MB) in test details is reporting slow page rendering. This is expected, but I've mostly given up on fighting this unless I absolutely have to. Degrade the interface more aggressively.

Test Plan:
  - Submitted a large block of test details in remarkup format.
  - Before patch: they rendered inline.
  - After patch: degraded display.
  - Verified small blocks are not changed.

{F7180727}

{F7180728}

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T10635

Differential Revision: https://secure.phabricator.com/D20970
---
 .../build/HarbormasterBuildUnitMessage.php    | 84 +++++++++++++++++--
 .../view/HarbormasterUnitPropertyView.php     |  6 +-
 2 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php
index 9e437efaba..f2632153c3 100644
--- a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php
+++ b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php
@@ -179,18 +179,23 @@ final class HarbormasterBuildUnitMessage
 
     $is_text = ($format !== self::FORMAT_REMARKUP);
     $is_remarkup = ($format === self::FORMAT_REMARKUP);
+    $message = null;
 
     $full_details = $this->getUnitMessageDetails();
+    $byte_length = strlen($full_details);
 
-    if (!strlen($full_details)) {
+    $text_limit = 1024 * 2;
+    $remarkup_limit = 1024 * 8;
+
+    if (!$byte_length) {
       if ($summarize) {
         return null;
       }
-      $details = phutil_tag('em', array(), pht('No details provided.'));
+      $message = phutil_tag('em', array(), pht('No details provided.'));
     } else if ($summarize) {
       if ($is_text) {
         $details = id(new PhutilUTF8StringTruncator())
-          ->setMaximumBytes(2048)
+          ->setMaximumBytes($text_limit)
           ->truncateString($full_details);
         $details = phutil_split_lines($details);
 
@@ -201,7 +206,34 @@ final class HarbormasterBuildUnitMessage
 
         $details = implode('', $details);
       } else {
-        $details = $full_details;
+        if ($byte_length > $remarkup_limit) {
+          $uri = $this->getURI();
+
+          if ($uri) {
+            $link = phutil_tag(
+              'a',
+              array(
+                'href' => $uri,
+                'target' => '_blank',
+              ),
+              pht('View Details'));
+          } else {
+            $link = null;
+          }
+
+          $message = array();
+          $message[] = phutil_tag(
+            'em',
+            array(),
+            pht('This test has too much data to display inline.'));
+          if ($link) {
+            $message[] = $link;
+          }
+
+          $message = phutil_implode_html(" \xC2\xB7 ", $message);
+        } else {
+          $details = $full_details;
+        }
       }
     } else {
       $details = $full_details;
@@ -212,19 +244,47 @@ final class HarbormasterBuildUnitMessage
     $classes = array();
     $classes[] = 'harbormaster-unit-details';
 
-    if ($is_remarkup) {
+    if ($message !== null) {
+      // If we have a message, show that instead of rendering any test details.
+      $details = $message;
+    } else if ($is_remarkup) {
       $details = new PHUIRemarkupView($viewer, $details);
     } else {
       $classes[] = 'harbormaster-unit-details-text';
       $classes[] = 'PhabricatorMonospaced';
     }
 
-    return phutil_tag(
+    $warning = null;
+    if (!$summarize) {
+      $warnings = array();
+
+      if ($is_remarkup && ($byte_length > $remarkup_limit)) {
+        $warnings[] = pht(
+          'This test result has %s bytes of Remarkup test details. Remarkup '.
+          'blocks longer than %s bytes are not rendered inline when showing '.
+          'test summaries.',
+          new PhutilNumber($byte_length),
+          new PhutilNumber($remarkup_limit));
+      }
+
+      if ($warnings) {
+        $warning = id(new PHUIInfoView())
+          ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
+          ->setErrors($warnings);
+      }
+    }
+
+    $content = phutil_tag(
       'div',
       array(
         'class' => implode(' ', $classes),
       ),
       $details);
+
+    return array(
+      $warning,
+      $content,
+    );
   }
 
   public function getUnitMessageDisplayName() {
@@ -262,6 +322,18 @@ final class HarbormasterBuildUnitMessage
     return implode("\0", $parts);
   }
 
+  public function getURI() {
+    $id = $this->getID();
+
+    if (!$id) {
+      return null;
+    }
+
+    return urisprintf(
+      '/harbormaster/unit/view/%d/',
+      $id);
+  }
+
   public function save() {
     if ($this->nameIndex === null) {
       $this->nameIndex = HarbormasterString::newIndex($this->getName());
diff --git a/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php
index ee8b80a7a7..2306f5ba64 100644
--- a/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php
+++ b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php
@@ -72,13 +72,13 @@ final class HarbormasterUnitPropertyView extends AphrontView {
       }
 
       $name = $message->getUnitMessageDisplayName();
-      $id = $message->getID();
+      $uri = $message->getURI();
 
-      if ($id) {
+      if ($uri) {
         $name = phutil_tag(
           'a',
           array(
-            'href' => "/harbormaster/unit/view/{$id}/",
+            'href' => $uri,
           ),
           $name);
       }