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

1892 commits

Author SHA1 Message Date
epriestley
6b99aac49d Digest changeset anchors into purely alphanumeric strings
Summary:
Ref T13045. See that task for discussion.

This replaces `digestForIndex()` with a "clever" algorithm in `digestForAnchor()`. The new digest is the same as `digestForIndex()` except when the original output was "." or "_". In those cases, a replacement character is selected based on entropy accumulated by the digest function as it iterates through the string.

Test Plan: Added unit tests.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13045

Differential Revision: https://secure.phabricator.com/D18909
2018-01-23 13:42:08 -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
7970cf0585 Add a bin/auth revoke revoker for temporary tokens
Summary: Ref T13043. Allows CLI revocation of temporary ("forgot password", "one-time login") tokens.

Test Plan: Used "Forgot Password?" to generate tokens, used `bin/auth revoke --type temporary` with `--from` and `--everywhere` to revoke them.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043

Differential Revision: https://secure.phabricator.com/D18891
2018-01-22 12:00:33 -08:00
epriestley
d9b6513a21 Respect tokenizer limits in the bulk editor
Summary: Ref T13025. This makes limits (for fields like "Assign To") work in the bulk editor, so you can't type "Assign to: x, y, z" anymore.

Test Plan: Hit limit for "Assign to" and a custom project field. No limit for "Add subscribers".

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13025

Differential Revision: https://secure.phabricator.com/D18888
2018-01-22 11:55:55 -08:00
epriestley
fbfcc37531 Respect token limits for "Assign to" and custom datasource fields in Herald
Summary:
See PHI173. Currently, Herald has an "Assign to" action for tasks, and you can specify custom fields with datasource values (like users or projects) that have a limit (like 1 "Owner", or 12 "Jury Members").

Herald doesn't support these limits right now, so you can write `[ Assign to ][ X, Y, Z ]`. This just means "Assign to X", but make it more clear by actually enforcing the limit in the UI.

Test Plan:
  - Created a "projects" custom field with limit 1.
  - Tried to create actions that 'assign to' or 'set custom field to' more than one thing, got helpfully rebuffed by the UI.
  - Created an "add subscribers" action with more than one value.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18887
2018-01-22 11:54:12 -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
7a43181337 Organize bulk edit actions into nice groups
Summary: Ref T13025. We're getting kind of a lot of actions, so put them in nice groups so they're easier to work with.

Test Plan: {F5386038}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13025

Differential Revision: https://secure.phabricator.com/D18880
2018-01-19 13:22:25 -08:00
epriestley
ae1b07bcfb Support "<select />" custom fields in bulk editor
Summary: Ref T13025. Fixes T5689. A straightforward change!

Test Plan: Used the bulk editor to modify a custom "select" field like the one in T5689.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13025, T5689

Differential Revision: https://secure.phabricator.com/D18879
2018-01-19 13:18:02 -08:00
epriestley
b6737554e1 Support tokenizer custom fields in bulk editor
Summary:
Ref T13025. This allows custom tokenizer fields, like a "Owning Group" field, to be edited with the bulk editor.

See PHI173 for some context.

Test Plan: Edited a custom "Owner" field (a project tokenizer) with the bulk editor.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13025

Differential Revision: https://secure.phabricator.com/D18877
2018-01-19 13:16:46 -08:00
epriestley
a26cf20dd1 Fix a bug with setting custom PHID list field values via Conduit and prepare for bulk edits
Summary:
Ref T13025. Custom field transactions work somewhat unusually: the values sometimes need to be encoded. We currently do not apply this encoding correctly via Conduit.

For example, setting some custom PHID field to `["PHID-X-Y"]` fails with a bunch of JSON errors.

Add an extra hook callback so that EditTypes can apply processing to transaction values, then apply the correct CustomField processing.

This only affects Conduit. In a future diff, this also allows bulk edit of custom fields to work correctly.

