@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Fix limits in queries
Summary: I think this is simpler? Includes test cases.
Test Plan: Ran tests. Loaded /paste/.
Reviewers: vrana, nh
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3209
+37
-25
+30
src/applications/policy/__tests__/PhabricatorPolicyTestCase.php
+30
src/applications/policy/__tests__/PhabricatorPolicyTestCase.php
···
137
137
138
138
139
139
/**
140
+
* Test limits.
141
+
*/
142
+
public function testLimits() {
143
+
$results = array(
144
+
$this->buildObject(PhabricatorPolicies::POLICY_USER),
145
+
$this->buildObject(PhabricatorPolicies::POLICY_USER),
146
+
$this->buildObject(PhabricatorPolicies::POLICY_USER),
147
+
$this->buildObject(PhabricatorPolicies::POLICY_USER),
148
+
$this->buildObject(PhabricatorPolicies::POLICY_USER),
149
+
$this->buildObject(PhabricatorPolicies::POLICY_USER),
150
+
);
151
+
152
+
$query = new PhabricatorPolicyTestQuery();
153
+
$query->setResults($results);
154
+
$query->setViewer($this->buildUser('user'));
155
+
156
+
$this->assertEqual(
157
+
3,
158
+
count($query->setLimit(3)->setOffset(0)->execute()),
159
+
'Limits work.');
160
+
161
+
$this->assertEqual(
162
+
2,
163
+
count($query->setLimit(3)->setOffset(4)->execute()),
164
+
'Limit + offset work.');
165
+
}
166
+
167
+
168
+
169
+
/**
140
170
* Test an object for visibility across multiple user specifications.
141
171
*/
142
172
private function expectVisibility(
+7
-25
src/infrastructure/query/policy/PhabricatorPolicyQuery.php
+7
-25
src/infrastructure/query/policy/PhabricatorPolicyQuery.php
···
153
153
$limit = (int)$this->getLimit();
154
154
$count = 0;
155
155
156
-
$need = null;
157
-
if ($offset) {
158
-
$need = $offset + $limit;
159
-
}
156
+
$need = $offset + $limit;
160
157
161
158
$this->willExecute();
162
159
163
160
do {
164
-
165
-
// Figure out how many results to load. "0" means "all results".
166
-
$load = 0;
167
-
if ($need && ($count < $offset)) {
168
-
// This cap is just an arbitrary limit to keep memory usage from going
169
-
// crazy for large offsets; we can't execute them efficiently, but
170
-
// it should be possible to execute them without crashing.
171
-
$load = min($need, 1024);
172
-
} else if ($limit) {
173
-
// Otherwise, just load the number of rows we're after. Note that it
174
-
// might be more efficient to load more rows than this (if we expect
175
-
// about 5% of objects to be filtered, loading 105% of the limit might
176
-
// be better) or fewer rows than this (if we already have 95 rows and
177
-
// only need 100, loading only 5 rows might be better), but we currently
178
-
// just use the simplest heuristic since we don't have enough data
179
-
// about policy queries in the real world to tweak it.
180
-
$load = $limit;
161
+
if ($need) {
162
+
$this->rawResultLimit = min($need - $count, 1024);
163
+
} else {
164
+
$this->rawResultLimit = 0;
181
165
}
182
-
$this->rawResultLimit = $load;
183
-
184
166
185
167
$page = $this->loadPage();
186
168
···
202
184
}
203
185
}
204
186
205
-
if (!$load) {
187
+
if (!$this->rawResultLimit) {
206
188
// If we don't have a load count, we loaded all the results. We do
207
189
// not need to load another page.
208
190
break;
209
191
}
210
192
211
-
if (count($page) < $load) {
193
+
if (count($page) < $this->rawResultLimit) {
212
194
// If we have a load count but the unfiltered results contained fewer
213
195
// objects, we know this was the last page of objects; we do not need
214
196
// to load another page because we can deduce it would be empty.