Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ListDiff#accept throws IndexOutOfBoundException #1713

Open
navasndm opened this issue Feb 22, 2024 · 1 comment
Open

ListDiff#accept throws IndexOutOfBoundException #1713

navasndm opened this issue Feb 22, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@navasndm
Copy link

navasndm commented Feb 22, 2024

When we have 2 list in specific order, ListDiff#accept may handle the ListDiffEntry list created by Diffs#computeListDiff incorrectly which results in IndexOutOfBoundException.

Recreation step : Running the attached snippet will reproduce the issue

Attached a snippet and output from the snippet below to explain the issue. Diffs#computeListDiff correctly generates the ListDiffEntry list and while processing the list accept() method clubs the 1st and 2nd entry together incorrectly, however it should be considering the 2nd and 3rd entry together as computeListDiff created 2nd and 3rd entry together to consider this for move(There is a code comment in Diffs#createListDiffs() to mention the move)

Based on the accept() method java doc it calls the handleReplace() if add entry is adjacent to remove entry, in this case it calculated the 2nd entries position as 1st entry and processing as replace.

Expected behaviour:
To correctly process the list it should call the handleAdd() first then handleMove() for 2nd and 3rd entry together followed by handleMove() for 4th and 5th entry then handleAdd() for 6th entry and 7th entry.

Proposed Code Change:
Before calling the visitor.handleReplace() it should check the next to next entry to confirm it is not same element as the next entry element.

Current Code - ListDiff#accept()

					if (removePos == addPos) {
						visitor.handleReplace(pos, removeElem, addElem);
						i++;
						continue;
					}

After Change - ListDiff#accept()

					if (removePos == addPos) {
						if (i + 2 < differences.length) {
							ListDiffEntry nextToNextEntry = differences[i + 2];
							Object nextToNextElem = nextToNextEntry.getElement();
							if ( Objects.equals(removeElem, nextToNextElem))
							{
								visitor.handleAdd(addPos, addElem);
								continue;
							}
						}

						visitor.handleReplace(pos, removeElem, addElem);
						i++;
						continue;
					}

Attached the snippet to recreate the issue
ListDiffSnippet.txt

Snippet generates the following output.

Old List : Item 7, Item 4, Item 5, Item 2
New List : Item 1, Item 2, Item 3, Item 4, Item 5, Item 6, Item 7

ListDiff contents :
	Position/Index : 0 - Is Addition : true - Entry : Item 1
	Position/Index : 1 - Is Addition : false - Entry : Item 7
	Position/Index : 4 - Is Addition : true - Entry : Item 7
	Position/Index : 3 - Is Addition : false - Entry : Item 2
	Position/Index : 1 - Is Addition : true - Entry : Item 2
	Position/Index : 2 - Is Addition : true - Entry : Item 3
	Position/Index : 5 - Is Addition : true - Entry : Item 6

ListDiff#accept :
	ListDiffSnippet#handleReplace - index = 0 - Old Element = Item 7 - New Element = Item 1
	ListDiffSnippet#handleReplace - index = 4 - Old Element = Item 2 - New Element = Item 7
Exception in thread "main" java.lang.IndexOutOfBoundsException: Index 4 out of bounds for length 4
	at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
	at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
	at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248)
	at java.base/java.util.Objects.checkIndex(Objects.java:372)
	at java.base/java.util.ArrayList.set(ArrayList.java:473)
	at com.snippet.test.ListDiffSnippet$1.handleReplace(ListDiffSnippet.java:65)
	at org.eclipse.core.databinding.observable.list.ListDiff.accept(ListDiff.java:136)
	at com.snippet.test.ListDiffSnippet.main(ListDiffSnippet.java:39)
@jukzi jukzi added the bug Something isn't working label Feb 23, 2024
@jukzi
Copy link
Contributor

jukzi commented Feb 23, 2024

Proposed Code Change:

Please provide a pull request together with a JUnit test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants