Page MenuHomePhabricator

Never raise policy exceptions for the omnipotent viewer
ClosedPublic

Authored by epriestley on Nov 18 2013, 10:54 PM.
Tags
None
Referenced Files
F14063407: D7603.diff
Mon, Nov 18, 6:28 PM
F14053501: D7603.diff
Fri, Nov 15, 4:42 PM
F14050530: D7603.id17157.diff
Thu, Nov 14, 7:02 PM
F14040306: D7603.diff
Mon, Nov 11, 8:53 AM
F14023215: D7603.diff
Thu, Nov 7, 12:05 AM
F14003215: D7603.id.diff
Sat, Oct 26, 3:31 AM
F13987402: D7603.id17156.diff
Oct 21 2024, 9:13 AM
Unknown Object (File)
Oct 11 2024, 6:21 AM
Subscribers

Details

Summary

Fixes T4109. If a revision has a bad repositoryPHID (for example, because the repository was deleted), DifferentialRevisionQuery calls didRejectResult() on it, which raises a policy exception, even if the viewer is omnipotent. This aborts the MessageParser, because it does not expect policy exceptions to be raised for an omnipotent viewer.

Fix this in two ways:

  1. Never raise a policy exception for an omnipotent viewer. I think this is the expected behavior and a reasonable rule.
  2. In this case, load the revision for an omnipotent viewer.

This feels a little gross, but it's the only place where we do this in the codebase right now. We can clean this up later on once it's more clear what the circumstances of checks like these are.

Test Plan

Set a revision to have an invalid repositoryPHID, ran message parser on it, got a clean parse.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped