1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 01:08:50 +02:00
Commit graph

3064 commits

Author SHA1 Message Date
epriestley
2b5c73fc3d In "Analyze Query Plans" mode, collect service call stack traces in DarkConsole
Summary: Ref T13106. When profiling service queries, there's no convenient way to easily get a sense of why a query was issued. Add a mode to collect traces for each query to make this more clear. This is rough, but works well enough to be useful.

Test Plan: Clicked "Analyze Query Plans", got stack traces for each service call.

Maniphest Tasks: T13106

Differential Revision: https://secure.phabricator.com/D19221
2018-03-14 20:34:34 -07:00
epriestley
dc7e40ff3f Fix the DarkConsole inline error log stack trace expansion behavior for Content-Security-Policy
Summary:
See PHI451. Ref T13102. DarkConsole uses an ancient inline "onclick" handler to expand the stack traces for errors.

The new Content-Security-Policy prevents this from functioning.

Replace this with a more modern behavior-driven action instead.

Test Plan:
  - Clicked some errors in DarkConsole, saw stack traces appear.
  - Grepped for `onclick` and `jsprintf()` to see if I could find any more of these, but came up empty.

Maniphest Tasks: T13102

Differential Revision: https://secure.phabricator.com/D19218
2018-03-13 16:45:20 -07:00
epriestley
0bf8e33bb6 Issue setup guidance recommending MySQLi and MySQL Native Driver
Summary:
Fixes T12994. We need `MYSQLI_ASYNC` to implement client-side query timeouts, and we need MySQLi + MySQL Native Driver to get `MYSQLI_ASYNC`.

Recommend users install MySQLi and MySQL Native Driver if they don't have them. These are generally the defaults and best practice anyway, but Ubuntu makes it easy to use the older stuff.

All the cases we're currently aware of stem from `apt-get install php5-mysql` (which explicitly selects the non-native driver) so issue particular guidance about `php5-mysqlnd`.

Test Plan:
  - Faked both issues locally, reviewed the text.
  - Will deploy to `secure`, which currently has the non-native driver.

Maniphest Tasks: T12994

Differential Revision: https://secure.phabricator.com/D19216
2018-03-13 12:38:09 -07:00
epriestley
2b19f91936 Allow Doorkeeper references to have multiple display variations (full, short, etc.)
Summary:
Ref T13102. An install has a custom rule for bridging JIRA references via Doorkeeper and would like to be able to render them as `JIRA-123` instead of `JIRA JIRA-123 Full JIRA title`.

I think it's reasonable to imagine future support upstream for `JIRA-123`, `{JIRA-123}`, and so on, although we do not support these today. We can take a small step toward eventual support by letting the rendering pipeline understand different view modes.

This adds an optional `name` (the default text rendered before we do the OAuth sync) and an optional `view`, which can be `short` or `full`.

Test Plan:
I tested this primarily with Asana, since it's less of a pain to set up than JIRA. The logic should be similar, hopefully.

I changed `DoorkeeperAsanaRemarkupRule` to specify `name` and `view`, e.g `'view' => (mt_rand(0, 1) ? 'short' : 'full')`. Then I made a bunch of Asana references in a comment and saw them randomly go short or long.

Maniphest Tasks: T13102

Differential Revision: https://secure.phabricator.com/D19215
2018-03-13 11:29:52 -07:00
epriestley
1e93b49b1b Allow custom actions in Differential to explicitly override "accept" stickiness
Summary:
See PHI431. Ref T13102. An install is interested in a custom "non-sticky" accept action, roughly.

On the one hand, this is a pretty hacky patch. However, I suspect it inches us closer to T731, and I'm generally comfortable with exploring the realms of "Accept Next Update", "Unblock Without Accepting", etc., as long as most of it doesn't end up enabled by default in the upstream.

Test Plan:
  - Accepted and updated revisions normally, saw accepts respect global stickiness.
  - Modified the "Accept" action to explicitly be unsticky, saw nonsticky accept behavior after update.

Maniphest Tasks: T13102

Differential Revision: https://secure.phabricator.com/D19211
2018-03-12 17:10:43 -07:00
epriestley
3c4f31e4b9 Dynamically composite favicons from customizable sources
Summary: Ref T13103. Make favicons customizable, and perform dynamic compositing to add marker to indicate things like "unread messages".

