Skip to content

Commit 75fca4c

Browse files
google-labs-jules[bot]paodb
authored andcommitted
fix: return modifiable collections from getValue()
Close #193
1 parent 79f307e commit 75fca4c

File tree

4 files changed

+122
-3
lines changed

4 files changed

+122
-3
lines changed

src/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColGrid.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,8 +618,7 @@ public void setValue(final Set<T> value) {
618618
*/
619619
@Override
620620
public Set<T> getValue() {
621-
return Collections.unmodifiableSet(
622-
collectValue(Collectors.<T, Set<T>>toCollection(LinkedHashSet::new)));
621+
return collectValue(Collectors.<T, Set<T>>toCollection(LinkedHashSet::new));
623622
}
624623

625624
/**

src/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColGridListAdapter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public void setValue(List<T> value) {
8585

8686
@Override
8787
public List<T> getValue() {
88-
return Collections.unmodifiableList(delegate.collectValue(Collectors.toList()));
88+
return delegate.collectValue(Collectors.toList());
8989
}
9090

9191
@Override
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package com.flowingcode.vaadin.addons.twincolgrid;
2+
3+
import org.junit.Assert;
4+
import org.junit.Test;
5+
import java.util.Arrays;
6+
import java.util.LinkedHashSet;
7+
import java.util.List;
8+
import java.util.Set;
9+
10+
public class TwinColGridListAdapterTest {
11+
12+
@Test
13+
public void testGetValueReturnsModifiableListAndDoesNotAffectInternalState() {
14+
// 1. Instantiate a TwinColGrid<String>
15+
TwinColGrid<String> twinColGrid = new TwinColGrid<>();
16+
17+
// 2. Instantiate TwinColGridListAdapter<String> with the created TwinColGrid
18+
TwinColGridListAdapter<String> adapter = new TwinColGridListAdapter<>(twinColGrid);
19+
20+
// 3. Add a few initial items (e.g., "Vixen", "Comet") to the TwinColGrid's selection.
21+
Set<String> initialSelection = new LinkedHashSet<>(Arrays.asList("Vixen", "Comet"));
22+
twinColGrid.setValue(initialSelection);
23+
24+
// 4. Call getValue() on the TwinColGridListAdapter instance.
25+
List<String> listFromAdapter = adapter.getValue();
26+
27+
// 5. Assert that the returned List<String> is not null and contains the initial items.
28+
Assert.assertNotNull("The list returned by getValue() should not be null.", listFromAdapter);
29+
Assert.assertEquals("The list should contain 2 items.", 2, listFromAdapter.size());
30+
Assert.assertTrue("The list should contain 'Vixen'.", listFromAdapter.contains("Vixen"));
31+
Assert.assertTrue("The list should contain 'Comet'.", listFromAdapter.contains("Comet"));
32+
33+
// 6. Attempt to add a new item (e.g., "Cupid") to the list obtained in step 4.
34+
// Verify that this operation is successful (no exception thrown) and the list now contains "Cupid".
35+
boolean added = false;
36+
try {
37+
added = listFromAdapter.add("Cupid");
38+
} catch (UnsupportedOperationException e) {
39+
Assert.fail("The list should be modifiable, but add operation threw UnsupportedOperationException.");
40+
}
41+
Assert.assertTrue("The add operation should return true, indicating the list was modified.", added);
42+
Assert.assertEquals("The list from adapter should now contain 3 items.", 3, listFromAdapter.size());
43+
Assert.assertTrue("The list from adapter should now contain 'Cupid'.", listFromAdapter.contains("Cupid"));
44+
45+
// 7. Call getValue() on the TwinColGridListAdapter again
46+
List<String> secondListFromAdapter = adapter.getValue();
47+
48+
// 8. Assert that this second list *only* contains the original items ("Vixen", "Comet")
49+
// and *does not* contain "Cupid".
50+
Assert.assertNotNull("The second list returned by getValue() should not be null.", secondListFromAdapter);
51+
Assert.assertEquals("The second list should still contain 2 items (original state).", 2, secondListFromAdapter.size());
52+
Assert.assertTrue("The second list should contain 'Vixen'.", secondListFromAdapter.contains("Vixen"));
53+
Assert.assertTrue("The second list should contain 'Comet'.", secondListFromAdapter.contains("Comet"));
54+
Assert.assertFalse("The second list should NOT contain 'Cupid'.", secondListFromAdapter.contains("Cupid"));
55+
56+
// Also, verify the underlying TwinColGrid's value directly
57+
Set<String> twinColGridValue = twinColGrid.getValue();
58+
Assert.assertNotNull("The TwinColGrid's value should not be null.", twinColGridValue);
59+
Assert.assertEquals("The TwinColGrid's value should contain 2 items.", 2, twinColGridValue.size());
60+
Assert.assertTrue("The TwinColGrid's value should contain 'Vixen'.", twinColGridValue.contains("Vixen"));
61+
Assert.assertTrue("The TwinColGrid's value should contain 'Comet'.", twinColGridValue.contains("Comet"));
62+
Assert.assertFalse("The TwinColGrid's value should NOT contain 'Cupid'.", twinColGridValue.contains("Cupid"));
63+
}
64+
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package com.flowingcode.vaadin.addons.twincolgrid;
2+
3+
import org.junit.Assert;
4+
import org.junit.Test;
5+
import java.util.Arrays;
6+
import java.util.HashSet;
7+
import java.util.Set;
8+
import java.util.LinkedHashSet;
9+
10+
public class TwinColGridTest {
11+
12+
@Test
13+
public void testGetValueReturnsModifiableSet() {
14+
// 1. Create an instance of TwinColGrid<String>
15+
TwinColGrid<String> twinColGrid = new TwinColGrid<>();
16+
17+
// 2. Add a couple of items to the selection side of the grid.
18+
// To do this, we first set items to the available side,
19+
// then simulate moving them to the selection side.
20+
// Alternatively, setValue directly manipulates the selection.
21+
Set<String> initialSelection = new LinkedHashSet<>(Arrays.asList("Dasher", "Dancer"));
22+
twinColGrid.setValue(initialSelection);
23+
24+
// 3. Call the getValue() method to get the set of selected items.
25+
Set<String> selectedItems = twinColGrid.getValue();
26+
27+
// 4. Assert that the returned set is not null and contains the expected items.
28+
Assert.assertNotNull("The set returned by getValue() should not be null.", selectedItems);
29+
Assert.assertEquals("The set should contain 2 items.", 2, selectedItems.size());
30+
Assert.assertTrue("The set should contain 'Dasher'.", selectedItems.contains("Dasher"));
31+
Assert.assertTrue("The set should contain 'Dancer'.", selectedItems.contains("Dancer"));
32+
33+
// 5. Attempt to add a new item (e.g., "Prancer") to the set returned by getValue().
34+
boolean added = false;
35+
try {
36+
added = selectedItems.add("Prancer");
37+
} catch (UnsupportedOperationException e) {
38+
Assert.fail("The set should be modifiable, but add operation threw UnsupportedOperationException.");
39+
}
40+
41+
// 6. Assert that the add operation was successful and the set now contains the newly added item.
42+
Assert.assertTrue("The add operation should return true, indicating the set was modified.", added);
43+
Assert.assertEquals("The set should now contain 3 items.", 3, selectedItems.size());
44+
Assert.assertTrue("The set should contain 'Prancer'.", selectedItems.contains("Prancer"));
45+
46+
// 7. Call getValue() on the TwinColGrid instance again to get a fresh set.
47+
Set<String> internalSelectionAfterModification = twinColGrid.getValue();
48+
49+
// 8. Assert that this new set contains only the original items and does not contain "Prancer".
50+
Assert.assertNotNull("The internal set fetched after modification should not be null.", internalSelectionAfterModification);
51+
Assert.assertEquals("The internal set should still contain 2 original items.", 2, internalSelectionAfterModification.size());
52+
Assert.assertTrue("The internal set should still contain 'Dasher'.", internalSelectionAfterModification.contains("Dasher"));
53+
Assert.assertTrue("The internal set should still contain 'Dancer'.", internalSelectionAfterModification.contains("Dancer"));
54+
Assert.assertFalse("The internal set should NOT contain 'Prancer'.", internalSelectionAfterModification.contains("Prancer"));
55+
}
56+
}

0 commit comments

Comments
 (0)