IndexOutOfBounds for DataProvider #6072

dennisunderwood · 2019-07-15T14:44:43Z

Created a DataProvider.fromCallbacks object. First page of paging works correctly, second load of data,however causes an indexoutofbounds error:
query.getOffset(): 0
query.getLimit(): 50
query.getOffset(): 50
query.getLimit(): 50
java.lang.IndexOutOfBoundsException: Index: 50, Size: 50

java.lang.IndexOutOfBoundsException: Index: 50, Size: 50
at java.util.ArrayList.rangeCheck(ArrayList.java:657) ~[na:1.8.0_212]
at java.util.ArrayList.get(ArrayList.java:433) ~[na:1.8.0_212]
at com.vaadin.flow.data.provider.DataCommunicator.lambda$getJsonItems$3(DataCommunicator.java:611) ~[flow-data-1.4.6.jar!/:1.4.6]

Minimal reproducible example
This is about as stripped down as I can make it quickly while keeping this verbose for you...

DataProvider<License,Void> licenseDataProvider = DataProvider.fromCallbacks(
                new CallbackDataProvider.FetchCallback<License, Void>() {
                    @Override
                    Stream<License> fetch(Query<License, Void> query) {
                        println "query.getOffset(): "+query.getOffset()
                        println "query.getLimit(): "+ query.getLimit()
                        Stream<License> licenseStream = licenseRepository.findByZenGroupId(user.getZenGroupId()).subList(query.getOffset(),query.getLimit()).stream()
                        //licensePage = licensePagingRepository.findAllByZenGroupId(user.getZenGroupId(), new PageRequest(query.getOffset(), limit))

                        return licenseStream
                    }
                },new CallbackDataProvider.CountCallback<License, Void>() {
            @Override
            int count(Query<License, Void> query) {
                //println "totalElements:"+licensePagingRepository.findAllByZenGroupId(user.getZenGroupId(), new PageRequest(query.getOffset(), query.getLimit())).getNumber()
                //println "numberOfElements:"+licensePagingRepository.countByZenGroupId(user.getZenGroupId(), new PageRequest(query.getOffset(), query.getLimit()))

                return licenseRepository.countByZenGroupId(user.getZenGroupId())
            }
        })

Expected behavior
DataProvider lazy loading population to a grid.
Actual behavior
Crash upon scroll.
Versions:
- Vaadin / Flow version
  13.0.10
- Java version
  Java: 1.8.212 zulu
  Spring: Spring boot 2.1.6.RELEASE
- OS version: Server: Ubuntu
- Browser version (if applicable)
  --N/A

The text was updated successfully, but these errors were encountered:

denis-anisimov · 2019-07-16T10:42:43Z

Hi, unfortunately I may not use your code to reproduce the issue.
Minimal reproducible example means the example which may be used to copy-paste to reproduce
an issue. All extra information shouldn't be included.
The data provider construction code doesn't allow me to reproduce anything.

There is no License bean (which is kind of extra here), there is no licenseRepository (which is definitely extra here).
There is no code which I may just run and see the issue: data provider doesn't do anything by itself.
It should be used inside something.
Where is the component which uses your data provider ? Is it Grid or something else?
How can I be sure that the issue is not in the component but rather in the DataProvider , DataCommunicator?

So : please simplify the code that it doesn't use unknown beans, the repository which may be avoided in the example ( and replaced by an in-memory collection ).
And please provide a full example which may doesn't require any additional code to execute it.
The most appropriate way is just to use https://github.com/vaadin/skeleton-starter-flow project , modify it to reproduce the issue and attach it.

dennisunderwood · 2019-07-16T14:56:39Z

OK, that type of response, that's fine.

It is out of scope, to provide you a fully functioning web application to demonstrate your method back to you.
This is a unit testing issue on core Vaadin.

We cannot be expected to use in-memory lists in production, even if lists work, and they appear to work, but do not work in production due to data sizes.