Test Plan: Viewed favicons in Safari, Firefox and Chrome. With unread messages, saw pink dot composited into icon.

Maniphest Tasks: T13103

Differential Revision: https://secure.phabricator.com/D19209
2018-03-12 15:28:41 -07:00
epriestley
3e992c6713 Add audit, review, and dominion information to "owners.search" API method
Summary:
See PHI439. This fills in additional information about Owners packages.

Also removes dead `primaryOwnerPHID`.

Test Plan: Called `owners.search` and reviewed the results. Grepped for `primaryOwnerPHID`.

Differential Revision: https://secure.phabricator.com/D19207
2018-03-09 12:11:13 -08:00
epriestley
e83cfa295b Fix image prev/next cycling in lightboxes
Summary:
See <https://discourse.phabricator-community.org/t/lightbox-not-working/1201/3>. The lightbox code is fragile and currently relies on simulating a click on the actual "<a />" tag surrounding other images in the document.

This breaks the prev/next links which ignore the event because it there's no "<img />".

Instead, don't simulate clicks and just call the code we want directly.

Test Plan: Added several images to a page, used lightbox prev/next buttons to cycle between them.

Differential Revision: https://secure.phabricator.com/D19197
2018-03-08 08:28:04 -08:00
epriestley
9d3a722eb1 When proxying an "{image ...}" image fails, show the user an error message
Summary:
Depends on D19192. Ref T4190. Ref T13101. Instead of directly including the proxy endpoint with `<img src="..." />`, emit a placeholder and use AJAX to make the request. If the proxy fetch fails, replace the placeholder with an error message.

This isn't the most polished implementation imaginable, but it's much less mysterious about errors.

Test Plan: Used `{image ...}` for valid and invalid images, got images and useful error messages respectively.

Maniphest Tasks: T13101, T4190

Differential Revision: https://secure.phabricator.com/D19193
2018-03-08 07:03:26 -08:00
epriestley
a4cc1373d3 Use a tokenizer, not a gigantic poorly-ordered "<select />", to choose repositories in Owners
Summary: Depends on D19190. Fixes T12590. Ref T13099. Replaces the barely-usable, gigantic, poorly ordered "<select />" control with a tokenizer. Attempts to fix various minor issues.

Test Plan:
  - Edited paths: include/exclude paths, from different repositories, different actual paths.
  - Used "Add New Path" to add rows, got repository selector prepopulated with last value.
  - Used "remove".
  - Used validation typeahead, got reasonable behaviors?

The error behavior if you delete the repository for a path is a little sketchy still, but roughly okay.

Maniphest Tasks: T13099, T12590

Differential Revision: https://secure.phabricator.com/D19191
2018-03-07 20:57:24 -08:00
epriestley
ab0ac7f61b Remove very old "owners-default-path" code from Owners
Summary: Ref T12590. This is ancient code which was used to prefill `/trunk/tfb/www/` or similar at Facebook. I don't think it ever had a UI and no install has asked for this feature since 2011.

Test Plan: Grepped for affected symbols, edited paths in Owners.

Maniphest Tasks: T12590

Differential Revision: https://secure.phabricator.com/D19189
2018-03-07 18:25:27 -08:00
epriestley
229d467770 Restore lightbox behavior for thumbnailed images
Summary: Ref T13099. See <https://discourse.phabricator-community.org/t/lightbox-not-working/1201>. The Content-Security-Policy changes rewrote some of this code and the handling for "Download" links is incorrectly catching clicks on thumbnailed images.

Test Plan: Clicked a thumbnailed image, got a lightbox. Command-clicked a download link, still got link behavior instead of a lightbox.

Maniphest Tasks: T13099

Differential Revision: https://secure.phabricator.com/D19188
2018-03-07 07:33:43 -08:00
epriestley
df1e9ce646 Treat Owners paths like "/src/backend" and "/src/backend/" identically
Summary:
Depends on D19183. Ref T11015. Currently, adding a trailing slash works great and omitting it mysteriously doesn't work.

