1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-11 09:22:40 +01:00
Commit graph

407 commits

Author SHA1 Message Date
epriestley
ea32c541e4 Remove all optimistic lock code from Lisk
Summary: We never use this and almost certainly never will. It's been in Lisk for ~7 years but is a solution in search of a problem. It causes a conflict with any DAO that has a `version` column.

Test Plan: Browsed around, performed inserts and updates. Edited a Phriction document.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran, leslie.chong

Differential Revision: https://secure.phabricator.com/D3625
2012-10-04 13:55:43 -07:00
epriestley
fe6022cd97 Load only valid properties in LiskDAO::loadFromArray()
Summary:
Fixes a TODO, and silences a warning introduced by D3601.

There are several cases where we load data like:

  SELECT *, ... AS extraData FROM ...

...and then pass it to `loadAllFromArray()`. Currently, this causes us to set an `extraData` property on the object.

This idiom seems fairly useful and non-dangerous, so I made `loadFromArray()` just drop extra keys.

Since we hit this loop a potentially huge number of times (10,000+ for full Maniphest pages) I did some microoptimization. Lisk is hot enough that it's one of the few places where it's worthwhile (see D1291).

Test Plan: Loaded homepage, no longer got warnings about `viewerIsMember` from Project queries. Browsed ~10 apps, didn't see any issues.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3606
2012-10-03 15:42:44 -07:00
vrana
4682e0c104 Warn against writing to undeclared properties
Summary:
I make this error quite often: I forget to declare a property I am writing to or I make a typo in it.
PHP implicitly creates a public property which I don't like.

I would much rather see a linter warning me against this than this runtime check but writing it is very difficult:

- We need to explore all parents of the class we are checking.
- It is even possible that children will declare that property but it's OK to treat this as error anyway.
- We can extend also builtin or external classes.
- It's somewhat doable for `$this` but even more complex for any `$obj` because we don't know the class of it.

This should catch significant part of these errors and I'm fine with that.

I don't plan escalating to exception because this error is not fatal and should not stop the application from working.

Test Plan: Loaded homepage, checked log.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3601
2012-10-03 11:54:03 -07:00
vrana
7c39c4ca7d Declare common Lisk properties
Summary:
Calling `->setPHID()` or other common Lisk setters creates an implicit public property `$phid`.
I don't like implicit properties and I see them as errors.
Its public visibility also makes me nervous and is vulnerable to bypassing any setters we may create.

Test Plan: Loaded homepage, checked log.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3600
2012-10-03 11:53:37 -07:00
Bob Trahan
9e1b643896 Phame - allow blogs to specify custom URIs
Summary: this then enables people to create blog.theircompany.com. And for us, blog.phacility.com...!

Test Plan:
 - created custom URIs of various goodness and verified the error messages were sensical.
 - verified if "false" in configuration then custom uri stuff disappears

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1373

Differential Revision: https://secure.phabricator.com/D3542
2012-09-30 17:10:27 -07:00
vrana
5a5d0b2b56 Allow custom request checker prior to displaying page
Summary: We want to allow a broader access to our installation but we need to check the request in that case.

Test Plan:
Created a simple `PhabricatorRequestChecker` returning a custom controller.
Verified that this controller is used when accessing any page.
Returned `null` from this checker and verified that all 209 Phabricator pages are accessible.

Reviewers: epriestley

Reviewed By: epriestley

CC: scottmac, aran, Korvin, btrahan

Differential Revision: https://secure.phabricator.com/D2488
2012-09-27 15:34:54 -07:00
hfcorriez
065e977b3c Fix $_SERVER['CONTENT_TYPE'] trigger a NOTICE error
See T1762.

Reviewed by: epriestley
2012-09-27 12:50:09 -07:00
vrana
d119ac672f Remember action in Differential comment draft
Summary:
It happens to me quite often that I leave the window with revision (by closing it or by visiting a link from it).
When I return then the comment draft is there so I clowncopterize it but forget that I wanted to take some other action than Comment.

Test Plan: Selected "Add Reviewers", added some reviewers, closed the window, opened it - the action and reviewers were still there.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3530
2012-09-21 13:05:09 -07:00
vrana
8e7ae7b33a Optimize reindex_everything.php
Summary:
We have two troubles with this script:

# Our revisions and commits don't fit in the memory. (Our tasks do :-).)
# Reindexing revisions is slow.

Test Plan: Ran it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3483
2012-09-17 13:34:41 -07:00
vrana
3c2cb13153 Fix bad rebase
Auditors: nh
2012-09-17 13:29:09 -07:00
Nick Harper
5978bbfc64 Do sampled profiling of requests
Summary:
People have occasionally complained about phabricator being slow. We have
the access log to look at to see when slowness happens, but it doesn't tell
us much about why it happened. Since it's usually a sporadic issue that's
reported, it's hard to reproduce and then profile. This change will allow us
to collect sampled profiles so we can look at them when slowness occurs.

Test Plan:
checking that sampling works correctly:
- set rate to 0; do several page loads; check no new entries in table
- set rate to 1; check that there's a new row in the table for each page load
- set rate to 10; check that some requests write to table and some don't
check new ui for samples:
- load /xhprof/list/all/, see a list with a lot of samples
- load /xhprof/list/sampled/, see only sampled runs
- load /xhprof/list/manual/, see only non-sampled runs
- load /xhprof/list/my-runs/, se only my manual runs

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3458
2012-09-17 10:53:45 -07:00
epriestley
a1df1f2b70 Allow projects to be set as policies
Summary:
  - Renames `PhabricatorPolicyQuery` to `PhabricatorPolicyAwareQuery` (a query which respects policy settings).
  - Introduces `PhabricatorPolicyQuery`, which loads available policies (e.g., "member of project X").
  - Introduces `PhabricatorPolicy`, which describes a policy.
  - Allows projects to be set as policies.
  - Allows Paste policies to be edited.
  - Covers crazy cases where you make projects depend on themselves or each other because you are a dastardly villan.

Test Plan: Set paste and project policies, including crazy policies like A -> B -> A, A -> A, etc.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3476
2012-09-13 10:15:08 -07:00
epriestley
b39175342d Add paste policy storage
Summary: Add storage to Pastes for view policies.

Test Plan: Set policies on pastes, see next diff.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3474
2012-09-13 10:11:14 -07:00
Pieter Hooimeijer
5883b4f50c adding comments to ponder
Summary: This is pretty spartan, but it does the job.

Test Plan:
Patch, update storage, add some comment
to your favorite question or answer.

Reviewers: nh, vrana, epriestley

