1. 07 Oct, 2014 2 commits
    • Joshua Spence's avatar
      Minor formatting changes · 3cf9a582
      Joshua Spence authored
      Summary: Apply some autofix linter rules.
      
      Test Plan: `arc lint` and `arc unit`
      
      Reviewers: #blessed_reviewers, epriestley
      
      Reviewed By: #blessed_reviewers, epriestley
      
      Subscribers: epriestley, Korvin, hach-que
      
      Differential Revision: https://secure.phabricator.com/D10585
      3cf9a582
    • epriestley's avatar
      Add Merchants to Phortune · fcd2025a
      epriestley authored
      Summary:
      Ref T2787. Currently, you add payment providers (Stripe, Paypal, etc) in global configuration.
      
      Generally, this approach is cumbersome, limiting, and often hard for users to figure out. It also doesn't provide a natural way to segment payment receivers or provide web access to administrative payment functions like issuing refunds, canceling orders, etc. I think that stuff definitely needs to be in the web UI, and the rule for access to it can't reasonably just be "all administrators" in a lot of reasonable cases.
      
      The only real advantage is that it prevents an attacker from adjusting settings and pointing something at an account they control. But this attack can be mitigated through notifications, some sort of CLI-only merchant lock, payment accounts being relatively identifiable, etc.
      
      So introduce "merchants", which are basically payable entities. An individual merchant will have attached Paypal, Stripe, etc., accounts, and access rules. When you buy something in an application, the merchant to pay is also specified. They also provide an umbrella for dealing with permissions down the line.
      
      This may get a //little// cumbersome because if there are several merchants your saved card information is not shared across them. I think that will be fine in the normal case (most installs will have only one merchant). Even if it isn't and we leave providers global, I think introducing this is the right call from a web UI / permissions point of view. I'll play around with it in the next couple of diffs and figure out exactly where the line goes.
      
      Test Plan: Listed, created, edited, viewed merchants.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10648
      fcd2025a
  2. 06 Oct, 2014 14 commits
    • epriestley's avatar
      Modernize Phortune PHID constants · 61b1fe78
      epriestley authored
      Summary:
      Ref T2787. These were still stuck in the stone ages.
      
      (The handles are pretty skeletal but most aren't used anywehre.)
      
      Test Plan: Funded an initiative without anything breaking. Grepped for removed constants.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10647
      61b1fe78
    • epriestley's avatar
      Give Balanced provider complete workflow logic in Phortune · 19c1c07d
      epriestley authored
      Summary: Ref T2787. Like Stripe, this one is pretty easy to get working correctly on the "good" path and fataling out in a safe way on bad paths.
      
      Test Plan: Funded an initiative with Balanced.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10645
      19c1c07d
    • Bob Trahan's avatar
      Storage - fix more query errors by escaping collate_text and collate_sort · 0bbe3a6d
      Bob Trahan authored
      Summary: second bit of https://github.com/phacility/phabricator/pull/729
      
      Test Plan: this is a weird pull request merge
      
      Reviewers: chad, epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin, epriestley
      
      Differential Revision: https://secure.phabricator.com/D10646
      0bbe3a6d
    • epriestley's avatar
      Disable Phortune Paypal payment provider · 31817b90
      epriestley authored
      Summary:
      Ref T2787. For test charges, Paypal is putting the charge in a "payment review" state. Dealing with this state requires way more infrastructure than other providers: we're supposed to pause delivery, then poll Paypal every 6 hours to see if the review has resolved.
      
      Since I can't seem to generate normal test charges, I can't test Paypal for now. Disable it until we have more infrastructure.
      
      (This diff gets us further along, up to the point where I hit this issue.)
      
      Test Plan: Read documentation, rolled eyes.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10644
      31817b90
    • epriestley's avatar
      Give Stripe complete payment workflow logic in Phortune · 83e1a1dd
      epriestley authored
      Summary:
      Ref T2787. This basically already works correctly since the hard logic is external to the provider on API providers. Tweak a couple of things.
      
      Failures still just fail the cart completely, for now.
      
      Test Plan: Completed a charge with Stripe.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10640
      83e1a1dd
    • epriestley's avatar
      Give WePay complete payment logic in Phortune · 4ef547f8
      epriestley authored
      Summary:
      Ref T2787. This doesn't get all the edge cases quite correct, but is generally a safe, complete payment workflow:
      
        - Shares the actual charging state logic.
        - Makes it appropriately stateful with locking and transactions.
        - Gets the main flow correct.
        - Detects failure cases, just tends to blow up rather than help the user resolve them.
      
      Test Plan:
        - Charged with WePay.
        - Charged with Infinite Free Money.
        - Resumed an abandoned cart.
        - Hit all failure states where we just dead-end the cart. Not ideal, but (seemingly) complete/safe/correct.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10639
      4ef547f8
    • epriestley's avatar
      Give applications control over Phortune cart logic · 0beb8228
      epriestley authored
      Summary: Ref T2787. Similar to D10634, give applications more control over the cart workflow. For now this just means they get to pick exit URIs, but in the future they can manage more details of cart behavior.
      
      Test Plan: Funded an initiative and got returned to the initiative instead of dead-ending in Phortune.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10638
      0beb8228
    • Chad Little's avatar
      Add new Payment Icons · ed0b23cb
      Chad Little authored
      Summary: Fixes T6244, adds icons for payment providers. May split into different sprites down the road.
      
      Test Plan: Photoshop
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin, epriestley
      
      Maniphest Tasks: T6244
      
      Differential Revision: https://secure.phabricator.com/D10642
      ed0b23cb
    • Bob Trahan's avatar
      Configuration - re-jigger how we handle bad configuration files · 69b1ff30
      Bob Trahan authored
      Summary: just explicitly check if the file doesn't exist *first*, and then do the standard include thing with the more generic error if that doesn't work. Fixes T6255.
      
      Test Plan: re-started apache and phabricator still worked; will ask csilvers to give it a whirl too
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin, epriestley
      
      Maniphest Tasks: T6255
      
      Differential Revision: https://secure.phabricator.com/D10643
      69b1ff30
    • Bob Trahan's avatar
      Storage - escape collation type in create database code pathway · 928b4edf
      Bob Trahan authored
      Summary: without escapage here, creating databases fails. Fixes T6251.
      
      Test Plan: ran the command CREATE DATABASE foo COLLATION binary and it failed; ran the command CREATE DATABASE foo2 COLLATION "binary" and it worked; trusting that the %T still works as advertised.
      
      Reviewers: chad, epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin, epriestley
      
      Maniphest Tasks: T6251
      
      Differential Revision: https://secure.phabricator.com/D10641
      928b4edf
    • Chad Little's avatar
      Fix incorrect maniphest.update conduit UI · 923f6251
      Chad Little authored
      Summary: Fixes T6254 and renames status as string. Though maybe this should go through `formatStringConstants`?
      
      Test Plan: Reload Conduit page, see new text.
      
      Reviewers: btrahan, epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin, epriestley
      
      Maniphest Tasks: T6254
      
      Differential Revision: https://secure.phabricator.com/D10637
      923f6251
    • epriestley's avatar
      Add more structure to Phortune product purchasing flow · 35dc510e
      epriestley authored
      Summary:
      Ref T2787. When a user purchases a product in Phortune, transition the cart through a purchased state and invoke product callbacks so applications can respond to the workflow.
      
      Also shore up some stuff like preventing negative amounts of funding.
      
      Test Plan: Backed an initiative and saw it show up on the initiative after completing the purcahsing workflow.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10635
      35dc510e
    • epriestley's avatar
      Move Phortune product logic into applications · e9615b74
      epriestley authored
      Summary: Ref T2787. `Product` is currently a fairly heavy object, but as Phortune develops it makes a lot of sense to make it a lighter object and put more product logic in applications. Convert it into a fairly lightweight reference to applications. The idea is that Phortune is mostly providing a cart flow, and applications manage the details of products.
      
      Test Plan: Funded an initiative for $1.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10634
      e9615b74
    • epriestley's avatar
      Make Currency a more formal type · f86f9dc5
      epriestley authored
      Summary:
      Ref T2787. Phortune currently stores a bunch of stuff as `...inUSDCents`. This ends up being pretty cumbersome and I worry it will create a huge headache down the road (and possibly not that far off if we do Coinbase/Bitcoin soon). Even now, it's more of a pain than I figured it would be.
      
      Instead:
      
        - Provide an application-level serialization mechanism.
        - Provide currency serialization.
        - Store currency in an abstract way (currently, as "1.23 USD") that can handle currencies in the future.
        - Change all `...inUSDCents` to `..asCurrency`.
        - This generally simplifies all the application code.
        - Also remove some columns which don't make sense or don't make sense anymore. Notably, `Product` is going to get more abstract and mostly be provided by applications.
      
      Test Plan:
        - Created a new product.
        - Purchased a product.
        - Backed an initiative.
        - Ran unit tests.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T2787
      
      Differential Revision: https://secure.phabricator.com/D10633
      f86f9dc5
  3. 03 Oct, 2014 2 commits
    • epriestley's avatar
      Create new databases with appropriate collation · 3463ce8a
      epriestley authored
      Summary: Ref T1191. We don't create new databases with appropriate collation yet.
      
      Test Plan:
      Created a new database and saw it issue:
      
      ```
      >>> [10] <query> CREATE DATABASE IF NOT EXISTS `phabricator2_testo` COLLATE utf8mb4_bin
      ```
      
      Reviewers: btrahan, hach-que
      
      Reviewed By: hach-que
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10632
      3463ce8a
    • James Rhodes's avatar
      Implement storage of a host ID and a public key for authorizing Conduit between servers · 8fbebce5
      James Rhodes authored
      Summary:
      Ref T4209.  This creates storage for public keys against authorized hosts, such that servers can be authorized to make Conduit calls as the omnipotent user.
      
      Servers are registered into this system by running the following command once:
      
      ```
      bin/almanac register
      ```
      
      NOTE: This doesn't implement authorization between servers, just the storage of public keys.
      
      Placing this against Almanac seemed like the most sensible place, since I'm imagining in future that the `register` command will accept more information (like the hostname of the server so it can be found in the service directory).
      
      Test Plan: Ran `bin/almanac register` and saw the host (and public key information) appear in the database.
      
      Reviewers: #blessed_reviewers, epriestley
      
      Reviewed By: #blessed_reviewers, epriestley
      
      Subscribers: epriestley, Korvin
      
      Maniphest Tasks: T4209
      
      Differential Revision: https://secure.phabricator.com/D10400
      8fbebce5
  4. 02 Oct, 2014 10 commits
    • epriestley's avatar
      Add a setup warning about innodb_buffer_pool_size · 0ddb1875
      epriestley authored
      Summary: Fixes T6119. This is a little fuzzy, but generally bumping up `innodb_buffer_pool_size` to something bigger than the default (which is often anemic, at `8M`) is desriable, and it seems like it will fix the specific issue a user encountered in T6119.
      
      Test Plan: {F211855}
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T6119
      
      Differential Revision: https://secure.phabricator.com/D10630
      0ddb1875
    • epriestley's avatar
      Remove old, confusing configuration files · b16527d9
      epriestley authored
      Summary:
      Fixes T6230. These files have not been read by default for a long time, but users are frequently confused and try to edit `default.conf.php`.
      
      Remove the actual files. Allow `phabricator_read_config_file(...)` to continue working as though they exist so as to not break config-file-based installs.
      
      Test Plan:
      I used this script to make sure that removing `default.conf.php` won't change things for installs which are still using config files:
      
      ```
      <?php
      
      require_once 'scripts/__init_script__.php';
      
      $file = require 'conf/default.conf.php';
      $global = new PhabricatorConfigDefaultSource();
      $global_values = $global->getAllKeys();
      
      foreach ($file as $key => $value) {
        $global_value = idx($global_values, $key, (object)array());
      
        if ($value !== $global_value) {
          echo "{$key}\n\n";
          echo "FILE VALUE\n";
          var_dump($value);
          echo "\n";
          echo "DEFAULT VALUE\n";
          var_dump($global_value);
          return;
        }
      }
      ```
      
      These were the keys that had issues:
      
        - `log.access.format` Not specified in default.conf.php, safe to speciy.
        - `mysql.pass` Empty string in file, null in global. Same effect.
        - `metamta.default-addrress` One used `noreply@example.com`, one `noreply@phabricator.example.com`. These are just human-readable examples so it's safe to change behavior.
        - `metamta.domain` same as above, `example.com` vs `phabricator.example.com`.
        - `phpmailer.smtp-host` One used null, one empty string.
        - `phpmailer.smtp-protocol` As above.
        - `files.viewable-mime-types` File version is out of date.
        - `repository.default-local-path` Null in file, set in global. This is correct to set to a default value now.
        - `pygments.dropdown-choices` File version is out of date.
        - `environment.append-paths` File version is empty, global version adds common paths. This //could// change behavior, but the web behavior is better and more reasonable in general, and a system would need to be configured in a very bizarre way for this to be relevant.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T6230
      
      Differential Revision: https://secure.phabricator.com/D10628
      b16527d9
    • epriestley's avatar
      Correct column mutations for old versions of MySQL · d67b7f0f
      epriestley authored
      Summary:
      Ref T1191. Although I fixed some of the mutations earlier (in D10598), I missed the column mutations under old versions of MySQL. In particular, this isn't valid:
      
        - `ALTER TABLE ... MODIFY columnName VARCHAR(64) COLLATE binary`
      
      Issue the permitted version of this instead, which is:
      
        - `ALTER TABLE ... MODIFY columnName VARBINARY(64)`
      
      Also fixed an issue where a clean schema had the wrong nullability for a column in the draft table. Force it to the expected nullability.
      
      The other trick here is around the one column with a FULLTEXT index on it, which needs a little massaging.
      
      Test Plan:
        - Forced my local install to return `false` for utf8mb4 support.
        - Did a clean adjust into `binary` columns.
        - Poked around, added emoji to things.
        - Reverted the fake check and did a clean adjust into `utf8mb4` columns.
        - Emoji survived.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: fabe, epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10627
      d67b7f0f
    • epriestley's avatar
      Strip email signatures from Mailbox · e333017c
      epriestley authored
      Summary: thanks mailbox
      
      Test Plan: unit tests
      
      Reviewers: btrahan, chad
      
      Reviewed By: chad
      
      Subscribers: epriestley
      
      Differential Revision: https://secure.phabricator.com/D10629
      e333017c
    • epriestley's avatar
      Remove UTF8 BMP unit test and replace it with a general UTF8 test · 410c2ecb
      epriestley authored
      Summary:
      Ref T1191. After utf8mb4 conversion, these tests no longer pass because MySQL allows emoji and gclefs and such.
      
      We could keep these tests running by keeping a `ut8f_bin` table around somewhere, but we have no other use cases for it and it does not seem worth the added complexity. All these BMP-only codepaths are on the way out.
      
      Update the `%s` / `%B` test to make sure it's rejecting invalid byte sequences, which are still not permitted.
      
      Test Plan: Tests now pass.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10621
      410c2ecb
    • epriestley's avatar
      Automatically build all Lisk schemata · 8fa8415c
      epriestley authored
      Summary:
      Ref T1191. Now that the whole database is covered, we don't need to do as much work to build expected schemata. Doing them database-by-database was helpful in converting, but is just reudndant work now.
      
      Instead of requiring every application to build its Lisk objects, just build all Lisk objects.
      
      I removed `harbormaster.lisk_counter` because it is unused.
      
      It would be nice to autogenerate edge schemata, too, but that's a little trickier.
      
      Test Plan: Database setup issues are all green.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley, hach-que
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10620
      8fa8415c
    • epriestley's avatar
      Use `sort`, not `text`, for search index table · e4e5a200
      epriestley authored
      Summary: Ref T1191. The index's case sensitivity depends on the column type. Using `text` makes the search case-sensitive, which is not desirable.
      
      Test Plan: After adjustment, searched for "PROJECTS" and found hits against "projects".
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10619
      e4e5a200
    • epriestley's avatar
      Make execution order of Herald rules explicit · 5f828050
      epriestley authored
      Summary: Fixes T6211. This gives Herald rules an explicit execution order, which seems generally good. See some discussion on T6211 and inline.
      
      Test Plan:
        - Added unit test.
        - Dry ran rules and saw rules appear in the expected order in the transcript.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T6211
      
      Differential Revision: https://secure.phabricator.com/D10624
      5f828050
    • epriestley's avatar
      Differentiate between "no pygmetnize" and "nonworking pygmentize" during setup · 3c6781b1
      epriestley authored
      Summary: Fixes T6210. The current messaging may be confusing if `pygmentize` is available but broken.
      
      Test Plan: Faked the binary names and hit the errors, which seemed helpful.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T6210
      
      Differential Revision: https://secure.phabricator.com/D10626
      3c6781b1
    • epriestley's avatar
      Fix TokenGiven stories for Asana · 70189094
      epriestley authored
      Summary: Ref T6201. This isn't quite perfect but should be good enough. At some point far in the future I plan to revamp feed rendering a bit. This should possibly become a real ApplicationTransaction story eventually, too.
      
      Test Plan: {F211777}
      
      Reviewers: btrahan, chad
      
      Reviewed By: chad
      
      Subscribers: epriestley
      
      Maniphest Tasks: T6201
      
      Differential Revision: https://secure.phabricator.com/D10625
      70189094
  5. 01 Oct, 2014 12 commits
    • epriestley's avatar
      Make `#🐳` work properly · fda0b086
      epriestley authored
      Summary:
      Ref T6223. Two issues:
      
        - We don't use `/u` mode on these regexps. Without `/u`, the `\w`/`\W`/`\s`/`\S` modifiers have bad behavior on non-ASCII bytes. Add the flag to use unicode mode, making `\w` and `\s` behave like we expect.
          - We might possibly want to do something different here eventually (for example, if the `/u` flag has some huge performance penalty) but this seems OK for now.
        - We use `\b` (word boundary) to terminate the match, but `🐳` is not a word character. Use `(?!\w)` instead ("don't match before a word character") which is what we mean.
      
      Test Plan: {F211498}
      
      Reviewers: btrahan, chad
      
      Reviewed By: chad
      
      Subscribers: epriestley
      
      Maniphest Tasks: T6223
      
      Differential Revision: https://secure.phabricator.com/D10618
      fda0b086
    • epriestley's avatar
      Purge readthrough caches before applying schema adjustments · 0a647313
      epriestley authored
      Summary: Ref T1191. The bulk of the slowness in T1191 is copying tables. In some cases, we can't avoid this, but we have various readthrough caches which may be very large and are safe to drop, and dropping them is very quick (much less than 1 second). In particular, dropping the `changeset_parse_cache` made the process at least ~8 minutes faster on `secure.phabricator.com` (I killed it after 8 minutes, so I'm not sure what the real number is).
      
      Test Plan: Ran `bin/storage adjust` and saw it drop caches before applying adjustments.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10616
      0a647313
    • epriestley's avatar
      Truncate very old, overlong Maniphest mail keys · 3a644cf6
      epriestley authored
      Summary:
      Ref T1191. Long ago, Maniphest generated with 40-character mail keys. These prevent the migration to `bytes20`. We had about 300 of these on secure.phabricator.com from several years ago.
      
      Just truncate them. This adjusts reply-to addresses, but it's very likely that none are relevant anymore.
      
      Test Plan: Ran migration on `secure.phabricator.com` to truncate keys.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10615
      3a644cf6
    • epriestley's avatar
      Drop very old schema_version table if it exists · 3629ebeb
      epriestley authored
      Summary: Ref T1191. This predates the mdoern patch stuff and may exist on very, very old installs. By the time they apply this patch, it's guaranteed it won't matter anymore. Drop it to make the schemata consistent with expectations.
      
      Test Plan: Ran patch on installs with and without the table.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10611
      3629ebeb
    • epriestley's avatar
      Convert SavedQuery / NamedQuery to text, not bytes · 4eb43996
      epriestley authored
      Summary: Ref T1191. Similar issue to D10613. This column usually has a hash exactly 12 bytes long, but sometimes stores an internal builtin query name like "open", "all", etc. It might be nice to promote those to 12-byte hashes of a consistent length eventually, but for now just make this a variable-length column.
      
      Test Plan: Ran migration, no longer saw issues with reordering builtin saved searches.
      
      Reviewers: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10614
      4eb43996
    • epriestley's avatar
      Convert DivinerLiveSymbol to text, not bytes · f881b5dd
      epriestley authored
      Summary:
      Ref T1191. The `bytes` types are BINARY(...), which is fixed-length and zero-pads. These hashes are not 64 characters long, so migrating them to `binary` ends up with a bunch of zero-padding.
      
      Instead, migrate them to `text` so we drop the zero padding. It would be vaguely nice to either introduce a `varbytes` type (ick) or change the hash size to a standard size (nicer) eventually, but this isn't very important.
      
      Test Plan: Will adjust `secure.phabricator.com`.
      
      Reviewers: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10613
      f881b5dd
    • epriestley's avatar
      Fix `adjust` phases for keys · 5ce3575f
      epriestley authored
      Summary: Ref T1191. I renamed the phases but missed these two since I didn't have any more key issues locally.
      
      Test Plan: Ran `bin/storage adjust` in production with key issues.
      
      Reviewers: btrahan
      
      Subscribers: chad, epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10612
      5ce3575f
    • epriestley's avatar
      Support AUTO_INCREMENT in `bin/storage adjust` · 300172e7
      epriestley authored
      Summary:
      Ref T1191. When changing the column type of an AUTO_INCREMENT column, we currently may lose the autoincrement attribute.
      
      Instead, support it. This is a bit messy because AUTO_INCREMENT columns interact with PRIMARY KEY columns (tables may only have one AUTO_INCREMENT column, and it must be a primary key). We need to migrate in more phases to avoid this issue.
      
      Introduce new `auto` and `auto64` types to represent autoincrement IDs.
      
      Test Plan:
        - Saw autoincrement show up correctly in web UI.
        - Fixed an autoincrement issue on the XHProf storage table with `bin/storage adjust` safely.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10607
      300172e7
    • epriestley's avatar
      Provide `bin/storage quickstart` to automate generation of `quickstart.sql` · 0d7489da
      epriestley authored
      Summary:
      Ref T1191. Currently, the `quickstart.sql` gets generated in a pretty manual fashion. This is a pain, and will become more of a pain in the world of utf8mb4.
      
      Provide a workflow which does upgrade + adjust + dump + destroy, then massages the output to produce a workable `quickstart.sql`.
      
      Test Plan: Inspected output; I'll test this more throughly before actually generating a new quickstart, but that's some ways away.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10603
      0d7489da
    • epriestley's avatar
      Use binary collations for most text · 1dfa94e5
      epriestley authored
      Summary:
      Ref T1191. For most text columns, we either don't care if "a" and "A" are the same, or we expect them to be different (for example: keys, domains, secrets, etc). Default text columns to the `_bin` collation so they are compared by strict character value. This is safer in cases where we aren't sure.
      
      For some text columns, we allow the user to sort by the column in the UI (like Maniphest task titles) or we do care that "A" and "a" are the same (for example: project names). Introduce a new class of virtual data types, the "sort..." types, to cover these columns. These are like the "text..." types but use sorting collations which treat "A" and "a" the same.
      
      Test Plan:
        - Made an effort to identify all columns where the UI relies on database collation.
        - Ran `bin/storage adjust` and cleared all warnings.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: beng, epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10602
      1dfa94e5
    • epriestley's avatar
      Fix almost all remaining schemata issues · 4fcc634a
      epriestley authored
      Summary:
      Ref T1191. This fixes nearly every remaining blocker for utf8mb4 -- primarily, overlong keys.
      
      Remaining issue is https://secure.phabricator.com/T1191#77467
      
      Test Plan: I'll annotate inline.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley, hach-que
      
      Maniphest Tasks: T6099, T6129, T6133, T6134, T6150, T6148, T6147, T6146, T6105, T1191
      
      Differential Revision: https://secure.phabricator.com/D10601
      4fcc634a
    • epriestley's avatar
      Allow `bin/storage adjust` to make key changes · a5ce56aa
      epriestley authored
      Summary:
      Ref T1191. These are a bit tricky because keys can interact with column changes, so basically we do three phases:
      
        1. Nuke all bad keys.
        2. Make all column (and database/table) changes.
        3. Fix all nuked keys.
      
      Test Plan: Ran migration locally. See note for remaining issues.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      Subscribers: epriestley
      
      Maniphest Tasks: T1191
      
      Differential Revision: https://secure.phabricator.com/D10599
      a5ce56aa