Here are basics on spring data jpa to get you started, so you can create a quick bean, then spring data repository:
https://www.petrikainulainen.net/programming/spring-framework/spring-data-jpa-tutorial-creating-database-queries-from-method-names/

It populates a grid via grid.setDataProvider(), but this is an issue with DataProvider, not with Grid, or ComboBox, or some other component. The same off by one error occurs.

It looks like this is binned as "feature work". We cannot use grid in a production environment while this bug exists, so please let us know if we need to start migrating away from vaadin.

alump · 2019-07-16T18:53:53Z

Make sure you check values you use:
Query.getLimit returns amount of responses expected, not end index
List.subList second parameter is exclusive end index, not size of subList
=> You shouldn't give Query.getLimit as second parameter to subList method, right?

But what value does your count method return? Is it 50?

Here is simple example code that works with Vaadin 13:

DataProvider<String,Void> dataProvider = DataProvider.fromCallbacks((query) -> {
	System.out.println("from index " + query.getOffset() + " to index "
			+ (query.getOffset() + query.getLimit() - 1));

	if(query.getOffset() + query.getLimit() > 50) {
		throw new IllegalStateException("Asking too many!");
	}

	List<String> data = new ArrayList<>();
	for(int i = 0; i < query.getLimit(); ++i) {
		int rowIndex = query.getOffset() + i;
		data.add("Row " + rowIndex);
	}
	return data.stream();
	}, (query) -> {
		return 50;
	});
Grid<String> grid = new Grid<>();
grid.setDataProvider(dataProvider);
grid.addColumn(String::toString).setHeader("Header");

add(grid);

The only line this prints with default page size (50) is:

from index 0 to index 49

Legioth · 2019-07-17T14:13:12Z

The immediate problem in this case is indeed as indicated by @alump, i.e. that subList is based on starting and ending index instead of starting index (offset) and size (limit) that is directly available from the data provider query.

This causes the data provider to return a stream of size 0 (starting from index 50 and ending at index 50) which indirectly causes the indexing error because the logic expected 50 items to be available from the stream. You can observe this by breaking up the example code to also print the size of the intermediate sub list:

List<License> subList = licenseRepository.findByZenGroupId(0).subList(query.getOffset(), query.getLimit());
System.out.println("Returning sublist of length " + subList.size() + " for query size " + query.getLimit());
return subList.stream();

This can be fixed by changing the example code to calculate the end index for the sub list based on the offset and the limit, i.e. subList(query.getOffset(), query.getOffset() + query.getLimit()).

At the same time, the commented-out lines in the example indicates that this is only an intermediate step towards lazy loading items from the database. This won't work as it is implemented in the commented-out code line because the (deprecated) PageRequest constructor takes the page index as its first parameter rather than the index of the first item in the desired page. This would probably quite quickly lead to the same symptoms since new PageRequest(50, 50) would lead to loading items starting from index 2500 from the database. If the database query finds less than 2500 lines, the returned list would be empty.

It might be tempting to try to compensate for this simply by dividing the requested offset by the limit in order to get a corresponding page index. This will seem to work in simple cases, but it will break down in cases when the component requests multiple logical pages at the same time. Instead, you need to use slightly more logic to align the requested item range to a page boundary that Spring Data can process. There's already code for doing this freely available in https://github.com/Artur-/spring-data-provider. The add-on also provides additional helper API that makes it easier to use Spring Data repositories together with Vaadin.

Legioth · 2019-07-17T14:44:20Z

On a completely different note, I'd also like to address this comment:

It is out of scope, to provide you a fully functioning web application to demonstrate your method back to you.
This is a unit testing issue on core Vaadin.

In order to determine that the problem was in the example code and not in Vaadin itself, I had to make assumptions about the implementation of the missing classes and how the data provider was used. This was a relatively straightforward case but as an example, the symptoms would have been different if I'd used the data provider with a Select component instead of Grid.

There have also been cases where we have made wrong assumptions without noticing and then ended up only implementing a partial fix or fixing a completely different related issue instead of fixing the issue faced by the reporter.