Store a normalized version with an unconditional trailing slash for the lookup logic to operate on, and a separate display version which tracks what the user actually typed.

Test Plan:
  - Entered "/src/main.c", "/src/main.c/", saw them de-duplicate.
  - Entered "/src/main.c", saw it stay that way in the UI but appear as "/src/main.c/" internally.
  - Added a rule for "/src/applications/owners" (no slash), created a revision touching paths in that directory, saw Owners fire for it.
  - Changed the display value of a path only ("/src/main.c" to "/src/main.c/"), saw the update reflected in the UI without any beahvioral change.

Maniphest Tasks: T11015

Differential Revision: https://secure.phabricator.com/D19184
2018-03-06 20:31:46 -08:00
epriestley
adde4089b4 Allow owners paths to be arbitrarily long and add storage for display paths
Summary:
Depends on D19182. Ref T11015. This changes `path` from `text255` to `longtext` because paths may be arbitrarily long.

It adds `pathDisplay` to prepare for display paths and storage paths having different values. For now, `pathDisplay` is copied from `path` and always has the same value.

Test Plan:
  - Ran migration, checked database for sanity (all `pathDisplay` and `path` values identical).
  - Added new paths, saw `pathDisplay` and `path` get the same values.
  - Added an unreasonably enormous path with far more than 255 characters.

Maniphest Tasks: T11015

Differential Revision: https://secure.phabricator.com/D19183
2018-03-06 20:31:22 -08:00
epriestley
8cb273a053 Add a unique key to OwnersPath on "<packageID, repositoryPHID, pathIndex>"
Summary:
Depends on D19181. Ref T11015. This nukes duplicates from the table if they exist, then adds a unique key.

(Duplicates should not exist and can not be added with any recent version of the web UI.)

Test Plan:
  - Tried to add duplicates with web UI, didn't have any luck.
  - Explicitly added duplicates with manual `INSERT`s.
  - Viewed packages in web UI and saw duplicates.
  - Ran migrations, got a clean purge and a nice unique key.
  - There's still no way to actually hit a duplicate key error in the UI (unless you can collide hashes, I suppose), this is purely a correctness/robustness change.

Maniphest Tasks: T11015

Differential Revision: https://secure.phabricator.com/D19182
2018-03-06 20:30:59 -08:00
epriestley
1bf4422c74 Add and populate a pathIndex column for OwnersPath
Summary: Ref T11015. This supports making path names arbitrarily long and putting a proper unique key on the table.

Test Plan:
  - Migrated, checked database, saw nice digested indexes.
  - Edited a package, saw new rows update with digested indexes.

Maniphest Tasks: T11015

Differential Revision: https://secure.phabricator.com/D19181
2018-03-06 20:30:33 -08:00
epriestley
dbccfb234f Perform a client-side redirect after OAuth server authorization
Summary:
Ref T13099. See that task for discussion. Chrome is unhappy with an MFA form submitting to an endpoint which redirects you to an OAuth URI.

Instead, do the redirect entirely on the client.

Chrome's rationale here isn't obvious, so we may be able to revert this at some point.

Test Plan: Went through the OAuth flow locally, was redirected on the client. Will verify in production.

Maniphest Tasks: T13099

Differential Revision: https://secure.phabricator.com/D19177
2018-03-06 12:18:27 -08:00
epriestley
743d1ac426 Mostly modularize the Differential "update" transaction
Summary: Ref T13099. Move most of the "Update" logic to modular transactions

Test Plan: Created and updated revisions. Flushed the task queue. Grepped for `TYPE_UPDATE`. Reviewed update transactions in the timeline and feed.

Maniphest Tasks: T13099

Differential Revision: https://secure.phabricator.com/D19175
2018-03-06 09:10:32 -08:00
epriestley
44f0664d2c Add a "lock log" for debugging where locks are being held
Summary: Depends on D19173. Ref T13096. Adds an optional, disabled-by-default lock log to make it easier to figure out what is acquiring and holding locks.

Test Plan: Ran `bin/lock log --enable`, `--disable`, `--name`, etc. Saw sensible-looking output with log enabled and daemons restarted. Saw no additional output with log disabled and daemons restarted.

Maniphest Tasks: T13096

