From efec05fd768e96290838dbb91daf11346270a249 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 15 Jan 2019 10:27:43 +0100 Subject: [PATCH] Move auxclasspath in view menu, record new exceptions --- .../fxdesigner/MainDesignerController.java | 4 +- .../util/fxdesigner/model/EventLogger.java | 71 +++++++- .../pmd/util/fxdesigner/model/LogEntry.java | 18 +- .../fxdesigner/popups/EventLogController.java | 159 ++++++++++-------- .../util/fxdesigner/util/DesignerUtil.java | 45 +++++ .../fxdesigner/util/SoftReferenceCache.java | 4 + .../pmd/util/fxdesigner/fxml/designer.fxml | 31 ++-- .../pmd/util/fxdesigner/less/designer.less | 4 + 8 files changed, 246 insertions(+), 90 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java index ca630219d8..af0631b188 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java @@ -138,8 +138,8 @@ public class MainDesignerController extends AbstractController { openEventLogMenuItem.setOnAction(e -> eventLogController.showPopup()); openEventLogMenuItem.textProperty().bind( - eventLogController.numLogEntriesProperty() - .map(i -> "Open event log (" + i + " entries)")); + designerRoot.getLogger().numNewLogEntriesProperty().map(i -> "Exception log (" + (i > 0 ? i : "no") + " new)") + ); } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/EventLogger.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/EventLogger.java index e0e34f4b5d..0179eff03e 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/EventLogger.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/EventLogger.java @@ -4,30 +4,87 @@ package net.sourceforge.pmd.util.fxdesigner.model; -import java.util.Objects; +import static net.sourceforge.pmd.util.fxdesigner.model.LogEntry.Category.PARSE_EXCEPTION; +import static net.sourceforge.pmd.util.fxdesigner.model.LogEntry.Category.PARSE_OK; +import static net.sourceforge.pmd.util.fxdesigner.model.LogEntry.Category.XPATH_EVALUATION_EXCEPTION; +import static net.sourceforge.pmd.util.fxdesigner.model.LogEntry.Category.XPATH_OK; + +import java.time.Duration; +import java.util.EnumSet; import org.reactfx.EventSource; import org.reactfx.EventStream; +import org.reactfx.EventStreams; +import org.reactfx.collection.LiveArrayList; +import org.reactfx.collection.LiveList; +import org.reactfx.value.Val; + +import net.sourceforge.pmd.util.fxdesigner.model.LogEntry.Category; +import net.sourceforge.pmd.util.fxdesigner.util.DesignerUtil; + /** - * Logs events. + * Logs events. Stores the whole log in case no view was open. * * @author Clément Fournier * @since 6.0.0 */ public class EventLogger { + /** + * Exceptions from XPath evaluation or parsing are never emitted + * within less than that time interval to keep them from flooding the tableview. + */ + private static final Duration PARSE_EXCEPTION_DELAY = Duration.ofMillis(3000); private final EventSource latestEvent = new EventSource<>(); + private final LiveList fullLog = new LiveArrayList<>(); + + + public EventLogger() { + + EventStream onlyParseException = + latestEvent.filter(x -> x.getCategory() == PARSE_EXCEPTION || x.getCategory() == PARSE_OK) + .successionEnds(PARSE_EXCEPTION_DELAY) + // don't output anything when the last state recorded was OK + .filter(x -> x.getCategory() != PARSE_OK); + + EventStream onlyXPathException = + latestEvent.filter(x -> x.getCategory() == XPATH_EVALUATION_EXCEPTION || x.getCategory() == XPATH_OK) + .successionEnds(PARSE_EXCEPTION_DELAY) + // don't output anything when the last state recorded was OK + .filter(x -> x.getCategory() != XPATH_OK); + + EnumSet otherExceptionSet = EnumSet.complementOf(EnumSet.of(PARSE_EXCEPTION, XPATH_EVALUATION_EXCEPTION, PARSE_OK, XPATH_OK)); + + EventStream otherExceptions = latestEvent.filter(x -> otherExceptionSet.contains(x.getCategory())); + + EventStreams.merge(onlyParseException, otherExceptions, onlyXPathException) + .subscribe(fullLog::add); + } + + + /** Number of log entries that were not yet examined by the user. */ + public Val numNewLogEntriesProperty() { + return DesignerUtil.countNotMatching(fullLog.map(LogEntry::wasExaminedProperty)); + } + + + /** Total number of log entries. */ + public Val numLogEntriesProperty() { + return fullLog.sizeProperty(); + } + public void logEvent(LogEntry event) { - latestEvent.push(event); + if (event != null) { + latestEvent.push(event); + } } /** - * Returns a stream that emits an event each time an exception is logged by some - * part of the application. + * Returns the full log. */ - public EventStream getLog() { - return latestEvent.filter(Objects::nonNull); + public LiveList getLog() { + return fullLog; } } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/LogEntry.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/LogEntry.java index a70deda233..1b0ff23eda 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/LogEntry.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/LogEntry.java @@ -7,6 +7,8 @@ package net.sourceforge.pmd.util.fxdesigner.model; import java.util.Date; import org.apache.commons.lang3.exception.ExceptionUtils; +import org.reactfx.value.Var; + /** * Log entry of an {@link EventLogger}. @@ -20,7 +22,7 @@ public class LogEntry { private final Throwable throwable; private final Category category; private final Date timestamp; - + private Var wasExamined = Var.newSimpleVar(false); public LogEntry(Throwable thrown, Category cat) { this.throwable = thrown; @@ -29,6 +31,20 @@ public class LogEntry { } + public boolean isWasExamined() { + return wasExamined.getValue(); + } + + + public void setExamined(boolean wasExamined) { + this.wasExamined.setValue(wasExamined); + } + + public Var wasExaminedProperty() { + return wasExamined; + } + + public Throwable getThrown() { return throwable; } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/popups/EventLogController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/popups/EventLogController.java index ba42c3b6ad..a26ab38825 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/popups/EventLogController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/popups/EventLogController.java @@ -4,24 +4,16 @@ package net.sourceforge.pmd.util.fxdesigner.popups; -import static net.sourceforge.pmd.util.fxdesigner.model.LogEntry.Category.PARSE_EXCEPTION; -import static net.sourceforge.pmd.util.fxdesigner.model.LogEntry.Category.PARSE_OK; -import static net.sourceforge.pmd.util.fxdesigner.model.LogEntry.Category.XPATH_EVALUATION_EXCEPTION; -import static net.sourceforge.pmd.util.fxdesigner.model.LogEntry.Category.XPATH_OK; - import java.io.IOException; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.time.Duration; import java.util.Collections; -import java.util.Date; -import java.util.EnumSet; import java.util.List; -import java.util.Objects; -import org.reactfx.EventStream; +import org.kordamp.ikonli.javafx.FontIcon; import org.reactfx.EventStreams; -import org.reactfx.value.Val; +import org.reactfx.Subscription; import org.reactfx.value.Var; import net.sourceforge.pmd.lang.ast.Node; @@ -34,6 +26,9 @@ import net.sourceforge.pmd.util.fxdesigner.util.DesignerUtil; import net.sourceforge.pmd.util.fxdesigner.util.SoftReferenceCache; import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.value.ChangeListener; +import javafx.collections.FXCollections; +import javafx.css.PseudoClass; import javafx.fxml.FXML; import javafx.fxml.FXMLLoader; import javafx.scene.Parent; @@ -41,6 +36,7 @@ import javafx.scene.Scene; import javafx.scene.control.TableCell; import javafx.scene.control.TableColumn; import javafx.scene.control.TableColumn.SortType; +import javafx.scene.control.TableRow; import javafx.scene.control.TableView; import javafx.scene.control.TextArea; import javafx.scene.control.cell.PropertyValueFactory; @@ -49,6 +45,8 @@ import javafx.stage.Stage; /** + * A presenter over the {@link net.sourceforge.pmd.util.fxdesigner.model.EventLogger}. + * * @author Clément Fournier * @since 6.0.0 */ @@ -59,6 +57,7 @@ public class EventLogController extends AbstractController { * within less than that time interval to keep them from flooding the tableview. */ private static final Duration PARSE_EXCEPTION_DELAY = Duration.ofMillis(3000); + private static final PseudoClass NEW_ENTRY = PseudoClass.getPseudoClass("new-entry"); private final DesignerRoot designerRoot; private final MainDesignerController mediator; @@ -66,7 +65,7 @@ public class EventLogController extends AbstractController { @FXML private TableView eventLogTableView; @FXML - private TableColumn logDateColumn; + private TableColumn logDateColumn; @FXML private TableColumn logCategoryColumn; @FXML @@ -79,7 +78,9 @@ public class EventLogController extends AbstractController { private SoftReferenceCache popupStageCache = new SoftReferenceCache<>(this::createStage); - private Var numLogEntries = Var.newSimpleVar(0); + // subscription allowing to unbind from the popup. If that's not done, + // the popup fxml is always bound to the controller we have a memory leak + private Subscription popupBinding = () -> {}; public EventLogController(DesignerRoot owner, MainDesignerController mediator) { this.designerRoot = owner; @@ -87,66 +88,41 @@ public class EventLogController extends AbstractController { } + // this is only called each time a popup is created @Override protected void beforeParentInit() { + popupBinding.unsubscribe(); + logCategoryColumn.setCellValueFactory(new PropertyValueFactory<>("category")); logMessageColumn.setCellValueFactory(new PropertyValueFactory<>("message")); final DateFormat dateFormat = new SimpleDateFormat("HH:mm:ss"); - logDateColumn.setCellValueFactory(entry -> new SimpleObjectProperty<>(entry.getValue().getTimestamp())); - logDateColumn.setCellFactory(column -> new TableCell() { + logDateColumn.setCellValueFactory(entry -> new SimpleObjectProperty<>(entry.getValue())); + logDateColumn.setCellFactory(column -> new TableCell() { + + Subscription sub = null; + + @Override - protected void updateItem(Date item, boolean empty) { + protected void updateItem(LogEntry item, boolean empty) { super.updateItem(item, empty); + + if (sub != null) { + sub.unsubscribe(); + } if (item == null || empty) { setText(null); setGraphic(null); } else { - setText(dateFormat.format(item)); + setText(dateFormat.format(item.getTimestamp())); + sub = item.wasExaminedProperty() + .map(wasExamined -> wasExamined ? null : new FontIcon("fas-exclamation-circle")) + .values() + .subscribe(graphicProperty()::setValue); } } }); - EventStream onlyParseException = designerRoot.getLogger().getLog() - .filter(x -> x.getCategory() == PARSE_EXCEPTION || x.getCategory() == PARSE_OK) - .successionEnds(PARSE_EXCEPTION_DELAY) - // don't output anything when the last state recorded was OK - .filter(x -> x.getCategory() != PARSE_OK); - - EventStream onlyXPathException = designerRoot.getLogger().getLog() - .filter(x -> x.getCategory() == XPATH_EVALUATION_EXCEPTION || x.getCategory() == XPATH_OK) - .successionEnds(PARSE_EXCEPTION_DELAY) - // don't output anything when the last state recorded was OK - .filter(x -> x.getCategory() != XPATH_OK); - - EnumSet otherExceptionSet = EnumSet.complementOf(EnumSet.of(PARSE_EXCEPTION, XPATH_EVALUATION_EXCEPTION, PARSE_OK, XPATH_OK)); - - EventStream otherExceptions = designerRoot.getLogger().getLog() - .filter(x -> otherExceptionSet.contains(x.getCategory())); - - EventStreams.merge(onlyParseException, otherExceptions, onlyXPathException) - .hook(e -> numLogEntries.setValue(numLogEntries.getValue() + 1)) - .subscribe(t -> eventLogTableView.getItems().add(t)); - - eventLogTableView.getSelectionModel() - .selectedItemProperty() - .addListener((obs, oldVal, newVal) -> onExceptionSelectionChanges(oldVal, newVal)); - - EventStreams.combine(EventStreams.changesOf(eventLogTableView.focusedProperty()), - EventStreams.changesOf(selectedErrorNodes)); - - EventStreams.valuesOf(eventLogTableView.focusedProperty()) - .successionEnds(Duration.ofMillis(100)) - .subscribe(b -> { - if (b) { - mediator.handleSelectedNodeInError(selectedErrorNodes.getValue()); - } else { - mediator.resetSelectedErrorNodes(); - } - }); - - selectedErrorNodes.values().subscribe(mediator::handleSelectedNodeInError); - eventLogTableView.resizeColumn(logMessageColumn, -1); logMessageColumn.prefWidthProperty() @@ -156,6 +132,55 @@ public class EventLogController extends AbstractController { .subtract(2)); // makes it work logDateColumn.setSortType(SortType.DESCENDING); + eventLogTableView.setRowFactory(tv -> { + TableRow row = new TableRow<>(); + ChangeListener examinedListener = (obs, oldVal, newVal) -> row.pseudoClassStateChanged(NEW_ENTRY, !newVal); + row.itemProperty().addListener((obs, previousEntry, currentEntry) -> { + if (previousEntry != null) { + previousEntry.wasExaminedProperty().removeListener(examinedListener); + } + if (currentEntry != null) { + currentEntry.wasExaminedProperty().addListener(examinedListener); + row.pseudoClassStateChanged(NEW_ENTRY, !currentEntry.isWasExamined()); + } else { + row.pseudoClassStateChanged(NEW_ENTRY, false); + } + }); + return row; + }); + + } + + + private Subscription bindPopupToController() { + Subscription binding = + EventStreams.valuesOf(eventLogTableView.getSelectionModel().selectedItemProperty()) + .distinct() + .subscribe(this::onExceptionSelectionChanges); + + binding = binding.and( + EventStreams.valuesOf(eventLogTableView.focusedProperty()) + .successionEnds(Duration.ofMillis(100)) + .subscribe(b -> { + if (b) { + mediator.handleSelectedNodeInError(selectedErrorNodes.getValue()); + } else { + mediator.resetSelectedErrorNodes(); + } + }) + ); + + binding = binding.and( + selectedErrorNodes.values().subscribe(mediator::handleSelectedNodeInError) + ); + + eventLogTableView.itemsProperty().setValue(designerRoot.getLogger().getLog()); + + binding = binding.and( + () -> eventLogTableView.itemsProperty().setValue(FXCollections.emptyObservableList()) + ); + + return binding; } @@ -164,6 +189,9 @@ public class EventLogController extends AbstractController { selectedErrorNodes.setValue(Collections.emptyList()); return; } + + entry.setExamined(true); + switch (entry.getCategory()) { case OTHER: break; @@ -182,25 +210,22 @@ public class EventLogController extends AbstractController { public void showPopup() { popupStageCache.getValue().show(); + popupBinding = bindPopupToController(); } public void hidePopup() { - popupStageCache.getValue().hide(); - } - - - private void onExceptionSelectionChanges(LogEntry oldVal, LogEntry newVal) { - logDetailsTextArea.setText(newVal == null ? "" : newVal.getStackTrace()); - - if (!Objects.equals(newVal, oldVal)) { - handleSelectedEntry(newVal); + if (popupStageCache.hasValue()) { + popupStageCache.getValue().hide(); + popupBinding.unsubscribe(); + popupBinding = () -> {}; } } - public Val numLogEntriesProperty() { - return numLogEntries; + private void onExceptionSelectionChanges(LogEntry newVal) { + logDetailsTextArea.setText(newVal == null ? "" : newVal.getStackTrace()); + handleSelectedEntry(newVal); } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerUtil.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerUtil.java index 6f25015009..5bcfb5cf15 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerUtil.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerUtil.java @@ -15,14 +15,21 @@ import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Optional; +import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; import org.apache.commons.lang3.exception.ExceptionUtils; +import org.reactfx.EventStream; +import org.reactfx.EventStreams; +import org.reactfx.collection.LiveList; +import org.reactfx.value.Val; import org.reactfx.value.Var; import net.sourceforge.pmd.lang.Language; @@ -34,6 +41,7 @@ import net.sourceforge.pmd.lang.rule.xpath.XPathRuleQuery; import javafx.beans.property.BooleanProperty; import javafx.beans.property.Property; import javafx.beans.value.ObservableValue; +import javafx.collections.ObservableList; import javafx.scene.control.ListCell; import javafx.scene.control.ListView; import javafx.scene.control.ToggleGroup; @@ -263,4 +271,41 @@ public final class DesignerUtil { public static Var booleanVar(BooleanProperty p) { return Var.mapBidirectional(p, Boolean::booleanValue, Function.identity()); } + + + // Creating a real function Val> => LiveList or LiveList> => LiveList would + // allow implementing LiveList.flatMap, which is a long-standing feature request in ReactFX + // These utilities are very inefficient, but sufficient for our use case... + public static Val> flatMapChanges(ObservableList> listOfObservables) { + + // every time an element changes an invalidation stream + EventStream invalidations = + LiveList.map(listOfObservables, EventStreams::valuesOf) + .reduce(EventStreams::merge) + .values() + .filter(Objects::nonNull) + .flatMap(Function.identity()); + + return Val.create(() -> LiveList.map(listOfObservables, ObservableValue::getValue), invalidations); + } + + + public static Val reduceWElts(ObservableList> list, U zero, BiFunction mapper) { + return flatMapChanges(list).map(l -> l.stream().reduce(zero, mapper, (u, v) -> v)); + } + + + public static Val countMatching(ObservableList> list, Predicate predicate) { + return reduceWElts(list, 0, (cur, t) -> predicate.test(t) ? cur + 1 : cur); + } + + + public static Val countMatching(ObservableList> list) { + return countMatching(list, b -> b); + } + + + public static Val countNotMatching(ObservableList> list) { + return countMatching(list, b -> !b); + } } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/SoftReferenceCache.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/SoftReferenceCache.java index 4508e2ca9c..747c6082be 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/SoftReferenceCache.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/SoftReferenceCache.java @@ -23,6 +23,10 @@ public class SoftReferenceCache { } + public boolean hasValue() { + return myRef != null && myRef.get() != null; + } + public T getValue() { if (myRef == null || myRef.get() == null) { T val = mySupplier.get(); diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/designer.fxml b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/designer.fxml index a1b5bf0982..bc7dad9cbb 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/designer.fxml +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/designer.fxml @@ -19,6 +19,7 @@ + @@ -27,7 +28,7 @@ - + @@ -35,26 +36,30 @@ - + - + + + + + + + + + + + - - - - - - - - + - + diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less index d7835c2ebf..fbe67042b3 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less @@ -61,6 +61,10 @@ } } + .table-row-cell:new-entry { + -fx-background-color: lightyellow; + } + .show-hide-columns-button { -fx-background-color: @darker-accent; -fx-border-color: @darker-accent-border;