1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 22:10:55 +01:00
Commit graph

740 commits

Author SHA1 Message Date
epriestley
e9309fdd6a When a Drydock lease schedules a resource to be reclaimed, awaken the lease update task when the reclaim completes
Summary:
Depends on D19751. Ref T13210. When Drydock needs to reclaim an existing unused resource in order to build a new resource to satisfy a lease, the lease which triggered the reclaim currently gets thrown back into the pool with a 15-second yield.

If the queue is pretty empty and the reclaim is quick, this can mean that we spend up to 15 extra seconds just waiting for the lease update task to get another shot at building a resource (the resource reclaim may complete in a second or two, but nothing else happens until the yield ends).

Instead, when a lease triggers a reclaim, have the reclaim reawaken the lease task when it completes. In the best case, this saves us 15 seconds of waiting. In other cases (the task already completed some other way, the resource gets claimed before the lease gets to it), it's harmless.

Test Plan:
  - Allocated A, A, A working copies with limit 3. Leased a B working copy.
  - Before patch: allocation took ~32 seconds.
  - After patch: allocation takes ~17 seconds (i.e., about 15 seconds less).

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13210

Differential Revision: https://secure.phabricator.com/D19752
2018-10-26 06:09:52 -07:00
epriestley
10f219fb82 Allow Drydock logs to be associated with RepositoryOperation objects
Summary: Ref T13197. See PHI873. I want to give RepositoryOperation objects access to Drydock logging like leases, resources, and blueprints currently have. This just does the schema/query changes, no actual UI or new logging yet.

Test Plan: Ran storage upgrade, poked around the UI looking for anything broken.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13197

Differential Revision: https://secure.phabricator.com/D19671
2018-09-15 07:55:14 -07:00
epriestley
40d5d5c984 Remove "mailKey" from "PhabricatorRepositoryCommit"
Summary: Ref T13197. Ref T13065. This continues the gradual purge of dedicated "mailKey" columns in favor of shared infrastructure.

Test Plan:
  - Ran migration.
  - Visually inspected database.
  - Grepped for `mailKey`.
  - Added some comments, saw the daemons generate corresponding mail.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13197, T13065

Differential Revision: https://secure.phabricator.com/D19670
2018-09-15 07:54:15 -07:00
epriestley
d63281cc54 Migrate remaining Audit database status constants
Summary: Depends on D19652. Ref T13197. See PHI851. This migrates the actual `auditStatus` on Commits, and older status transactions.

Test Plan:
  - Ran migrations.
  - Spot-checked the database for sanity.
  - Ran some different queries, got unchanged results from before migration.
  - Reviewed historic audit state transactions, and accepted/raised concern on new audits. All state transactions appeared to generate properly.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13197

Differential Revision: https://secure.phabricator.com/D19655
2018-09-12 12:21:27 -07:00
epriestley
09703938fb Migrate old commit saved queries to new audit status constants
Summary: Depends on D19651. Ref T13197. The application now accepts either numeric or string values. However, for consistency and to reduce surprise in the future, migrate existing saved queries to use string values.

Test Plan: Saved some queries on `master` with numeric constants, ran the migration, saw string constants in the database and equivalent behavior in the UI.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13197

Differential Revision: https://secure.phabricator.com/D19652
2018-09-12 12:21:02 -07:00
epriestley
dc0924deed Properly namespace the query in the spliced-in content cleanup patch
Yikes. Actually ran it this time!
2018-08-31 12:07:07 -07:00
epriestley
c5960c71f9 Splice in a patch to remove Phriction content rows with no document
The unique key on <documentPHID, version> may fail to apply if any content
rows don't have a valid document. This is rare, but we have some old random
garbage rows on "secure.phabricator.com" which prevent the next patch from
applying. Just toss these rows, they're junk.
2018-08-31 12:02:52 -07:00
epriestley
3b1294cf45 Store Phriction max version on Document, improve editing rules for editing documents with drafts
Summary:
Ref T13077. We need to know the maximum version of a document in several cases, so denormalize it onto the Document object.

Then clean up some behaviors where we edit a document with, e.g., 7 versions but version 5 is currently published. For now, we: edit starting with version 7, save as version 8, and immediately publish the new version.

Test Plan:
  - Ran migration.
  - Edited a draft page without hitting any weird version errors.
  - Checked database for sensible `maxVersion` values.

Reviewers: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13077

Differential Revision: https://secure.phabricator.com/D19625
2018-08-30 10:12:51 -07:00
epriestley
0a77b0e53e Work around an issue in MariaDB where dropping a column from a UNIQUE KEY fails
Summary:
See T13193. See T13077. If we drop a column which is part of a UNIQUE KEY, MariaDB raises an error.

This is probably a bad idea on our side anyway, but in this case it wasn't an obviously bad idea.

To get around this:

  - Drop the unique key, if it exists, before dropping the column.
  - Explicitly add the new unique key afterward.

Test Plan: Ran `bin/storage upgrade` locally without issue, but I'm on MySQL. Will follow up on T13193.

Reviewers: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D19624
2018-08-30 06:25:39 -07:00
epriestley
50f4adef64 Remove on-object mailkeys from Phriction
Summary: Depends on D19619. Ref T13065. Ref T13077. Migrate Phriction mail keys to the new infrastructure and drop the column.

Test Plan: Ran migrations, spot-checked the database.

Reviewers: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13077, T13065

Differential Revision: https://secure.phabricator.com/D19620
2018-08-29 13:43:13 -07:00
epriestley
64cee4a902 Move Phriction internal document/content references from IDs to PHIDs
Summary:
Ref T13077. This is mostly just a small cleanup change, even though the actual change is large.

We currently reference content and document objects from one another with `contentID` and `documentID`, but this means that `contentID` must be nullable. Switching to PHIDs allows the column to be non-nullable.

This also supports reorienting some current and future transactions around PHIDs, which is preferable for the API. In particular, I'm adding a "publish version X" transaction soon, and would rather callers pass a PHID than an ID or version number, since this will make the API more consistent and powerful.

Today, `contentID` gets used as a cheaty way to order documents by (content) edit time. Since PHIDs aren't orderable and stuff is going to become actually-revertible soon, replace this with an epoch timestamp.

Test Plan:
  - Created, edited, moved, retitled, and deleted Phriction documents.
  - Grepped for `documentID` and `contentID`.
  - This probably breaks //something// but I'll be in this code for a bit and am likely to catch whatever breaks.

Reviewers: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13077

Differential Revision: https://secure.phabricator.com/D19619
2018-08-29 13:41:24 -07:00
epriestley
2f5c6541fc Add an "Activated Epoch" and an "Acquired Epoch" to Drydock Leases
Summary: Ref T13189. See PHI690. When a lease is first acquired or activated, note the time. This supports better visibility into queue lengths. For now, this is only queryable via DB and visible in the UI, but can be more broadly exposed in the future.

Test Plan: Landed a revision, saw the leases get sensible timestamps for acquisition/activation.

Reviewers: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13189

Differential Revision: https://secure.phabricator.com/D19613
2018-08-27 11:27:45 -07:00
Austin McKinley
a6951a0a5a Add migration to encourage rebuilding repository identities
Summary: Ref T12164. Defines a new manual activity that suggests rebuilding repository identities before Phabricator begins to rely on them.

Test Plan:
- Ran migration, observed expected setup issue: {F5788217}
- Ran `bin/config done identities` and observed setup issue get marked as done.
- Ran `/bin/storage upgrade --apply phabricator:20170912.ferret.01.activity.php` to make sure I didn't break the reindex migration; observed reindex setup issue appear as expected.
- Ran `./bin/config done reindex` and observed reindex issue cleared as expected.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T12164

Differential Revision: https://secure.phabricator.com/D19497
2018-08-10 13:47:03 -07:00
epriestley
8d8086fccf Add Spaces support to Phriction
Summary:
Ref T13164. See PHI774. Fixes T12435.

Since Phriction is hierarchical, there isn't a super strong motivation to support Spaces: you can generally set policies on a small number of documents to get the desired effective policy behavior.

However, it still improves consistency and there's no reason //not// to support Spaces. In the case where you have some moderately weird/complex policy on one or more Spaces, using Spaces to define the policy behavior can make things a bit simpler and easier to understand.

This probably doesn't actually fix whatever the root problem in T12435 was (complicated, non-hierarchical access policies?). See also a bunch of discussion in T12442. So we might end up going beyond this to address other use cases, but I think this is reasonable regardless.

Test Plan: Created and edited Phriction documents and shifted them between Spaces. Searched by Space, etc.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13164, T12435

Differential Revision: https://secure.phabricator.com/D19553
2018-07-31 10:24:28 -07:00
epriestley
13cac5c362 Add Spaces to Projects
Summary:
See PHI774. Ref T13164. There is no reason projects //don't// support Spaces, just a vague concern that it's not hugely useful and might be a bit confusing.

However, it's at least somewhat useful (to improve consistency and reduce special casing) and doesn't necessarily seem more confusing than Projects are anyway. Support is trivial from a technical point of view, so just hook it up.

Test Plan: Created new projects, shifted projects between spaces. The support is all pretty much automatic.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19549
2018-07-31 10:15:41 -07:00
Austin McKinley
fe5fde5910 Assign RepositoryIdentity objects to commits
Summary: Depends on D19429. Depends on D19423. Ref T12164. This creates new columns `authorIdentityPHID` and `committerIdentityPHID` on commit objects and starts populating them. Also adds the ability to explicitly set an Identity's assignee to "unassigned()" to null out an incorrect auto-assign. Adds more search functionality to identities. Also creates a daemon task for handling users adding new email address and attempts to associate unclaimed identities.

Test Plan: Imported some repos, watched new columns get populated. Added a new email address for a previous commit, saw daemon job run and assign the identity to the new user. Searched for identities in various and sundry ways.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T12164

Differential Revision: https://secure.phabricator.com/D19443
2018-05-31 07:28:23 -07:00
Austin McKinley
f191a66490 Add controllers/search/edit engine functionality to RepositoryIdentity
Summary: Depends on D19423. Ref T12164. Adds controllers capable of listing and editing `PhabricatorRepositoryIdentity` objects. Starts creating those objects when commits are parsed.

