Do a lot of preparatory work for adding local-site rooting to the webapi.

Review Request #1950 — Created Nov. 27, 2010 and submitted


Review Board


Do a lot of preparatory work for adding local-site rooting to the webapi.

This change adds a bunch of infrastructure to prepare for my next changes which
actually add knowledge of local-site infrastructure and URLs to the API. This

- New test fixtures.

- Local site checking in manager.accessible() and model.is_accessible_by() for
  Group, LocalSite, Repository, and ReviewRequest.

- A fix for a circular import in webapi/ (this import was only
  circular when running -only- the webapi tests, for some reason).

- A fix for the to_group_deprecated mess (this actually adds local-site
  knowledge to a bit of the deprecated json API, which wasn't planned yet but
  it's nice to clean it up).

- Add a check_local_site decorator which will be used to check for access to the
  local site before returning data or links.

- Add an override of WebAPIResource.get_href which knows how to serialize links
  including the local-site name.
Ran unit tests.
  1. Overall, this looks good.
    One additional thing that I'd like you to do, which is easy and can be done separately. Since we're adding new unit test fixtures, can you update the codebase/unit-tests/fixtures docs and add this one to the list of things to load/dump?
  2. reviewboard/site/ (Diff revision 1)
    Potentially, you could have several sites but have them all anonymous. So we should check if anonymous access is enabled for the site before requiring an authenticated user.
    1. LocalSite has no concept of anonymous access. We can maybe add that later but I don't want to deal with it now.
  3. reviewboard/site/ (Diff revision 1)
    The assert_ functions are being deprecated in Python 2.7 and I believe in Django. The docs say to just use assertTrue, which should do the right thing.
  4. reviewboard/webapi/ (Diff revision 1)
    Can we maybe move this import into the class definition? Or will that continue to break things? I think we end up taking a small performance hit every time encode() is called, and that's called quite a lot.
    1. I'm going to change this by moving status_to_string and string_to_status into encoder and importing it from the other places. This unifies the two versions of them that we had, too.
  5. reviewboard/webapi/ (Diff revision 1)
    This should probably go in webapi/
  6. reviewboard/webapi/ (Diff revision 1)
    If a name is specified but it doesn't exist in the database, this will return None and we'll check if the user has access on the default site. Is that what we want? Feels like we should instead error out with a 404.
  7. reviewboard/webapi/ (Diff revision 1)
    Will that work for subdirectory installs?
    1. Uh, I really don't know. How does reverse() work with subdirectory installs?
Review request changed
  1. Awesome :)