Differential Revision: https://secure.phabricator.com/D19174
2018-03-05 17:55:34 -08:00
epriestley
1f40e50f7e Improve live Harbormaster log follow behaviors
Summary:
Depends on D19166. Ref T13088. When the user scrolls away from a followed log, break the focus lock.

Let users stop following a live log.

Show when lines are added more clearly.

Don't refresh quite as quickly give users a better shot at clicking the stop button.

These behaviors can probably be refined but are at least more plausible and less actively user-hostile than the first version of this behavior was.

Test Plan: Used `write-log --rate` to write a large log slowly. Clicked "Follow Log", followed for a bit. Scrolled away, still got live updates but no more scroll lock. Clicked stop, no more updates.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19167
2018-03-01 13:11:22 -08:00
epriestley
4e91ad276d Prevent copying Harbormaster build log line numbers with CSS psuedocontent instead of ZWS
Summary:
Depends on D19165. Ref T13088. Currently, in other applications, we use Zero Width Spaces and Javascript "copy" listeners to prevent line numbers from being copied. This isn't terribly elegant.

Modern browsers support a second approach: using psuedo-elements with `content`. Try this in Harbormaster since it's conceptually cleaner, at least. One immediate drawback is that Command-F can't find this text either.

Test Plan: In Safari, Chrome and Firefox, highlighted ranges of lines and copy/pasted text. Got just text (no line numbers) in all cases.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19166
2018-03-01 13:03:40 -08:00
epriestley
73619c4643 Share the Paste line highlighting behavior for Harbormaster build logs
Summary: Depends on D19164. Ref T13088. Now that the JS behaviors are generic, use them on the Harbormaster standalone page.

Test Plan: Clicked lines and dragged across line ranges. Reloaded pages. Saw expected highlighting behavior in the client and on the server across reloads.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19165
2018-03-01 12:57:30 -08:00
epriestley
fe3de5dd58 Make Paste source code line highlighting behavior more generic
Summary: Depends on D19163. Ref T13088. Increase the generality of this code so it can be shared with Harbormaster.

Test Plan: Clicked individual lines, clicked-and-dragged, etc., in Paste. Got sensible URI and highlight behaviors.

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19164
2018-03-01 12:46:36 -08:00
epriestley
49af4165bc Support rendering arbitrary sections in the middle of a Harbormaster build log so links to line 3500 work
Summary:
Depends on D19162. Ref T13088. When a user links to `$1234`, we need to render a default view of the log with a piece at the head, a piece at the end, and a piece in the middle.

We also need to figure out the offset for line 1234, or multiple offsets for "1234-2345".

Since the logic views/reads mostly anticipated this it isn't too much of a mess, although there are a couple of bugs this exposes with view specifications that use combinations of parameters which were previously impossible.

Test Plan: Viewed a large log with no line marker. Viewed `$1`. Viewed `$end`. Viewed `$35-40`, etc. Expanded context around logs.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19163
2018-03-01 11:18:21 -08:00
epriestley
a2fdf14275 Stop using forms to download files in file embed and lightbox elements
Summary: Depends on D19156. Ref T13094. This replaces the remaining forms in the file embed view and lightbox with normal download links.

Test Plan: Clicked "Download" and lightbox -> download for embedded files.

Maniphest Tasks: T13094

Differential Revision: https://secure.phabricator.com/D19157
2018-02-28 17:21:07 -08:00
epriestley
ab579f2511 Never generate file download forms which point to the CDN domain, tighten "form-action" CSP
Summary:
Depends on D19155. Ref T13094. Ref T4340.

We can't currently implement a strict `form-action 'self'` content security policy because some file downloads rely on a `<form />` which sometimes POSTs to the CDN domain.

Broadly, stop generating these forms. We just redirect instead, and show an interstitial confirm dialog if no CDN domain is configured. This makes the UX for installs with no CDN domain a little worse and the UX for everyone else better.

Then, implement the stricter Content-Security-Policy.

This also removes extra confirm dialogs for downloading Harbormaster build logs and data exports.