We are receiving hundreds of reports through open source issue trackers every week. We want to dedicate our time to fixing actual issues or implementing new features rather than filling in blanks in reported issues or making changes that are not helping the reporter. By providing a fully functional example that can be run as-is, you are significantly increasing the chances that you will receive help quickly and efficiently.

In this case, a functionally equivalent self-contained example could have been implemented like this:

@Route
public class Test extends Div {
    public Test() {
        List<String> items = IntStream.range(0, 500).mapToObj(Integer::toString)
                .collect(Collectors.toList());

        DataProvider<String, Void> licenseDataProvider = DataProvider
                .fromCallbacks(query -> {
                    Stream<String> licenseStream = items
                            .subList(query.getOffset(), query.getLimit())
                            .stream();
                    return licenseStream;
                }, query -> items.size());

        Grid<String> grid = new Grid<>();
        grid.setDataProvider(licenseDataProvider);
        grid.addColumn(item -> item);
        add(grid);
    }
}

denis-anisimov · 2019-07-17T15:23:40Z

This causes the data provider to return a stream of size 0 (starting from index 50 and ending at index 50) which indirectly causes the indexing error because the logic expected 50 items to be available from the stream.

But it indicates that our contract is not obvious/well defined.
So an additional question is : should we add some logic which verifies the contract like we did in this case:
https://github.com/vaadin/flow/blob/master/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java#L374
https://github.com/vaadin/flow/blob/master/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java#L378
And whether it's feasible ?

dennisunderwood · 2019-07-17T16:06:07Z

HI everyone, thank you for the responses, and we'll look at this in a bit, over the weekend. We've moved to other parts of our product from the web app (machine learning automation, etc. on different servers) while some type of decision is made internally. It appears there is a potential way forward with Vaadin customer-facing then, which is good. I understand - to Denis' point, and now I understand the length item. Could we rename the attributes and/or make it more obvious in the documentation what length you are looking at for the documentation? While we confirmed the location we saw in our stacktraces, in vaadin's github repo, we did/do not have time in the near term to trace how the attributes are used in your code, given the other pieces to the product. The commented portions were just left to show some other efforts to enumerate what is going on, versus us deleting them. Sorry to be be confusing there. Cheers, Dennis Underwood CEO Cyber Crucible, Inc. dennis.underwood@cybercrucible.com +1.410.216.0369 x1001

…

On Wed, Jul 17, 2019 at 11:23 AM Denis ***@***.***> wrote: This causes the data provider to return a stream of size 0 (starting from index 50 and ending at index 50) which indirectly causes the indexing error because the logic expected 50 items to be available from the stream. But it indicates that our contract is not obvious/well defined. So an additional question is : should we add some logic which verifies the contract like we did in this case: https://github.com/vaadin/flow/blob/master/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java#L374 https://github.com/vaadin/flow/blob/master/flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java#L378 And whether it's feasible ? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#6072?email_source=notifications&email_token=AB6ONGW5CWRPPRERJGKXYCTP742P7A5CNFSM4IDXTAJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2EYHOA#issuecomment-512328632>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB6ONGRIFBJPBJ6FGWY52NLP742P7ANCNFSM4IDXTAJQ> .

-- www.cybercrucible.com <http://www.cybercrucible.com/> 550M Ritchie Hwy. #135 Severna Park, MD 21146 This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you have received this email in error please notify the system manager. This message contains confidential information and is intended only for the individual named. If you are not the named addressee you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. If you are not the intended recipient you are notified that disclosing, copying, distributing or taking any action in reliance on the contents of this information is strictly prohibited.

With the original phrasing, "limit" could potentially also be understood as the index of the last item to fetch. Related to #6072

Legioth · 2019-07-19T08:33:29Z

Seems like it's not completely trivial to improve the error message since we also have logic added by #4889 that tries to deal with situations where there is a size mismatch if rows have been removed from the database since the last time size() was called.