Reviewed By: epriestley

CC: aran, Korvin, starruler, syrneus, me.here, victorzarate7

Maniphest Tasks: T1645

Differential Revision: https://secure.phabricator.com/D3471
2012-09-11 12:13:20 -07:00
vrana
99949bec8d Link Phabricator tests
Test Plan: This diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3457
2012-09-07 15:04:44 -07:00
epriestley
dbc8218f06 Add 'viewer' to some Remarkup callsites
Summary:
I want to implement a `{P123}` rule to embed pastes, but we need viewers everywhere before it will work with privacy.

This is not exhaustive; many Remarkup callsites haven't been converted to `PhabricatorMarkupInterface` yet.

Test Plan: Looked at Maniphest, Differential, Diffusion, Phriction; added markup, made edits and hit previews.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3428
2012-09-05 11:40:48 -07:00
vrana
8ff52c0b6c Set viewer for all handles loaded in controllers
Summary:
I've replaced all `id(new PhabricatorObjectHandleData(...))->loadHandles()` by `$this->loadViewerHandles(...)`.
Lint caught one usage in a static method.

Test Plan: Displayed revision with sporadic author.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3432
2012-09-04 23:14:26 -07:00
KorvinSzanto
da2fc57d77 Fix irc server login
Summary: Previously, the identification string was thrown at the server long before you were connected, I've moved this to the end of the motd raw, and now errthangz gud

Test Plan: Register an account for your bot to use, give your bot the correct nick and password, then watch