Test Plan:
  - Went through the plain data export, data export with bulk jobs, ssh key generation, calendar ICS download, Diffusion data, Paste data, Harbormaster log data, and normal file data download workflows with a CDN domain.
  - Went through all those workflows again without a CDN domain.
  - Grepped for affected symbols (`getCDNURI()`, `getDownloadURI()`).
  - Added an evil form to a page, tried to submit it, was rejected.
  - Went through the ReCaptcha and Stripe flows again to see if they're submitting any forms.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13094, T4340

Differential Revision: https://secure.phabricator.com/D19156
2018-02-28 17:20:12 -08:00
epriestley
f114b2dd7d When viewing a live build log, trap users in a small personal hell where nothing but slavish devotion to the log exists
Summary: Depends on D19152. Ref T13088. This adds live log tailing. It is probably not the final version of this feature because it prevents escape once you begin tailing a log.

Test Plan: Used `bin/harbormaster write-log --rate ...` to write a log slowly. Viewed it in the web UI. Clicked "Follow Log". Followed the log until the write finished, a lifetime later.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19153
2018-02-28 12:38:41 -08:00
epriestley
143033dc1f When showing a small piece of a Harbormaster build log, load a small piece of data instead of the entire log
Summary: Depends on D19148. Ref T13088. The new rendering always executes range requests for data it needs, and we can satisfy these requests by loading the smallest number of chunks which span that range.

Test Plan: Piped 50,000 lines of Apache log into Harbormaster, viewed it in the new UI, got sensible rendering times and a reasonable amount of data actually going over the wire.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19149
2018-02-28 12:32:26 -08:00
epriestley
dba4c4bdf6 Emit a "Content-Security-Policy" HTTP header
Summary:
See PHI399. Ref T4340. This header provides an additional layer of protection against various attacks, including XSS attacks which embed inline `<script ...>` or `onhover="..."` content into the document.

**style-src**: The "unsafe-inline" directive affects both `style="..."` and `<style>`. We use a lot of `style="..."`, some very legitimately, so we can't realistically get away from this any time soon. We only use one `<style>` (for monospaced font preferences) but can't disable `<style>` without disabling `style="..."`.

**img-src**: We use "data:" URIs to inline small images into CSS, and there's a significant performance benefit from doing this. There doesn't seem to be a way to allow "data" URIs in CSS without allowing them in the document itself.

**script-src** and **frame-src**: For a small number of flows (Recaptcha, Stripe) we embed external javascript, some of which embeds child elements (or additional resources) into the document. We now whitelist these narrowly on the respective pages.

This won't work with Quicksand, so I've blacklisted it for now.

**connect-src**: We need to include `'self'` for AJAX to work, and any websocket URIs.

**Clickjacking**: We now have three layers of protection:

  - X-Frame-Options: works in older browsers.
  - `frame-ancestors 'none'`: does the same thing.
  - Explicit framebust in JX.Stratcom after initialization: works in ancient IE.

We could probably drop the explicit framebust but it wasn't difficult to retain.

**script tags**: We previously used an inline `<script>` tag to start Javelin. I've moved this to `<data data-javelin-init ...>` tags, which seems to work properly.

**`__DEV__`**: We previously used an inline `<script>` tag to set the `__DEV__` mode flag. I tried using the "initialization" tags for this, but they fire too late. I moved it to `<html data-developer-mode="1">`, which seems OK everywhere.

**CSP Scope**: Only the CSP header on the original request appears to matter -- you can't refine the scope by emitting headers on CSS/JS. To reduce confusion, I disabled the headers on those response types. More headers could be disabled, although we're likely already deep in the land of diminishing returns.

**Initialization**: The initialization sequence has changed slightly. Previously, we waited for the <script> in bottom of the document to evaluate. Now, we go fishing for tags when domcontentready fires.

