Skip to content

Commit 2ec3ef1

Browse files
mcollovatimcollovati
authored
fix: limit the requested items to a given threshold (#18417) (#18425)
Limits the number of requested items from backend to a given threshold of ten pages. Exactly the same as #12003, but applied to HierarchicalDataCommunicator. Fixes #18403
1 parent 253436a commit 2ec3ef1

File tree

3 files changed

+70
-26
lines changed

3 files changed

+70
-26
lines changed

‎flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,17 +164,28 @@ public DataCommunicator(DataGenerator<T> dataGenerator,
164164
* @param length the end of the requested range
165165
*/
166166
public void setRequestedRange(int start, int length) {
167+
requestedRange = computeRequestedRange(start, length);
168+
requestFlush();
169+
}
170+
171+
/**
172+
* Computes the requested range, limiting the number of requested items to a
173+
* given threshold of ten pages.
174+
*
175+
* @param start
176+
* the start of the requested range
177+
* @param length
178+
* the end of the requested range
179+
*/
180+
protected final Range computeRequestedRange(int start, int length) {
167181
if (length > MAXIMUM_ALLOWED_ITEMS) {
168182
getLogger().warn(
169183
"Attempted to fetch more items from server than allowed "
170184
+ "in one go: number of items requested '{}', maximum "
171185
+ "items allowed '{}'.",
172186
length, MAXIMUM_ALLOWED_ITEMS);
173187
}
174-
requestedRange = Range.withLength(start,
175-
Math.min(length, MAXIMUM_ALLOWED_ITEMS));
176-
177-
requestFlush();
188+
return Range.withLength(start, Math.min(length, MAXIMUM_ALLOWED_ITEMS));
178189
}
179190

180191
/**

‎flow-data/src/main/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalDataCommunicator.java

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -143,33 +143,32 @@ public void reset() {
143143

144144
Collection<T> expandedItems = getHierarchyMapperExpandedItems();
145145
if (!expandedItems.isEmpty()) {
146-
update.enqueue("$connector.expandItems",
147-
expandedItems
148-
.stream()
149-
.map(getKeyMapper()::key)
150-
.map(key -> {
151-
JsonObject json = Json.createObject();
152-
json.put("key", key);
153-
return json;
154-
}).collect(
155-
JsonUtils.asArray()));
146+
update.enqueue("$connector.expandItems", expandedItems.stream()
147+
.map(getKeyMapper()::key).map(key -> {
148+
JsonObject json = Json.createObject();
149+
json.put("key", key);
150+
return json;
151+
}).collect(JsonUtils.asArray()));
156152
}
157153

158154
requestFlush(update);
159155
}
160156
}
161157

162158
@Override
163-
protected void handleDataRefreshEvent(DataChangeEvent.DataRefreshEvent<T> event) {
159+
protected void handleDataRefreshEvent(
160+
DataChangeEvent.DataRefreshEvent<T> event) {
164161
if (event.isRefreshChildren()) {
165162
T item = event.getItem();
166163
if (isExpanded(item)) {
167164
String parentKey = getKeyMapper().key(item);
168165

169166
if (!dataControllers.containsKey(parentKey)) {
170-
setParentRequestedRange(0, mapper.countChildItems(item), item);
167+
setParentRequestedRange(0, mapper.countChildItems(item),
168+
item);
171169
}
172-
HierarchicalCommunicationController<T> dataController = dataControllers.get(parentKey);
170+
HierarchicalCommunicationController<T> dataController = dataControllers
171+
.get(parentKey);
173172
if (dataController != null) {
174173
dataController.setResendEntireRange(true);
175174
requestFlush(dataController);
@@ -188,7 +187,6 @@ public Stream<T> fetchFromProvider(int offset, int limit) {
188187

189188
public void setParentRequestedRange(int start, int length, T parentItem) {
190189
String parentKey = getKeyMapper().key(parentItem);
191-
192190
HierarchicalCommunicationController<T> controller = dataControllers
193191
.computeIfAbsent(parentKey,
194192
key -> new HierarchicalCommunicationController<>(
@@ -199,7 +197,8 @@ parentKey, getKeyMapper(), mapper,
199197
(pkey, range) -> mapper.fetchChildItems(
200198
getKeyMapper().get(pkey), range)));
201199

202-
controller.setRequestRange(start, length);
200+
Range range = computeRequestedRange(start, length);
201+
controller.setRequestRange(range.getStart(), range.length());
203202
requestFlush(controller);
204203
}
205204

@@ -602,8 +601,8 @@ private Collection<T> getExpandedItems(T parent) {
602601
if (hierarchyMapper.isExpanded(parent)) {
603602
expandedItems.add(parent);
604603
}
605-
hierarchyMapper.fetchChildItems(parent, null)
606-
.forEach(child -> expandedItems.addAll(getExpandedItems(child)));
604+
hierarchyMapper.fetchChildItems(parent, null).forEach(
605+
child -> expandedItems.addAll(getExpandedItems(child)));
607606
return expandedItems;
608607
}
609608
}

‎flow-data/src/test/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicatorDataTest.java

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.util.List;
2121
import java.util.stream.IntStream;
2222

23-
import com.vaadin.flow.function.SerializablePredicate;
2423
import org.junit.Assert;
2524
import org.junit.Before;
2625
import org.junit.Test;
@@ -32,6 +31,7 @@
3231
import com.vaadin.flow.data.provider.DataCommunicatorTest;
3332
import com.vaadin.flow.data.provider.hierarchy.HierarchicalArrayUpdater.HierarchicalUpdate;
3433
import com.vaadin.flow.dom.Element;
34+
import com.vaadin.flow.function.SerializablePredicate;
3535
import com.vaadin.flow.function.ValueProvider;
3636
import com.vaadin.flow.server.VaadinRequest;
3737
import com.vaadin.flow.server.VaadinService;
@@ -204,8 +204,9 @@ public void uniqueKeyProviderIsSet_keysGeneratedByProvider() {
204204
fakeClientCommunication();
205205

206206
Arrays.asList("ROOT", "FOLDER", "LEAF")
207-
.forEach(key -> Assert.assertNotNull("Expected key '" + key
208-
+ "' to be generated when unique key provider used",
207+
.forEach(key -> Assert.assertNotNull(
208+
"Expected key '" + key
209+
+ "' to be generated when unique key provider used",
209210
communicator.getKeyMapper().get(key)));
210211
}
211212

@@ -225,8 +226,9 @@ public void uniqueKeyProviderIsNotSet_keysGeneratedByKeyMapper() {
225226

226227
// key mapper should generate keys 1,2,3
227228
IntStream.range(1, 4).mapToObj(String::valueOf)
228-
.forEach(i -> Assert.assertNotNull("Expected key '" + i
229-
+ "' to be generated when unique key provider is not set",
229+
.forEach(i -> Assert.assertNotNull(
230+
"Expected key '" + i
231+
+ "' to be generated when unique key provider is not set",
230232
communicator.getKeyMapper().get(i)));
231233
}
232234

@@ -287,6 +289,38 @@ public void expandItem_requestNonOverlappingRange_expandedItemPersistsInKeyMappe
287289
communicator.reset();
288290
}
289291

292+
@Test
293+
public void expandItem_tooMuchItemsRequested_maxItemsAllowedRequested() {
294+
int startingChildId = 10000;
295+
int children = 3000;
296+
int maxAllowedItems = 1000;
297+
treeData.clear();
298+
treeData.addItems(null, ROOT);
299+
treeData.addItems(ROOT, IntStream.range(0, children).mapToObj(
300+
i -> new Item(startingChildId + i, "ROOT CHILD " + i)));
301+
302+
communicator.expand(ROOT);
303+
fakeClientCommunication();
304+
305+
communicator.setParentRequestedRange(0, children, ROOT);
306+
fakeClientCommunication();
307+
308+
IntStream.range(0, children).forEach(i -> {
309+
String treeItemId = Integer.toString(startingChildId + i);
310+
if (i < maxAllowedItems) {
311+
Assert.assertNotNull(
312+
"Expecting item " + treeItemId
313+
+ " to be fetched, but was not",
314+
communicator.getKeyMapper().get(treeItemId));
315+
} else {
316+
Assert.assertNull(
317+
"Expecting item " + treeItemId
318+
+ " not to be fetched because of max allowed items rule, but it was fetched",
319+
communicator.getKeyMapper().get(treeItemId));
320+
}
321+
});
322+
}
323+
290324
private void assertKeyItemPairIsPresentInKeyMapper(String key, Item item) {
291325
Assert.assertTrue(communicator.getKeyMapper().has(item));
292326
Assert.assertEquals(key, communicator.getKeyMapper().key(item));

0 commit comments

Comments
 (0)