Skip to content

Commit 78ae4a8

Browse files
author
Jeanette Winzenburg
committed
8269871: CellEditEvent: must not throw NPE in accessors
Reviewed-by: aghaisas
1 parent 2267b11 commit 78ae4a8

File tree

6 files changed

+406
-20
lines changed

6 files changed

+406
-20
lines changed

‎modules/javafx.controls/src/main/java/javafx/scene/control/TableColumn.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,8 @@ public TableColumn(String text) {
290290
**************************************************************************/
291291

292292
private EventHandler<CellEditEvent<S,T>> DEFAULT_EDIT_COMMIT_HANDLER = t -> {
293-
int index = t.getTablePosition().getRow();
294-
List<S> list = t.getTableView().getItems();
293+
int index = t.getTablePosition() != null ? t.getTablePosition().getRow() : -1;
294+
List<S> list = t.getTableView() != null ? t.getTableView().getItems() : null;
295295
if (list == null || index < 0 || index >= list.size()) return;
296296
S rowData = list.get(index);
297297
ObservableValue<T> ov = getCellObservableValue(rowData);
@@ -795,7 +795,7 @@ public CellEditEvent(TableView<S> table, TablePosition<S,T> pos,
795795
* @return The TableView control upon which this event occurred.
796796
*/
797797
public TableView<S> getTableView() {
798-
return pos.getTableView();
798+
return pos != null ? pos.getTableView() : null;
799799
}
800800

801801
/**
@@ -804,7 +804,7 @@ public TableView<S> getTableView() {
804804
* @return The TableColumn that the edit occurred in.
805805
*/
806806
public TableColumn<S,T> getTableColumn() {
807-
return pos.getTableColumn();
807+
return pos != null ? pos.getTableColumn() : null;
808808
}
809809

810810
/**
@@ -853,10 +853,10 @@ public T getOldValue() {
853853
* @return the value for the row
854854
*/
855855
public S getRowValue() {
856-
List<S> items = getTableView().getItems();
856+
List<S> items = getTableView() != null ? getTableView().getItems() : null;
857857
if (items == null) return null;
858858

859-
int row = pos.getRow();
859+
int row = pos != null ? pos.getRow() : -1;
860860
if (row < 0 || row >= items.size()) return null;
861861

862862
return items.get(row);

‎modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableColumn.java

+8-8
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,9 @@ public TreeTableColumn(String text) {
285285

286286
private EventHandler<TreeTableColumn.CellEditEvent<S,T>> DEFAULT_EDIT_COMMIT_HANDLER =
287287
t -> {
288-
ObservableValue<T> ov = getCellObservableValue(t.getRowValue());
288+
TreeItem<S> rowValue = t.getRowValue();
289+
if (rowValue == null) return;
290+
ObservableValue<T> ov = getCellObservableValue(rowValue);
289291
if (ov instanceof WritableValue) {
290292
((WritableValue)ov).setValue(t.getNewValue());
291293
}
@@ -772,7 +774,7 @@ public CellEditEvent(TreeTableView<S> table, TreeTablePosition<S,T> pos,
772774
* @return The TableView control upon which this event occurred.
773775
*/
774776
public TreeTableView<S> getTreeTableView() {
775-
return pos.getTreeTableView();
777+
return pos != null ? pos.getTreeTableView() : null;
776778
}
777779

778780
/**
@@ -781,7 +783,7 @@ public TreeTableView<S> getTreeTableView() {
781783
* @return The TreeTableColumn that the edit occurred in.
782784
*/
783785
public TreeTableColumn<S,T> getTableColumn() {
784-
return pos.getTableColumn();
786+
return pos != null ? pos.getTableColumn() : null;
785787
}
786788

787789
/**
@@ -830,12 +832,10 @@ public T getOldValue() {
830832
* @return the row value
831833
*/
832834
public TreeItem<S> getRowValue() {
833-
// List<S> items = getTreeTableView().getItems();
834-
// if (items == null) return null;
835-
836835
TreeTableView<S> treeTable = getTreeTableView();
837-
int row = pos.getRow();
838-
if (row < 0 || row >= treeTable.getExpandedItemCount()) return null;
836+
int row = pos != null ? pos.getRow() : -1;
837+
int expandedItemCount = treeTable != null ? treeTable.getExpandedItemCount() : 0;
838+
if (row < 0 || row >= expandedItemCount) return null;
839839

840840
return treeTable.getTreeItem(row);
841841
}

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/CellEditEventOfTableColumnTest.java

+190
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
/*
2+
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation. Oracle designates this
8+
* particular file as subject to the "Classpath" exception as provided
9+
* by Oracle in the LICENSE file that accompanied this code.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
26+
package test.javafx.scene.control;
27+
28+
import org.junit.After;
29+
import org.junit.Before;
30+
import org.junit.Ignore;
31+
import org.junit.Test;
32+
33+
import static javafx.scene.control.TableColumn.*;
34+
import static org.junit.Assert.*;
35+
36+
import javafx.beans.property.SimpleStringProperty;
37+
import javafx.collections.FXCollections;
38+
import javafx.collections.ObservableList;
39+
import javafx.event.Event;
40+
import javafx.scene.control.TableColumn;
41+
import javafx.scene.control.TableColumn.CellEditEvent;
42+
import javafx.scene.control.TablePosition;
43+
import javafx.scene.control.TableView;
44+
45+
/**
46+
* Test cell edit event for TableColumn: must not throw NPE in accessors (JDK-8269871).
47+
*/
48+
public class CellEditEventOfTableColumnTest {
49+
50+
private TableView<String> table;
51+
private TableColumn<String, String> editingColumn;
52+
53+
//---------------- default commit handler
54+
55+
@Test
56+
public void testDefaultOnCommitHandlerTablePositionWithNullTable() {
57+
String edited = "edited";
58+
TablePosition<String, String> pos = new TablePosition<>(null, 1, editingColumn);
59+
CellEditEvent<String, String> event = new CellEditEvent<>(table, pos, editCommitEvent(), edited);
60+
Event.fireEvent(editingColumn, event);
61+
}
62+
63+
@Test
64+
public void testDefaultOnCommitHandlerNullTablePosition() {
65+
String edited = "edited";
66+
CellEditEvent<String, String> event = new CellEditEvent<>(table, null, editCommitEvent(), edited);
67+
Event.fireEvent(editingColumn, event);
68+
}
69+
70+
//---------------- accessors in CellEditEvent
71+
72+
@Test
73+
public void testNullTablePositionGetTableView() {
74+
CellEditEvent<String, String> ev = new CellEditEvent<>(table, null, editAnyEvent(), null);
75+
assertNull("table must be null for null pos", ev.getTableView());
76+
}
77+
78+
@Test
79+
public void testNullTablePositionGetTableColumn() {
80+
CellEditEvent<String, String> ev = new CellEditEvent<>(table, null, editAnyEvent(), null);
81+
assertNull("column must be null for null pos", ev.getTableColumn());
82+
}
83+
84+
@Test
85+
public void testNullTablePositionGetOldValue() {
86+
CellEditEvent<String, String> ev = new CellEditEvent<>(table, null, editAnyEvent(), null);
87+
assertNull("oldValue must be null for null pos", ev.getOldValue());
88+
}
89+
90+
@Test
91+
public void testNullTablePositionGetRowValue() {
92+
CellEditEvent<String, String> ev = new CellEditEvent<>(table, null, editAnyEvent(), null);
93+
assertNull("rowValue must be null for null pos", ev.getRowValue());
94+
}
95+
96+
@Test
97+
public void testNullTablePositionGetNewValue() {
98+
String editedValue = "edited";
99+
CellEditEvent<String, String> ev = new CellEditEvent<>(table, null, editAnyEvent(), editedValue);
100+
assertEquals("editedValue must be available for null pos", editedValue, ev.getNewValue());
101+
}
102+
103+
@Test
104+
public void testTablePositionWithNullTable() {
105+
String editedValue = "edited";
106+
TablePosition<String, String> pos = new TablePosition<>(null, 1, editingColumn);
107+
CellEditEvent<String, String> ev = new CellEditEvent<>(table, pos, editAnyEvent(), editedValue);
108+
assertNull("rowValue must be null for null pos", ev.getRowValue());
109+
}
110+
111+
//---------- event source
112+
113+
@Ignore("JDK-8271474")
114+
@Test
115+
public void testNullTable() {
116+
new CellEditEvent<Object, Object>(null, // null table must not throw NPE
117+
new TablePosition<>(null, -1, null), editAnyEvent(), null);
118+
}
119+
120+
@Test
121+
public void testCellEditEventDifferentSource() {
122+
assertCellEditEvent(new TableView<>());
123+
}
124+
125+
@Test
126+
public void testCellEditEventSameSource() {
127+
assertCellEditEvent(table);
128+
}
129+
130+
@Ignore("JDK-8271474")
131+
@Test
132+
public void testCellEditEventNullSource() {
133+
assertCellEditEvent(null);
134+
}
135+
136+
/**
137+
* Creates a CellEditEvent with the given source and TablePosition
138+
* having default values and asserts its state.
139+
*/
140+
private void assertCellEditEvent(TableView<String> source) {
141+
int editingRow = 1;
142+
String editedValue = "edited";
143+
String rowValue = table.getItems().get(editingRow);
144+
String oldValue = editingColumn.getCellData(editingRow);
145+
TablePosition<String, String> pos = new TablePosition<>(table, editingRow, editingColumn);
146+
CellEditEvent<String, String> event = new CellEditEvent<>(source, pos, editAnyEvent(), editedValue);
147+
if (source != null) {
148+
assertEquals(source, event.getSource());
149+
}
150+
assertCellEditEventState(event, table, editingColumn, pos, editedValue, oldValue, rowValue);
151+
}
152+
153+
/**
154+
* Asserts state of the CellEditEvent.
155+
*/
156+
private <S, T> void assertCellEditEventState(CellEditEvent<S, T> event,
157+
TableView<S> table, TableColumn<S, T> tableColumn, TablePosition<S, T> pos,
158+
T newValue, T oldValue, S rowValue) {
159+
assertEquals(newValue, event.getNewValue());
160+
assertEquals(oldValue, event.getOldValue());
161+
assertEquals(rowValue, event.getRowValue());
162+
assertEquals(tableColumn, event.getTableColumn());
163+
assertEquals(pos, event.getTablePosition());
164+
assertEquals(table, event.getTableView());
165+
}
166+
167+
//------------ init
168+
169+
@Before public void setup() {
170+
Thread.currentThread().setUncaughtExceptionHandler((thread, throwable) -> {
171+
if (throwable instanceof RuntimeException) {
172+
throw (RuntimeException)throwable;
173+
} else {
174+
Thread.currentThread().getThreadGroup().uncaughtException(thread, throwable);
175+
}
176+
});
177+
178+
ObservableList<String> model = FXCollections.observableArrayList("Four", "Five", "Fear");
179+
table = new TableView<String>(model);
180+
editingColumn = new TableColumn<>("TEST");
181+
editingColumn.setCellValueFactory(e -> new SimpleStringProperty(e.getValue()));
182+
table.getColumns().addAll(editingColumn);
183+
}
184+
185+
@After
186+
public void cleanup() {
187+
Thread.currentThread().setUncaughtExceptionHandler(null);
188+
}
189+
190+
}

0 commit comments

Comments
 (0)