Test Plan: Added a custom field to Maniphest with a list of projects. Used Conduit to bulk edit it (which now works, but did not before). Used the web UI to bulk edit it.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13025

Differential Revision: https://secure.phabricator.com/D18876
2018-01-19 12:51:35 -08:00
epriestley
8dccf05c4c Manually set "max_allowed_packet" to 1GB for "mysqldump"
Summary:
We have one production instance with failing database backups since they recently uploaded a 52MB hunk. The production configuration specifies a 64MB "max_allowed_packet" in `[mysqld]`, but this doesn't apply to `mysqldump` (we'd need to specify it in a separate `[mysqldump]` section) and `mysqldump` runs with an effective limit of the default (16MB).

We could change our production config to specify a value in `[mysqldump]`, but just change it unconditionally at execution time since there's no reason for any user to ever want this command to fail because they have too much data.

Test Plan: Dumped locally, will verify production backup goes through cleanly.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18834
2017-12-20 10:29:02 -08:00
epriestley
c7d6fd198c Support "Set X to" as an action in Herald for tokenizer/datasource custom fields
Summary:
See PHI173. Adds custom field support for Herald actions, and implements actions for "Datasource/Tokenizer" fields.

The only action available for now is "set field to...". Other actions ("Add values", "Remove values") might make sense in the future for these fields, but there's currently no use case. For most other field types (text, select, checkbox, etc) only "Set to" makes sense.

Test Plan:
  - Added a "datasource" custom field to the custom field definition in Config.
  - Added a "if field is empty, set field to default value X" rule to Herald.
  - Created a task with a nonempty field: no Herald trigger.
  - Created a task with an empty field: Herald fired.
  - Reviewed rule and transcripts for text strings.

{F5297615}

{F5297616}

{F5297617}

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18784
2017-11-28 13:41:52 -08:00
epriestley
1d213dc1fa Clean up virtual "_ft_rank" column for query construction of Ferret objects
Summary:
Ref T12974. Ferret object queries SELECT a virtual "_ft_rank" column for relevance ordering.

Currently, they always SELECT this column. That's fine and doesn't hurt anything, but makes developing and debugging things kind of a pain since every query has this `, blah blah _ft_rank` junk.

Instead, construct this column only if we're actually going to use it.

Mostly, this cleans up DarkConsole / query logs a bit.

Test Plan:
Viewed normal query results on various pages, viewed global search results, ordered Maniphest tasks by normal stuff and by "Relevance".

Viewed DarkConsole, saw no more "_ft_rank" junk on normal pages.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12974

Differential Revision: https://secure.phabricator.com/D18728
2017-10-23 16:18:04 -07:00
epriestley
157f47cd14 Rewrite CommitQuery to use UNION for performance
Summary:
Ref T12680. See PHI167. See that task for discussion.

Rewrite `DiffusionCommitQuery` to work more like `DifferentialRevisionQuery`, and use a UNION to find "all revisions you need to audit OR respond to".

I tried to get this working a little more cleanly than RevisionQuery does, and can probably simplify that now.

Test Plan: Poked at the UI locally without hitting any apparent issues, but my local data is pretty garbage at this point. I'll take a look at how the query plans work on `secure`.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12680

Differential Revision: https://secure.phabricator.com/D18722
2017-10-23 10:32:24 -07:00
epriestley
c5e8de9450 Make bin/storage dump insert CREATE DATABASE and USE statements
Summary:
Ref T13000. The new approach for dumping database-by-database means that we don't get CREATE DATABASE or USE statements, which makes importing the dump again inconvenient.

Manually stitch these into the dump.

Test Plan:
  - Used `bin/storage dump --namespace ...` to dump a smaller local instance.
  - Used `bin/storage destroy --namespace ...`, to destroy the namespace, then inported the dump cleanly.
  - Verified that each CREATE DATABASE statement appears only once.
  - Verified that `bin/storage renamespace --live` can correctly process this file.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13000