Test Plan: Reparsed some revisions, observed objects getting created in the database. Altered some `Identity` objects using the controllers and observed effects in the database. No attempts made to validate behavior under "challenging" author/committer strings.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T12164

Differential Revision: https://secure.phabricator.com/D19429
2018-05-31 07:03:25 -07:00
Austin McKinley
cd84e53c44 Begin building out RepositoryIdentity indirection layer
Summary: Ref T12164. Start building initial objects for managing `RepositoryIdentity` objects. This won't land until much more of the infrastructure is in place.

Test Plan: Ran `bin/storage upgrade` and observed expected table.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T12164

Differential Revision: https://secure.phabricator.com/D19423
2018-05-31 07:01:16 -07:00
epriestley
4a98e0ff65 Allow Owners packages to be configured to ignore generated paths in Differential
Summary:
Depends on D19427. Ref T13130. See PHI251. Support configuring owners packages so they ignore generated paths.

This is still a little rough. A couple limitations:

  - It's hard to figure out how to use this control if you don't know what it's for, but we don't currently have a "CheckboxesEditField". I may add that soon.
  - The attribute ignore list doesn't apply to Diffusion, only Differential, which isn't obvious. I'll either try to make it work in Diffusion or note this somewhere.
  - No documentation yet (which could mitigate the other two issues a bit).

But the actual behavior seems to work fine.

Test Plan:
  - Set a package to ignore paths with the "generated" attribute. Saw the package stop matching generated paths in Differential.
  - Removed the attribute from the ignore list.
  - Tried to set invalid attributes, got sensible errors.
  - Queried a package with Conduit, got the ignored attribute list.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13130

Differential Revision: https://secure.phabricator.com/D19428
2018-05-05 08:47:29 -07:00
epriestley
dc510354c3 Remove explicit "mailKey" from Owners packages
Summary:
Depends on D19426. Ref T13130. Ref T13065. While I'm making changes to Owners for "Ignore generated paths", clean up the "mailKey" column.

We recently (D19399) added code to automatically generate and manage mail keys so we don't need a ton of `mailKey` properties in the future. Migrate existing mail keys and blow away the explicit column on packages.

Test Plan: Ran migration, manually looked at the database and saw sensible data. Edited a package to send some mail, which looked good.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13130, T13065

Differential Revision: https://secure.phabricator.com/D19427
2018-05-05 08:47:08 -07:00
epriestley
1b24b486f5 Manage object mailKeys automatically in Mail instead of storing them on objects
Summary:
Ref T13065. `mailKey`s are a private secret for each object. In some mail configurations, they help us ensure that inbound mail is authentic: when we send you mail, the "Reply-To" is "T123+456+abcdef".

  - The `T123` is the object you're actually replying to.
  - The `456` is your user ID.
  - The `abcdef` is a hash of your user account with the `mailKey`.

Knowing this hash effectively proves that Phabricator has sent you mail about the object before, i.e. that you legitimately control the account you're sending from. Without this, anyone could send mail to any object "From" someone else, and have comments post under their username.