I suspect that logic gets triggered in this case, which makes it call size() again and then we get the IndexOutOfBoundsException since size() and the actually returned data is still not in sync.

This in turn might imply that the logic from #4889 is actually also broken if the database contents change again between the time when fetch() is called and size() is called again. This means that the only bombproof approach would be to only treat size() as a hint, i.e. vaadin/framework#8492. In that mode of operation, the misunderstanding that caused this issue would just silently cause the data to be truncated to the 50 first rows.

My conclusion for now is thus that the only thing we could reasonably do is to clarify the documentation. I wouldn't consider changing any method names since those would cause a mess with backwards compatibility.

I'm quite blind to the ways in which the documentation might be misunderstood since I already know exactly what it wants to say. I only spotted one immediately obvious case that is addressed in #6099.

Please write a comment here or create a new issue if you encounter something else in the documentation that could be clarified or if you have an idea on how error reporting could be improved in a way that also makes sense in the case when the database size has actually changed.

With the original phrasing, "limit" could potentially also be understood as the index of the last item to fetch. Related to #6072

With the original phrasing, "limit" could potentially also be understood as the index of the last item to fetch. Related to #6072 (cherry picked from commit aa23fcd)

jhult · 2019-10-03T20:49:09Z

This seems like it may be related to #3830.

The self-contained app (mentioned again in this comment as a copy and paste from above comment) errors out for me once I scroll past the first 49 results.

@Route
public class Test extends Div {
    public Test() {
        final List<String> items = IntStream.range(0, 500).mapToObj(Integer::toString)
                .collect(Collectors.toList());

        final DataProvider<String, Void> licenseDataProvider = DataProvider
                .fromCallbacks(query -> {
                    final Stream<String> licenseStream = items
                            .subList(query.getOffset(), query.getLimit())
                            .stream();
                    return licenseStream;
                }, query -> items.size());

        final Grid<String> grid = new Grid<>();
        grid.setDataProvider(licenseDataProvider);
        grid.addColumn(item -> item);
        add(grid);
    }
}

Stack trace:

