Summary: Fixes T6608, though I'll also clean up the comment for PhutilStringTruncator in another diff. If I understand correctly, before T1191, MySQL column length was by character count and post T1191 its by byte count. Ergo, most of these changes are going from codepoint -> bytes. See test plan for complete list of what was and was not done.
Test Plan:
Thought very carefully about each callsite and made changes as appropos. "Display" means the string is clearly used for display-only purposes and correctly uses "glyph" already.
grep -rn PhutilUTF8StringTruncator *
applications/calendar/query/PhabricatorCalendarEventSearchEngine.php:217: ->addAttribute(id(new PhutilUTF8StringTruncator()) -- display
applications/chatlog/controller/PhabricatorChatLogChannelLogController.php:111: $author = id(new PhutilUTF8StringTruncator()) -- display
applications/conduit/method/ConduitConnectConduitAPIMethod.php:62: $client_description = id(new PhutilUTF8StringTruncator()) -- was codepoint, changed to bytes
applications/conpherence/view/ConpherenceFileWidgetView.php:22: ->setFileName(id(new PhutilUTF8StringTruncator()) -- display
applications/differential/controller/DifferentialDiffViewController.php:65: id(new PhutilUTF8StringTruncator()) -- display
applications/differential/event/DifferentialHovercardEventListener.php:69: id(new PhutilUTF8StringTruncator()) -- display
applications/differential/parser/DifferentialCommitMessageParser.php:144: $short = id(new PhutilUTF8StringTruncator()) -- was glyphs, made to bytes
applications/differential/view/DifferentialLocalCommitsView.php:80: $summary = id(new PhutilUTF8StringTruncator()) -- display
applications/diffusion/controller/DiffusionBrowseFileController.php:686: id(new PhutilUTF8StringTruncator()) -- display
applications/feed/story/PhabricatorFeedStory.php:392: $text = id(new PhutilUTF8StringTruncator()) -- display, unless people are saving the results of renderSummary() somewhere...
applications/harbormaster/storage/build/HarbormasterBuild.php:216: $log_source = id(new PhutilUTF8StringTruncator()) -- was codepoints now bytes
applications/herald/storage/transcript/HeraldObjectTranscript.php:55: // NOTE: PhutilUTF8StringTruncator has huge runtime for giant strings. -- not applicable
applications/maniphest/export/ManiphestExcelDefaultFormat.php:107: id(new PhutilUTF8StringTruncator()) -- bytes
applications/metamta/storage/PhabricatorMetaMTAMail.php:587: $body = id(new PhutilUTF8StringTruncator()) -- bytes
applications/people/event/PhabricatorPeopleHovercardEventListener.php:62: id(new PhutilUTF8StringTruncator()) -- display
applications/phame/conduit/PhameCreatePostConduitAPIMethod.php:93: id(new PhutilUTF8StringTruncator()) -- was codepoints, now bytes
applications/pholio/storage/PholioTransaction.php:300: id(new PhutilUTF8StringTruncator()) -- display
applications/phortune/provider/PhortuneBalancedPaymentProvider.php:147: $charge_as = id(new PhutilUTF8StringTruncator()) -- bytes
applications/ponder/storage/PonderAnswerTransaction.php:86: id(new PhutilUTF8StringTruncator()) -- display
applications/ponder/storage/PonderQuestionTransaction.php:267: id(new PhutilUTF8StringTruncator()) -- display
applications/ponder/storage/PonderQuestionTransaction.php:276: id(new PhutilUTF8StringTruncator()) -- display
applications/repository/storage/PhabricatorRepositoryCommitData.php:43: $summary = id(new PhutilUTF8StringTruncator()) -- was codepoints, now bytes
applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php:20: $data->setAuthorName(id(new PhutilUTF8StringTruncator()) -- was codepoints, now bytes
applications/slowvote/query/PhabricatorSlowvoteSearchEngine.php:158: $item->addAttribute(id(new PhutilUTF8StringTruncator()) -- display
infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php:317: $host = id(new PhutilUTF8StringTruncator()) -- bytes
view/form/control/AphrontFormPolicyControl.php:61: $policy_short_name = id(new PhutilUTF8StringTruncator()) -- glyphs, probably display only
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6608
Differential Revision: https://secure.phabricator.com/D11219
Summary: Increase font size and contrast of all Object Headers.
Test Plan:
Sample a few, find it easier to read. I've been using this locally for a while.
{F261868}
{F261869}
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11221
Summary: This appears to be a typo, identified by `ArcanistXHPASTLinter::LINT_DUPLICATE_SWITCH_CASE` (see D11171).
Test Plan: `arc lint`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11194
Summary: Ref T6861. Some discussion in IRC. The behavior of `sort` is somewhat broken when dealing with mixed types. In this particular case, we have both integers and strings.
Test Plan: @epriestley confirmed that this made the ordering of the Celerity map slightly-more-sane.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6861
Differential Revision: https://secure.phabricator.com/D11210
Summary:
Ref T1751. When a commit reverts another commit:
- Add an edge linking them;
- Show the edge in Diffusion.
Next steps are:
- If the reverted commit is associated with a Differential revision, leave a comment;
- Also leave a comment on the commit (no API yet);
- Also trigger an audit by the original commit's author.
Test Plan: Used `scripts/repository/reparse.php --message ...` to parse commits with revert language. Verified they appear correctly in Diffusion, and update Differential.
Reviewers: btrahan, epriestley
Reviewed By: btrahan, epriestley
Subscribers: Korvin, epriestley, cburroughs, joshuaspence, sascha-egerer, aran
Maniphest Tasks: T4896, T1751
Differential Revision: https://secure.phabricator.com/D5846
Summary: Show the full unit test name, including the namespace. Depends on D11208.
Test Plan: Inspected the "Table of Contents" of a diff created //with// D11208 and //without// D11208 applied.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11209
Summary: Adding a set of 16 icons that match the typeahead icons.
Test Plan: review in photoshop
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6856
Differential Revision: https://secure.phabricator.com/D11183
Summary: Moves from 33% to 50% widths, allows more room for text on smaller screens. (Don't think this gets much use anyways). Fixes T6855
Test Plan: Reload page, shrink, still 50%
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6855
Differential Revision: https://secure.phabricator.com/D11184
Summary: The default behavior was inadvertedly changed in D11074. This restores the original behavior.
Test Plan: Added a project reviewer to a diff, saw no inverse transaction recorded.
Reviewers: Krenair, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11181
Summary: This class is no longer used after D11125.
Test Plan: `grep`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11170
Summary: These classes are no longer used after D10649.
Test Plan: `grep`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11168
Summary: This class is no longer required after D6675.
Test Plan: `grep`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11169
Summary: This class is no longer used after D6673.
Test Plan: `grep`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11167
Summary: Ref T5655. Fixes T6849. This is another take on D11131, which was missing the DB migration and was reverted in rP7c4de0f6be77ddaea593e1f41ae27211ec179a55.
Test Plan: Ran `./bin/storage upgrade` and verified that the classes were renamed in the `phabricator_policy.policy` table.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6849, T5655
Differential Revision: https://secure.phabricator.com/D11166
Summary: This class is no longer used after D10965.
Test Plan: `grep`
Reviewers: btrahan, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11133
Summary: Modernize remaining edges to subclass `PhabricatorEdgeType`. Largely based on D11045.
Test Plan: Browsed around and performed various actions include subscribing, unsubscribing and watching.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11116
Summary: I was going to fix the variable name as it violates convention, but it is not used anyway.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11139
Summary:
Ref T2783.
This updates PhabricatorRepositoryManagementLookupUsersWorkflow to use ConduitCall to retrieve information about the commit.
Test Plan:
Ran `bin/repository lookup-users rTESTe9683b64d3283f0b2d355fdbf231bc918b5ac0ab --trace` and saw the information returned (by making a request to `diffusion.querycommits` as the omnipotent user, signed with the device key).
Mucked with `cluster.addresses` and saw requests rejected.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
Subscribers: Krenair, epriestley, Korvin
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D10403
Summary:
Ref T2783. Ref T6706.
- Add `cluster.addresses`. This is a whitelist of CIDR blocks which define cluster hosts.
- When we recieve a request that has a cluster-based authentication token, require the cluster to be configured and require the remote address to be a cluster member before we accept it.
- This provides a general layer of security for these mechanisms.
- In particular, it means they do not work by default on unconfigured hosts.
- When cluster addresses are configured, and we receive a request //to// an address not on the list, reject it.
- This provides a general layer of security for getting the Ops side of cluster configuration correct.
- If cluster nodes have public IPs and are listening on them, we'll reject requests.
- Basically, this means that any requests which bypass the LB get rejected.
Test Plan:
- With addresses not configured, tried to make requests; rejected for using a cluster auth mechanism.
- With addresses configred wrong, tried to make requests; rejected for sending from (or to) an address outside of the cluster.
- With addresses configured correctly, made valid requests.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6706, T2783
Differential Revision: https://secure.phabricator.com/D11159
Summary:
Ref T2783. This is basically a more refined version of D10400, which churned a bit on things like SSH key storage, the actual way the signing protocol shook out, etc.
- When Phabricator tries to make an intra-cluster service call as the omnipotent user, sign it with the host's device key.
- Add `bin/almanac register` to say "this host is X device, identified by private key Y". This stores the keypair locally, adds the public key to Almanac, and trusts it.
Net effect is that once a host has been registered, the daemons can make calls to other nodes as the omnipotent user. This is primarily necessary so they can access repository API methods on remote hosts.
Test Plan:
- Ran `bin/almanac register` with various valid and invalid inputs.
- Verified keys get generated/added/stored properly.
- Made a device-signed cluster Conduit call.
- Made a normal old user-signed cluster Conduit call.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11158
Summary: We don't need to handle any request data for the `/status/` route, so we can simplify this code slightly.
Test Plan:
```lang=bash
> curl http://127.0.0.1:22281/status/
{"uptime":2543,"clients.active":0,"clients.total":0,"messages.in":0,"messages.out":0,"log":"/var/log/aphlict.log","version":6}
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11145
Summary: This class is unused after D6679.
Test Plan: `grep`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11149
Summary: This class is no longer used after D8168.
Test Plan: `grep`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11152
Summary: This class is no longer required after D10869.
Test Plan: `grep`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11154
Summary: Fixes T6846, cleans up spacing, makes it look scary red.
Test Plan: Fake an exception, see new layout
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6846
Differential Revision: https://secure.phabricator.com/D11161
Summary:
These didn't get translated quite right:
- We need to use `$total_count` because some languages have different words for 1, 2-3, and 4+ things (for example). So the strings might translate as:
- alincoln added a reviewer-one ...
- alincoln added reviewers-few ...
- alincoln added reviewers-many ...
- That is, while English has only "reviewer" and "reviewers", other languages have more plural forms, and "reviewer", "reviewers-few" and "reviewers-many" may be completely different words.
- In English, because we know we always have 2+ in this branch and the only special word is for 1, we can just drop this.
- Anyway, the %4$s stuff is counting assuming that $total_count is included in the string, so these were a off by one.
- See also D11160.
There a probably a couple more of these, but they should be easy enough to hunt down as they crop up.
Test Plan: Saw nice strings instead of empty strings, or invalid strings (after D11160).
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11162
Summary: The actual author here usually gets truncated by the extra text, which doesn't seem needed in most (all?) cases.
Test Plan: Look at list of recent commits.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11127
Summary: If we have a byline, but no attributes, the height of the entire object is off by 4 pixels.
Test Plan: Review list of recent commits, see correct padding.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11128
