1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-27 01:02:42 +01:00
Commit graph

14846 commits

Author SHA1 Message Date
epriestley
a1d9a2389d Improve Ferret engine indexing performance for large blocks of text
Summary:
See PHI87. Ref T12974. Currently, we do a lot more work here than we need to: we call `phutil_utf8_strtolower()` on each token, but can do it once at the beginning on the whole block.

Additionally, since ngrams don't care about order, we only need to convert unique tokens into ngrams. This saves us some `phutil_utf8v()`. These calls can be slow for large inputs.

Test Plan:
  - Created a ~4MB task description.
  - Ran `bin/search index Txxx --profile ...` to profile indexing performance before and after the change.
  - Saw total runtime drop form 38s to 9s.
  - Before: <https://secure.phabricator.com/xhprof/profile/PHID-FILE-wiht5d7lkyazaywwxovw/>
  - After: <https://secure.phabricator.com/xhprof/profile/PHID-FILE-efxv56q2hulr6kjrxbx6/>

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12974

Differential Revision: https://secure.phabricator.com/D18647
2017-09-27 08:15:40 -07:00
Aviv Eyal
9f11f310f8 Make PHUITwoColumnView a little more printable
Summary: Hide navbar, and make curtain behave like on a phone, when printing.

Test Plan: {F5197340}

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18583
2017-09-25 19:56:22 +00:00
epriestley
1ac52c09e7 Improve search highlighting for CJK and substring queries
Summary:
Fixes T12995. Currently, the result highlighter (which shows //where// terms matched) only works in "term" mode, not in "substring" mode.

Provide better feedback and behvaior:

  - When a term is a substring term, color it a little differently and add a tooltip. (This is partly to make it easier to debug/diagnose things, probably not enormously valuable to users.)
  - When a term is a substring term, highlight it anywhere in the results.

Test Plan:
Queried for latin and CJK terms.

Here is CJK being highlighted:

{F5192195}

Here is substring vs non-substring implicit behavior:

{F5192196}

Here's ONLY terms being highlighted:

{F5192198}

Here's terms and substrings, since the query now has a substring:

{F5192201}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12995

Differential Revision: https://secure.phabricator.com/D18635
2017-09-22 11:34:46 -07:00
epriestley
36df39761e Create revisions into "Draft", publish them when builds finish
Summary:
Ref T2543. This doesn't stand alone since mail still goes out normally, but gets this piece working: new revisions start as "Draft", then after updates if there are no builds they go into "Needs Review".

This should work in general because builds update revisions when they complete, to publish a "Harbormaster finished build yada yada" transaction. So either we'll un-draft immediately, or un-draft after the last build finishes.

I'll hold this until the mail and some other stuff (like UI hints) are in slightly better shape since I think it's probably too rough on its own.

Test Plan: Created revisions locally, saw them un-draft after builds.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18628
2017-09-21 07:21:21 -07:00
epriestley
fca553f142 Prepare revision mail for the "Draft" status
Summary:
Ref T2543. Currently, we always do some special things when a revision is created, mostly adding more stuff to the mail.

With drafts, we want to suppress initial mail and send this big, rich mail only when the revision actually moves out of "draft".

Prepare the code for this, with the actual methods hard-coded to the current behavior. This will probably take some tweaking but I think I got most of it.

Test Plan: Banged around in Differential so it sent some mail, saw normal mail without anything new.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18627
2017-09-21 07:21:07 -07:00
epriestley
c7af663523 Align most revision actions to the new "Draft" state
Summary:
Ref T2543. Most actions are not available for drafts.

Authors can "Request Review" (move out of draft to become a normal revision) or "Abandon".

Non-authors can't do anything (maybe we'll let them do something later -- like "Commandeer"? -- if there's a good reason).

Test Plan: Viewed a draft revision as an author and non-author, saw fewer actions available.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18626
2017-09-21 07:20:48 -07:00
epriestley
5112dac491 Update an old SSH redirect URI when editing a bot's SSH keys
Summary: See PHI79. When you edit another user's SSH keys (normally, for a bot account) we currently redirect you to an older URI.

Test Plan:
  - Viewed a bot's profile page.
  - Clicked "Edit Settings" on the Manage page.
  - Went to "SSH Keys".
  - Uploaded an SSH key.
  - Before: redirected to a 404 after finishing the workflow.
  - After: redirected to the same page after the workflow.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18633
2017-09-20 14:46:31 -07:00
epriestley
3fbad684c1 More completely explain why we're refusing to send reset mail to an unverified address
Summary:
See PHI78. The user was getting this message and (reasonably) interpreted it to mean "reset mail can never be sent to unverified addresses".

Reword it to be more clear, albeit an entire paragraph long. I don't really have a good solution in these cases where we'd need a whole page to explain what's happening (this, plus "we can't tell you which address you should use because an attacker could get information if we did" and "this rule defuses the risk that an opportunistic attacker may try to compromise your account after you add an email you don't own by mistake"). We could write it up separately and link to it, but I feel like that stuff tends to get out of date.

Just land somewhere in the middle.

Test Plan: {F5189105}

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18630
2017-09-20 10:46:22 -07:00
epriestley
03e5d69817 Fix an error in Diffusion when the Owners application is uninstalled
Summary:
See <https://discourse.phabricator-community.org/t/undefined-view-when-owners-is-uninstalled/451>.

When Owners is not installed, Diffusion can fatal with a bad `$view`.

Test Plan:
  - Uninstall Owners.
  - View the content of any file in Diffusion.
  - Before: fatal on `$view` undefined.
  - After: Valid page with no owners information.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18629
2017-09-19 09:42:42 -07:00
epriestley
23867c1487 Add a "Draft" state for revisions, and action bucket support
Summary:
Ref T2543. There's no way to put revisions into this state yet, but start adding support for when there is.

Adds the status constant, plus support for bucketing them.

Test Plan:
  - Manually put a revision in "Draft" state by updating the database directly.
  - Verified my drafts showed up in a "Drafts" section on the bucket view.
  - Verified others' drafts did not appear on the action bucket view.
  - Viewed revisions, queried for "Draft" revisions, etc (stuff we get for free).

{F5186781}

{F5186782}

{F5186783}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18625
2017-09-18 14:01:00 -07:00
epriestley
156adccef0 Fix an issue where "bin/differential migrate-hunk" could decompress data
Summary:
Fixes T12986. I caught this bug in the changes from D18584: when we moved a large hunk to file storage, we would decompress it but keep the "deflated" flag. This could cause confusion when loading it later. I missed this in testing since I wasn't exhaustive enough in checking hunks and didn't run into a compressed one.

Instead of compressing on `save()`, compress during the normal workflow.

We currently never advise users to run this workflow so I didn't bother trying to clean up possible existing migrations.

Test Plan:
  - Ran `bin/differential migrate-hunk` on compressed hunks, moving them to and from file storage. Saw them work correctly and remain compressed.
  - Created new small (uncompressed) and large (compressed) hunks, verified they work properly and get compressed (if applicable).
  - Used `bin/cache purge --caches changeset` to clear changeset caches and make sure the actual table was being hit.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12986

Differential Revision: https://secure.phabricator.com/D18624
2017-09-18 14:00:41 -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
94ab0c9afb Spell "Relevance" correctly
Summary: Despite how I (and everyone else?) pronounce it, it is spelled with an "a". See PHI38.

Test Plan: Googled both spellings.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18622
2017-09-18 09:36:55 -07:00
epriestley
51b810b0eb Fix "Author's projects" Herald rules for revisions and diffs
Summary:
See PHI71. These didn't get properly updated when we wrote Subprojects and Milestones, and should use materialized members, not raw members. Swap the query so projects you are an indirect member of (e.g., milestones you are a member of the parent for, and parent projects you are a member of a subproject of) are included in the result list.

Also fix a bad typeahead datasource.

Test Plan:
  - Ran a dry run with the test console, saw project PHIDs for milestones and parent projects in the raw field value.
  - Tried to set "Author's projects" to a user, no longer could.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18619
2017-09-15 17:59:49 -07:00
epriestley
b352cacdd9 Swap "-R" and "serve" argument order for Mercurial
Summary: See <https://discourse.phabricator-community.org/t/unable-to-use-current-mercurial-on-debian-stretch/391/13>. I missed that `-R` is order-sensitive.

Test Plan: Verified both orders work on 3.5.2.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18616
2017-09-15 13:07:43 -07:00
epriestley
49b7181780 Update utility "bin/repository parents" workflow to work with RefPosition
Summary:
Ref T11823. I think this is the last callsite which relies on the old data format: `bin/repository parents` rebuilds a cache which we don't currently use very heavily.

Update it to work with the new data.

Test Plan: Ran `bin/repository parents <repository> --trace`, saw successful script execution and reasonable-looking output.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11823

Differential Revision: https://secure.phabricator.com/D18615
2017-09-15 10:21:51 -07:00
epriestley
8982e3e52d Update major RefCursor callsites to work properly with RefPosition
Summary:
Ref T11823. This is the meaty part of the change, and updates `RefEngine` to use separate RefCursor (for names) and RefPosition (for actual commit positions) tables.

I'll hold this whole series until after the release cut so it has some time to bake on `secure` to look for issues. It's also not a huge problem if there are bugs here since these tables are just caches anyway, although they do feed into some other things, and obviously it's never good to have bugs.

Test Plan:
  - This logic can be invoked directly with `bin/repository refs <repository> --trace --verbose`.
  - Ran that on unchanged repositories, new branches, removed branches, and modified branches. Saw appropriate output and cursor positions.
  - Ran on a mercurial repository to test the close/open logic, saw it correct open/closed state of incorrect positions.
  - Browed around Diffusion in various repositories.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11823

Differential Revision: https://secure.phabricator.com/D18614
2017-09-15 10:21:32 -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
bd923d1ce0 Provide an explicit "-R" flag to "hg serve"
Summary:
See <https://discourse.phabricator-community.org/t/unable-to-use-current-mercurial-on-debian-stretch/391>.

The Mercurial commit is helpful in particular: <https://www.mercurial-scm.org/repo/hg/rev/77eaf9539499>

We weren't vulnerable to the security issue (users can not control any part of the command) but pass the working directory explicitly to get past the new safety check.

I left `setCWD()` in place (a few lines below) just because it can't hurt, and in some other contexts it sometimes matter (for example, if commit hooks execute, they might inherit the parent CWD here or in other VCSes).

Test Plan:
  - Cloned from a Mercurial repo locally over HTTP.
  - Verified that SSH cloning already uses `-R` (it does, see `DiffusionMercurialServeSSHWorkflow`).
  - Did not actually upgrade to Mercurial 4.0/4.1.3 to completely verify this, but a user in the Discourse thread asserted that a substantially similar fix worked correctly.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18611
2017-09-15 08:57:11 -07:00
epriestley
5ae3af6691 Fix an outdated HTML anchor link in Diffusion table of contents
Summary:
See <https://discourse.phabricator-community.org/t/navigating-to-changed-files-in-diffusion-does-not-work-anymore/433>.

In D18465, I updated these but this hard-coded the anchor for some reason (???) and I missed it while `grep`-ing.

Test Plan: Viewed a commit (`/rXYZaaaa`) and clicked a file link in the table of contents. Got modern `#change-...` anchor and navigation into the document.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18609
2017-09-15 08:37:36 -07:00
Austin McKinley
c71cb944a4 Add edit methods for Almanac services and devices
Summary: See T12414. This just gets started; we still need edit endpoints for network interfaces and bindings.

Test Plan: Created some devices/services from the conduit UI.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18605
2017-09-14 14:32:58 -07:00
epriestley
939008e64c Correct an issue where Maniphest's awful legacy "reports" UI was extra broken on merges
Summary:
See PHI66. See that issue for context. This UI is bad broken legacy junk, but was especially broken when reporting merges.

These do not currently generate a "status" transaction, so they were never counted as task closures. Pretend they're normal closures.

This is still wrong, but should be much closer to the real numbers. Specifically, if you merge a closed task into another task, it will incorrectly be counted as an extra close. This could result in negative tasks, but the numbers should be much closer to reality than they are today even so.

The "Facts" application (T1562) is the real pathway forward here in the longer term.

Test Plan:
  - Moved my `maniphest_transactions` table aside with `RENAME TABLE ...`.
  - Created a new empty table with `CREATE TABLE ... LIKE ...`.
  - Reloaded reports UI, saw empty chart.
  - Created, closed, and reopened tasks while reloading the chart, saw accurate reporting.
  - Merged an open task into another task, saw bad reporting.
  - Applied patch, saw the right chart again.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18601
2017-09-14 10:09:46 -07:00
epriestley
c310f08b7a Work around workflow blocking error with duplicate "master" refs in "Land Revision"
Summary:
Ref T11823. See PHI68. T11823 has a full description of this issue and a plan to fix it, but the full plan is relatively complicated.

Until that can happen, provide a workaround for the biggest immediate issue, where multiple copies of a ref cursor can cause `executeOne()` to throw, since it expects a single result. In practice, these copies are always identical so we can just pick the first one.

This will get cleaned up once T11823 is fixed properly.

Test Plan:
Forced the table into a duplicate/ambiguous state, reproduced a similar-looking error:

{F5180999}

Applied the patch, got the "Land" to work as expected:

{F5181000}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T11823

Differential Revision: https://secure.phabricator.com/D18599
2017-09-13 16:03:10 -07:00
epriestley
6fb3f857fb Stop the bleeding caused by attaching enormous patches to revision mail
Summary:
Ref T12033. This is a very narrow fix for this issue, but it should fix the major error: don't attach patches if they're bigger than the mail body limit (by default, 512KB).

Specifically, the logs from an install in T12033 show a 112MB patch being attached, and that's the biggest practical problem here.

I'll follow up on the tasks with more nuanced future work.

Test Plan: Enabled `differential.attach-patches`, saw a patch attached to email. Set the byte limit very low, saw patches get thrown away.

Reviewers: chad, amckinley

Reviewed By: amckinley

Maniphest Tasks: T12033

Differential Revision: https://secure.phabricator.com/D18598
2017-09-13 15:32:55 -07:00
epriestley
29f625ef68 Make "No Notifications" setting less broad, and fix a bug with default display behavior
Summary:
Fixes T12979. In D18457, we added a "No Notifications" setting to let users disable the blue and yellow pop-up notifications that alert you when an object has been updated, since some users found them distracting.

However, the change made "do nothing" the default, so all other `JX.Notification` callsites -- which never pass a preference -- were effectively turned off no matter what your setting was set to. This includes the "Read-Only" mode warning (grey), the "High Security" mode warning (purple), the "timezone" warning, and a few others.

Tweak things a little bit so the setting applies to ONLY blue and yellow ("object you're following was updated" / "this object was updated") notifications, not other types of popup notifications.

Test Plan:
  - With notifications on in settings, got blue notifications and "Read-only".
  - With notifications off in settings, got "Read-only" but no blue notifications.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12979

Differential Revision: https://secure.phabricator.com/D18600
2017-09-13 15:32:46 -07:00
epriestley
6cedd4a95c Revert quickstart for tables with native FULLTEXT indexes to MyISAM
See D18594.
2017-09-12 12:24:23 -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
fdc0d8c2f6 Fix an issue with selecting the right stemmed ngrams with Ferret engine queries
Summary:
Ref T12819. In D18581, I corrected one bug (ngram selection for terms) but introduced a minor new bug. We now pass `' query '` (term corpus with boundary spaces) to the stemmer, but it bails out on this since English words don't start with spaces.

Trim these extra boundary spaces off before invoking the stemmer.

The practical effect of this is that searching for non-stem variations of a word ("detection") now finds stemmed variations again ("detect"). Prior to fixing this bug, the stem could find longer variations but not the other way around.

Test Plan: Searched for "detection", found results matching "detect" after patch (and saw same results for "detect" and "detection").

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18593
2017-09-12 12:13:42 -07:00
epriestley
e6f0f86518 Document Ferret engine fulltext search features
Summary: Ref T12819. Adds some documentation for `-term`, `~term`, `title:term`, etc.

Test Plan: Read documentation.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18592
2017-09-12 12:13:23 -07:00
epriestley
da0a08a7e1 Make "mysql" mean "Ferret engine" in Fulltext search
Summary: Ref T12819. Swaps constants so existing configurations that use a "mysql" engine now use the Ferret engine, not an InnoDB/MyISAM FULLTEXT engine.

Test Plan: Swapped my local config back to "mysql" (the default), saw Ferret engine results in the UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18590
2017-09-11 18:05:12 -07:00
epriestley
39b74572e6 Return fulltext tokens from the Ferret fulltext engine
Summary:
Ref T12819. These render the little "Searched For: X, Y, U V" hint about how something was parsed.

(This might get a "substring" color or "title only" color or something in the future.)

Test Plan: {F5178807}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18589
2017-09-11 18:04:56 -07:00
epriestley
dd3f05ec25 Remove "Name Contains" query constraint from Diffusion for Repositories
Summary:
Ref T12819. Obsoleted by the Ferret engine "Query" field.

This is a compatibility break, I'll note it in the changelog.

Test Plan: Searched for repositories by name with "Query" instead of "Name Contains".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18588
2017-09-11 18:04:39 -07:00
epriestley
6edf98eb3b Unprototype the Ferret UI fields
Summary:
Ref T12819. Show the new Ferret engine fields (and enable the indexer) unconditionally.

Also pull them to the top since they're fairly general-purpose and appear more broadly now, and also they actually work correctly (WOW).

Some redundant fields (like "Name Contains" in Repositories and Owners) could probably be removed now, I may clean those up in a followup.

Test Plan: Browsed around, saw Ferret fields in UI without "(Prototype)" suffix.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18587
2017-09-11 18:04:25 -07:00
epriestley
495ab7363b Remove "Contains Words" constraint from Maniphest
Summary:
Ref T12819. Obsoleted by the Ferret engine, which is unprototyping shortly.

This breaks compatibility in two ways:

  - `maniphest.query` no longer supports "fullText" (now throws an explicit exception).
  - Existing saved searches with a "Contains Words" constraint will no longer have that constraint.

It seems unlikely (?) that either of these are seeing too much use, and they should be easy to fix. I'll note them in the changelog.

Test Plan: Viewed Maniphest, no more "Contains Words" field. Called `maniphest.query` with "fullText", got explicit exception.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18586
2017-09-11 18:04:12 -07:00
epriestley
d15fb20fe6 Support storage of Differential hunk data in Files
Summary:
Ref T12932. For long-lived installs, one of the largest tables tends to be the hunk data table. Although it doesn't grow tremendously fast, it's also well suited to storage in Files instead of the database (infrequent access, relatively large blobs of data, mostly one-at-a-time access), and earlier work anticipated eventually adding support for Files storage.

Make Files storage work, and provide `bin/differential migrate-hunk` to manually test/migrate hunks. This is currently the only way hunks get moved to file storage, but I expect to add a GC step which moves them to File storage after 30 days shortly.

The immediate motivation for this is to relieve storage pressure on db001/db002 so we have more headroom for deploying the Ferret engine and its larger indexes (see also T12819).

Test Plan:
  - Used `bin/differential migrate-hunk` to move a hunk to and from file storage, verified it survived intact.
  - Downloaded the actual stored file, sanity-checked it. Verified permissions.
  - Destroyed a diff with `bin/remove destroy`, saw the hunk and file storage destroyed.
  - Verified that going from file -> text destroys the old file properly with `migrate-hunk --trace ...`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12932

Differential Revision: https://secure.phabricator.com/D18584
2017-09-11 16:09:02 -07:00
epriestley
c662dda0f1 When selecting Ferret ngrams, select term ngrams (not raw ngrams) for term search
Summary:
Ref T12819. For queries like `v0.2`, we would incorrectly search for ngrams including `0.2`, but this is only a substring ngram: the term corpus splits this into `v0` and `2`, so `0.2` is not in the ngrams table.

When executing term queries, search for term ngrams instead. This makes "v0.2" work properly again.

Test Plan: Searched for "v0.2", found a task with "v0.2" in the title.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18581
2017-09-08 09:47:58 -07:00
epriestley
d67cc8e5c5 Remove some redundant information from the Ferret engine index
Summary:
Ref T12819. The "full" field has all other fields, and the "core" field has "title" and "body". Due to the way the "full" and "core" fields were being built, the "core" field also got included in the "full" field, so the "full" field has two copies of the title, two copies of the body, and then one copy of everything else.

Put only one copy of each distinct thing in each "full" and "core". Also, simplify the logic a little bit so we build these virtual fields in a more consistent way.

Test Plan: Ran `bin/search index` and looked at the fields in the database, saw less redundant information.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18580
2017-09-08 09:40:12 -07:00
epriestley
7ea6de6e9c Split Ferret engine strings for tokenization on any sequence of whitespace
Summary:
Ref T12819. Currently, strings are split only on spaces, but newlines (and, if they exist, tabs) should also split strings.

Without this, we can fail to get the proper term boundary tokens for words which begin at the start of a line or end at the end of a line.

Test Plan: Reindexed a document with "xyz\nabc", saw `"yz "` and `" ab"` term boundary tokens generate properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18579
2017-09-08 09:39:57 -07:00
epriestley
4cae4a3b76 Correct bin/storage analyze internal API for cluster environments
Summary:
Ref T12819. This worked right in a non-cluster environment, but `bin/storage upgrade` iterates over each master in a partitioned cluster environment.

Tweak the API so `bin/storage analyze` targets a single host but `bin/storage upgrade` can hit all the masters.

Test Plan: Will run `bin/storage upgrade` in production again. Ran `upgrade` and `analyze` locally, still work fine.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18576
2017-09-07 16:35:26 -07:00
Chad Little
a46a9ff165 Update VCS password UI
Summary: Miss this with earlier pass, updates the VCS password page.

Test Plan: Try to set a vcs password

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18574
2017-09-07 15:50:05 -07:00
epriestley
8e9f049626 Provide "bin/storage analyze" and make "bin/storage upgrade" run analysis automatically
Summary:
Ref T12819. Normallly "ANALYZE TABLE" is like sprinkling magic pixie dust on the database and hoping it will make "good vibes" that cause it to go faster, but in at least some concrete cases with the ngrams tables there really was a key cardinality issue which ANALYZE TABLE corrected, fixing bogus query plans.

Add `bin/storage analyze` to analyze all tables, and make `bin/storage upgrade` run it after adjustment if `--no-adjust` is not specified, and make `bin/storage adjust` run it always.

This runs in a couple seconds and should never hurt anything, so it should be fine to sprinkle lots of pixie dust into the `bin/storage` workflow.

Test Plan: Ran `bin/storage analyze`. Ran `bin/storage upgrade`, saw analyze run. Totally felt great vibes and really aligned chakras on the database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18573
2017-09-07 14:44:34 -07:00
Chad Little
f1193afa94 Update passphrase edit UI
Summary: Updates creating credentials

Test Plan: create some notes

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18570
2017-09-07 14:13:40 -07:00
Chad Little
f9b109dc0d Fix info view error layout in config boxes
Summary: Adds some side margin here.

Test Plan: error out a form field in a white box

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Differential Revision: https://secure.phabricator.com/D18571
2017-09-07 14:09:03 -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