[INFO] java.lang.IndexOutOfBoundsException: Index 50 out of bounds for length 50
[INFO] 	at jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64) ~[?:?]
[INFO] 	at jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70) ~[?:?]
[INFO] 	at jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248) ~[?:?]
[INFO] 	at java.util.Objects.checkIndex(Objects.java:372) ~[?:?]
[INFO] 	at java.util.ArrayList.get(ArrayList.java:458) ~[?:?]
[INFO] 	at com.vaadin.flow.data.provider.DataCommunicator.lambda$getJsonItems$3(DataCommunicator.java:611) ~[flow-data-2.0.14.jar:2.0.14]
[INFO] 	at java.util.stream.IntPipeline$1$1.accept(IntPipeline.java:180) ~[?:?]
[INFO] 	at java.util.stream.Streams$RangeIntSpliterator.forEachRemaining(Streams.java:104) ~[?:?]
[INFO] 	at java.util.Spliterator$OfInt.forEachRemaining(Spliterator.java:699) ~[?:?]
[INFO] 	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?]
[INFO] 	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
[INFO] 	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913) ~[?:?]
[INFO] 	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
[INFO] 	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578) ~[?:?]
[INFO] 	at com.vaadin.flow.data.provider.DataCommunicator.getJsonItems(DataCommunicator.java:613) ~[flow-data-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.data.provider.DataCommunicator.lambda$collectChangesToSend$2(DataCommunicator.java:571) ~[flow-data-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.data.provider.DataCommunicator.applyIfNotEmpty(DataCommunicator.java:627) ~[flow-data-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.data.provider.DataCommunicator.withMissing(DataCommunicator.java:621) ~[flow-data-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.data.provider.DataCommunicator.collectChangesToSend(DataCommunicator.java:570) ~[flow-data-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.data.provider.DataCommunicator.flush(DataCommunicator.java:473) ~[flow-data-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.data.provider.DataCommunicator.lambda$requestFlush$2f364bb9$1(DataCommunicator.java:421) ~[flow-data-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.internal.StateTree.lambda$runExecutionsBeforeClientResponse$1(StateTree.java:364) ~[flow-server-2.0.14.jar:2.0.14]
[INFO] 	at java.util.ArrayList.forEach(ArrayList.java:1540) ~[?:?]
[INFO] 	at com.vaadin.flow.internal.StateTree.runExecutionsBeforeClientResponse(StateTree.java:361) ~[flow-server-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.server.communication.UidlWriter.encodeChanges(UidlWriter.java:392) ~[flow-server-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.server.communication.UidlWriter.createUidl(UidlWriter.java:182) ~[flow-server-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.server.communication.UidlRequestHandler.writeUidl(UidlRequestHandler.java:116) ~[flow-server-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:89) ~[flow-server-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:40) ~[flow-server-2.0.14.jar:2.0.14]
[INFO] 	at com.vaadin.flow.server.VaadinService.handleRequest(VaadinService.java:1540) ~[flow-server-2.0.14.jar:2.0.14]

dennisunderwood · 2019-10-03T20:56:22Z

@jhult I found a solution...we had moved on shortly afterwards.

project-bot bot added this to Inbox - needs triage in OLD Vaadin Flow ongoing work (Vaadin 10+) Jul 15, 2019

denis-anisimov assigned dennisunderwood Jul 16, 2019

project-bot bot moved this from Inbox - needs triage to In progress in OLD Vaadin Flow ongoing work (Vaadin 10+) Jul 16, 2019

denis-anisimov added the waiting for author label Jul 16, 2019

denis-anisimov moved this from In progress to Product backlog in OLD Vaadin Flow ongoing work (Vaadin 10+) Jul 16, 2019

Legioth added a commit that referenced this issue Jul 19, 2019

Clarify that "limit" is the number of items to fetch

fd865fa

With the original phrasing, "limit" could potentially also be understood as the index of the last item to fetch. Related to #6072

Legioth mentioned this issue Jul 19, 2019

Clarify that "limit" is the number of items to fetch #6099

Merged

Legioth closed this as completed Jul 19, 2019

OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Product backlog to Done - pending release Jul 19, 2019

joheriks pushed a commit that referenced this issue Jul 19, 2019

Clarify that "limit" is the number of items to fetch (#6099)

aa23fcd

With the original phrasing, "limit" could potentially also be understood as the index of the last item to fetch. Related to #6072

denis-anisimov added this to the Abandoned milestone Jul 24, 2019

jhult mentioned this issue Oct 7, 2019

DataProvider IndexOutOfBoundsException when providing offset/limit #6631

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IndexOutOfBounds for DataProvider #6072

IndexOutOfBounds for DataProvider #6072

dennisunderwood commented Jul 15, 2019 •

edited by denis-anisimov

denis-anisimov commented Jul 16, 2019

dennisunderwood commented Jul 16, 2019

alump commented Jul 16, 2019

Legioth commented Jul 17, 2019

Legioth commented Jul 17, 2019

denis-anisimov commented Jul 17, 2019

dennisunderwood commented Jul 17, 2019 via email

Legioth commented Jul 19, 2019

jhult commented Oct 3, 2019

dennisunderwood commented Oct 3, 2019

IndexOutOfBounds for DataProvider #6072

IndexOutOfBounds for DataProvider #6072

Comments

dennisunderwood commented Jul 15, 2019 • edited by denis-anisimov

denis-anisimov commented Jul 16, 2019

dennisunderwood commented Jul 16, 2019

alump commented Jul 16, 2019

Legioth commented Jul 17, 2019

Legioth commented Jul 17, 2019

denis-anisimov commented Jul 17, 2019

dennisunderwood commented Jul 17, 2019 via email

Legioth commented Jul 19, 2019

jhult commented Oct 3, 2019

dennisunderwood commented Oct 3, 2019

dennisunderwood commented Jul 15, 2019 •

edited by denis-anisimov