Differential Revision: https://secure.phabricator.com/D18707
2017-10-13 14:35:18 -07:00
Dmitri Iouchtchenko
9bd6a37055 Fix spelling
Summary: Noticed a couple of typos in the docs, and then things got out of hand.

Test Plan:
  - Stared at the words until my eyes watered and the letters began to swim on the screen.
  - Consulted a dictionary.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D18693
2017-10-09 10:48:04 -07:00
epriestley
4fd9d2d4bb Fix "bin/storage dump" with no "--output"
Ref T13004. (I distinctly remember testing this, but must have tweaked things afterward.)
2017-10-07 13:23:18 -07:00
epriestley
1ee7b3ab8c Correct "bin/storage dump" command construction with passwords
Fixes T13004. This should mirror the other branch.
2017-10-07 04:59:29 -07:00
epriestley
c767c971ca Add "persistence" types (data, cache, or index) to tables, and tweak what "storage dump" dumps
Summary:
Ref T13000. This marks each table as either "data" (normal data), "cache" (automatically rebuilt, no need to ever dump) or "index" (can be manually rebuilt).

By default, `bin/storage dump` dumps data and index tables, but not cache tables.

With `--no-indexes`, it dumps only data tables. Indexes can be rebuilt after a restore with `bin/search index --all ...`.

Test Plan:
  - Ran `--no-indexes` and normal dumps with `--trace`, verified that cache and index (former case) or cache only (latter case) tables were dumped with `--no-data`.
  - Verified dump has the same number of `CREATE TABLE` statements as before the changes.
  - Reviewed persistence tags in the web UI (note Ferret engine tables are "Index"):

{F5210886}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13000

Differential Revision: https://secure.phabricator.com/D18682
2017-10-04 12:09:33 -07:00
epriestley
02e1440ef2 Dump tables one at a time, rather than all at once
Summary:
Ref T13000. This allows us to be more selective about which tables we dump data for, to reduce the size of backups and exports. The immediate goal is to make large `ngrams` tables more manageable in the cluster, but this generally makes all backups and exports faster and easier.

Here, tables are dumped one at a time. A followup change will sometimes add the `--no-data` flag, to skip dumping readthrough caches and (optionally) rebuildable indexes.

Test Plan: Compared a dump from `master` and from this branch, found them to be essentially identical. The new dump has a little more header information in each section. Verified each contains the same number of `CREATE TABLE` statements.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13000

Differential Revision: https://secure.phabricator.com/D18679
2017-10-04 12:08:52 -07:00
epriestley
0ea5d668d1 Enable hovercards for the "Task Graph" UI in Maniphest
Summary: See PHI118. Enables hovercards to support peeking at tags and other details if you, e.g., create numerous identical subtasks of each task.

