Merge pull request #2441 from ControlSystemStudio/CSSTUDIO-1772

Updated log property provider interface

Author: shroffk

app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/LogPropertiesEditorController.java

@@ -4,7 +4,6 @@
 import javafx.beans.binding.Bindings;
 import javafx.beans.binding.BooleanBinding;
 import javafx.beans.property.SimpleStringProperty;
-import javafx.beans.value.ChangeListener;
 import javafx.beans.value.ObservableValue;
 import javafx.collections.FXCollections;
 import javafx.collections.ListChangeListener;
@@ -31,6 +30,7 @@
 import javafx.util.converter.DefaultStringConverter;
 import org.phoebus.framework.jobs.JobManager;
 import org.phoebus.logbook.LogClient;
+import org.phoebus.logbook.LogPropertiesProvider;
 import org.phoebus.logbook.LogService;
 import org.phoebus.logbook.LogbookPreferences;
 import org.phoebus.logbook.Property;
@@ -51,7 +51,7 @@ public class LogPropertiesEditorController {
 
     /**
      * List of properties user may add to log entry. It is constructed from the properties as
-     * provided by the log service, plus optional properties providers, see {@link LogPropertyProvider}.
+     * provided by the log service, plus optional properties providers, see {@link LogPropertiesProvider}.
      */
     private final ObservableList<Property> availableProperties = FXCollections.observableArrayList();
     /**
@@ -114,18 +114,15 @@ public void initialize() {
                 new PropertyValueTreeTableCell(new DefaultStringConverter()));
 
         // Hide the headers
-        selectedPropertiesTree.widthProperty().addListener(new ChangeListener<Number>() {
-            @Override
-            public void changed(ObservableValue<? extends Number> ov, Number t, Number t1) {
-                // Get the table header
-                Pane header = (Pane) selectedPropertiesTree.lookup("TableHeaderRow");
-                if (header != null && header.isVisible()) {
-                    header.setMaxHeight(0);
-                    header.setMinHeight(0);
-                    header.setPrefHeight(0);
-                    header.setVisible(false);
-                    header.setManaged(false);
-                }
+        selectedPropertiesTree.widthProperty().addListener((ov, t, t1) -> {
+            // Get the table header
+            Pane header = (Pane) selectedPropertiesTree.lookup("TableHeaderRow");
+            if (header != null && header.isVisible()) {
+                header.setMaxHeight(0);
+                header.setMinHeight(0);
+                header.setPrefHeight(0);
+                header.setVisible(false);
+                header.setManaged(false);
             }
         });
 
@@ -150,9 +147,9 @@ private void constructTree(Collection<Property> properties) {
                         PropertyTreeNode node = new PropertyTreeNode(property.getName(), " ");
                         rowCount.set(rowCount.get() + 1);
                         TreeItem<PropertyTreeNode> treeItem = new TreeItem<>(node);
-                        property.getAttributes().entrySet().stream().forEach(entry -> {
+                        property.getAttributes().forEach((key, value1) -> {
                             rowCount.set(rowCount.get() + 1);
-                            treeItem.getChildren().add(new TreeItem<>(new PropertyTreeNode(entry.getKey(), entry.getValue())));
+                            treeItem.getChildren().add(new TreeItem<>(new PropertyTreeNode(key, value1)));
                         });
                         treeItem.setExpanded(true);
                         return treeItem;
@@ -196,7 +193,7 @@ private void removeSelectedProperty(String propertyName) {
         }
     }
 
-    private class PropertyTreeNode {
+    private static class PropertyTreeNode {
         private SimpleStringProperty name;
         private SimpleStringProperty value;
 
@@ -237,7 +234,7 @@ public void setValue(String value) {
     }
 
     /**
-     * Refreshes the list of available properties. Properties provided by {@link LogPropertyProvider} implementations
+     * Refreshes the list of available properties. Properties provided by {@link LogPropertiesProvider} implementations
      * are considered first, and then properties available from service. However, adding items to the list of properties
      * always consider equality, i.e. properties with same name are added only once. SPI implementations should therefore
      * not support properties with same name, and should not implement properties available from service.
@@ -256,24 +253,26 @@ private void setupProperties() {
         JobManager.schedule("Fetch Properties from service", monitor ->
         {
             // First add properties from SPI implementations
-            List<LogPropertyProvider> factories = new ArrayList<>();
-            ServiceLoader<LogPropertyProvider> loader = ServiceLoader.load(LogPropertyProvider.class);
+            List<LogPropertiesProvider> factories = new ArrayList<>();
+            ServiceLoader<LogPropertiesProvider> loader = ServiceLoader.load(LogPropertiesProvider.class);
             loader.stream().forEach(p -> {
-                if (p.get().getProperty() != null) {
+                if (p.get().getProperties() != null && !p.get().getProperties().isEmpty()) {
                     factories.add(p.get());
                 }
             });
             // List of property names added if user is replying to a log entry.
             List<String> selectedPropertyNames =
                     selectedProperties.stream().map(Property::getName).collect(Collectors.toList());
             factories.stream()
-                    .map(LogPropertyProvider::getProperty)
-                    .forEach(property -> {
-                        // Do not add a property that is already selected
-                        if (!selectedPropertyNames.contains(property.getName())) {
-                            selectedProperties.add(property);
-                            selectedPropertyNames.add(property.getName());
-                        }
+                    .map(LogPropertiesProvider::getProperties)
+                    .forEach(propertiesList -> {
+                        // Do not add a property that is already present
+                        propertiesList.forEach(property -> {
+                            if (!selectedPropertyNames.contains(property.getName())) {
+                                selectedProperties.add(property);
+                                selectedPropertyNames.add(property.getName());
+                            }
+                        });
                     });
 
             LogClient logClient =
@@ -283,7 +282,7 @@ private void setupProperties() {
             Platform.runLater(() ->
             {
                 propertyList.forEach(property -> {
-                    // Do not add a property that is already selected or already added from provider
+                    // Do not add already selected property, or added from provider.
                     if (!selectedPropertyNames.contains(property.getName())) {
                         list.add(property);
                     }
@@ -323,8 +322,8 @@ protected void updateItem(String item, boolean empty) {
                 // Binding to determine if the selected cell is a leaf or not.
                 // Property name cells are not leaves.
                 BooleanBinding binding = Bindings.createBooleanBinding(() ->
-                        getTreeTableRow().getTreeItem() != null &&
-                                !getTreeTableRow().getTreeItem().leafProperty().get());
+                        getTableRow().getTreeItem() != null &&
+                                !getTableRow().getTreeItem().leafProperty().get());
                 // Action must specify the property name subject for removal
                 menuItem.setOnAction(e -> removeSelectedProperty(item));
                 contextMenuProperty().bind(Bindings
@@ -355,7 +354,7 @@ private class PropertyValueTreeTableCell extends TextFieldTreeTableCell<Property
         @Override
         public void commitEdit(String value) {
             super.commitEdit(value);
-            TreeTableRow row = getTreeTableRow();
+            TreeTableRow row = getTableRow();
             PropertyTreeNode parent = (PropertyTreeNode) row.getTreeItem().getParent().getValue();
             String propertyName = parent.getName();
             String attributeName = ((PropertyTreeNode) row.getTreeItem().getValue()).getName();

app/logbook/olog/ui/src/main/java/org/phoebus/logbook/olog/ui/write/LogPropertyProvider.java

@@ -0,0 +1,35 @@
+package org.phoebus.logbook;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * An interface to subscribe property providers. A custom logbook module can use this
+ * to - for instance - add a list of {@link Property} objects calculated dynamically when a new
+ * log entry is created.
+ */
+public interface LogPropertiesProvider {
+
+    /**
+     * Clients should consider that implementations may take some time to complete, e.g. query a remote
+     * endpoint for data.
+     * @param logEntry The {@link LogEntry} object as created by the client. Implementations can use this to apply filtering
+     *            based on the fields of the log entry.
+     * @return A {@link Property} added to a new log entry. An implementation should return <code>null</code>
+     * if it applies a filter that rules out the log entry.
+     */
+    @SuppressWarnings("unused")
+    default List<Property> getProperties(LogEntry logEntry){
+        return Collections.emptyList();
+    }
+
+    /**
+     * Clients should consider that implementations may take some time to complete, e.g. query a remote
+     * endpoint for data.
+     * @return A {@link Property} added to a new log entry. An implementation should return <code>null</code>
+     * if it applies a filter that rules out the log entry.
+     */
+    default List<Property> getProperties(){
+        return Collections.emptyList();
+    }
+}