To generate this hash, we need a stable secret per object. (We can't use properties like the PHID because the secret has to be legitimately secret.)

Today, we store these in `mailKey` properties on the actual objects, and manually generate them. This results in tons and tons and tons of copies of this same ~10 lines of code.

Instead, just store them in the Mail application and generate them on demand. This change also anticipates possibly adding flags like "must encrypt" and "original subject", which are other "durable metadata about mail transmission" properties we may have use cases for eventually.

Test Plan:
  - See next change for additional testing and context.
  - Sent mail about Herald rules (next change); saw mail keys generate cleanly.
  - Destroyed a Herald rule with a mail key, saw the mail properties get nuked.
  - Grepped for `getMailKey()` and converted all callsites I could which aren't the copy/pasted boilerplate present in 50 places.
  - Used `bin/mail receive-test --to T123` to test normal mail receipt of older-style objects and make sure that wasn't broken.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13065

Differential Revision: https://secure.phabricator.com/D19399
2018-04-25 06:46:58 -07:00
Austin McKinley
4dc8e2de56 Add unique constraint to AlmanacInterfaces
Summary: See discussion in D19379. The 4-tuple of (device, network, address, port) should be unique.

Test Plan: Created lots of duplicate interfaces, bound those interfaces to various services, observed migration script clean things up correctly.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D19388
2018-04-19 19:16:50 -07:00
Austin McKinley
e81b2173ad Add edge tables for Phlux
Summary: Fixes T13129. This at least makes the existing UI work again before we banish Phlux to the shadow realm.

Test Plan: Edited the visibility for a Phlux variable, didn't get an error. Nothing showed up in the edge tables when I made those changes, but at least it doesn't error out anymore.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13129

Differential Revision: https://secure.phabricator.com/D19387
2018-04-19 15:49:08 -07:00
Austin McKinley
0a83f253ed Add unique constraint for Almanac network names
Summary:
The name of networks should be unique.

Also adds support for exact-name queries for AlamanacNetworks.

Test Plan: Applied migration with existing duplicates, saw networks renamed, attempted to add duplicates, got a nice error message.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D19379
2018-04-19 13:41:15 -07:00
epriestley
f9c6a69d9c Add skeleton code for Almanac Interfaces to have real transactions
Summary:
Depends on D19322. Ref T13120. Ref T12414.

Currently, `AlmanacDevice` has a bit of a beast of a `TYPE_INTERFACE` transaction that fully creates a complex Interface object. This isn't very flexible or consistent, and Interfaces are complex enough to reasonably have their own object behaviors (for example, they have their own PHIDs).

The complexity of this transaction makes modularizing `AlmanacDevice` transactions tricky. To simplify this, move Interface toward having its own set of normal transactions.

This change just adds some reasonable-looking transactions; it doesn't actually hook them up in the UI or make them reachable. I'll test that they actually work as I swap the UI over.

We may also have some code using the `TYPE_INTERFACE` transaction in Phacility support stuff, so that may need to wait a week to actually phase out.

Test Plan: Ran `bin/storage upgrade` and `arc liberate`. This code isn't reachable yet.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13120, T12414

Differential Revision: https://secure.phabricator.com/D19323
2018-04-11 10:29:26 -07:00
epriestley
615d27c8e9 Show an additional "Draft" tag on non-broadcasting revisions in a non-draft state
Summary:
Depends on D19284. Ref T13110. It's now possible to get a revision into a "Abandoned + But, Never Promoted From Draft" state. Show this in the header and provide the draft hint above the comment area.

Also, remove `shouldBroadcast()`. The method `getShouldBroadcast()` now has the same meaning.

Finally, migrate existing drafts to `shouldBroadcast = false` and default `shouldBroadcast` to `true`. If we don't do this, every older revision becomes a non-broadcasting revision because this flag was not explicitly set on revision creation before, only on promotion out of draft.

Test Plan: Ran migration; abandoned draft revisions and ended up in a draft + abandoned state.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13110

Differential Revision: https://secure.phabricator.com/D19285
2018-04-03 11:09:49 -07:00
epriestley
f583406ba9 Drop uniqueness constraint on PushEvent request ID
Summary: See <https://discourse.phabricator-community.org/t/pushing-to-mercurial-repository-fails/1275/1>. Mercurial may invoke hooks multiple times per push.

Test Plan: Pushed to Mercurial, saw key constraint failure.

Differential Revision: https://secure.phabricator.com/D19257
2018-03-26 07:02:15 -07:00
epriestley
df3c937dab Record lock timing information on PushEvents
Summary:
Depends on D19249. Ref T13109. Add timing information to the `PushEvent`:

  - `writeWait`: Time spent waiting for a write lock.
  - `readWait`: Time spent waiting for a read lock.
  - `hostWait`: Roughly, total time spent on the leaf node.

The primary goal here is to see if `readWait` is meaningful in the wild. If it is, that motivates smarter routing, and the value of smarter routing can be demonstrated by looking for a reduction in read wait times.

Test Plan: Pushed some stuff, saw reasonable timing values in the table. Saw timing information in "Export Data".

Maniphest Tasks: T13109

Differential Revision: https://secure.phabricator.com/D19250
2018-03-22 13:46:01 -07:00
epriestley
69bff489d4 Generate a random unique "Request ID" for SSH requests so processes can coordinate better
Summary:
Depends on D19247. Ref T13109. When we receive an SSH request, generate a random unique ID for the request. Then thread it down through the process tree.

The immediate goal is to let the `ssh-exec` process coordinate with `commit-hook` process and log information about read and write lock wait times. Today, there's no way for `ssh-exec` to interact with the `PushEvent`, but this is the most helpful place to store this data for users.

Test Plan: Made pushes, saw the `PushEvent` table populate with a random request ID. Exported data and saw the ID preserved in the export.

Maniphest Tasks: T13109

Differential Revision: https://secure.phabricator.com/D19249
2018-03-22 13:44:30 -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
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
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
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
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
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
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
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
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
epriestley
ffc5c95c2f Correct flipped transaction constants in "Closed Date" migration
Summary: These transaction constants are flipped, which can produce the wrong result in some cases.

Test Plan: `./bin/storage upgrade -f --apply phabricator:20180208.maniphest.02.populate.php`

Differential Revision: https://secure.phabricator.com/D19054
2018-02-10 06:10:55 -08:00
epriestley
0470125d9e Add skeleton code for webhooks
Summary: Ref T11330. Adds general support for webhooks. This is still rough and missing a lot of pieces -- and not yet useful for anything -- but can make HTTP requests.

Test Plan: Used `bin/webhook call ...` to complete requests to a test endpoint.

Maniphest Tasks: T11330

Differential Revision: https://secure.phabricator.com/D19045
2018-02-09 13:55:04 -08:00
epriestley
f028aa6f60 Track closed date and closing user for tasks explicitly
Summary:
Ref T4434. Although some of the use cases for this data are better fits for Facts, this data is reasonable to track separately.

I have an approximate view of it already ("closed, ordered by date modified") that's useful to review things that were fixed recently. This lets us make that view more effective.

This just adds (and populates) the storage. Followups will add Conduit, Export, Search, and UI support.

This is slightly tricky because merges work oddly (see T13020).

Test Plan:
  - Ran migration, checked database for sensible results.
  - Created a task in open/closed status, got the right database values.
  - Modified a task to close/open it, got the right values.
  - Merged an open task, got updates.

Maniphest Tasks: T4434

Differential Revision: https://secure.phabricator.com/D19037
2018-02-08 15:40:49 -08:00
epriestley
aa74af1983 Remove all "originalTitle"/"originalName" fields from objects
Summary:
Depends on D19012. Ref T13053. In D19012, I've changed "Thread-Topic" to always use PHIDs.

This change drops the selective on-object storage we have to track the original, human-readable title for objects.

Even if we end up backing out the "Thread-Topic" change, we'd be better off storing this in a table in the Mail app which just has `<objectPHID, first subject we used when sending mail for that object>`, since then we get the right behavior without needing every object to have this separate field.

Test Plan: Grepped for `original`, `originalName`, `originalTitle`, etc.

Reviewers: amckinley

Maniphest Tasks: T13053

Differential Revision: https://secure.phabricator.com/D19013
2018-02-08 06:22:03 -08:00
epriestley
fd49acd033 Fix Herald repetition policy migration for NULL
When we change a nullable column to a non-nullable column, we can get a
data truncation error if any value was "NULL".

This is exceptionally unusual, but our two very oldest Herald rules have
a "NULL" policy on `secure`.
2018-01-26 13:17:15 -08:00
epriestley
204d1de683 Convert storage for Herald repetition policy to "text32"
Summary:
Depends on D18926. Ref T6203. Ref T13048. Herald rule repetition policies are stored as integers but treated as strings in most contexts.

After D18926, the integer stuff is almost totally hidden inside `HeraldRule` and getting rid of it completely isn't too tricky.

Do so now.

Test Plan:
  - Created "only the first time" and "every time" rules. Did a SELECT on their rows in the database.
  - Ran migrations, got a clean bill of health from `storage adjust`.
  - Did another SELECT on the rows, saw a faithful conversion to strings "every" and "first".
  - Edited and reviewed rules, swapping them between "every" and "first".

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13048, T6203

Differential Revision: https://secure.phabricator.com/D18927
2018-01-26 11:05:37 -08:00
epriestley
21e415299f Mark all existing password hashes as "legacy" and start upgrading digest formats
Summary:
Depends on D18907. Ref T13043. Ref T12509. We have some weird old password digest behavior that isn't terribly concerning, but also isn't great.

Specifically, old passwords were digested in weird ways before being hashed. Notably, account passwords were digested with usernames, so your password stops working if your username is chagned. Not the end of the world, but silly.

Mark all existing hashes as "v1", and automatically upgrade then when they're used or changed. Some day, far in the future, we could stop supporting these legacy digests and delete the code and passwords and just issue upgrade advice ("Passwords which haven't been used in more than two years no longer work."). But at least get things on a path toward sane, modern behavior.

Test Plan: Ran migration. Spot-checked that everthing in the database got marked as "v1". Used an existing password to login successfully. Verified that it was upgraded to a `null` (modern) digest. Logged in with it again.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043, T12509

Differential Revision: https://secure.phabricator.com/D18908
2018-01-23 14:01:09 -08:00
epriestley
cab2bba6f2 Remove "passwordHash" and "passwordSalt" from User objects
Summary:
Ref T13043. After D18903, this data has migrated to shared infrastructure and has no remaining readers or writers.

Just delete it now, since the cost of a mistake here is very small (users need to "Forgot Password?" and pick a new password).

Test Plan: Grepped for `passwordHash`, `passwordSalt`, and variations.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043

Differential Revision: https://secure.phabricator.com/D18904
2018-01-23 13:44:26 -08:00
epriestley
abc030fa00 Move account passwords to shared infrastructure
Summary:
Ref T13043. This moves user account passwords to the new shared infrastructure.

There's a lot of code changes here, but essentially all of it is the same as the VCS password logic in D18898.

Test Plan:
- Ran migration.
- Spot checked table for general sanity.
- Logged in with an existing password.
- Hit all error conditions on "change password", "set password", "register new account" flows.
- Verified that changing password logs out other sessions.
- Verified that revoked passwords of a different type can't be selected.
- Changed passwords a bunch.
- Verified that salt regenerates properly after password change.
- Tried to login with the wrong password, which didn't work.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043

Differential Revision: https://secure.phabricator.com/D18903
2018-01-23 13:43:07 -08:00
epriestley
5a8a56f414 Prepare the new AuthPassword infrastructure for storing account passwords
Summary:
Ref T13043. In D18898 I moved VCS passwords to the new shared infrastructure.

Before account passwords can move, we need to make two changes:

  - For legacy reasons, VCS passwords and Account passwords have different "digest" algorithms. Both are more complicated than they should be, but we can't easily fix it without breaking existing passwords. Add a `PasswordHashInterface` so that objects which can have passwords hashes can implement custom digest logic for each password type.
  - Account passwords have a dedicated external salt (`PhabricatorUser->passwordSalt`). This is a generally reasonable thing to support (since not all hashers are self-salting) and we need to keep it around so existing passwords still work. Add salt support to `AuthPassword` and make it generate/regenerate when passwords are updated.

Then add a nice story about password digestion.

Test Plan: Ran migrations. Used an existing VCS password; changed VCS password. Tried to use a revoked password. Unit tests still pass. Grepped for callers to legacy `PhabricatorHash::digestPassword()`, found none.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043

Differential Revision: https://secure.phabricator.com/D18900
2018-01-23 10:57:40 -08:00
epriestley
753c4c5ff1 Remove the "PhabricatorRepositoryVCSPassword" class and table
Summary:
Ref T13043. After D18898, this has been migrated to new, more modern storage and no longer has any readers or writers.

One migration from long ago (early 2014) is affected. Since this is ancient and the cost of dropping this is small (see inline), I just dropped it.

I'll note this in the changelog.

Test Plan: Ran migrations, got a clean bill of health from `storage status`. Grepped for removed symbol.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043

Differential Revision: https://secure.phabricator.com/D18899
2018-01-23 10:56:37 -08:00
epriestley
dd8f588ac5 Migrate VCS passwords to new shared password infrastructure
Summary:
Ref T13043. Migrate VCS passwords away from their dedicated table to new the new shared infrastructure.

Future changes will migrate account passwords and remove the old table.

Test Plan:
- Ran migrations.
  - Cloned with the same password that was configured before the migrations (worked).
  - Cloned with a different, invalid password (failed).
- Changed password.
  - Cloned with old password (failed).
  - Cloned with new password (worked).
- Deleted password in web UI.
  - Cloned with old password (failed).
- Set password to the same password as it currently is set to (worked, no "unique" collision).
- Set password to account password. !!This (incorrectly) works for now until account passwords migrate, since the uniqueness check can't see them yet.!!
- Set password to a new unique password.
  - Cloned (worked).
  - Revoked the password with `bin/auth revoke`.
  - Verified web UI shows "no password set".
  - Verified that pull no longer works.
  - Verified that I can no longer select the revoked password.
- Verified that accounts do not interact:
  - Tried to set account B to account A's password (worked).
  - Tried to set account B to a password revoked on account A (worked).
- Spot checked the `password` and `passwordtransaction` tables for saniity.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043

Differential Revision: https://secure.phabricator.com/D18898
2018-01-23 10:56:13 -08:00
epriestley
9c00a43784 Add a more modern object for storing password hashes
Summary:
Ref T13043. Currently:

  - Passwords are stored separately in the "VCS Passwords" and "User" tables and don't share as much code as they could.
  - Because User objects are all over the place in the code, password hashes are all over the place too (i.e., often somewhere in process memory). This is a very low-severity, theoretical sort of issue, but it could make leaving a stray `var_dump()` in the code somewhere a lot more dangerous than it otherwise is. Even if we never do this, third-party developers might. So it "feels nice" to imagine separating this data into a different table that we rarely load.
  - Passwords can not be //revoked//. They can be //deleted//, but users can set the same password again. If you believe or suspect that a password may have been compromised, you might reasonably prefer to revoke it and force the user to select a //different// password.

This change prepares to remedy these issues by adding a new, more modern dedicated password storage table which supports storing multiple password types (account vs VCS), gives passwords real PHIDs and transactions, supports DestructionEngine, supports revocation, and supports `bin/auth revoke`.

It doesn't actually make anything use this new table yet. Future changes will migrate VCS passwords and account passwords to this table.

(This also gives third party applications a reasonable place to store password hashes in a consistent way if they have some need for it.)

Test Plan: Added some basic unit tests to cover general behavior. This is just skeleton code for now and will get more thorough testing when applications move.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043

Differential Revision: https://secure.phabricator.com/D18894
2018-01-22 15:35:28 -08:00
epriestley
3038d564a6 Allow bulk edits to be made silently if you have CLI access
Summary:
Fixes T13042. This hooks up the new "silent" mode from D18882 and makes it actually work.

The UI (where we tell you to go run some command and then reload the page) is pretty clumsy, but should solve some problems for now and can be cleaned up eventually. The actual mechanics (timeline aggregation, Herald interaction,  etc.) are on firmer ground.

Test Plan:
  - Made a normal bulk edit, got mail and feed stories.
  - Made a silent bulk edit, no mail and no feed.
  - Saw "Silent Edit" marker in timeline for silent edits:

{F5386245}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13042

Differential Revision: https://secure.phabricator.com/D18883
2018-01-19 13:24:54 -08:00
epriestley
6d36eb9113 Denormalize Diff PHIDs onto Revisions
Summary:
Ref T12539. See PHI190. Currently, each Diff has a `revisionID`, but Revisions do not point at the current active diff. To find the active diff for a given revision, we need to issue a separate query.

Furthermore, this query is inefficient for bulk loads: if we have a lot of revisions, we end up querying for all diff IDs for all those revisions first, then selecting the largest ones and querying again to get the actual diff objects. This strategy could likely be optimized but the query is a mess in any case.

In several cases, it's useful to have the active diff PHID without needing to do a second query -- sometimes for convenience, and sometimes for performance.

T12539 is an example of such a case: it would be nice to refine the bucketing logic (which only depends on active diff PHIDs), but it feels bad to make the page heavier to do it.

For now, this is unused. I'll start using it to fix the bucketing issue, and then we can expand it gradually to address other performance/convenience issues.

Test Plan:
  - Ran migrations, inspected database, saw sensible values.
  - Created a new revision, saw a sensible database value.
  - Updated an existing revision, saw database update properly.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12539

Differential Revision: https://secure.phabricator.com/D18756
2017-11-01 17:19:38 -07:00
epriestley
f1204c8c45 Convert Ponder Questions to Ferret engine
Summary: See PHI177. Ref T12974. PonderQuestion was overlooked during the Ferret engine conversions.

Test Plan:
Ran migrations, searched for questions, got results:

{F5241185}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12974

Differential Revision: https://secure.phabricator.com/D18736
2017-10-26 18:18:04 -07:00
epriestley
1de130c9f5 Allow the Ferret engine to remove "common" ngrams from the index
Summary:
Ref T13000. This adds support for tracking "common" ngrams, which occur in too many documents to be useful as part of the ngram index.

If an ngram is listed in the "common" table, it won't be written when indexing documents, or queried for when searching for them.

In this change, nothing actually writes to the "common" table. I'll start writing to the table in a followup change.

Specifically, I plan to do this:

  - A new GC process updates the "common" table periodically, by writing ngrams which appear in more than X% of documents to it, for some value of X, if there are at least a minimum number of documents (maybe like 4,000).
  - A new GC process deletes ngrams that have been added to the common table from the existing indexes.

Hopefully, this will pare down the ngrams index to something reasonable over time without requiring any manual tuning.

Test Plan:
  - Ran some queries and indexes.
  - Manually inserted ngrams `xxx` and `yyy` into the ngrams table, searched and indexed, saw them ignored as viable ngrams for search/index.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13000

Differential Revision: https://secure.phabricator.com/D18672
2017-10-03 13:27:42 -07:00
epriestley
3ad727ba78 Guarantee the key_position key is created properly
Summary:
Ref T12987. I was focused on the RefCursor table and overlooked that we need some care on this key.

It's currently possible to run `bin/storage upgrade --no-adjust`, then start Phabricator, and end up with duplicate records in this table. If you try to run `bin/storage adjust` later, it will try to add the unique key but fail. This is unusual for normal installs (they usually do not use `--no-adjust`) but we do it in the cluster and I did this exact thing on `secure`.

Normally, to avoid this, when a new table with a unique key is introduced, we also add a migration to explicitly add that key.

This is mostly harmless in this case. Fix this mistake (force the table to contain only unique rows; add the key) and try using `LOCK TABLES` to make this atomic. If this doesn't cause problems we can use this in similar situations in the future.

The "alter table may unlock things" warning comes from here:

https://dev.mysql.com/doc/refman/5.7/en/lock-tables.html

It seems like it's fine to issue `UNLOCK TABLES` even if you don't have any locks, so I think this script should always do the right thing now, regardless of ALTER TABLE unlocking or not unlocking tables.

Test Plan: Ran `bin/storage upgrade -f`, saw table end up in the right state. I'll also check this on `secure`, where the starting state is a little messier.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12987

Differential Revision: https://secure.phabricator.com/D18623
2017-09-18 14:00:22 -07:00
epriestley
5cf62f86d7 Remove obsolete columns from RefCursor table
Summary:
Ref T11823. This change isn't standalone, but prepares for the more involved code change by dropping obsolete columns from the RefCursor table and adding the unique key we need to prevent the ambiguous/duplicate refs issue.

This data was moved to the RefPosition table in D18612.

Test Plan: Ran storage upgrade. See next revision for more substantial testing of this change series.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11823

Differential Revision: https://secure.phabricator.com/D18613
2017-09-15 10:21:12 -07:00
epriestley
782b18e7e2 Migrate RefCursor data to RefPosition table
Summary:
Ref T11823. This populates the new RefPosition table based on the existing RefCursor table, and deletes now-duplicate rows in the RefCursor table so the next change can add a unique key.

This change is not standalone, and there need to be separate code updates. I have a rough version of that written, but this migration needs to happen first to test it.

I'll hold this whole series of changes until after the release cut and until the code is updated.

Test Plan: Ran migration, spot-checked database tables. Saw redundant rows remove and correct-looking rows populated into the new RefPosition table.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11823

Differential Revision: https://secure.phabricator.com/D18612
2017-09-15 10:19:32 -07:00
epriestley
9d5a2b3b4f Add a RefPosition table to hold branch/tag positions once the RefCursor table is split
Summary:
Ref T11823. Currently, we have a "RefCursor" table which stores rows like `<branch or tag name, commit it is pointing at>` with some more data.

Because Mercurial can have a single branch pointing at several different places, this table must allow multiple rows with the same branch or tag name.

Among other things, this means there isn't a single PHID which can be used to identify a branch name in a stable way. However, we have several UIs where we want to be able to do this.

Some specific examples where we run into trouble: in Mercurial, if there are 5 heads for "default", that means there are 5 phids. And currently, if someone deletes a branch, we lose the PHID for it. Instead, we'd rather retain it so the whole world doesn't break if you accidentally delete a branch and then fix it a little later.

(I'll likely hold this until the rest of the logic is fleshed out a little more in followup changes.)

Test Plan: Ran `bin/storage upgrade`, saw the table get created without warnings.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11823

Differential Revision: https://secure.phabricator.com/D18602
2017-09-15 10:19:17 -07:00
epriestley
124e580f6e Issue upgrade guidance to rebuild indexes for the Ferret engine
Summary:
Ref T12819. This is shipping, so issue upgrade guidance to instruct installs to rebuild the index.

Also generate a new `quickstart.sql` since we haven't regenerated in a bit and there's been a large amount of table churn fairly recently.

Test Plan: Ran `bin/storage upgrade`, saw guidance notification in UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18594
2017-09-12 12:21:20 -07:00
epriestley
b1b638bd14 Support the Ferret engine in Diffusion
Summary: Ref T12819. More ferret engine support.

Test Plan: Indexed and searched commits and repositories.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18572
2017-09-07 13:41:04 -07:00
epriestley
d8132db75b Support Ferret engine in Pholio
Summary: Ref T12819. Support for Pholio.

Test Plan: Indexed and searched mocks.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18569
2017-09-07 13:25:29 -07:00
epriestley
e0f3de9c64 Support Ferret engine in Calendar
Summary: Ref T12819. Adds ferret engine support for Calendar events.

Test Plan: Indexed and queried calendar events.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18568
2017-09-07 13:25:12 -07:00
epriestley
a25bbc1dca Support Ferret engine in Phriction
Summary: Ref T12819. Adds Ferret engine support.

Test Plan: Indexed and searched for documents.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18567
2017-09-07 13:24:40 -07:00
epriestley
184f201ce2 Support Ferret engine in Projects
Summary: Ref T12819. Adds support for projects.

Test Plan: Indexed and searched for projects.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18566
2017-09-07 13:24:23 -07:00
epriestley
b1703c8801 Support Ferret engine in Phame
Summary: Ref T12819. Mostly straightforward, with a couple of minor query modernization things.

Test Plan: Indexed and searched for posts and blogs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18565
2017-09-07 13:24:07 -07:00
epriestley
c9152b586b Support Ferret engine in Owners
Summary: Ref T12819. Same deal as before, but smaller diffs after D18559.

Test Plan: Indexed and searched for packages.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18564
2017-09-07 13:23:46 -07:00
epriestley
2020c1e7bd Support Ferret engine for Passphrase credentials
Summary: Ref T12819. Adds Ferret support to Passphrase.

Test Plan: Indexed credentials, searched for credentials.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18556
2017-09-07 13:23:13 -07:00
epriestley
f23717b416 Support Ferret engine in Fund initiatives
Summary: Ref T12819. Adds Ferret engine support to initiatives.

Test Plan: Indexed and searched for initiatives.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18555
2017-09-07 13:22:57 -07:00
epriestley
3ff9d4a4ca Support Ferret engine for searching users
Summary:
Ref T12819. Adds support for indexing user accounts so they appear in global fulltext results.

Also, always rank users ahead of other results.

Test Plan: Indexed users. Searched for a user, got that user.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18552
2017-09-07 13:22:12 -07:00
epriestley
f40f3ca74c Add Ferret engine index support to Differential
Summary: Ref T12819. Adds storage and indexing for the Ferret engine to Differential.

Test Plan: Ran `bin/search index D123 --force`, saw indexes appear in database. No UI/user impact yet.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18540
2017-09-05 16:45:37 -07:00
epriestley
0e2e525bb4 Add a "terms" corpus to Ferret fields
Summary:
Ref T12819. Ferret currently does substring search, but this is not the default mode users expect: when you search for the "RICO" act, you do not expect to find documents containing "apRICOt" even though "RICO" is a substring.

To support term search, index the corpus as a list of terms with puncutation removed and whitespace normalized so the engine can match against it.

Test Plan:
Ran `storage upgrade`, ran `search index`, saw sensible database results:

```
   rawCorpus: This is the task description.

Hark! Whom'st'dve eaten this "food" shall surely ~perish~?? #blessed
normalCorpus: thi the task descript hark whom dve eaten food shall sure perish bless
  termCorpus:  This is the task description Hark Whom'st'dve eaten this food shall surely perish blessed
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18498
2017-08-30 11:29:14 -07:00
epriestley
77ef38f9a8 Aggregate corpus data in Ferret field rows
Summary:
Ref T12819. This addresses two issues:

  - One practical issue is that right now, if you search for "dog cat", and they appear in different fields (for example, "dog" appears ONLY in the title, while "cat" appears ONLY in a comment) we won't find the document. This is somewhat rare -- usually, if "dog" appears in the title, it's also repeated in the description -- but I think clearly a bug. To attack this, start automatically creating a virtual "ALL" field with the full document text which we'll use as the primary thing we match against.
  - For fields which may occur more than once -- today, only comments -- aggregate them all into one big "all of the text" row instead of writing one row per comment. This partly addresses the first point ("dog" in one comment and "cat" in a different comment won't be found) and partly makes some of the query gymnastics easier.

Test Plan:
Ran `bin/storage upgrade`, ran `bin/search index <Txxx>`, saw sensible corpus values in the database:

```
mysql> select * from maniphest_task_ffield\G
*************************** 1. row ***************************
          id: 3
  documentID: 1981
    fieldKey: full
   rawCorpus: This is the task title
This is the task description.
normalCorpus: thi the task titl
thi the task descript
*************************** 2. row ***************************
          id: 4
  documentID: 1981
    fieldKey: titl
   rawCorpus: This is the task title
normalCorpus: thi the task titl
*************************** 3. row ***************************
          id: 5
  documentID: 1981
    fieldKey: body
   rawCorpus: This is the task description.
normalCorpus: thi the task descript
3 rows in set (0.00 sec)
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18497
2017-08-30 11:28:30 -07:00
epriestley
f97157e7ed Build a prototype fulltext engine ("Ferret") using only basic MySQL primitives
Summary:
Ref T12819. I gave this stuff a sweet code name because all the terms related to "fulltext" and "search" already mean 5 different things. It, uh, ferrets out documents for you?

I'm building this to work a lot like the existing ngram index, which seems to work pretty well. If this sticks, it will auto-resolve the join issue (in T12443) by letting us do the entire thing locally in a JOIN and thus dodge a lot of mess.

This index gets built alongside other indexes, but only shows up in the UI if you have prototypes enabled. If you do, it appears under the existing fulltext field in Maniphest. No existing functionality is affected or disrupted.

NOTE: The query engine half of this is still EXTREMELY primitive, and this probably performs worse than the existing field for now. If this doesn't show obvious signs of being awful on `secure` I'll improve that in followup changes.

Test Plan:
Indexed my tasks, ran some simple queries, got the results I wanted, even for queries "ko", "k", "v0.1".

{F5147746}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819, T12443

Differential Revision: https://secure.phabricator.com/D18484
2017-08-28 14:52:59 -07:00
Chad Little
79c6b50049 Fix fatal on logged out Phame Post
Summary: Just deletes the view code until I have time to better plan this out, or just not ship.

Test Plan: Visit Phame post on public logged out page, view count doesnt cause transaction fatal.

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18475
2017-08-25 08:47:59 -07:00
epriestley
bc0963d54b Remove rows for personal saved builtin queries
Summary:
Ref T12956. After this change, individual users will no longer be able to modify builtin queries on a user-by-user basis: they will always appear at the bottom of the list, under their personal queries, and can only be managed by administrators.

To support this, clean up the old rows which could be hanging around from before: delete any personal saved queries where the saved query is a builtin query.

To ease this transition, try to pin the query we're deleting //if// the user had reordered things to put it on top.

Test Plan:
  - Ran the migration, saw no changes in the UI but fewer rows.
  - Went back to `master`, reordered queries to put a builtin one on top.
  - Ran the migration.
  - Saw that builtin one drop to the bottom (since it can't be on top anymore) but be pinned, preserving the behavior of `/maniphest/`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12956

Differential Revision: https://secure.phabricator.com/D18464
2017-08-24 15:25:00 -07:00
epriestley
58b889c5b0 Make the default ApplicationSearch query explicit, not just the first item in the list
Summary:
Ref T12956. Currently, when you visit `/maniphest/` (or any other ApplicationSearch application) we execute the first query in the list by default.

In T12956, I plan to make changes so that personal queries are always first, then global/builtin queries. Without changing the "default query" rule, this will make it harder to have, for example, some custom queries in Differential but still run a global query like "Active" by default. To make this work, you'd have to save a personal copy of the "Active" query, then put it at the top.

This feels a bit cumbersome and this rule is kind of implicit and a little weird anyway. To make this work a little better as we make changes here, add an explicit pinning action, like the one we have in Project ProfileMenus.

You can now explicitly choose a query to make default.

Test Plan:
  - Browsed without pinning anything, saw normal behavior.
  - Pinned queries, viewed `/maniphest/`, saw a non-initial query selected by default.
  - Pinned a query, deleted it, nothing exploded.

{F5098484}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12956

Differential Revision: https://secure.phabricator.com/D18422
2017-08-24 15:21:00 -07:00
Chad Little
e40c002a6d Add a basic view count to Phame
Summary: This adds a very very basic view count to Phame, so bloggers can get some idea which posts are more popular than others. Anything more than this I think should be Facts or Google Analytics.

Test Plan: Write a new post, see post count. Reload page, post count goes up. Archive post, post count stays the same.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18446
2017-08-21 14:03:21 -07:00
epriestley
48a74de0b6 Move all revision status transactions to modern values and mechanics
Summary:
Ref T2543. This updates and migrates the status change transactions:

  - All storage now records the modern modular transaction ("differential.revision.status"), not the obsolete non-modular transaction ("differential:status").
  - All storage now records the modern constants ("accepted"), not the obsolete numeric values ("2").

Test Plan:
  - Selected all the relevant rows before/after migration, data looked sane.
  - Browsed around, reviewed timelines, no changes after migration.
  - Changed revision states, saw appropriate new transactions in the database and timeline rendering.
  - Grepped for `differential:status`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18419
2017-08-12 04:05:57 -07:00
epriestley
7b695aa43b Migrate revision storage to modern status constants ("accepted") instead of legacy numeric values ("2")
Summary:
Ref T2543. Rewrites all the storage to use constants.

Note that transactions still use legacy values, I'll migrate and update them separately.

Test Plan:
  - Ran migration.
  - Browsed around, changed revision states, viewed dashboard, etc.
  - Selected `DISTINCT()` and `GROUP_CONCAT()` of the `status` field in the database, saw sane/expected before and after values.
  - Verified that old Conduit methods still return numeric constants for compatibility.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18418
2017-08-12 04:02:10 -07:00
epriestley
212d4d0dc7 Migrate Differential Revision SavedQueries to the new "statuses" tokenizer
Summary: Ref T2543. This migrates existing saved queries so they use the right modern values for the new tokenizer control, introduced in D18393.

Test Plan:
  - Saved a query with "Abandoned" selected as the status in the old "<select />", prior to D18393.
  - Upgraded to D18393, which broke the query (it no longer selected any status filter).
  - Ran the migration to fix things.
  - Saw the query now execute with "Abandoned" selected in the tokenizer, preseving the original behavior accurately.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18394
2017-08-11 13:36:37 -07:00
Chad Little
e7f94d7528 Properly version Legalpad documents
Summary: Fixes T12933. This now creates a new DocumentBody when creating or editing a legalpad document.

Test Plan:
Create a new document, edit document. Check database that version is saved as new row, and timestamps are correct.

```mysql> select * from legalpad_documentbody;
+----+--------------------------------+--------------------------------+--------------------------------+---------+---------------+--------+-------------+--------------+
| id | phid                           | creatorPHID                    | documentPHID                   | version | title         | text   | dateCreated | dateModified |
+----+--------------------------------+--------------------------------+--------------------------------+---------+---------------+--------+-------------+--------------+
|  1 | PHID-LEGB-nsgzqklzfmjahlcgobm7 | PHID-USER-72xwu7eurrpsu2kxgrvw | PHID-LEGD-v7mc3xyithjvbiqeksbj |       2 | Legal Title 1 | Body 2 |  1501037011 |   1501037081 |
|  2 | PHID-LEGB-2kaytwmjusljib6pjycc | PHID-USER-72xwu7eurrpsu2kxgrvw | PHID-LEGD-v7mc3xyithjvbiqeksbj |       3 | Legal Title 1 | Body 3 |  1501037521 |   1501037521 |
|  3 | PHID-LEGB-h6q6bi42w4rgxrhk3qdb | PHID-USER-72xwu7eurrpsu2kxgrvw | PHID-LEGD-7gxuhafvkoy2izkv4gdd |       1 | New 2         | asdf   |  1501037553 |   1501037553 |
+----+--------------------------------+--------------------------------+--------------------------------+---------+---------------+--------+-------------+--------------+
3 rows in set (0.00 sec)```

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: tmakarios, Korvin

Maniphest Tasks: T12933

Differential Revision: https://secure.phabricator.com/D18280
2017-07-26 09:29:56 -07:00
Austin McKinley
8008ade9af Use keywords instead of ints to update task priority in ManiphestEditEngine
Summary: Fixes T12124. Changes `ManiphestEditEngine` to populate the select using priority keywords instead of the integer value. Marks `maniphest.querystatuses` as frozen. Adds a new Conduit method for fetching potential task statuses.

Test Plan: Created tasks and changed their priorities, observed that transactions in the DB still have the same type (integers as strings). Invoked `maniphest.update` with `priority => '90'` and observed that it still works. Invoked `maniphest.edit` with `priority => 'unbreak'` and observed that it now works.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T12124

Differential Revision: https://secure.phabricator.com/D18111
2017-06-14 14:43:03 -07:00
Chad Little
83a89166ee Add profile images to Repositories
Summary: Builds out some images to use to identify repositories. Fixes T12825.

Test Plan:
Try setting custom, built in, and null images.

{F4998175}

{F4998192}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12825

Differential Revision: https://secure.phabricator.com/D18116
2017-06-12 07:51:39 -07:00
epriestley
8ef9490f66 Add a retroactive migration to expand the contentHash field
Summary:
See D18037. The migration there may cause us to write new file records as a side effect.

Ideally, we would rewrite that migration to not ever have this kind of side effect. However, that would make it much more complicated, and it's already very complicated.

Instead, retroactively expand the size of this field before `storage adjust` does it, so it has the right size by the time we hit the migration in D18037.

Test Plan:
@chad, can you `arc patch` this and see if it works?

It's possible that it will get us about five lines deeper and then we'll just hit another similar exception, and that this isn't really a viable way forward.

Reviewers: chad, amckinley

Reviewed By: amckinley

Subscribers: amckinley, chad

Differential Revision: https://secure.phabricator.com/D18107
2017-06-08 07:42:40 -07:00
Austin McKinley
fb9d036e57 Show task duplicates as related objects in Maniphest and migrate old duplicates
Summary: Does the UI work that's part of T12234 and adds migrations for both of the old-style duplicate transactions.

Test Plan:
 - Started with a clean DB.
 - Checked out really old code that marks tasks as dupes using comments.
 - Made a bunch of tasks and closed some as dupes. Made a bunch of additional comments.
 - Checked out D10427 and did a `storage upgrade`.
 - Made a bunch more new tasks and dupes.
 - Snapshotted DB.
 - Ran migration repeatedly until all expected edges showed up in the `phabricator_maniphest.edge`table.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T12234

Differential Revision: https://secure.phabricator.com/D18037
2017-06-07 13:30:20 -07:00
Austin McKinley
04fd93e51e Drop DifferentialDraft storage
Summary: Fixes T12104.

Test Plan: Ran `bin/storage upgrade` and observed table dun got dropped.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T12104

Differential Revision: https://secure.phabricator.com/D18034
2017-05-26 13:59:26 -07:00
Austin McKinley
b27c2ed6d1 Index Project milestones to accurately reflect milestone membership
Summary: Fixes T12505. `PhabricatorProjectsMembershipIndexEngineExtension->materializeProject()` was incorrectly bailing early for milestone objects, which prevented milestone members from being calculated correctly. This was causing problems where (for example) an Owners package owned by a milestone wasn't being satisfied when a member of the milestone approved a revision.

Test Plan: Invoked migration, observed that a user's milestones correctly showed up when searched for. Also observed that accepting a revision on behalf of a milestone now satisfies Owners rules.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12505

Differential Revision: https://secure.phabricator.com/D18033
2017-05-26 13:10:41 -07:00
epriestley
7e91f42b02 Issue commands to Nuance items, at least roughly
Summary:
Ref T12738. This makes clicking "Throw In Trash" technically do something, sort of.

In Nuance, the default mode of operation for actions is asynchronous -- so you don't have to wait for a response from Twitter or GitHub after you mash the "send default reply tweet" / "close this pull request with a nice response" button and can move directly to the next item instead.

In the future, some operations will attempt to apply synchronously (e.g., local actions like "ignore this item forever"). This fakes our way through that for now.

There's also no connection to the action actually doing anything yet, but I'll probably rig that up next.

Test Plan: {F4975227}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12738

Differential Revision: https://secure.phabricator.com/D18010
2017-05-24 11:04:57 -07:00
epriestley
272a5d668f Fix a handful of Nuance fatals
Summary: Ref T12738. Some of the Nuance "form" workflows currently fatal after work on the GitHub stuff. Try to make everything stop fataling, at least.

Test Plan: Using "Complaints Form" no longer fatals, and now lodges a complaint instead.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12738

Differential Revision: https://secure.phabricator.com/D18007
2017-05-24 11:02:55 -07:00
Chad Little
d6a620be45 Update Maniphest for modular transactions
Summary: Ref T12671. This modernized Maniphest transactions to modular transactions.

Test Plan:
- Create Task
- Edit Task
- Raise Priority
- Change Status
- Merge as a duplicate
- Create Subtask
- Claim Task
- Assign Project
- Move on Workboard
- Set a cover image
- Assign story points
- Change story points
- Generate lots via lipsum
- Bulk edit tasks
- Leave comments
- Award Token

I'm sure I'm missing something.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: hazelyang, Korvin

Maniphest Tasks: T12671

Differential Revision: https://secure.phabricator.com/D17844
2017-05-15 10:29:06 -07:00
Austin McKinley
0dc90b891a Reimplement Slowvote transactions using modular transactions
Summary:
Fixes T12623. Adds new modular transactions to Slowvote. Also converts
the `shuffle` column to `bool` for consistency with other boolean-ish columns.

Test Plan:
Create a new vote, modified everything that could be modified from the web UI,
observed expected timeline.

Example timeline: {F4938843}

Example transaction values in DB: {F4938850}

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T12623

Differential Revision: https://secure.phabricator.com/D17830
2017-05-04 20:20:00 -07:00
epriestley
d2baa88171 Allow Owners packages to have arbitrarily long names
Summary:
  - Change column type from `sort128` to `sort`.
  - Remove `originalName`. This column is unused. Long ago, we used it to generate a `Thread-Topic` header for mail, but just use PHIDs now (the value just needs to be stable for a given object, users normally don't see it).

Test Plan:
  - Created a package with a beautifully long name. Magnificent!
  - Grepped for `originalName` / `getOriginalName()`, found no Owners hits.
  - Verified that there isn't any name-length validation code to remove.

{F4925637}

Reviewers: chad, amckinley

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17798
2017-04-27 18:04:03 -07:00
epriestley
e2a94019b1 Migrate accounts to correct user email verification state flag
Summary:
Depends on D17785. Fixes T12635. There was a bug where users could verify their primary email without getting the "isEmailVerified" flag set on their accounts.

D17785 fixes this bug. This change migrates affected account to fix their state, now that they can't get in trouble any more (hopefully).

Test Plan:
  - Explicitly removed this flag from a bunch of accounts.
  - Ran migration, saw the accounts get fixed.
  - Ran migration again (`storage upgrade --apply ...`), saw the accounts not get touched.
  - We have 117 affected accounts on `secure`, so I'll verify that this fixes them.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12635

Differential Revision: https://secure.phabricator.com/D17786
2017-04-24 10:24:51 -07:00
epriestley
52c4715bbc No-op the Conpherence thumbnail resizing migration
Summary:
Fixes T12628. After later changes to `PhabricatorFile`, this migration no longer runs if you upgrade through it to a recent `HEAD` while your data has some room images.

Since this isn't critical and has been available for ~6 months, I just nuked it as a first pass. I can find a more careful approach which lets us continue to run this migration instead if you're hesitant to skip this step, although it may be a little involved.

In 95% of cases we avoid this by updating the storage table as it existed at the time the migraiton ran, but Files are much too complicated for that to be realistic.

Test Plan: Ran `bin/storage upgrade -f --apply phabricator:20161005.conpherence.image.2.php`, saw it do nothing.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12628

Differential Revision: https://secure.phabricator.com/D17770
2017-04-23 10:46:09 -07:00
epriestley
95dd9dbf43 Make Applications extend LiskDAO
Summary:
Ref T11476. This is a bit hacky, but makes `Application` extend `LiskDAO` so we can apply transactions to it with an `Editor` class.

Also fixes schema stuff so builds should produce a clean bill of health again.

This might only get you slightly further, yell if you run into more trouble.

Test Plan:
  - Ran `bin/storage upgrade -f` and got no warnings.
  - Browsed around, nothing exploded?

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11476

Differential Revision: https://secure.phabricator.com/D17738
2017-04-19 16:06:14 -07:00
Austin McKinley
febd68039f Add initial infrastructure for adding ModularTransaction support to Application config changes
Summary: Part of the groundwork for T11476.

Test Plan: ran `./bin/storage upgrade` and observed expected DB tables

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11476

Differential Revision: https://secure.phabricator.com/D17736
2017-04-19 15:44:57 -07:00
epriestley
76d0b67d91 Remove "dateTouched" from ConpherenceParticipant
Summary:
Pathway to D17685. This column is (mostly) a denormalization of `dateModified` on the thread.

Just use a JOIN instead.

This isn't //exactly// the same: we'll bump threads to the top now for non-message changes (e.g., a topic or title change). That seems fine, but we could put a `lastMessageDate` on Thread later if we want to refine it.

Also got rid of a lot of other unused stuff. There's a big garbage TODO here, I'll fix that in the next change.

Test Plan:
  - Grepped for `dateTouched`.
  - Grepped for `participantCursor`.
  - Grepped for `ConpherenceParticipantQuery::LIMIT`.
  - Looked for callsites to `setOrder()`, found none.
  - Added a message to an older thread, saw it bump up to the top.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17731
2017-04-19 13:58:54 -07:00
epriestley
0a335f91cd Remove "participationStatus" from ConpherenceParticipant
Summary:
Pathway to D17685. This column is a very complicated cache of: is participant.messageCount equal to thread.messageCount?

We can just ask this question with a JOIN instead and simplify things dramatically.

Test Plan:
  - Ran migration.
  - Browsed around.
  - Sent a message, saw unread count go up.
  - Read the message, saw unread count go down.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17730
2017-04-19 13:58:42 -07:00
epriestley
c96e17f5ea Remove "behindTransactionPHID" from ConpherenceParticipant
Summary: Pathway to D17685. Nothing reads this field and it has no use or value.

Test Plan:
  - Ran migration.
  - Grepped for `behindTransactionPHID`.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17729
2017-04-19 13:58:29 -07:00
Austin McKinley
be00264ae7 Make daemons perform file deletion
Summary:
Deletion is a possibly time-intensive process, especially with large
files that are backed by high-latency, chunked storage (such as
S3). Even ~200mb objects take minutes to delete, which makes for an
unhappy experience. Fixes T10828.

Test Plan:
Delete a large file, and stare in awe of the swiftness with
which I am redirected to the main file application.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: thoughtpolice, Korvin

Maniphest Tasks: T10828

Differential Revision: https://secure.phabricator.com/D15743
2017-04-18 11:09:41 -07:00
Austin McKinley
976fbee877 Implement ngram search for File objects
Summary: Follows the outline in D15656 for implementing ngram search for names of File objects. Also created FileFullTextEngine, because without implementing `PhabricatorFulltextInterface`, `./bin/search` complains that `File` is not an indexable type.

Test Plan:
  - ran `./bin/storage upgrade` to apply the schema change
  - confirmed the presence of a new `file_filename_ngrams` table
  - added a couple file objects
  - ran `bin/search index --type file --force`
  - confirmed the presence of rows in `file_filename_ngrams`
  - did a few keyword searches and saw expected results

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T8788

Differential Revision: https://secure.phabricator.com/D17702
2017-04-17 17:37:20 -07:00
Chad Little
5587abf04c Remove recentParticipants from ConpherenceThread
Summary: We no longer display this any more in the UI, so go ahead and remove the callsites and db column.

Test Plan: New Room, with and without participants.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17683
2017-04-13 13:55:08 -07:00
Chad Little
a9845b0b1d Remove picture crop transaction from Conpherence
Summary: Fixes T11730. Removes an old transaction that hasn't been used in a year.

Test Plan: Run sql, check various rooms.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11730

Differential Revision: https://secure.phabricator.com/D17666
2017-04-12 14:18:54 -07:00
epriestley
d4d46501ed Add a repair migration to fix legacy Calendar events without "utcInstanceEpoch"
Summary: Fixes T12488. Some events appear to have survived earlier migrations without getting completely fixed. Fix them.

Test Plan:
  - Ran migration locally with `bin/storage upgrade` (but: I could not reproduce this problem locally).
  - Ran migration in production and saw ICS import stop fataling.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12488

Differential Revision: https://secure.phabricator.com/D17642
2017-04-10 08:47:07 -07:00
epriestley
d450a08890 Support HMAC+SHA256 with automatic key generation and management
Summary:
Ref T12509. This adds support for HMAC+SHA256 (instead of HMAC+SHA1). Although HMAC+SHA1 is not currently broken in any sense, SHA1 has a well-known collision and it's good to look at moving away from HMAC+SHA1.

The new mechanism also automatically generates and stores HMAC keys.

Currently, HMAC keys largely use a per-install constant defined in `security.hmac-key`. In theory this can be changed, but in practice essentially no install changes it.

We generally (in fact, always, I think?) don't use HMAC digests in a way where it matters that this key is well-known, but it's slightly better if this key is unique per class of use cases. Principally, if use cases have unique HMAC keys they are generally less vulnerable to precomputation attacks where an attacker might generate a large number of HMAC hashes of well-known values and use them in a nefarious way. The actual threat here is probably close to nonexistent, but we can harden against it without much extra effort.

Beyond that, this isn't something users should really have to think about or bother configuring.

Test Plan:
  - Added unit tests.
  - Used `bin/files integrity` to verify, strip, and recompute hashes.
  - Tampered with a generated HMAC key, verified it invalidated hashes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12509

Differential Revision: https://secure.phabricator.com/D17630
2017-04-06 15:42:59 -07:00
epriestley
aea46e55da Fix an issue where "Request Review" of a fully-accepted revision would transition to "Accepted"
Summary:
Ref T10967. This is explained in more detail in T10967#217125

When an author does "Request Review" on an accepted revision, void (in the sense of "cancel out", like a bank check) any "accepted" reviewers on the current diff.

Test Plan:
  - Create a revision with author A and reviewer B.
  - Accept as B.
  - "Request Review" as A.
  - (With sticky accepts enabled.)
  - Before patch: revision swithced back to "accepted".
  - After patch: the earlier review is "voided" by te "Request Review", and the revision switches to "Review Requested".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17566
2017-03-28 11:50:15 -07:00
Mukunda Modell
e41c25de50 Support multiple fulltext search clusters with 'cluster.search' config
Summary:
The goal is to make fulltext search back-ends more extensible, configurable and robust.

When this is finished it will be possible to have multiple search storage back-ends and
potentially multiple instances of each.

Individual instances can be configured with roles such as 'read', 'write' which control
which hosts will receive writes to the index and which hosts will respond to queries.

These two roles make it possible to have any combination of:

* read-only
* write-only
* read-write
* disabled

This 'roles' mechanism is extensible to add new roles should that be needed in the future.

In addition to supporting multiple elasticsearch and mysql search instances, this refactors
the connection health monitoring infrastructure from PhabricatorDatabaseHealthRecord and
utilizes the same system for monitoring the health of elasticsearch nodes. This will
allow Wikimedia's phabricator to be redundant across data centers (mysql already is,
elasticsearch should be as well).

The real-world use-case I have in mind here is writing to two indexes (two elasticsearch clusters
in different data centers) but reading from only one. Then toggling the 'read' property when
we want to migrate to the other data center (and when we migrate from elasticsearch 2.x to 5.x)

Hopefully this is useful in the upstream as well.

Remaining TODO:

* test cases
* documentation

Test Plan:
(WARNING) This will most likely require the elasticsearch index to be deleted and re-created due to schema changes.

Tested with elasticsearch versions 2.4 and 5.2 using the following config:

```lang=json
  "cluster.search": [
    {
      "type": "elasticsearch",
      "hosts": [
        {
          "host": "localhost",
          "roles": { "read": true, "write": true }
        }
      ],
      "port": 9200,
      "protocol": "http",
      "path": "/phabricator",
      "version": 5
    },
    {
      "type": "mysql",
      "roles": { "write": true }
     }
  ]

Also deployed the same changes to Wikimedia's production Phabricator instance without any issues whatsoever.
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Tags: #elasticsearch, #clusters, #wikimedia

Differential Revision: https://secure.phabricator.com/D17384
2017-03-26 08:16:47 +00:00
epriestley
aa91dc992e Record which user accepted on behalf of packages/owners reviewers
Summary:
Ref T12271. Don't do anything with this yet, but store who accepted/rejected/whatever on behalf of reviewers.

In the future, we could use this to render stuff like "Blessed Committers (accepted by epriestley)" or whatever. I don't know that this is necessarily super useful, but it's easy to track, seems likely to be useful, and would be a gigantic pain to backfill later if we decide we want it.

Test Plan: Accepted/rejected a revision, saw reviewers update appropriately.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12271

Differential Revision: https://secure.phabricator.com/D17537
2017-03-22 14:26:37 -07:00
epriestley
8ad5d28686 Migrate old reviewer edges to new storage
Summary:
Ref T10967. We still have double writes, so all reviewers are being written to both old and new storage. This migrates all the data in the old storage to the new storage, so both storage tables should have a complete set of data and be getting identical updates as we move forward.

After this, I can move readers over one at a time and eventually get rid of the old writes and old storage.

This loads all of the edge data into memory in a big chunk. I reached out to one install to get some more information about their data size. Ours is quite manageable and I think even large installs will probably fit into memory, but we can do this in chunks if not.

However, because the Edge table doesn't have an `id` column, we can't use either the `RawMigrationIterator` or the `MigrationIterator`, and would need to write a new `EdgeMigrationIterator`. This isn't tons of work but might not be necessary.

Test Plan: Ran the migration locally, spot-checked the results in the database for sanity and correctness.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17515
2017-03-20 16:44:40 -07:00
epriestley
794b456530 Store "last comment" and "last action" diffs on reviewers
Summary:
Ref T10967. We have a "commented" state to help reviewers get a better sense of who is part of a discussion, and a "last action" state to help distinguish between "accept" and "accepted an older version", for the purposes of sticky accepts and as a UI hint.

Currently, these are first-class states, partly beacuse we were somewhat limited in what we could do with edges. However, a more flexible way to represent them is as flags separate from the primary state flag.

In the new storage, write them as separate state information: `lastActionDiffPHID` stores the Diff PHID of the last review action (accept, reject, etc). `lastCommentDiffPHID` stores the Diff PHID of the last comment (top-level or inline).

Test Plan: Applied storage changes, commented and acted on a revision. Saw appropriate state reflected in the database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17514
2017-03-20 16:44:05 -07:00
epriestley
7626ec0ce1 Correct an issue where "View Raw File" in Differential generated a file with overbroad permissions
Summary:
Via HackerOne. When you view a raw file in Differential, we currently generate a permanent file with default permissions. This may be incorrect: default permissions may be broader than the diff's permissions.

The other three methods of downloading/viewing raw files ("Download" in Diffusion and Differential, "View Raw" in Diffusion and Differential) already apply policies correctly and generate temporary files. However, this workflow was missed when other workflows were updated.

Beyond updating the workflow, delete any files we've generated in the past. This wipes the slate clean on any security issues and frees up a little disk space.

Test Plan:
  - Ran migration script, saw existing files get purged.
  - Did "View Raw File", got a new file.
  - Verified that the file was temporary and properly attached to the diff, with "NO ONE" permissions.
  - Double-checked that Diffusion already runs policy logic correctly and applies appropriate policies.
  - Double-checked that "Download Raw Diff" in Differential already runs policy logic correctly.
  - Double-chekced that "Download Raw Diff" in Diffusion already runs policy logic correctly.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17504
2017-03-16 09:51:48 -07:00
epriestley
251ee9b660 Add dedicated "reviewers" storage to Differential and do double writes
Summary:
Ref T10967. This is an incremental step toward removing "reviewers" back to a dedicated storage table so we can handle changes like T11050.

This adds the storage table, and starts doing double writes to it (so new or updated reviewers write to both the old edge table and the new "reviewers" table).

Then we can do a migration, swap readers over one at a time, and eventually remove the old write and old storage and then implement new features.

This change has no user-facing impact, it just causes us to write new data to two places instead of one.

This is not completely exhaustive: the Herald "Add Reviewers" action is still doing a manual EDGE transaction. I'll clean that up next and do another pass to look for anything else I missed.

This is also a bit copy/pastey for now but the logic around "RESIGN" is a little different in the two cases until T11050. I'll unify it in future changes.

Test Plan:
  - Did a no-op edit.
  - Did a no-op comment.
  - Added reviewers.
  - Removed reviewers.
  - Accepted and rejected revisions.

After all of these edits, did a `SELECT * FROM differential_reviewer` manually and saw consistent-looking rows in the database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17495
2017-03-14 11:51:51 -07:00
Chad Little
f2e013c2e9 Prep user table for default images
Summary: Ref T10319. Adds in database columns for upcoming default generated avatar support.

Test Plan: Ran storage upgrade, log into local site to verify it didn't blow up.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10319

Differential Revision: https://secure.phabricator.com/D17459
2017-03-04 08:18:07 -08:00
epriestley
dc7ecf5875 Add "subtype" storage to Maniphest tasks
Summary: Ref T12314. Provides a field on tasks for storing subtypes. Does nothing interesting yet.

Test Plan:
  - Ran storage upgrade.
  - Created some tasks.
  - Looked in the database.
  - Used Conduit to query some tasks.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17441
2017-03-02 04:17:47 -08:00
epriestley
91ef237290 Add a "subtype" field to EditEngine forms
Summary:
Ref T12314. This adds storage so EditEngine forms can later be marked as edit fields for particular types of objects (like an "animal edit form" vs a "plant edit form").

We'll take you to the right edit form when you click "Edit" by selecting among forms with the same subtype as the task.

This doesn't do anything very interesting on its own.

Test Plan:
  - Ran `bin/storage upgrade`.
  - Verified database got the field with proper values.
  - Created a new form, checked the database.
  - Ran unit tests.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17439
2017-03-02 04:16:27 -08:00
epriestley
ad01e26af7 Redesign Home/Profile/Projects side navigation
Summary: Ref T11957. Needs some more polish, but I think everything here is square.

Test Plan: Add personal/global items to home, test mobile. Test workboards / colors.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: 20after4, rfreebern, Korvin

Maniphest Tasks: T11957

Differential Revision: https://secure.phabricator.com/D17259
2017-01-31 08:59:01 -08:00
Chad Little
58c857a681 Remove motivator panel
Summary: Removes the often funny, but never really used but will cause us bug reports someday.... cat facts.

Test Plan: Install cat facts, run storage upgrade, see no cat facts in menu.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12126

Differential Revision: https://secure.phabricator.com/D17233
2017-01-19 14:55:19 -08:00
epriestley
11f3a8cfca Force 'changes' to a string in hunk migration
Summary: Fixes T12090. In obscure situations lost to the mists of time, the `changes` column could be `null`. Force a string cast so the migration finishes, even though these changesets are likely meaningless.

Test Plan:
I did a force-reapply as a sanity check:

```
$ ./bin/storage upgrade -f --apply phabricator:20161213.diff.01.hunks.php
```

That went cleanly; it would only have caught dramatic errors, but I didn't completely butcher things.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12090

Differential Revision: https://secure.phabricator.com/D17168
2017-01-10 12:28:09 -08:00
epriestley
2dfe79cfc7 When updating revisions in response to commits, reuse previously generated diffs
Summary:
Fixes T10968. In rare situations, we can generate a diff, then hit an error which causes this update to fail.

When it does, we tend to get stuck in a loop creating diffs, which can fill the database up with garbage. We saw this once in the Phacility cluster, and one instance hit it, too.

Instead: when we create a diff, keep track of which commit we generated it from. The next time through, reuse it if we already built it.

Test Plan:
  - Used `bin/differential attach-commit <commit> <revision>` to hit this code.
  - Simulated a filesystem write failure, saw the diff get reused.
  - Also did a normal update, which worked properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10968

Differential Revision: https://secure.phabricator.com/D17164
2017-01-09 12:13:44 -08:00
Chad Little
8a85ee7c15 Add CustomPHID to PhabricatorProfileMenuEngineConfiguration
Summary: Ref T5867, adds a customPHID field, nullable, and lets you query by it... i think? Not fully able to grok all the EditEngine stuff, but I think this is the right place for the query.

Test Plan: Not wired to anything, but pulling up project menu, editing, all still works.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T5867

Differential Revision: https://secure.phabricator.com/D17149
2017-01-07 10:49:54 -08:00
Sébastien Santoro
178466b826 Fix 20160122.project.1.boarddefault.php database upgrade patch
Summary: Fixes T12031.

Test Plan: Upgraded a December 2015 Phabricator installation to current schema.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T12031

Differential Revision: https://secure.phabricator.com/D17094
2016-12-19 12:52:55 -08:00
Chad Little
c6bdd2c56b Add Ngram support to Dashboards / Panels
Summary: Build ngram indexs, adds search by name capability.

Test Plan: Search for a dashboard by partial name, search for a panel by partial name.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17090
2016-12-16 12:09:06 -08:00
epriestley
3d8d98bd8d Probably (?) fix two very old project slug migrations
Summary: Fixes T12020. These callsites to `getPhrictionSlug()` were missed when that method was removed. They're very old (early 2014, late 2011).

Test Plan:
These are tricky to test because the migrations are so ancient, but `bin/storage upgrade --force --apply phabricator:20140521.projectslug.2.mig.php` gave me //plausible// results.

The other migration is so ancient that it can't apply to a modern database so I'm just kind of winging that one. We probably have essentially no installs which will ever apply it again, though.

Reviewers: chad, avivey

Reviewed By: avivey

Maniphest Tasks: T12020

Differential Revision: https://secure.phabricator.com/D17060
2016-12-14 14:14:21 -08:00
epriestley
c67e87c809 Force hunk storage migration to the modern format
Summary:
Ref T8475. This forces installs to migrate hunks to the modern format.

We stopped writing to the legacy format a very long time ago (2+ years?) without issues.

This doesn't destroy any data. T8623 has guidance and I'll publish more changelog guidance.

Test Plan: Faked some legacy data and migrated it.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8475

Differential Revision: https://secure.phabricator.com/D17039
2016-12-13 14:34:08 -08:00
epriestley
9d2feacfab Rename a missed Profile -> MenuItem
Summary: Didn't grep this good enough.

Test Plan: `bin/storage upgrade -f --apply ..`, got a clean apply.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17046
2016-12-13 14:27:00 -08:00
Chad Little
26127b9c5f Allow Dashboards to set an icon
Summary: Allows users set an icon (for reuse on upcoming home) for their dashboard based on 16 descriminating choices.

Test Plan: Create a new dashboard, set new icon. Edit an existing dashboard, set icon.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17042
2016-12-13 11:30:22 -08:00
Chad Little
c03a412d5c Add authorPHID to Dashboard Panels
Summary: Adds authorPHID to panels so we can default to the panels you made.

Test Plan: Run upgrade, visit manage panels, see my panels. Create a new panel. Edit a panel.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17036
2016-12-13 10:07:16 -08:00
epriestley
776a1f3aec Rename dashboard author patches so that the backfill patch happens
after the column it is adding is created

Auditors: chad
2016-12-12 15:44:28 -08:00
Chad Little
59f3b5125d Add authorPHID to Dashboards
Summary: Adds an authorPHIDs, populates olds ones.

Test Plan: Make a new Dashboard, see that I created it.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17022
2016-12-12 15:26:43 -08:00
epriestley
8480776ccd Rename "ProfilePanelConfiguration" to "ProfileMenuItemConfiguration"
Summary:
Ref T11957. This renames the Configuration storage, transaction, query, and PHID type.

No rename on the actual menu item types yet, that's next (and should be the end of this, I think).

Test Plan:
  - Viewed projects.
  - Viewed profiles.
  - Edited a project menu.
  - Grepped for all renamed symbols, I think?

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11957

Differential Revision: https://secure.phabricator.com/D17027
2016-12-11 11:44:22 -08:00
epriestley
29a3cd5121 Add "Manual Activities", to tell administrators to rebuild the search index
Summary:
Ref T11922. After updating to HEAD of `master`, you need to manually rebuild the index. We don't do this during `bin/storage upgrade` because it can take a very long time (`secure.phabricator.com` took roughly an hour) and can happen while Phabricator is running.

However, if we don't warn users about this they'll just get a broken index unless they go read the changelog (or file an issue, then we tell them to go read the changelog).

This adds a very simple table for notes to administrators so we can write a "you need to go rebuild the index" note, then adds one.

Administrators clear the note by completing the activity and running `bin/config done reindex`. This isn't automatic because there are various strategies you can use to approach the issue, which I'll discuss in greater detail in the linked documentation.

Also, fix an issue where `bin/storage upgrade --apply <patch>` could try to re-mark an already-applied patch as applied.

Test Plan:
  - Ran storage ugrades.
  - Got instructions to rebuild search index.
  - Cleared instructions with `bin/config done reindex`.

Reviewers: chad

Reviewed By: chad

Subscribers: avivey

Maniphest Tasks: T11922

Differential Revision: https://secure.phabricator.com/D16965
2016-11-30 11:23:54 -08:00
epriestley
7c5b5327c8 Use stemming in the MySQL fulltext search engine
Summary:
Ref T6740. When we index a document, also save a copy of the stemmed version.

When querying, search the combined corpus for the terms.

(We may need to tune this a bit later since it's possible for literal, quoted terms to match in the stemmed section, but I think this wil rarely cause issues in practice.)

A downside here is that search sort of breaks if you upgrade into this and don't reindex. I wasn't able to find a way to issue the query that remained compatible with older indexes and didn't have awful performance, so my plan is:

  - Put this on `secure`.
  - Rebuild the index.
  - If things look good after a couple of days, add a way that we can tell people they need to rebuild the search index with a setup warning.

We might get some reports between now and then, but if this is super awful we should know by the end of the weekend.

Test Plan:
WOW AMAZING

{F2021466}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6740

Differential Revision: https://secure.phabricator.com/D16947
2016-11-25 15:30:50 -08:00
epriestley
ff3333548f Create and populate a stopwords table for InnoDB fulltext indexes to use in the future
Summary:
Ref T11741. InnoDB uses a stopwords table instead of a stopwords file.

During `storage upgrade`, synchronize the table from the stopwords file on disk.

Test Plan:
  - Ran `storage upgrade`.
  - Ran `select * from stopwords`, saw stopwords.
  - Added some garbage to the table.
  - Ran `storage upgrade`, saw it remove it.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11741

Differential Revision: https://secure.phabricator.com/D16940
2016-11-25 15:13:08 -08:00
epriestley
4da74166fe When storage is partitioned, refuse to serve requests unless web and databases agree on partitioning
Summary:
Ref T11044. One popular tool in a modern operations environment is Puppet. The primary purpose of this tool is to randomly revert hosts to older or different configurations.

Introducing an element of chaotic unpredictability into operations trains staff to be on high alert at all times, rather than lulled into complacency by predictability or consistency.

When Puppet reverts a Phabricator host's configuration to an older version, we might start writing data to a lot of crazy places where it shouldn't go. This will create a big sticky mess that is virtually impossible to undo, mostly because we'll get two files with ID 123 or two tasks with ID 456 or whatever else and good luck with that.

Instead, after changing the partition layout, require `bin/storage partition` to be run. This writes a copy of the config everywhere.

Then, when we start serving web requests, make sure every database has the exact same config. This will foil Puppet by refusing to run requests on hosts it has reverted.

Test Plan:
  - Changed partition configuration.
  - Ran Phabricator.
  - FOILED!
  - Ran `bin/storage partition` to sync config.
  - Things worked again.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11044

Differential Revision: https://secure.phabricator.com/D16910
2016-11-22 04:15:46 -08:00
Chad Little
ce0cb115ca Add Hero Image to Phame Post
Summary: Adds a headerimage and lets you set it on posts for added reverence. Is that a word?

Test Plan:
Add an image, see an image.

{F1923010}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16873
2016-11-15 13:44:47 -08:00
Chad Little
015ead6e7b Add subtitle to PhamePost
Summary: No view engine yet (adding header image next), but adds subtitle to display like PhameBlog

Test Plan: Add a subtitle, remove a subtitle.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16871
2016-11-15 12:02:03 -08:00
epriestley
e337029769 Allow users to mark themselves as "Available", "Busy" or "Away" while attending an event
Summary:
Ref T11816.

  - Now that we can do something meaningful with them, bring back the yellow dots for "busy".
  - Default to "busy" when attending events (we could make this "busy" for short events and "away" for long events or something).
  - Let users pick how to display their attending status on the event page.
  - Also show which event the user is attending since I had to mess with the cache code anyway. We can get rid of this again if it doesn't feel good.

Test Plan:
{F1904179}

{F1904180}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16802
2016-11-04 16:55:44 -07:00