Test Plan: {F5210816}

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18681
2017-10-04 11:12:01 -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
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
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
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
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
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
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
epriestley
2218caee0f Reduce the amount of boilerplate that implementing FerretInterface requires
Summary:
See brief discussion in D18554. All the index tables are the same for every application (and, at this point, seem unlikely to change) and we never actually pass these objects around (they're only used internally).

In some other cases (like Transactions) not every application has the same tables (for example, Differential has extra field for inline comments), and/or we pass the objects around (lots of stuff uses `$xactions` directly).

However, in this case, and in Edges, we don't interact with any representation of the database state directly in much of the code, and it doesn't change from application to application.

Just automatically define document, field, and ngram tables for anything which implements `FerretInterface`. This makes the query and index logic a tiny bit messier but lets us delete a ton of boilerplate classes.

Test Plan: Indexed objects, searched for objects. Same results as before with much less code. Ran `bin/storage upgrade`, got a clean bill of health.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18559
2017-09-07 13:23:31 -07:00
epriestley
a2a2b3f7f4 Sort global fulltext results by overall relevance
Summary:
Ref T12819. Currently, under the Ferret engine, we query each application's index separately and then aggregate the results.

At the moment, results are aggregated by type first, then by actual rank. For example, all the revisions appear first, then all the tasks.

Instead, surface the internal ranking data from the underlying query and sort by it.

Test Plan: Searched for "A B" with a task named "A B" and a revision named "A". Saw task first. Broadly, saw mixed task and revision order in result sets.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18551
2017-09-07 13:21:58 -07:00
epriestley
8059db894d Use the Ferret engine fulltext document table to drive auxiliary fulltext constraints
Summary:
Ref T12819. I started trying to get individual engines to drive these constraints (e.g., `ManiphestTaskQuery` can do most of the work) but this is a big pain, especially since most engines don't support "any owner" or "no owner", and not everything has an owner, and so on and so on. Going down this path would have meant a huge pile of stub functions everywhere, I think.

Instead, drive these through the main engine using the fulltext document table, which already has everything we need to apply these constraints in a uniform way.

Also tweak some parts of query construction and result ordering.

Test Plan: Searched for documents by author, owner, unowned, any owner, tags, subscribers, fulltext in global search. Got sensible results without any application-specific code.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18550
2017-09-07 13:21:42 -07:00
epriestley
395a2ed6d1 Add an "only()" edge logic constraint, meaning "only the other constraints, exactly"
Summary:
See PHI57. For example, a query for "ios, only()" finds tags tasked with iOS, exactly, and no other tags.

I called this "only()" instead of "exact()" because we use the term/function "Exact" elsewhere with a different meaning, e.g. in Differential.

Test Plan:
Basic query for a tag:

{F5168857}

Same query with "only", finds tasks tagged with only that tag:

{F5168858}

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18543
2017-09-06 12:16:06 -07:00
epriestley
64b7778f32 Add support for relevance-ranking Ferret engine results
Summary: Ref T12819. "Relevance" here just means "how many of your search terms are present in the title?" but that's about the best we can do anyway.

Test Plan: Indexed tasks "A B", "A Z", "Z B", and "Z Z" (all with "A B" in comments). Searched for "A B". Got results ranked in the listed order, with "A B" as the most relevant hit for query "A B".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18539
2017-09-05 16:45:20 -07:00
epriestley
20aad35e60 Move Ferret engine "title:..." field definitions to the engine itself
Summary: Ref T12819. Move these out of the core engine into the Ferret engine. In the future different applications can define different functions, like "summary:..." or whatever. This may get more formalization when I possibly do "author:" and such some time down the road.

Test Plan: Searched for "title:...". Searched for "dog:...", got a useful error.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18536
2017-09-05 11:57:51 -07:00
epriestley
46abc11114 Reduce the number of magic strings in the Ferret implementation
Summary:
Ref T12819. Push more of the magic `' '` stuff into the engine and simplify calls to ngram construction.

Also fixes a bug where a task with title "apple banana" and description "cherry doughnut" could match query "banana cherry" by separating separate term segments with newlines instead of spaces.

Test Plan:
  - Indexed some objects.
  - Searched (term, substring, quoted terms).
  - Viewed index in database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18534
2017-09-05 11:57:35 -07:00
epriestley
4a7593f47f Consolidate more Ferret engine code into FerretEngine
Summary: Ref T12819. Earlier I separated some ngram code into an "ngram engine" hoping to share it across the simple Ngrams stuff and the full Ferret stuff, but they actually use slightly different rules. Just pull more of this stuff into FerretEngine to reduce the number of moving pieces and the amount of code duplication.

Test Plan: Searched for terms, rebuilt indexes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18533
2017-09-05 11:57:18 -07:00
epriestley
577d498033 Create a virtual "core" field in the Ferret engine for "title and body together"
Summary: See PHI46. The `core:` function means "find results in either the title or body, but not other auxiliary fields like comments".

Test Plan: Searched for text present in the title (yes), body (yes), and comments (no) with the `core:...` prefix.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18514
2017-09-01 09:40:56 -07:00
epriestley
f4f73e0a7e Separate fulltext engine extensions into "enrich" and "index" phases
Summary:
Ref T12819. Some of the extensions "enrich" the document (adding more fields or relationships), while others "index" it (insert it into some kind of index for later searching).

Currently, these are all muddled under a single "index" phase. However, the Ferret extension cares about fields and relationships which other extensions may add.

Split this into two phases: "enrich" adds fields and relationships so other extensions can read them later if they want. "Index" happens after the document is built and has all the fields and relationships.

The specific problem this solves is that comments may not have been added to the document when the Ferret extension runs. By moving them to the "enrich" phase, the Ferret engine will be able to see and index comments.

Test Plan: Ran `bin/search index ...`, grepped for `indexFulltextDocument`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18513
2017-09-01 09:40:11 -07:00
epriestley
3b43a70773 Add "title:..." support to the Ferret engine
Summary:
Ref T12819. Adds (hacky, hard-coded) field support (for now, only for "title").

I've written this so `title:quick ferret` is the same as `title:quick title:ferret`. I think this is what users probably mean.

You can do the other thing as `ferret title:quick`, or `title:quick all:ferret`.

Test Plan: Searched for `title:x`, `title:"x"`, `title:~"x"`, etc. Searched for "garbage:y", got an exception since that's not a recognized function. Searched for `title:x y`, saw both do title search.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18503
2017-08-30 11:30:42 -07:00
epriestley
048aa36c23 Support "-term" in Ferret engine queries
Summary:
Ref T12819. Supports negating search terms, e.g. "apple -honeycrisp".

When negating a term, we're a little more strict about what can match (that is, what can //prevent// a document from being returned) since it's easy for a user to type "apple -honeycrisp -honey -crisp -crispies -olcrispers -honeyyums" to keep refining their search, but hard/impossible to split apart an overboard term.

Test Plan:
  - Ran `apple -smith`, `apple -"granny smith"`, etc.
  - Verified `phone -tact` does not exclude `phone contact`.
  - (In theory, `phone -~tact` would, but the parser currently doesn't support this, and I'm not champing at the bit to add support.)

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18502
2017-08-30 11:30:24 -07:00
epriestley
df9c24e750 Provide some "term vs substring" support for the Ferret engine
Summary:
Ref T12819. Distinguishes between "term" queries and "substring" queries, and tries to match them correctly most of the time. For example:

  - `example` matches "example", obviously.
  - `~amp` matches "example", but `amp` does not.
  - `examples` matches "example" through stemming.
  - `"examples"` does not match "example" (quoted text does not stem).
  - `"an examp"` does not match "an example" (quoted text is still term text).
  - `~"an examp"` matches "an example" (quoted, substring-operator text uses substring search).

Test Plan: Ran searches similar to the above, they seemed to do what they should.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18500
2017-08-30 11:30:04 -07:00
epriestley
e5a495f435 Parse raw Ferret queries into tokens before processing them
Summary:
Ref T12819. Depends on D18492. Instead of passing a raw query into the Query layer, parse it first.

This allows the query layer to figure out which parts should be substring vs term match, and would allow the SearchEngine layer to do `author:...` eventually by picking it out before sending it to the Ferret engine.

Test Plan: Ran some Ferret queries. They work like before, except that nonsense like `-+"quack"` raises an exception now.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12819

Differential Revision: https://secure.phabricator.com/D18499
2017-08-30 11:29:46 -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
epriestley
70088f7eec Continue reducing callsites to ArcanistDifferentialRevisionStatus
Summary:
Ref T2543. Further consolidates status management into DifferentialRevisionStatus.

One change I'm making here is internally renaming "CLOSED" to "PUBLISHED". The UI will continue to say "Closed", at least for now, but this should make the code more clear because we care about "is closed, exactly" vs "is any closed status (closed, abandoned, sometimes accepted)". This distinction is more obvious as `isClosed()` vs `isPublished()` than, e.g., `isClosedWithExactlyTheClosedStatus()` or something. I think "Published" is generally more clear, too, and more consistent with modern language (e.g., "pre-publish review" replacing "pre-commit review" to make it more clear what we mean in Git/Mercurial).

I've removed the IN_PREPARATION status since this was just earlier groundwork for "Draft" and not actually used, and under the newer plan I'm trying to just abandon `ArcanistDifferentialRevisionStatus` entirely (or, at least, substantially).

Test Plan:
- Viewed revisions.
- Viewed revision list.
- Viewed revisions linked to a task in Maniphest.
- Viewed revision graph of dependencies in Differential.
- Grepped for `COLOR_STATUS_...` constants.
- Grepped for removed method `getRevisionStatusIcon()` (no callsites).
- Grepped for removed method `renderFullDescription()` (one callsite, replaced with just building a `TagView` inline).
- Grepped for removed method `isClosedStatus()` (no callsites after other changes).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18340
2017-08-09 11:05:22 -07:00
epriestley
e9208ed3da Fix a spelling error in worker triggers
Summary: This word is not spelled properly.

Test Plan: Read the word.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18250
2017-07-20 14:20:44 -07:00
epriestley
b46e2bb4cc Convert cluster/projects config options to newer modular structure
Summary: Ref T12845. Converts the cluster and project config options to the new stuff; this is mostly just shifting boilerplate around.

Test Plan: Edited, deleted, and mangled these options from the web UI and CLI.

Reviewers: chad, amckinley

Reviewed By: amckinley

Maniphest Tasks: T12845

Differential Revision: https://secure.phabricator.com/D18166
2017-06-27 12:35:54 -07:00
epriestley
988a52cf1a Fix ambiguous URI parsing in Youtube Remarkup rule
Summary:
Fixes T12867. Also:

  - Simplify the code a little.
  - Stop mutating this on text/mobile -- there's no inherent value in the "youtu.be" link so I think this just changes the text the user wrote unnecessarily.

Test Plan: {F5013804}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12867

Differential Revision: https://secure.phabricator.com/D18149
2017-06-23 08:43:15 -07:00
epriestley
c71d9c601f Pass all Throwables to Exception Handlers, not just Exceptions
Summary:
Ref T12855. PHP7 introduced "Throwables", which are sort of like super exceptions. Some errors that PHP raises at runtime have become Throwables instead of old-school errors now.

The major effect this has is blank pages during development under PHP7 for certain classes of errors: they skip all the nice "show a pretty error" handlers and

This isn't a compelete fix, but catches the most common classes of unexpected Throwable and sends them through the normal machinery. Principally, it shows a nice stack trace again instead of a blank page for a larger class of typos and minor mistakes.

Test Plan:
Before: blank page. After:

{F5007979}

Reviewers: chad, amckinley

Reviewed By: chad

Maniphest Tasks: T12855

Differential Revision: https://secure.phabricator.com/D18136
2017-06-20 05:44:51 -07:00
epriestley
3be36783b3 Consider inline comments with draft checkmarks as "unsubmitted"
Summary:
Ref T12733. When a revision has unsubmitted checkmarks:

  - Color the banner yellow.
  - Show them in the "X unsubmitted" count.
  - Make the "X unsubmitted" button cycle between all drafts (written but unpublished comments) and "draft done" (checked but unsubmitted "Done" checkbox comments).

Test Plan:
  - Checked a "Done" box, saw "1 unsubmitted" and yellow banner.
  - Clicked "5 unsubmitted" repeatedly, saw it cycle through all unsubmitted comments and checkboxes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18127
2017-06-15 05:22:58 -07:00
epriestley
887bd2d66e In the UI, rename "Hide Inline" to "Collapse Inline"
Summary:
Ref T12733. This paves the way for a separate "hide" operation which completely hides things.

(I didn't extend this to the server side because that would require schema changes and the new "hide" state is client-only.)

Test Plan: Collapsed and expanded inlines, viewed tooltips.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18126
2017-06-15 05:22:44 -07:00