Issue
I have a TableView which uses a RowFactory to style rows depending on a specific property of the item of the row. The RowFactory uses worker threads to check the validity of this specific property against a call to the database. The problem is that correct rows are sometimes marked as incorrect (red through a PseudoClass) and incorrect rows not marked. I have created a Minimal Reproducible Example below. This example should mark only rows that are even...but it also marks other rows.
Test Entity
public class TestEntity
{
public TestEntity(String firstName, String lastName, int c)
{
setFirstName(firstName);
setLastName(lastName);
setC(c);
}
private StringProperty firstName = new SimpleStringProperty();
private StringProperty lastName = new SimpleStringProperty();
private IntegerProperty c = new SimpleIntegerProperty();
public int getC()
{
return c.get();
}
public IntegerProperty cProperty()
{
return c;
}
public void setC(int c)
{
this.c.set(c);
}
public String getFirstName()
{
return firstName.get();
}
public StringProperty firstNameProperty()
{
return firstName;
}
public void setFirstName(String firstName)
{
this.firstName.set(firstName);
}
public String getLastName()
{
return lastName.get();
}
public StringProperty lastNameProperty()
{
return lastName;
}
public void setLastName(String lastName)
{
this.lastName.set(lastName);
}
}
Main
public class TableViewProblemMain extends Application
{
public static void main(String[] args)
{
launch(args);
AppThreadPool.shutdown();
}
@Override
public void start(Stage stage)
{
TableView<TestEntity> tableView = new TableView();
TableColumn<TestEntity, String> column1 = new TableColumn<>("First Name");
column1.setCellValueFactory(new PropertyValueFactory<>("firstName"));
TableColumn<TestEntity, String> column2 = new TableColumn<>("Last Name");
column2.setCellValueFactory(new PropertyValueFactory<>("lastName"));
TableColumn<TestEntity, String> column3 = new TableColumn<>("C");
column3.setCellValueFactory(new PropertyValueFactory<>("c"));
tableView.getColumns().addAll(column1, column2, column3);
tableView.setRowFactory(new TestRowFactory());
for (int i = 0; i < 300; i++)
{
tableView.getItems().add(new TestEntity("Fname" + i, "Lname" + i, i));
}
VBox vbox = new VBox(tableView);
Scene scene = new Scene(vbox);
scene.getStylesheets().add(this.getClass().getResource("/style.css").toExternalForm());
// Css has only these lines:
/*
.table-row-cell:invalid {
-fx-background-color: rgba(240, 116, 116, 0.18);
}
* */
stage.setScene(scene);
stage.show();
}
}
Row Factory
public class TestRowFactory implements Callback<TableView<TestEntity>, TableRow<TestEntity>>
{
private final PseudoClass INVALID_PCLASS = PseudoClass.getPseudoClass("invalid");
@Override
public TableRow<TestEntity> call(TableView param)
{
TableRow<TestEntity> row = new TableRow();
Thread validationThread = new Thread(() ->
{
try
{
if(row.getItem() != null)
{
Thread.sleep(500); // perform validation and stuff...
if(row.getItem().getC() % 2 == 0)
{
Tooltip t = new Tooltip("I am a new tooltip that should be shown only on red rows");
row.setTooltip(t);
row.pseudoClassStateChanged(INVALID_PCLASS, true);
}
}
} catch (InterruptedException e)
{
e.printStackTrace();
}
});
ChangeListener changeListener = (obs, old, current) ->
{
row.setTooltip(null);
AppThreadPool.perform(validationThread);
};
row.itemProperty().addListener((observable, oldValue, newValue) ->
{
row.setTooltip(null);
if (oldValue != null)
{
oldValue.firstNameProperty().removeListener(changeListener);
}
if (newValue != null)
{
newValue.firstNameProperty().removeListener(changeListener);
AppThreadPool.perform(validationThread);
}
else
{
row.pseudoClassStateChanged(INVALID_PCLASS, false);
}
});
row.focusedProperty().addListener(changeListener);
return row;
}
}
AppThreadPool
public class AppThreadPool
{
private static final int threadCount = Runtime.getRuntime().availableProcessors();
private static final ThreadPoolExecutor executorService = (ThreadPoolExecutor) Executors.newFixedThreadPool(threadCount * 2 + 1);
public static <R extends Runnable> void perform(R runnable)
{
executorService.submit(runnable);
}
public static void shutdown()
{
executorService.shutdown();
}
}
Screenshot
Solution
There are several misunderstandings in your code. The first is about cells and reuse (a TableRow is a Cell). Cells can be reused arbitrarily, and potentially frequently (especially during user scrolling) to stop displaying one item and display a different one.
In your code, if the row is used to display an entity that is invalid, the listener on the row's itemProperty will trigger the runnable on the background thread, which will at some point set the pseudoclass state to true.
If the cell is subsequently reused to display a valid item, however, the next runnable that is executed does not change the pseudoclass state. So that state remains true and the row color remains red.
Consequently, a row is red if it ever displayed an invalid item at some point. (Not if it is currently displaying an invalid item.) If you scroll enough, eventually all cells will be red.
Secondly, you must not update any UI that is part of the scene graph from any thread other than the FX Application Thread. Additionally, some other operations, such as creating Window instances (Tooltip is a subclass of Window) must be performed on the FX Application Thread. Note that this includes modifying model properties which are bound to the UI, including properties used in the table columns. You violate this in your validationThread, where you create a Tooltip, set it on the row, and change the pseudoclass state, all in a background thread.
A good approach here is to use the JavaFX concurrency API. Use Tasks which, as far as possible, use only immutable data and return an immutable value. If you do need to update properties which are displayed in the UI, use Platform.runLater(...) to schedule those updates on the FX Application Thread.
In terms of MVC design, it is a good practice for your Model class(es) to store all the data needed by the View. Your design runs into trouble because there is no real place where the validation status is stored. Moreover, the validation status is really more than just "valid" or "invalid"; there is a phase while the thread is running but not completed where the validation status is unknown.
Here's my solution, which addresses these issues. I am assuming:
- Your entity has a notion of validity.
- Establishing the validity of an entity is a long-running process
- Validity depends on one or more properties which may change while the UI is displayed
- The validity should be "lazily" established, on an as-need basis.
- The UI prefers not to display "unknown" validity, and if an entity is displayed whose validity is unknown, it should be established and redisplayed.
I created an enum for ValidationStatus, which has four values:
public enum ValidationStatus {
VALID, INVALID, UNKNOWN, PENDING ;
}
UNKNOWN indicates the validity is not known and validation has not been requested; PENDING indicates that validation has been requested but is not yet complete.
Then I have a wrapper for your entity which adds the validation status as an observable property. If the property on which validation depends in the underlying entity changes, the validation is reset to UNKNOWN.
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleObjectProperty;
public class ValidatingTestEntity {
private final TestEntity entity ;
private final ObjectProperty<ValidationStatus> validationStatus = new SimpleObjectProperty<>(ValidationStatus.UNKNOWN);
public ValidatingTestEntity(TestEntity entity) {
this.entity = entity;
entity.firstNameProperty().addListener((obs, oldName, newName) -> setValidationStatus(ValidationStatus.UNKNOWN));
}
public TestEntity getEntity() {
return entity;
}
public ValidationStatus getValidationStatus() {
return validationStatus.get();
}
public ObjectProperty<ValidationStatus> validationStatusProperty() {
return validationStatus;
}
public void setValidationStatus(ValidationStatus validationStatus) {
this.validationStatus.set(validationStatus);
}
}
The ValidationService provides a service to validate entities on a background thread, updating the appropriate properties with the result. This is managed through a thread pool and with JavaFX Tasks. This just mimics a database call by sleeping for a random amount of time and then returning alternating results.
When the task changes state (i.e. as it progresses though its lifecycle), the validation property of the entity is updated: UNKNOWN if the task fails to complete normally, PENDING if the task is in an incomplete state, and either VALID or INVALID, depending on the result of the task, if the task succeeds.
import javafx.application.Platform;
import javafx.concurrent.Task;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadLocalRandom;
public class ValidationService {
private final Executor exec = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() * 2 + 1,
r -> {
Thread thread = new Thread(r);
thread.setDaemon(true);
return thread;
}
);
public Task<Boolean> validateEntity(ValidatingTestEntity entity) {
// task runs on a background thread and should not access mutable data,
// so make final copies of anything needed here:
final String firstName = entity.getEntity().getFirstName();
final int code =entity.getEntity().getC();
Task<Boolean> task = new Task<Boolean>() {
@Override
protected Boolean call() throws Exception {
try {
Thread.sleep(500 + ThreadLocalRandom.current().nextInt(500));
} catch (InterruptedException exc) {
// if interrupted other than being cancelled, reset thread's interrupt status:
if (! isCancelled()) {
Thread.currentThread().interrupt();
}
}
boolean result = code % 2 == 0;
return result;
}
};
task.stateProperty().addListener((obs, oldState, newState) ->
entity.setValidationStatus(
switch(newState) {
case CANCELLED, FAILED -> ValidationStatus.UNKNOWN;
case READY, RUNNING, SCHEDULED -> ValidationStatus.PENDING ;
case SUCCEEDED ->
task.getValue() ? ValidationStatus.VALID : ValidationStatus.INVALID ;
}
)
);
exec.execute(task);
return task ;
}
}
This is the TableRow implementation. It has a listener which observes the validation status of the current item, if there is one. If the item changes, the listener is removed from the old item (if there is one), and attached to the new item (if there is one). If either the item changes, or the validation state of the current item changes, the row is updated. If the new validation status is UNKNOWN, a request is sent to the service to validate the current item. There are two pseudoclass states: invalid (red) and unknown (orange), which are updated any time the item or its validation status change. The tooltip is set if the item is invalid, and set to null otherwise.
import javafx.beans.value.ChangeListener;
import javafx.css.PseudoClass;
import javafx.scene.control.TableRow;
import javafx.scene.control.Tooltip;
public class ValidatingTableRow extends TableRow<ValidatingTestEntity> {
private final ValidationService validationService ;
private final PseudoClass pending = PseudoClass.getPseudoClass("pending");
private final PseudoClass invalid = PseudoClass.getPseudoClass("invalid");
private final Tooltip tooltip = new Tooltip();
private final ChangeListener<ValidationStatus> listener = (obs, oldStatus, newStatus) -> {
updateValidationStatus();
};
public ValidatingTableRow(ValidationService validationService){
this.validationService = validationService ;
itemProperty().addListener((obs, oldItem, newItem) -> {
setTooltip(null);
if (oldItem != null) {
oldItem.validationStatusProperty().removeListener(listener);
}
if (newItem != null) {
newItem.validationStatusProperty().addListener(listener);
}
updateValidationStatus();
});
}
private void updateValidationStatus() {
if (getItem() == null) {
pseudoClassStateChanged(pending, false);
pseudoClassStateChanged(invalid, false);
setTooltip(null);
return ;
}
ValidationStatus validationStatus = getItem().getValidationStatus();
if( validationStatus == ValidationStatus.UNKNOWN) {
validationService.validateEntity(getItem());
}
if (validationStatus == ValidationStatus.INVALID) {
tooltip.setText("Invalid entity: "+getItem().getEntity().getFirstName() + " " +getItem().getEntity().getC());
setTooltip(tooltip);
} else {
setTooltip(null);
}
pseudoClassStateChanged(pending, validationStatus == ValidationStatus.PENDING);
pseudoClassStateChanged(invalid, validationStatus == ValidationStatus.INVALID);
}
}
Here's the Entity, which is the same as in the question:
import javafx.beans.property.IntegerProperty;
import javafx.beans.property.SimpleIntegerProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
public class TestEntity
{
public TestEntity(String firstName, String lastName, int c)
{
setFirstName(firstName);
setLastName(lastName);
setC(c);
}
private StringProperty firstName = new SimpleStringProperty();
private StringProperty lastName = new SimpleStringProperty();
private IntegerProperty c = new SimpleIntegerProperty();
public String getFirstName() {
return firstName.get();
}
public StringProperty firstNameProperty() {
return firstName;
}
public void setFirstName(String firstName) {
this.firstName.set(firstName);
}
public String getLastName() {
return lastName.get();
}
public StringProperty lastNameProperty() {
return lastName;
}
public void setLastName(String lastName) {
this.lastName.set(lastName);
}
public int getC() {
return c.get();
}
public IntegerProperty cProperty() {
return c;
}
public void setC(int c) {
this.c.set(c);
}
}
And here's the application class. I added the ability to edit the first name, which lets you see an item reverting to unknown and then re-establishing its validity (you need to change selection quickly after committing the edit).
import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;
import javafx.scene.control.cell.TextFieldTableCell;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
public class TableViewProblemMain extends Application
{
public static void main(String[] args)
{
launch(args);
}
@Override
public void start(Stage stage)
{
TableView<ValidatingTestEntity> tableView = new TableView();
tableView.setEditable(true);
TableColumn<ValidatingTestEntity, String> column1 = new TableColumn<>("First Name");
column1.setCellValueFactory(cellData -> cellData.getValue().getEntity().firstNameProperty());
column1.setEditable(true);
column1.setCellFactory(TextFieldTableCell.forTableColumn());
TableColumn<ValidatingTestEntity, String> column2 = new TableColumn<>("Last Name");
column2.setCellValueFactory(cellData -> cellData.getValue().getEntity().lastNameProperty());
TableColumn<ValidatingTestEntity, Number> column3 = new TableColumn<>("C");
column3.setCellValueFactory(cellData -> cellData.getValue().getEntity().cProperty());
tableView.getColumns().addAll(column1, column2, column3);
ValidationService service = new ValidationService();
tableView.setRowFactory(tv -> new ValidatingTableRow(service));
for (int i = 0; i < 300; i++)
{
tableView.getItems().add(new ValidatingTestEntity(
new TestEntity("Fname" + i, "Lname" + i, i)));
}
VBox vbox = new VBox(tableView);
Scene scene = new Scene(vbox);
scene.getStylesheets().add(this.getClass().getResource("/style.css").toExternalForm());
stage.setScene(scene);
stage.show();
}
}
Finally, for completeness, the stylesheet:
.table-row-cell:invalid {
-fx-background-color: rgba(240, 116, 116, 0.18);
}
.table-row-cell:pending {
-fx-background-color: rgba(240, 120, 0, 0.18);
}
Answered By - James_D
Answer Checked By - David Goodson (JavaFixing Volunteer)
