Page MenuHomePhabricator

Decouple some aspects of request routing and construction
ClosedPublic

Authored by epriestley on Oct 13 2014, 9:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 9, 12:30 AM
Unknown Object (File)
Sun, Apr 7, 4:01 PM
Unknown Object (File)
Thu, Mar 28, 1:45 AM
Unknown Object (File)
Mon, Mar 25, 1:46 PM
Unknown Object (File)
Mon, Mar 25, 1:46 PM
Unknown Object (File)
Fri, Mar 22, 10:10 AM
Unknown Object (File)
Fri, Mar 22, 10:10 AM
Unknown Object (File)
Fri, Mar 22, 10:04 AM
Subscribers

Details

Summary

Ref T5702. This is a forward-looking change which provides some very broad API improvements but does not implement them. In particular:

  • Controllers no longer require $request to construct. This is mostly for T5702, directly, but simplifies things in general. Instead, we call setRequest() before using a controller. Only a small number of sites activate controllers, so this is less code overall, and more consistent with most constructors not having any parameters or effects.
  • $request now offers getURIData($key, ...). This is an alternate way of accessing $data which is currently only available on willProcessRequest(array $data). Almost all controllers which implement this method do so in order to read one or two things out of the URI data. Instead, let them just read this data directly when processing the request.
  • Introduce handleRequest(AphrontRequest $request) and deprecate (very softly) processRequest(). The majority of processRequest() calls begin $request = $this->getRequest(), which is avoided with the more practical signature.
  • Provide getViewer() on $request, and a convenience getViewer() on $controller. This fixes $viewer = $request->getUser(); into $viewer = $request->getViewer();, and converts the $request + $viewer two-liner into a single $this->getViewer().
Test Plan
  • Browsed around in general.
  • Hit special controllers (redirect, 404).
  • Hit AuditList controller (uses new style).

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Decouple some aspects of request routing and construction.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
This revision is now accepted and ready to land.Oct 14 2014, 12:02 AM

I've built the Almanac stuff on top of this with the new APIs and it seems like smooth sailing so far, but yell if you see anything suspicious after this lands.

This revision was automatically updated to reflect the committed changes.