Summary: It's dumb to execute a query which we know will return an empty result.
Test Plan: Looked at comment preview with "11", didn't see "1 = 0" in DarkConsole.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5177
Summary:
Two issues:
- When we read the content out of cache, it has lost its "safe html" flag, since the cache is raw-string oriented. Restore it.
- explode() isn't safe-html-safe. Use phutil_split_lines() instead, which is.
Test Plan: Looked at /paste/
Reviewers: codeblock, chad
Reviewed By: codeblock
CC: aran
Differential Revision: https://secure.phabricator.com/D4980
Summary:
- When a setup issue is nonfatal (i.e., a warning), instruct the user to edit the value from the web UI instead of using `bin/config`.
- When the user edits configuration in response to a setup issue, send them back to the issue when they're done.
- When an issue relates to PHP configuration, link to the PHP documentation on configuration.
- Add new-style setup check for timezone issues.
Test Plan: Mucked with my timezone config, resolved the issues I created.
Reviewers: codeblock, btrahan, vrana
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2221, T2228
Differential Revision: https://secure.phabricator.com/D4298
Summary:
See discussion in D4204. Facebook currently has a 314MB remarkup cache with a 55MB index, which is slow to access. Under the theory that this is an index size/quality problem (the current index is on a potentially-384-byte field, with many keys sharing prefixes), provide a more general index with fancy new features:
- It implements PhutilKeyValueCache, so it can be a component in cache stacks and supports TTL.
- It has a 12-byte hash-based key.
- It automatically compresses large blocks of data (most of what we store is highly-compressible HTML).
Test Plan:
- Basics:
- Loaded /paste/, saw caches generate and save.
- Reloaded /paste/, saw the page hit cache.
- GC:
- Ran GC daemon, saw nothing.
- Set maximum lifetime to 1 second, ran GC daemon, saw it collect the entire cache.
- Deflate:
- Selected row formats from the database, saw a mixture of 'raw' and 'deflate' storage.
- Used profiler to verify that 'deflate' is fast (12 calls @ 220us on my paste list).
- Ran unit tests
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4259
Summary:
D4188 adds a preview of Paste contents to the list, but gets slow for large lists if you have Pygments installed since it has to spend ~200ms/item highlighting them. Instead, cache the highlighted output.
- Adds a Paste highlighting cache. This uses RemarkupCache because it has automatic GC and we don't have a more generalized cache for the moment.
- Uses the Paste cache on paste list and paste detail.
- Adds a little padding to the summary.
- Adds "..." if there's more content.
- Adds line count and language, if available. These are hidden on Mobile.
- Nothing actually uses needRawContent() but I left it there since it doesn't hurt anything and is used internally (I thought the detail view did, but it uses the file content directly, and must for security reasons).
Test Plan:
{F27710}
- Profiled paste list, saw good performance and few service calls.
- Viewed paste.
- Viewed raw paste content.
Reviewers: codeblock, btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4204
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.
Reviewers: epriestley, davidrecordon
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3886
Summary:
Provides a simple way for policy-aware queries to pre-filter results without needing to maintain separate cursors, and fixes a bunch of filter-related edge cases.
- For reverse-paged cursor queries, we previously reversed each individual set of results. If the final result set is built out of multiple pages, it's in the wrong order overall, with each page in the correct order in sequence. Instead, reverse everything at the end. This also simplifies construction of queries.
- `AphrontCursorPagerView` would always render a "<< First" link when paging backward, even if we were on the first page of results.
- Add a filtering hook to let queries perform in-application pre-policy filtering as simply as possible (i.e., without maintaing their own cursors over the result sets).
Test Plan: Made feed randomly prefilter half the results, and paged forward and backward. Observed correct result ordering, pagination, and next/previous links.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3787
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
Summary:
I just put them in the property table instead of a list at the foot, they looked weird down there and were too bulky relative to their importance.
This won't scale great if someone forks a paste ten thousand times or whatever, but we can deal with that when we get there.
Also clean up a few things and tweak some styles,
Test Plan: Looked at forked, unforked pastes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3295
Summary:
- Add a PhabricatorApplication.
- Make most of the views work well on tablets / phones. The actual "Create" form doesn't, but everything else is good -- need to make device-friendly form layouts before I can do the form.
Test Plan: Will attach screenshots.
Reviewers: btrahan, chad, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1569
Differential Revision: https://secure.phabricator.com/D3293
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
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.
NOTE: `arc diff` timed out so I'm pushing it without review.
Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.
Auditors: epriestley
Maniphest Tasks: T1103
Summary:
Provides a basic start for access policies. Objects expose various capabilities, like CAN_VIEW, CAN_EDIT, etc., and set a policy for each capability. We currently implement three policies, PUBLIC (anyone, including logged-out), USERS (any logged-in) and NOONE (nobody). There's also a way to provide automatic capability grants (e.g., the owner of an object can always see it, even if some capability is set to "NOONE"), but I'm not sure how great the implementation feels and it might change.
Most of the code here is providing a primitive for efficient policy-aware list queries. The problem with doing queries naively is that you have to do crazy amounts of filtering, e.g. to show the user page 6, you need to filter at least 600 objects (and likely more) before you can figure out which ones are 500-600 for them. You can't just do "LIMIT 500, 100" because that might have only 50 results, or no results. Instead, the query looks like "WHERE id > last_visible_id", and then we fetch additional pages as necessary to satisfy the request.
The general idea is that we move all data access to Query classes and have them do object filtering. The ID paging primitive allows efficient paging in most cases, and the executeOne() method provides a concise way to do policy checks for edit/view screens.
We'll probably end up with mostly broader policy UIs or configuration-based policies, but there are at least a few cases for per-object privacy (e.g., marking tasks as "Security", and restricting things to the members of projects) so I figured we'd start with a flexible primitive and the simplify it in the UI where we can.
Test Plan: Unit tests, played around in the UI with various policy settings.
Reviewers: btrahan, vrana, jungejason
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D2210