Reviewers: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D3410
2012-08-30 13:52:33 -07:00
epriestley
7fbcdfc52c Make CelerityController extend PhabricatorController
Summary:
Currently, CelerityController extends AphrontController, not PhabricatorController. (I think I imagined Celerity being somewhat stand-alone and didn't want to create a dependency.)

This creates a concrete problem if a static resource is missing, since we throw an exception, but the higher-level exception handlers depend on the User existing in order to show an appropriate response page. This is the only controller which doesn't extend PhabricatorController, and it doesn't seem worthwhile to make a weird edge case out of it.

Specific repro case is:

  - Remove `externals/javelin/` (or forget to run `git submodule update --init`).
  - Load a static resource.
  - Get "[Rendering Exception] Argument 1 passed to PhabricatorMainMenuView::setUser() must be an instance of PhabricatorUser, null given, called in /services/apache/phabricator/phabricator/src/view/page/PhabricatorStandardPageView.php on line 435 and defined"

Test Plan:
  - Followed above steps, no more fataling.
  - Verified this is the only weird controller.

Reviewers: voldern, vrana, btrahan

Reviewed By: voldern

CC: aran

Differential Revision: https://secure.phabricator.com/D3389
2012-08-28 13:46:35 -07:00
vrana
5f3dc3b7ae Make storage.mysql-engine.max-size independent on max_allowed_packet
Summary:
I like systems that just work. It is possible to store files larger than max_allowed_packet in MySQL and we shouldn't demand it.

It also fixes a problem when file was smaller than `storage.mysql-engine.max-size` but its escaped version was larger than `max_allowed_packet`.

Test Plan: Reduced the size to 5e4, uploaded 90 kB file, checked the queries in DarkConsole, downloaded the file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3392
2012-08-27 15:56:45 -07:00
Evan Priestley
1d72cde41f Merge pull request #190 from KorvinSzanto/master
Fix "where is symbol" ircbot handler
2012-08-25 17:06:37 -07:00
KorvinSzanto
6c44587717 Fix "where is symbol" ircbot handler
Summary: In my haste, I forgot a trailing ?

Test Plan: Try both "Where is Derp?" and "Where in the world is Derp?"

Reviewers: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D3387
2012-08-25 16:54:48 -07:00
epriestley
85bf88e400 Allow pastes to be flagged
Summary:
This does a few things:

  - Allows you to flag pastes. This is straightforward.
  - Allows Applications to register event listeners.
  - Makes object action lists emit a 'didrenderactions' event, so other applications can add more actions. The Flags application injects its action in this way. This should generally make it much easier to add actions to objects when we add new applications, with less code duplication and better modularity. We have a really hacky version of this in Differential that I want to get rid of in lieu of this more general approach. I'm going to make object lists do the same thing, so any application can jump in and add stuff.

Test Plan: Flagged and unflagged pastes. Viewed home page, differential, flags list.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3377
2012-08-24 13:19:47 -07:00
vrana
45e93495e4 Add method for loading relative edges
Summary:
More and more relations are going under edges and I can't work with them from Relatives framework.

This doesn't have the nice transitive property of normal relatives (loading relative objects from relatives loads all of them at once) but I can add it when I need it.

I plan to use it in D3085 (after converting relationships to edges).

Test Plan:
  $task = id(new ManiphestTask())
    ->loadOneWhere('phid = %s', $phid);
  print_r($task->loadRelativeEdges(4));

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3344
2012-08-20 21:11:55 -07:00
epriestley
772a942366 Detect 'post_max_size' more robustly
Summary:
Currently, when a user runs "arc diff" and the diff exceeds PHP's 'post_max_size', they get a very confusing and irrelevant error about a missing Conduit session token. The reason for this is that 'post_max_size' doesn't build $_POST, so //all// the data is missing.

We try to detect this, but currently only do so effectively for specific file upload forms. Broaden the detection to cover all cases.

Previously, we ran into an issue where Firefox + HTML5 drag-and-drop uploads would get a false positive on this detection. I dug into this and added the Content-Type checks, which correctly handle that case.

Test Plan: With small and large 'post_max_size', ran small and large normal, HTML5 and multipart/form-data POST requests against Phabricator in Safari and Firefox. Got desired beahviors.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: tido, aran

Differential Revision: https://secure.phabricator.com/D3320
2012-08-17 13:41:57 -07:00
epriestley
2628c91454 Minor, MySQL requires -pxxx, not -p xxx for passwords. 2012-08-17 08:17:23 -07:00
Evan Priestley
c839dc29a6 Merge pull request #185 from KorvinSzanto/master
Added novelty Where in the world is Symbol? match to IRCbot.
2012-08-16 17:46:33 -07:00
vrana
f770900983 Save edge type as number
Summary: We use numbers here and I see no reason for strings.

Test Plan:
  $ bin/storage upgrade

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3303
2012-08-16 14:43:03 -07:00
KorvinSzanto
528589edc6 Added novelty Where in the world is Symbol? match to IRCbot.
Summary: Added match to the novel statement: Where in the world is derp?

Test Plan: Say something like "Where in the world is CarmenSandiego?"

Reviewers: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D3318
2012-08-16 14:31:44 -07:00
epriestley
5342bb1073 Don't fatal on daemon status updates from phd
Summary:
See D3126, T1667, T1658. Prior to D3126, `phd` did not use MySQL directly. Now that it does, there are at least two specific problems (see inline comment).

In the long term, we should probably break this dependency and use Conduit. However, we don't currently have access to the daemon log ID and getting it is a mess (the overseer generates it), and I think I want to rewrite how all this works at some point anyway (the daemon calls are currently completely unauthenticated, which is silly -- we should move them to an authenticated channel at some point, I think).

Test Plan: Ran `phd stop` with a bad MySQL config against a non-running daemon, didn't get a query error.

Reviewers: nh, vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1667, T1658

Differential Revision: https://secure.phabricator.com/D3314
2012-08-16 14:13:24 -07:00
epriestley
2a815e0715 Fix a PhutilOpaqueEnvelope issue with bin/storage dump
Summary: We need to open the envelope here.

Test Plan: Ran `bin/storage dump` without errors.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3315
2012-08-16 14:13:10 -07:00
vrana
6623a721d3 Fix letter case 2012-08-15 17:22:46 -07:00
vrana
14cfdeca92 Fix lint error 2012-08-15 13:16:06 -07:00
Alan Huang
f736ca047a Make countdowns (internally) embeddable
Summary:
You can now embed countdowns in Remarkup! Not sure what it's
useful for, but there you have it.

Also I may have made a hash of the markup code; I don't really know what
I'm doing.

Test Plan: Make a new countdown, put `{C###}` in a Differential comment.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1053

Differential Revision: https://secure.phabricator.com/D3290
2012-08-14 19:19:23 -07:00
Nick Harper
3908f7db2e Show list of non-exited daemons
Summary: This is arguably a more useful view than listing all daemons.

Test Plan: Looked at list, only saw daemons that haven't exited

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3286
2012-08-14 18:01:15 -07:00
epriestley
6f3d15bb82 Remove hard-coded tests against 'phabricator' namespace in setup
Summary: See https://github.com/facebook/arcanist/issues/49

Test Plan:
  - Turned on setup mode with non-default namespace.
  - Verified that setup tests passed.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3274
2012-08-13 17:10:51 -07:00
vrana
1379876db7 Remove subsets when cleaning Lisk set 2012-08-13 11:39:59 -07:00
vrana
f841491524 Use Lisk sets in fact update iterator
Summary:
Fact engines loading dependent objects are super slow because they load them one by one.
This diff put each page in a Lisk set allowing engines to use `loadRelatives()`.

It also introduces `clearSet()` method which is somewhat neccessary in PHP < 5.3 or with disabled cyclic [[ http://php.net/gc | GC ]].

Test Plan:
  $iterator = new PhabricatorFactUpdateIterator(new DifferentialRevision());
  foreach ($iterator as $revision) {
    $diffs = $revision->loadRelatives(new DifferentialDiff(), 'revisionID');
    echo memory_get_usage() . "\n";
  }

Experienced not-steadily-increasing memory usage and much faster loading.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3247
2012-08-13 10:26:17 -07:00
epriestley
7b068d3e46 Reverse project paging order
Summary:
Currently, we're showing projets in reverse order (Z..A) because most cursor pagers go from high IDs to low IDs.

Allow sequence to be reversed; reverse it.

Also simplify some query/paging stuff.

Test Plan: Set page size to 1, paged back and forth.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3221
2012-08-11 07:05:45 -07:00
epriestley
bd0be1c650 Add View, Edit and Join policies to PhabricatorProject
Summary:
  - In ProjectQuery, always load the viewer's membership in the project because we need it to perform a CAN_VIEW test.
  - Add storage for the view, edit and join policies.
  - A user can always view a project if they are a member.
  - A user can always join a project if they can edit it.
  - Editing a project requires both "view" and "edit" permissions, and edit does not imply view.
  - This has no effect on the application yet.

Test Plan: See next diff.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3219
2012-08-11 07:05:01 -07:00
epriestley
6cbc67ea75 Improve PolicyFilter and PolicyQuery
Summary:
  - Allow PolicyQuery to require specific sets of capabilities other than "CAN_VIEW", like edit, etc. The default set is "view".
  - Add some convenience methods to PolicyFilter to test for capabilities.

Test Plan: Viewed pastes, projects, etc. Used other stuff in future diff.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3212
2012-08-11 07:02:31 -07:00
vrana
b2c9edd17d Fix doc links 2012-08-10 14:21:55 -07:00
epriestley
62b06f0f5d Fix a memory leak in PhabricatorGlobalLock
Summary:
We currently cache all connections in LiskDAO so we can roll back transactions when fixtured unit tests complete.

Since we establish a new connection wrapper each time we establish a global lock, this cache currently grows without bound.

Instead, pool global lock connections so we never have more than the largest number of locks we've held open at once (in PullLocalDaemon, always 1).

Another way to solve this is probably to add an "onclose" callback to `AphrontDatabaseConnection` so that it can notify any caches that it been closed. However, we currently allow a connection to be later reopened (which seeems reasonable) so we'd need a callback for that too. This is much simpler, and this use case is unusual, so I'd like to wait for more use cases before pursing a more complicated fix.

Test Plan:
Ran this in a loop:

    while (true) {
      for ($ii = 0; $ii < 100; $ii++) {
        $lock = PhabricatorGlobalLock::newLock('derp');
        $lock->lock();
        $lock->unlock();
      }
      $this->sleep(1);
    }

Previously it leaked ~100KB/sec, now has stable memory usage.

Reviewers: vrana, nh, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1636

Differential Revision: https://secure.phabricator.com/D3239
2012-08-10 11:28:43 -07:00
Pieter Hooimeijer
64472dd7b8 Adding Ponder-related files.
Summary:
Ponder is similar in spirit to the Wiki tool, but uses a Q&A
format and up/downvotes to signal user sentiment. Popular
questions are moved to the top of the feed on a 5-minute
cycle based on age (younger is better) and vote count (higher
is better).

Pre-apologies for noob diff.

Test Plan:
- `./bin/phd list` Should include `PonderHeatDaemon`; phd launch it
  if necessary.

- Navigate to /ponder/ ; observe sanity when adding questions,
  voting on them, and adding answers.

- Confirm that questions and answers are linkable using Q5 / Q5#A5 formatted object links.

- Confirm that searching for Ponder Questions works using built-in
  search.

Feedback on code / schema / whatever organization very welcome.

Reviewers: nh, vrana, epriestley

Reviewed By: epriestley

CC: gmarcotte, aran, Korvin, starruler

Differential Revision: https://secure.phabricator.com/D3136
2012-08-10 10:44:04 -07:00
epriestley
d4cbb00d3b Fix offset-without-limit case in Policy query
Summary: Apparently I am not qualified to do basic math.

Test Plan: Unit test.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3218
2012-08-09 11:40:55 -07:00
epriestley
d32926e5f7 Work-in-progress schema for Facts app
Summary: See discussion in D3078 for why I've separated this. Pretty sure it's not quite ready yet -- I want to build a couple of things on it so we have a better idea of what we need (autoincrement ID? <factType, objectA, epoch> primary key? objectB column? valueZ?) and don't need to do a ton of schema patches.

Test Plan: Applied patches, ran D3078.

Reviewers: vrana, btrahan, majak

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1581, T1562

Differential Revision: https://secure.phabricator.com/D3088
2012-08-09 08:40:56 -07:00
epriestley
3460da5f34 Fix limits in queries
Summary: I think this is simpler? Includes test cases.

Test Plan: Ran tests. Loaded /paste/.

Reviewers: vrana, nh

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3209
2012-08-08 18:58:49 -07:00
Pieter Hooimeijer
9debf779d6 Allow edge query filtering by destination PHIDs
Summary: See title. Adds features needed for D3136.

Test Plan:
Observe sanity (or run D3136 in a sandbox
and observe that voting works).

Reviewers: epriestley

Reviewed By: epriestley

CC: gmarcotte, aran, Korvin

Differential Revision: https://secure.phabricator.com/D3205
2012-08-08 18:57:38 -07:00
epriestley
ab92242e00 Extend PhabricatorPolicyQuery from PhabricatorOffsetPagedQuery
Summary:
A few goals here:

  - Slightly simplify the Query classtree -- it's now linear: `Query` -> `OffsetPagedQuery` (adds offset/limit) -> `PolicyQuery` (adds policy filtering) -> `CursorPagedPolicyQuery` (adds cursors).
  - Allow us to move from non-policy queries to policy queries without any backward compatibility breaks, e.g. Conduit methods which accept 'offset'.
  - Separate the client limit ("limit") from the datafetch hint limit ("rawresultlimit") so we can make the heurstic smarter in the future if we want. Some discussion inline.

Test Plan: Expanded unit tests to cover offset behaviors.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3192
2012-08-08 12:15:58 -07:00
Bob Trahan
8a4c08b01d Allow commits to be associated with projects and associated goodies
Summary:
- Commit detail view
 - List of projects
 - "edit" action which takes the user to a simple form where they can only add / remove projects.
-  Integrated the project relationship into the commit search indexer
 - fixed a bug from D790; it seems you must select the column if you're going to join against it later. Without this change searching for author or projectfails 100% for me.

Test Plan: added and removed projects. verified appropriate projects showed up in detail and edit view. searched for commits by project and found the ones I was supposed to...!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1614

Differential Revision: https://secure.phabricator.com/D3189
2012-08-08 10:03:41 -07:00
vrana
523cba5da4 Use Remarkup document link 2012-08-07 18:51:52 -07:00
epriestley
f9fcaa1f84 Migrate project membership to edges
Summary:
  - Store project members in edges.
  - Migrate existing members to edge storage.
  - Delete PhabricatorProjectAffiliation.
  - I left the actual underlying data around just in case something goes wrong; we can delete it evenutally.

Test Plan:
  - Ran migration.
  - Created a new project.
  - Joined and left a project.
  - Added and removed project members.
  - Manually called PhabricatorOwnersOwner::loadAffiliatedUserPHIDs() to verify its behavior.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3186
2012-08-07 18:02:05 -07:00
epriestley
ed4a155c91 Rename "IDPaged" to "CursorPaged", "executeWithPager" to "executeWith[Cursor|Offset]Pager"
Summary:
I'm trying to make progress on the policy/visibility stuff since it's a blocker for Wikimedia.

First, I want to improve Projects so they can serve as policy groups (e.g., an object can have a visibility policy like "Visible to: members of project 'security'"). However, doing this without breaking anything or snowballing into a bigger change is a bit awkward because Projects are name-ordered and we have a Conduit API which does offset paging. Rather than breaking or rewriting this stuff, I want to just continue offset paging them for now.

So I'm going to make PhabricatorPolicyQuery extend PhabricatorOffsetPagedQuery, but can't currently since the `executeWithPager` methods would clash. These methods do different things anyway and are probably better with different names.

This also generally improves the names of these classes, since cursors are not necessarily IDs (in the feed case, they're "chronlogicalKeys", for example). I did leave some of the interals as "ID" since calling them "Cursor"s (e.g., `setAfterCursor()`) seemed a little wrong -- it should maybe be `setAfterCursorPosition()`. These APIs have very limited use and can easily be made more consistent later.

Test Plan: Browsed around various affected tools; any issues here should throw/fail in a loud/obvious way.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3177
2012-08-07 11:54:06 -07:00
Alan Huang
bcb9de4ea1 Add a context field to symbol objects
Summary:
See T1602.

This is just the minimal functional patch; the scripts will continue
working because of the `DEFAULT ''`.

Test Plan:
Can't fully test this until I get more code working, but
nothing broke horribly yet.

Reviewers: epriestley

Reviewed By: epriestley

CC: nh, aran, Korvin

Maniphest Tasks: T1602

Differential Revision: https://secure.phabricator.com/D3147
2012-08-06 12:20:45 -07:00
Marcel Beck
99e9a26192 Separates the PID and log directories of daemons
Summary: The Log and PID directory should be separable in the config file

Test Plan: Start the daemons, and check if the pid and log files are stored in directories that were specified in the config file.

Reviewers: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3149
2012-08-05 08:35:24 +02:00
epriestley
194dc40672 Add a meta-application
Summary:
  - Adds a new "Applications" application.
  - Builds an application list via application config instead of via hard-coding, so we can move toward better concepts of installing/uninstalling applications, etc.
  - Applications indicate that they need attention with notice counts and brief status messages rathern than 50 giant tables of all sorts of app data.

I want to try replacing the home screen with this screen, pretty much. Not sure if this is totally crazy or not. What does everyone else think?

Test Plan: Will add screenshots.

Reviewers: btrahan, chad, vrana, alanh

Reviewed By: vrana

CC: aran, davidreuss, champo

Maniphest Tasks: T1569

Differential Revision: https://secure.phabricator.com/D3129
2012-08-02 14:07:21 -07:00
Nick Harper
88caa45854 Save daemon state to database
Summary:
To make it easier to monitor daemons, let's store their current state
(running, died, exited, or unknown) to the db. The purpose of this is to
provide more information on the daemon console about the status of daemons,
especially when they are running on multiple machines. This is mostly backend
work, with only a few frontend changes. (It is also dependent on a change
to libphutil.)

These changes will make dead or stuck daemons more obvious, and will allow
more work on the frontend to hide daemons (and logs) that have exited cleanly,
i.e. ones we don't care about any more.

Test Plan:
- run db migration, check in db that all daemons were marked as exited
- start up a daemon, check in db that it is marked as running
- open web interface, check that daemon is listed as running
- after daemon has been running for a little bit, check in db that dateModified
  is being updated (indicating daemon is properly sending heartbeat)
- kill -9 daemon (but don't run bin/phd yet), and check that db still shows it
  as running
- edit daemon db entry to show it as being on a different host, and backdate
  dateModified field by 3 minutes, and check the web ui to show that the status
  is unknown.
- change db entry to have proper host, check in web ui that daemon status is
  displayed as dead. Check db to see that the status was saved.
- run bin/phd stop, and see that the formerly dead daemon is now exited.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3126
2012-08-01 17:06:04 -07:00
Nick Harper
e7eac67cf3 Fix documentation on deprecated phd repository-launch-readonly
Summary: You need to use -- to separate arguments for phd and the daemon.

Test Plan: Ran with the extra --.

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3106
2012-07-30 15:58:52 -07:00
epriestley
486f7c1e8e Add aggregated facts to the Facts application
Summary:
Some facts are aggregations of other facts. For example, we may compute how many times each macro is used in each object as a "raw fact":

  Dnnn uses macro "psyduck" 6 times.

But we want to present this data in aggregate form, e.g. "order macros by popularity". We can do this at runtime and it probably won't be too awful a query, but we can also aggregate it cheaply:

  Macro "psyduck" is used 3920 times across all objects.

...and then do a query like "select macros ordered by usage".

"Aggregate" facts support facts like this. The aggregate facts I've implemented are:

  - Count of all objects.
  - Count of objects of type X.
  - Last time facts were updated.

These clearly fit the "aggregate" facts template well. I'm not 100% sure macros do. We can use this table to answer a question like "What are the most popular macros, ordered by use?" We can also use it to answer a question like "What are the most popular macros in the last 6 months?", if we build a specific fact for that. But we can't use it to answer a question like "What are the most popular macros between times X and Y?". Maybe that's important; maybe not.

This seems like a good fit for at least some types of facts.

I'll de-magic the keys a bit in the next diff.

Test Plan: Ran the engines and got some aggregated facts about other facts.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1562

Differential Revision: https://secure.phabricator.com/D3089
2012-07-27 13:46:01 -07:00
epriestley
7c934e4176 Add a basic "fact" application
Summary:
Basic "Fact" application with some storage, part of a daemon, and a control binary.

= Goals =

The general idea is that we have various statistics we'd like to compute, like the frequency of image macros, reviewer responsiveness, task close rates, etc. Computing these on page load is expensive and messy. By building an ETL pipeline and running it in a daemon, we can precompute statistics and just pull them out of "stats" tables.

One way to do this is just to completely hard-code everything, e.g. have a daemon that runs every hour which issues a big-ass query and dumps results into a table per-fact or per fact-group. But this has a bunch of drawbacks: adding new stuff to the pipeline is a pain, various fact aggregators can't share much code, updates are slow and expensive, we can never build generic graphs on top of it, etc.

I'm hoping to build an ETL pipeline which is generic enough that we can use it for most things we're interested in without needing schema changes, and so that installs can use it also without needing schema changes, while still being specific enough that it's fast and we can build useful stuff on top of it. I'm not sure if this will actually work, but it would be cool if it does so I'm starting pretty generally and we'll see how far I get. I haven't built this exact sort of thing before so I might be way off.

I'm basing the whole thing on analyzing entire objects, not analyzing changes to objects. So each part of the pipeline is handed an object and told "analyze this", not handed a change. It pretty much deletes all the old data about that thing and then writes new data. I think this is simpler to implement and understand, and it protects us from all sorts of weird issues where we end up with some kind of garbage in the DB and have to wipe the whole thing.

= Facts =

The general idea is that we extract "facts" out of objects, and then the various view interfaces just report those facts. This change has on type of fact, a "raw fact", which is directly derived from an object. These facts are concerete and relate specifically to the object they are derived from. Some examples of such facts might be:

  D123 has 9 comments.
  D123 uses macro "psyduck" 15 times.
  D123 adds 35 lines.
  D123 has 5 files.
  D123 has 1 object.
  D123 has 1 object of type "DREV".
  D123 was created at epoch timestamp 89812351235.
  D123 was accepted by @alincoln at epoch timestamp 8397981839.

The fact storage looks like this:

  <factType, objectPHID, objectA, valueX, valueY, epoch>

Currently, we supprot one optional secondary key (like a user PHID or macro PHID), two optional integer values, and an optional timestamp. We might add more later. Each fact type can use these fields if it wants. Some facts use them, others don't. For instance, this diff adds a "N:*" fact, which is just the count of total objects in the system. These facts just look like:

  <"N:*", "PHID-xxxx-yyyy", ...>

...where all other fields are ignored. But some of the more complex facts might look like:

  <"DREV:accept", "PHID-DREV-xxxx", "PHID-USER-yyyy", ..., ..., nnnn> # User 'yyyy' accepted at epoch 'nnnn'.
  <"FILE:macro", "PHID-DREV-xxxx", "PHID-MACR-yyyy", 17, ..., ...> # Object 'xxxx' uses macro 'yyyy' 17 times.

Facts have no uniqueness constraints. For @vrana's reviewer responsiveness stuff, we can insert multiple rows for each reviewer, e.g.

  <"DREV:reviewed", "PHID-DREV-xxxx", "PHID-USER-yyyy", nnnn, ..., mmmm> # User 'yyyy' reviewed revision 'xxxx' after 'nnnn' seconds at 'mmmm'.

The second value (valueY) is mostly because we need it if we sample anything (valueX = observed value, valueY = sample rate) but there might be other uses. We might need to add "objectB" at some point too -- currently we can't represent a fact like "User X used macro Y on revision Z", so it would be impossible to compute macro use rates //for a specific user// based on this schema. I think we can start here though and see how far we get.

= Aggregated Facts =

These aren't implemented yet, but the idea is that we can then take the "raw facts" and compute derived/aggregated/rollup facts based on the raw fact table. For example, the "count" fact can be aggregated to arrive at a count of all objects in the system. This stuff will live in a separate table which does have uniqueness constraints, and come in the next diff.

We might need some kind of time series facts too, not sure about that. I think most of our use cases today are covered by raw facts + aggregated facts.

Test Plan: Ran `bin/fact` commands and verified they seemed to do reasonable things.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, majak

Maniphest Tasks: T1562

Differential Revision: https://secure.phabricator.com/D3078
2012-07-27 13:34:21 -07:00
epriestley
ff61dba7ac Extend LiskMigrationIterator from PhutilBufferedIterator
Summary: PhutilBufferedIterator now implements all the nonspecific logic here.

Test Plan:
Created a test script like this:

  $iterator = new LiskMigrationIterator(new DifferentialRevision());
  $iterator->setPageSize(3);

  foreach ($iterator as $key => $rev) {
    echo "{$key}: ".$rev->getID()."\n";
  }

Ran it and verified sensible iteration results.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1562

Differential Revision: https://secure.phabricator.com/D3076
2012-07-26 12:01:57 -07:00
epriestley
fc09bcf0a3 Move qsprintf() test cases from libphutil to Phabricator
Summary: Also move the other tests up so they'll trigger when this stuff is touched.

Test Plan: liberate

Reviewers: nh, btrahan, vrana

Reviewed By: nh

CC: aran

Maniphest Tasks: T1283

Differential Revision: https://secure.phabricator.com/D3074
2012-07-26 12:01:47 -07:00
epriestley
7ffe802671 Remove queryfx() from phabricator/
Summary: Seee D3057.

Test Plan: Loaded site.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, chad

Maniphest Tasks: T1283

Differential Revision: https://secure.phabricator.com/D3058
2012-07-24 12:34:02 -07:00
epriestley
514ee3526c Add an event for looking up names from repositories
Summary:
Currently, we have this cumbersome `PhabricatorRepositoryCommitMessageDetailParser` hook. This is really old and outdated; I want to just use the Differential custom field parser. See T945 for a specific application.

However, it allows installs to override author/committer association. Instead, provide an event hook for doing this.

Test Plan: Added a listener, made every commit resolve to "turtle", parsed some commits, verified the events looked sane and they now correctly were all attributed to "turtle".

Reviewers: btrahan, vrana, nh

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1337

Differential Revision: https://secure.phabricator.com/D3040
2012-07-24 11:59:28 -07:00
epriestley
17e20bc363 Remove AphrontConnection from Phabricator
Summary: See D3055.

Test Plan: Loaded pages and such.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1283

Differential Revision: https://secure.phabricator.com/D3056
2012-07-24 11:50:19 -07:00
epriestley
27f6cc3b27 Support PhabricatorOpaqueEnvelope for managing database passwords
Summary: Currently, MySQL/MySQLi connections store passwords in plain text on the object. Allow them to be stored in PhutilOpaqueEnvelopes instead. See D3053.

Test Plan: Loaded site.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3054
2012-07-24 11:13:53 -07:00
epriestley
5d4a6bcf95 Break AphrontDatabaseConnection dependencies on PhabricatorEnv
Summary: We pull "retries" and a doc link from PhabricatorEnv directly. Break these dependencies so the classes can move to libphutil.

Test Plan: Browsed site, triggered a schema exception and verified I still got the useful footer text.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1283

Differential Revision: https://secure.phabricator.com/D3053
2012-07-24 10:47:27 -07:00
epriestley
f1270315e9 Allow connections to be closed; close connections for global locks
Summary: Add an explicit close() method to connections and call it in GlobalLock.

Test Plan:
Wrote a script like this:

  $lock = PhabricatorGlobalLock::newLock('test');
  echo "LOCK";
  $lock->lock();
  sleep(10);
  echo "UNLOCK";
  $lock->unlock();
  sleep(9999);

Using `SHOW FULL PROCESSLIST`, verified the connection closed after 10 seconds with both the "MySQL" and "MySQLi" implementations.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1470

Differential Revision: https://secure.phabricator.com/D3035
2012-07-23 19:06:58 -07:00
Bob Trahan
bc29a3e8a2 Make inline comment preview work in Diffusion
Summary: created a PhabricatorInlineCommentPreviewController so controllers in Diffusion and Differential respectively just have to handle the URI mapping and data loading like good little controllers.

Test Plan:
left inline comments on commits, deleted inline commits, submitted inline comments -- all worked well
did the same on some diffs

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1176

Differential Revision: https://secure.phabricator.com/D3034
2012-07-23 11:01:28 -07:00
epriestley
9be12551a9 Move Task <=> Revision storage to Edges
Summary:
  - Add edges for this relationship.
  - Use edges to store this data.
  - Migrate old data.
  - Fix some warnings with generating feed stories about Aux and Edge transactions.
  - Fix a task-task edge issue with "Create Subtask".

Test Plan:
  - Migrated data, verified reivsions showed up.
  - Attached and detached tasks to revisions and vice versa.
  - Created a new revision with attached tasks.
  - Created a subtask.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3018
2012-07-20 08:59:39 -07:00
epriestley
ba4fb05d91 Fix translations
Summary: Theses are sort of silly anyway since they should all have the actor in them rather than being sentence fragments, but make them work OK for English at least. See D3013.

Test Plan:
Ran:

  echo pht('added %d dependencie(s): %s', 1, 'derp')."\n";
  echo pht('added %d dependencie(s): %s', 2, 'derp, derp')."\n";

Got:

  added dependency: derp
  added dependencies: derp, derp

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3015
2012-07-19 11:45:08 -07:00
Evan Priestley
e746ccfeeb Merge pull request #163 from floatingLomas/master
Fixed some @{method} links in PhabricatorEdgeEditor
2012-07-19 09:53:38 -07:00
Jonathan Lomas
0be6d87a45 Fixed some @{method} links in the PhabricatorEdgeEditor documentation. 2012-07-19 09:49:10 -07:00
Bob Trahan
ae13d33859 Phame - introduce blogs
Summary:
blogs are collections of posts. a blog also has metadata like a name, description and "bloggers" that can edit the metadata of the blog and contribute posts.

changes include the post edit flow where bloggers can now select which blogs to publish to. also made various small tweaks throughout the UI to make things sensical and clean as the concept of blogs is introduced.

there's edges powering this stuff.  bloggers <=> blogs and posts <=> blogs in particular.

Test Plan:
made blogs, deleted blogs, tried to make blogs with no bloggers. all went well.
verified ui to publish only showed up for public posts, published posts to blogs, un-published posts to blogs, re-published posts to blogs, deleted posts and verified they disappeared from blogs.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1373

Differential Revision: https://secure.phabricator.com/D3003
2012-07-19 09:03:10 -07:00
epriestley
ee709a0543 Use Edges to store dependencies between revisions in Differential
Summary: See D3006. Move this data to the edge store.

Test Plan:
  - Created dependencies, migrated, verified dependencies were preserved.
  - Added new dependencies, they worked.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D3007
2012-07-18 20:42:06 -07:00
epriestley
9196a6bd9f Use Edges to store dependencies between tasks in Maniphest
Summary:
  - Use edges to store "X depends on Y" information in Maniphest.
  - Show both "Depends On" and "Dependent Tasks".
  - Migrate all the old edges.

Test Plan:
  - Added some relationships, migrated, verified they were preserved.
  - Added some new valid relationships, verified tasks got updated with sensible transactions and sent reasonable emails.
  - Tried to add a cycle, got an ugly but effective error.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D3006
2012-07-18 20:41:42 -07:00
epriestley
409974fbd6 Add getDestinationPHIDs() to PhabricatorEdgeQuery
Summary: This should simplify a bunch of stuff in D3006 and D3003.

Test Plan: Will update D3006.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3010
2012-07-18 20:41:26 -07:00
epriestley
a7bcc532da Add an iterator to make it easier to perform database migrations
Summary: See D3006, D3007. Make it easier to do migrations like that without holding all results in memory.

Test Plan:
Ran this code with an artificially small page size (2):

  foreach (new LiskMigrationIterator(new DifferentialRevision()) as $rev) {
    echo "Revision ".$rev->getID()."\n";
  }

Verified each revision as loaded and processed.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D3008
2012-07-18 20:01:23 -07:00
Owen Jacobson
e098f5d275 Mention non-zero exit from 'phd status' in 'phd help'. 2012-07-17 15:20:53 -04:00
Owen Jacobson
883e11f761 Retain pid files for dead daemons in 'phd status'.
'phd status' should have a stable result when invoked multiple times.
Automatically removing PID files for dead daemons every time 'phd status' is
invoked prevents tools from noticing that a daemon has died if something
happens to invoke 'phd status' before the tool looks. This affects Puppet
noticably, since it probably runs the status command every half hour.
2012-07-17 13:48:24 -04:00
Owen Jacobson
420d6426f9 'phd status' should exit with non-zero if daemons are not running.
'phd status' may be invoked by tools (such as puppet) which need to make
automated decisions about whether to start/restart the daemon. To enable this,
'phd status' now exits with 0 if all daemons are running, 1 if no daemons are
running, and 2 if some (but not all) daemons are running.
2012-07-17 13:48:15 -04:00
epriestley
22660cff2a OMG
Summary: NOOOO

Test Plan: Image macros work again.

Reviewers: kdeggelman, btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2979
2012-07-16 10:35:36 -07:00
epriestley
cb8120551f Don't special-case LiskDAO->load(0)
Summary:
Lisk currently behaves in two different ways if you call it like `load("cow")` (throws) versus `load(99999999)` (returns null), where neither ID exists.

This was intended to catch programming errors as distinct from missing data, but in practice the former is very rare and you have to handle the latter in most cases anyway. The case where you pass "0" is particularly confusing. See D2971 for an example.

On the balance, I think this ends up being far more confusing than helpful. Instead, just return NULL if we're sure there's no such object.

Test Plan: Reasoned about program behavior.

Reviewers: alanh, btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2977
2012-07-16 09:50:23 -07:00
Bob Trahan
dc75e79cb5 Make IRC Bot connect on both successful end of MOTD (376) and non-successful MOTD (422)
Summary: http://www.networksorcery.com/enp/protocol/irc.htm

Test Plan: augment code with an additional debug line (phlog('hi');) so I can see my case was trigged and it will fall through. setup an ill-configured IRC server with ngircd. Configure an ircbot to connect to said ill-configured IRC server. verify ircbot connected to channel. verify in irc bot logs that debug line was invoked.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1452

Differential Revision: https://secure.phabricator.com/D2962
2012-07-11 16:27:41 -07:00
epriestley
fcd04708d2 Add markup cache collection to the GC daemon
Summary: Allow the GC daemon to collect the new markup cache.

Test Plan: Ran gc daemon in "debug" mode, saw it collect cache entries.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2947
2012-07-11 11:40:18 -07:00
epriestley
5d8b75b4da Use the unified markup cache for Maniphest
Summary:
  - See D2945.
  - Drop `cache` field from ManiphestTransaction.
  - Render task descriptions and transactions through PhabricatorMarkupEngine.
  - Also pull the list of macros more lazily.

Test Plan:
  - Verified transactions and transaction preview work correctly and interact with cache correctly.
  - Verified tasks descriptions and task preview work correctly.
  - Verified we don't hit the imagemacro table when we're rendering everything from cache anymore.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2946
2012-07-11 11:40:10 -07:00
epriestley
e2e9aed4fa Fix symbol handling in symbol query and IRC "Where is x?" handler
Summary: If a symbol's project has no linked repository, we currently explode. Instead, decline to generate a URI and fall back gracefully.

Test Plan: https://secure.phabricator.com/chatlog/channel/%23phabricator/?at=22345

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1465

Differential Revision: https://secure.phabricator.com/D2948
2012-07-09 17:51:42 -07:00
epriestley
2b0b9a1573 Add a generic multistep Markup cache
Summary:
The immediate issue this addresses is T1366, adding a rendering cache to Phriction. For wiki pages with code blocks especially, rerendering them each time is expensive.

The broader issue is that out markup caches aren't very good right now. They have three major problems:

**Problem 1: the data is stored in the wrong place.** We currently store remarkup caches on objects. This means we're always loading it and passing it around even when we don't need it, can't genericize cache management code (e.g., have one simple script to drop/GC caches), need to update authoritative rows to clear caches, and can't genericize rendering code since each object is different.

To solve this, I created a dedicated cache database that I plan to move all markup caches to use.

**Problem 2: time-variant rules break when cached.** Some rules like `**bold**` are time-invariant and always produce the same output, but some rules like `{Tnnn}` and `@username` are variant and may render differently (because a task was closed or a user is on vacation). Currently, we cache the raw output, so these time-variant rules get locked at whatever values they had when they were first rendered. This is the main reason Phriction doesn't have a cache right now -- I wanted `{Tnnn}` rules to reflect open/closed tasks.

To solve this, I split markup into a "preprocessing" phase (which does all the parsing and evaluates all time-invariant rules) and a "postprocessing" phase (which evaluates time-variant rules only). The preprocessing phase is most of the expense (and, notably, includes syntax highlighting) so this is nearly as good as caching the final output. I did most of the work here in D737 / D738, but we never moved to use it in Phabricator -- we currently just do the two operations serially in all cases.

This diff splits them apart and caches the output of preprocessing only, so we benefit from caching but also get accurate time-variant rendering.

**Problem 3: cache access isn't batched/pipelined optimally.** When we're rendering a list of markup blocks, we should be able to batch datafetching better than we do. D738 helped with this (fetching is batched within a single hunk of markup) and this improves batching on cache access. We could still do better here, but this is at least a step forward.

Also fixes a bug with generating a link in the Phriction history interface ($uri gets clobbered).

I'm using PHP serialization instead of JSON serialization because Remarkup does some stuff with non-ascii characters that might not survive JSON.

Test Plan:
  - Created a Phriction document and verified that previews don't go to cache (no rows appear in the cache table).
  - Verified that published documents come out of cache.
  - Verified that caches generate/regenerate correctly, time-variant rules render properly and old documents hit the right caches.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1366

Differential Revision: https://secure.phabricator.com/D2945
2012-07-09 15:20:56 -07:00
epriestley
1089a48d4a Allow edges to be configured to prevent cycles
Summary:
Certain types of things we should be storing in edges (notably, Task X depends on Task Y) should always be acyclic. Allow `PhabricatorEdgeEditor` to enforce this, since we can't correctly enforce it outside of the editor without being vulnerable to races.

Each edge type can be marked acyclic. If an edge type is acyclic, we perform additional steps when writing new edges of that type:

  - We acquire a global lock on the edge type before performing any reads or writes. This ensures we can't produce a cycle as a result of a race where two edits add edges which independently do not produce a cycle, but do produce a cycle when combined.
  - After performing writes but before committing transactions, we load the edge graph for each acyclic type and verify that it is, in fact, acyclic. If we detect cycles, we abort the edit.
  - When we're done, we release the edge type locks.

This is a relatively high-complexity change, but gives us a simple way to flag an edge type as acyclic in a robust way.

Test Plan: Ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D2940
2012-07-09 10:39:38 -07:00
epriestley
bf8cbf55b1 Namespace GlobalLocks to storage namespaces
Summary:
Currently, multiple unit tests that acquire global locks will interfere with each other. Namespace the locks so they don't.

(Possibly we should also rename this to PhabricatorStorageNamespaceLock or something since it's not really global any more, but that's kind of unwieldy...)

Test Plan: Acquired locks with --trace and verified they were namespaced properly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D2939
2012-07-09 10:39:30 -07:00
epriestley
d86c4e0366 Store forced connections in the Lisk connection cache
Summary:
In unit tests which use fixtures, we open transactions on every connection we establish. However, since we don't track connections that are established with "$force_new" (currently, only GlobalLock connections) we never close these transactions normally.

Instead of not tracking these connections, track them using unique keys so we'll never get a cache hit on them.

Test Plan: Built unit tests on top of this, had them stop dying from unclosed transactions.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D2938
2012-07-09 10:39:21 -07:00
epriestley
7cf6313be9 Add a generic object for unit tests
Summary:
A later diff adds unit tests against edges, but we need real objects to connect with edges. Add some trivial objects to the Harbormaster database to compliment the similar HarbormasterScratchTable.
On its own, this does nothing interesting.

Test Plan: Built unit tests on this in a followup.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1162

Differential Revision: https://secure.phabricator.com/D2937
2012-07-09 10:39:14 -07:00
epriestley
63be89ba00 Improve error message for error 2006
Summary:
See discussion here:

https://secure.phabricator.com/chatlog/channel/%23phabricator/?at=21186

Basically, MySQL usually raises a good error if we exceed "max_allowed_packet":

  EXCEPTION: (AphrontQueryException) #1153: Got a packet bigger than 'max_allowed_packet' bytes

But sometimes it gives us a #2006 instead. This is documented, at least:

>"With some clients, you may also get a Lost connection to MySQL server during query error if the communication packet is too large."

  http://dev.mysql.com/doc/refman//5.5/en/packet-too-large.html

Try to improve the error message to point at this as a possible explanation.

Test Plan: Faked an error, had it throw, read exception message. See also chatlog.

Reviewers: btrahan, skrul

Reviewed By: skrul

CC: aran

Differential Revision: https://secure.phabricator.com/D2923
2012-07-05 16:03:58 -07:00
epriestley
ce360926b7 Allow PhabricatorGlobalLock to block
Summary: See D2924.

Test Plan: Ran locks with blocking timeouts.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2925
2012-07-05 16:03:43 -07:00
epriestley
ddf67fce58 Add an example event listener, improve documentation, and add a commit discovery event
Summary: Improve documentation around Phabricator events.

Test Plan: Generated and read documentation. Ran test script.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T1092

Differential Revision: https://secure.phabricator.com/D2917
2012-07-03 16:46:27 -07:00
epriestley
bda5c670bc Add useful text descriptions to edge transactions
Summary: See D2906. This just adds text so they render pretty.

Test Plan:
Got pretty emails and rendered transactions.

{F13706}

Reviewers: btrahan, davidreuss

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2907
2012-07-02 15:42:16 -07:00
epriestley
9f4cfd40bc Insert Maniphest transactions when edges are edited
Summary:
  - See D2741.
  - When EdgeEditor performs edits, emit events.
  - Listen for Maniphest edge events and save the changes as transactions.
  - Do all this in a reasonably generic way that won't take too much rewriting as we use edges more generally.

Test Plan: Attached and detached commits from tasks, saw reasonable-looking transactions spring into existence.

Reviewers: btrahan, davidreuss

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2906
2012-07-02 15:42:06 -07:00
epriestley
310cf00fc3 Consolidate feed query code
Summary: Simplify FeedQuery by making it extend from PhabricatorIDPagedPolicyQuery

Test Plan: Looked at feed on home, projects, user profile, and called `feed.query`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2905
2012-07-02 15:41:19 -07:00
epriestley
a33e84e1e5 Add table markup to Phabricator
Summary: See D2902.

Test Plan: Made tables, generated docs.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2903
2012-07-02 14:44:38 -07:00
epriestley
534c0aa326 Minor, move all storage/query/db code to src/infrastructure/storage 2012-07-02 10:49:00 -07:00