Test Plan:
  - Browsed around in Firefox, Safari and Chrome looking for console warnings. Interacted with various Javascript behaviors. Enabled Quicksand.
  - Disabled all the framebusting, launched a clickjacking attack, verified that each layer of protection is individually effective.
  - Verified that the XHProf iframe in Darkconsole and the PHPAST frame layout work properly.
  - Enabled notifications, verified no complaints about connecting to Aphlict.
  - Hit `__DEV__` mode warnings based on the new data attribute.
  - Tried to do sketchy stuff with `data:` URIs and SVGs. This works but doesn't seem to be able to do anything dangerous.
  - Went through the Stripe and Recaptcha workflows.
  - Dumped and examined the CSP headers with `curl`, etc.
  - Added a raw <script> tag to a page (as though I'd found an XSS attack), verified it was no longer executed.

Maniphest Tasks: T4340

Differential Revision: https://secure.phabricator.com/D19143
2018-02-27 10:17:30 -08:00
epriestley
f450c6c55b Fix some of the most egregious errors in Harbormaster log paging
Summary:
Depends on D19141. Ref T13088. Some of the fundamental log behaviors like "loading the correct rows" are now a bit better behaved.

The UI is a little less garbage, too.

Test Plan: Viewed some logs and loaded more context by clicking the buttons.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19142
2018-02-26 17:59:13 -08:00
epriestley
11d1dc484b Sort of make Harbormaster build logs page properly
Summary: Depends on D19139. Ref T13088. This doesn't actually work, but is close enough that a skilled attacker might be able to briefly deceive a small child.

Test Plan:
  - Viewed some very small logs under very controlled conditions, saw content.
  - Larger logs vaguely do something resembling working correctly.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19141
2018-02-26 17:58:33 -08:00
epriestley
6dc341be87 As Harbormaster logs are processed, build a sparse map of byte offsets to line numbers
Summary:
Depends on D19138. Ref T13088. When we want to read the last part of a logfile //and show accurate line numbers//, we need to be able to get from byte offsets to line numbers somehow.

Our fundamental unit must remain byte offsets, because a test can emit an arbitrarily long line, and we should accommodate it cleanly if a test emits 2GB of the letter "A".

To support going from byte offsets to line numbers, compute a map with periodic line markers throughout the offsets of the file. From here, we can figure out the line numbers for arbitrary positions in the file with only a constant amount of work.

Test Plan: Added unit tests; ran unit tests.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19139
2018-02-26 17:56:52 -08:00
epriestley
d6311044bb Store the Harbormaster log chunk format on the log record
Summary: Depends on D19137. Ref T13088. This allows `rebuild-log` to skip work if the chunks are already compressed. It also prepares for a future GC which is looking for "text" or "gzip" chunks to throw away in favor of archival into Files; such a GC can use this column to find collectable logs and then write "file" to it, meaning "chunks are gone, this data is only available in Files".

Test Plan: Ran migration, saw logs populate as "text". Ran `rebuild-log`, saw logs rebuild as "gzip".

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19138
2018-02-26 17:56:14 -08:00
epriestley
57e3d607f5 In Harbormaster, record byte length on the build logs
Summary: Depends on D19135. Ref T13088. Denormalize the total log size onto the log itself. This makes reasoning about the log at display time easier, and we don't need to fish around in the database as much to figure out what we're dealing with.

Test Plan: Ran `bin/harbormaster rebuild-log`, saw an existing log populate. Ran `bin/harbormaster write-log`, saw new log write with proper length information.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19136
2018-02-26 17:54:47 -08:00
epriestley
8a2604cf06 Add a "filePHID" to HarbormasterBuildLog and copy logs into Files during finalization
Summary: Depends on D19131. Ref T13088. During log finalization, stream the log into Files to support "Download Log", archive to Files, and API access.

Test Plan: Ran `write-log` and `rebuild-log`, saw Files objects generate with log content and appropriate permissions.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19132
2018-02-26 17:52:39 -08:00
epriestley
4c7370a1a3 Make the filetree view width sticky across show/hide and reload
Summary:
Ref T13090. The default width changed recently to become much wider, but the behavior on this control isn't great. Instead:

  - Pick a default width somewhere between the two.
  - Make the width sticky across show/hide (pressing "f" twice remembers your width instead of resetting it).
  - Make the width sticky across reloads (dragging the bar, then reloading the page keeps the bar in the same place).

Test Plan:
  - Without settings, loaded page: got medium-width bar.
  - Dragged bar wide/narrow, toggled on/off with "f", got persistent width.
  - Dragged bar wide/narrow, reloaded page, got persistent width.
  - Dragged bar wide/narrow, toggled it off, reloaded page, toggled it on, got persistent width.

Maniphest Tasks: T13090

Differential Revision: https://secure.phabricator.com/D19129
2018-02-22 13:47:41 -08:00
epriestley
0dee34b3fa Make Facts more modern, DRY, and dimensional
Summary:
Ref T13083. Facts has a fair amount of weird hardcoding and duplication of responsibilities. Reduce this somewhat: no more hard-coded fact aggregates, no more database-driven list of available facts, etc. Generally, derive all objective truth from FactEngines. This is more similar to how most other modern applications work.

For clarity, hopefully: rename "FactSpec" to "Fact". Rename "RawFact" to "Datapoint".

Split the fairly optimistic "RawFact" table into an "IntDatapoint" table with less stuff in it, then dimension tables for the object PHIDs and key names. This is primarily aimed at reducing the row size of each datapoint. At the time I originally wrote this code we hadn't experimented much with storing similar data in multiple tables, but this is now more common and has worked well elsewhere (CustomFields, Edges, Ferret) so I don't anticipate this causing issues. If we need more complex or multidimension/multivalue tables later we can accommodate them. The queries a single table supports (like "all facts of all kinds in some time window") don't make any sense as far as I can tell and could likely be UNION ALL'd anyway.

Remove all the aggregation stuff for now, it's not really clear to me what this should look like.

Test Plan: Ran `bin/fact analyze` and viewed web UI. Nothing exploded too violently.

Subscribers: yelirekim

Maniphest Tasks: T13083

Differential Revision: https://secure.phabricator.com/D19119
2018-02-19 12:05:19 -08:00
epriestley
eb3fd2b7f5 Fix an issue with marking aborted buildables failed when more than one build is aborted
Summary: See <https://discourse.phabricator-community.org/t/upgrade-issue-2018-week-7-mid-february/1139>.

Test Plan: Used `bin/storage upgrade -f --apply ...` to re-apply the migration.

Differential Revision: https://secure.phabricator.com/D19116
2018-02-17 04:36:25 -08:00
epriestley
8796a6036e Let users escape more easily from the autosuggester after typing "[" or "("
Summary:
Ref T13077. The autosuggester is a little too eager right now, and will eat carriage returns after typing `[` if you never activate the tokenizer.

To fix this, try just canceling sooner. If that doesn't work, we might need to cancel more eagerly by testing to see if the tokenizer is actually open.

Test Plan: Typed `[x]<return>`, got my return instead of getting trapped by the autosuggester.

Maniphest Tasks: T13077

Differential Revision: https://secure.phabricator.com/D19110
2018-02-16 11:02:48 -08:00
epriestley
0202c36b62 Suggest Phurl URLs on "((..." in Remarkup text areas
Summary: Depends on D19108. Ref T12241. Ref T13077. See D19108. This extends the `[[ ...` autocompleter to `((...` for Phurl URLs.

Test Plan: Typed `((th`, got `((thing))` suggested.

Reviewers: avivey

Reviewed By: avivey

Maniphest Tasks: T13077, T12241

Differential Revision: https://secure.phabricator.com/D19109
2018-02-16 09:56:39 -08:00
epriestley
8771b7d5c4 Add autocomplete for Phriction documents on "[[ ..." in Remarkup
Summary: Depends on D19107. Ref T13077. The underlying datasource may need some adjustment but this appears to work properly locally.

Test Plan: Typed `[[ por` locally, was suggested "Porcupine Facts". Typed `[[ / ]]`, saw it render as a reference to the wiki root instead of the install root.

Maniphest Tasks: T13077

Differential Revision: https://secure.phabricator.com/D19108
2018-02-16 09:56:18 -08:00
epriestley
f82206a4d1 Add a rough Quick Search datasource for Phriction documents
Summary:
Depends on D19106. Fixes T5941. Ref T13077. Allows you to find Phriction documents as suggestions from global quick search.

Also supports `w` to jump to Phriction and `w query` to query Phriction.

The actual query logic for the datasource may need some tweaking after it collides with reality, but seems to produce fairly reasonable results in local testing against synthetic data.

Test Plan: Searched for "Porcupine Facts", "Travel Companions", and other useful local pages. Searched for `w`. Searched for `w zebra facts`.

Maniphest Tasks: T13077, T5941

Differential Revision: https://secure.phabricator.com/D19107
2018-02-16 09:55:54 -08:00
epriestley
143350fdba Give Phriction documents modern string status constants instead of numeric constants
Summary:
Depends on D19099. Ref T13077. Updates Phriction documents to string constants to make API interactions cleaner and statuses more practical to extend.

This does not seem to require any transaction migrations because none of the Phriction transactions actually store status values: status is always a side effect of other edits.

Test Plan: Created, edited, deleted, moved documents. Saw appropriate UI cues. Browsed and filtered documents by status in the index.

Maniphest Tasks: T13077

Differential Revision: https://secure.phabricator.com/D19100
2018-02-15 18:23:41 -08:00
epriestley
a965d8d6ae Make PhrictionContent "description" non-nullable
Summary:
Depends on D19095. Ref T6203. Ref T13077. This column is nullable in an inconsistent way. Make it non-nullable.

Also clean up one more content query on the history view.

Test Plan: Ran migration, then created and edited documents without providing a descriptino or hitting `NULL` exceptions.

Maniphest Tasks: T13077, T6203

Differential Revision: https://secure.phabricator.com/D19096
2018-02-15 17:55:11 -08:00
epriestley
e492c717c6 Give PhrictionContent objects (older versions of wiki pages) legitimate PHIDs
Summary: Ref T13077. Prepares for modern API access to document history using standard "v3" APIs.

Test Plan: Ran migration, verified PHIDs appeared in the database. Created/edited a document, got even more PHIDs in the database.

Maniphest Tasks: T13077

Differential Revision: https://secure.phabricator.com/D19092
2018-02-15 17:39:07 -08:00
epriestley
2b0f98900b Fail outstanding buildables with aborted builds
Summary:
Ref T13072. See PHI361. The bug in T10746 where aborting builds didn't propagate properly to the buildable was fixed, but existing builds are still stuck "Building".

Since it doesn't look like anything will moot this before these changes promote to `stable`, just migrate these builds into "failed".

Test Plan: Ran migration, saw it affect only relevant builds and correctly fail them.

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D19091
2018-02-15 03:56:58 -08:00
epriestley
a4053bb580 When a ChangesetList sleeps after a Quicksand navigation, also hide any visible banner
Summary: Fixes T13080. The banner wasn't properly included in the sleep/wake logic.

Test Plan:
Mentioned `Dxxx` on a task. Enabled persistent chat to activate Quicksand. Reloaded page. Clicked `Dxxx`. Scrolled down until a changeset header appeared. Pressed back button.

  - Before patch: ended up on task, with header still around.
  - After patch: ended up on task, with header properly vanquished.

Pressed "forward", ended up back on the revision with the header again.

Maniphest Tasks: T13080

Differential Revision: https://secure.phabricator.com/D19086
2018-02-14 15:25:25 -08:00
epriestley
c42bbd6f5c Rename HarbormasterBuildMessage "buildTargetPHID" to "receiverPHID"
Summary: Ref T13054. Companion storage change for D19062.

Test Plan: Applied migration and adjustments. Viewed messages in Harbormaster; created them with `harbormaster.sendmessage`; processed them with `bin/phd debug task`.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054

Differential Revision: https://secure.phabricator.com/D19063
2018-02-12 12:17:44 -08:00
epriestley
f43d08c2bb Completely remove the legacy hunk table
Summary: Depends on D19056. Fixes T8475. Ref T13054. Merges "ModernHunk" back into "Hunk".

Test Plan: Grepped for `modernhunk`. Reviewed revisions. Created a new revision. Used `bin/differential migrate-hunk` to migrate hunks between storage formats and back.

Maniphest Tasks: T13054, T8475

Differential Revision: https://secure.phabricator.com/D19057
2018-02-10 16:12:50 -08:00
epriestley
b0d1d46a73 Drop the legacy hunk table
Summary: Ref T13054. Ref T8475. This table has had no readers or writers for more than a year after it was migrated to the modern table.

Test Plan: Ran migration, verified that all the data was still around.

Maniphest Tasks: T13054, T8475

Differential Revision: https://secure.phabricator.com/D19056
2018-02-10 16:09:31 -08:00