Page MenuHomePhabricator

Reduce the amount of weird "static" and "cache" behavior in Pholio query classes

Authored by epriestley on Dec 20 2018, 8:18 PM.



Depends on D19922. Ref T11351. These query classes have some slightly weird behavior, including public static function loadImages(...). Convert all this stuff into more standard query patterns.

Test Plan

Grepped for callsites, browsed around in Pholio.

Diff Detail

rP Phabricator
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Dec 20 2018, 8:18 PM
epriestley requested review of this revision.Dec 20 2018, 8:20 PM
epriestley added inline comments.Dec 20 2018, 8:21 PM

withMocks(...) now has the effect of working like a cache and implying withMockPHIDs(...), similar to withRepositories(...) elsewhere.


There's a behavioral change here: we'll now load Images if they have a null value for mockPHID (i.e., an image not yet attached to a mock). Previously, we would always reject them.


(Not actually that scary: it does some extra fetches, but hits the cache the second time through.)


This fixes a sort-of-bug. The mpull() intends to rekey the array (the lines below care about the array keys) but changes the values instead. The code worked before because the array happened to always be keyed by ID anyway.

amckinley accepted this revision.Dec 20 2018, 10:42 PM
This revision is now accepted and ready to land.Dec 20 2018, 10:42 PM
This revision was automatically updated to reflect the committed changes.