Code cleanup, javadoc and doc

Author: georgweiss

app/save-and-restore/app/doc/index.rst

@@ -58,9 +58,13 @@ A word of caution
 -----------------
 
 Save-and-restore data is persisted in a central service and is therefore accessible by multiple
-clients. Users should keep in mind that changes (e.g. new or deleted nodes) are not pushed to all clients.
-Caution is therefore advocated when working on the nodes in the tree, in particular when changing the structure by
-copying, deleting or moving nodes.
+clients. Users should keep in mind that changes (e.g. new or deleted nodes) are pushed by the service to
+all connected clients. If any other user is working in the save-and-restore app, saved changes may update
+the current view. For instance, if a folder node is expanded and another user adds an object (folder
+or configuration) to that folder, the new object will automatically be added to the expanded folder.
+
+In other words, changes in the current view are triggered not only by the current user, but may be triggered as a result of
+changes done by others.
 
 Tree View Context Menu
 ----------------------

app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java

@@ -977,8 +977,7 @@ private void exportToCSV() {
     protected void addTagToSnapshots() {
         ObservableList<TreeItem<Node>> selectedItems = browserSelectionModel.getSelectedItems();
         List<Node> selectedNodes = selectedItems.stream().map(TreeItem::getValue).collect(Collectors.toList());
-        List<Node> updatedNodes = TagUtil.addTag(selectedNodes);
-        //updatedNodes.forEach(this::nodeChanged);
+        TagUtil.addTag(selectedNodes);
     }
 
     /**

app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/websocket/MessageType.java

@@ -4,10 +4,13 @@
 
 package org.phoebus.applications.saveandrestore.model.websocket;
 
+/**
+ * Enum to indicate what type of web socket message the service is sending to clients.
+ */
 public enum MessageType {
     NODE_ADDED,
     NODE_UPDATED,
     NODE_REMOVED,
     FILTER_ADDED_OR_UPDATED,
-    FILTER_REMOVED;
+    FILTER_REMOVED
 }

app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/websocket/SaveAndRestoreWebSocketMessage.java

@@ -4,6 +4,10 @@
 
 package org.phoebus.applications.saveandrestore.model.websocket;
 
-
+/**
+ * Record encapsulating a {@link MessageType} and a payload.
+ * @param messageType The {@link MessageType} of a web socket message
+ * @param payload The payload, e.g. {@link String} or {@link org.phoebus.applications.saveandrestore.model.Node}
+ */
 public record SaveAndRestoreWebSocketMessage<T>(MessageType messageType, T payload) {
 }

app/save-and-restore/model/src/main/java/org/phoebus/applications/saveandrestore/model/websocket/WebMessageDeserializer.java

@@ -10,38 +10,43 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
-import org.phoebus.applications.saveandrestore.model.Node;
-import org.phoebus.applications.saveandrestore.model.search.Filter;
 
+/**
+ * Custom JSON deserializer of {@link SaveAndRestoreWebSocketMessage}s.
+ */
 public class WebMessageDeserializer extends StdDeserializer<SaveAndRestoreWebSocketMessage> {
 
-    private ObjectMapper objectMapper = new ObjectMapper();
+    private final ObjectMapper objectMapper = new ObjectMapper();
 
     public WebMessageDeserializer(Class<?> clazz) {
         super(clazz);
     }
 
+    /**
+     * Deserializes a {@link SaveAndRestoreWebSocketMessage}-
+     *
+     * @param jsonParser Parsed used for reading JSON content
+     * @param context    Context that can be used to access information about
+     *                   this deserialization activity.
+     * @return A {@link SaveAndRestoreWebSocketMessage} object, or <code>null</code> if deserialization fails.
+     */
     @Override
     public SaveAndRestoreWebSocketMessage<? extends Object> deserialize(JsonParser jsonParser,
                                                                         DeserializationContext context) {
         try {
             JsonNode rootNode = jsonParser.getCodec().readTree(jsonParser);
             String messageType = rootNode.get("messageType").asText();
             switch (MessageType.valueOf(messageType)) {
-                case NODE_ADDED, NODE_REMOVED, FILTER_REMOVED-> {
-                    SaveAndRestoreWebSocketMessage<String> saveAndRestoreWebSocketMessage =
-                            objectMapper.readValue(rootNode.toString(), SaveAndRestoreWebSocketMessage.class);
-                    return saveAndRestoreWebSocketMessage;
+                case NODE_ADDED, NODE_REMOVED, FILTER_REMOVED -> {
+                    return objectMapper.readValue(rootNode.toString(), SaveAndRestoreWebSocketMessage.class);
                 }
                 case NODE_UPDATED -> {
-                    SaveAndRestoreWebSocketMessage<Node> saveAndRestoreWebSocketMessage = objectMapper.readValue(rootNode.toString(), new TypeReference<>() {
+                    return objectMapper.readValue(rootNode.toString(), new TypeReference<>() {
                     });
-                    return saveAndRestoreWebSocketMessage;
                 }
                 case FILTER_ADDED_OR_UPDATED -> {
-                    SaveAndRestoreWebSocketMessage<Filter> saveAndRestoreWebSocketMessage = objectMapper.readValue(rootNode.toString(), new TypeReference<>() {
+                    return objectMapper.readValue(rootNode.toString(), new TypeReference<>() {
                     });
-                    return saveAndRestoreWebSocketMessage;
                 }
             }
         } catch (Exception e) {

core/websocket/src/main/java/org/phoebus/core/websocket/WebSocketClient.java

@@ -29,18 +29,26 @@ public class WebSocketClient implements WebSocket.Listener {
 
     /**
      * @param uri The URI of the web socket peer.
+     * @param onTextCallback A callback method the API client will use to process web socket messages.
      */
     public WebSocketClient(URI uri, Consumer<CharSequence> onTextCallback) {
         this.uri = uri;
         this.onTextCallback = onTextCallback;
     }
 
+    /**
+     * Attempts to connect to the remote web socket.
+     */
     public void connect() {
         attemptConnect.set(true);
-        reconnect();
+        doConnect();
     }
 
-    private void reconnect() {
+    /**
+     * Internal connect implementation. This is done in a loop with 10 s intervals until
+     * connection is established.
+     */
+    private void doConnect() {
         new Thread(() -> {
             while (attemptConnect.get()) {
                 logger.log(Level.INFO, "Attempting web socket connection to " + uri);
@@ -63,6 +71,12 @@ private void reconnect() {
         }).start();
     }
 
+    /**
+     * Called when connection has been established. An API client may optionally register a
+     * {@link #connectCallback} which is called when connection is opened.
+     * @param webSocket
+     *         the WebSocket that has been connected
+     */
     @Override
     public void onOpen(WebSocket webSocket) {
         WebSocket.Listener.super.onOpen(webSocket);
@@ -87,6 +101,16 @@ public void sendText(String message) {
     }
 
 
+    /**
+     * Called when connection has been closed, e.g. by remote peer. An API client may optionally register a
+     * {@link #disconnectCallback} which is called when connection is opened.
+     *
+     * <p>
+     *     Note that reconnection will be attempted immediately.
+     * </p>
+     * @param webSocket
+     *         the WebSocket that has been connected
+     */
     @Override
     public CompletionStage<?> onClose(WebSocket webSocket,
                                       int statusCode,
@@ -95,7 +119,7 @@ public CompletionStage<?> onClose(WebSocket webSocket,
         if (disconnectCallback != null) {
             disconnectCallback.run();
         }
-        reconnect();
+        doConnect();
         return null;
     }
 
@@ -131,15 +155,34 @@ public CompletionStage<?> onText(WebSocket webSocket,
         return WebSocket.Listener.super.onText(webSocket, data, last);
     }
 
+    /**
+     *
+     * <b>NOTE:</b> this <b>must</b> be called by the API client when web socket messages are no longer
+     * needed, otherwise reconnect attempts will continue as these run on a separate thread.
+     *
+     * <p>
+     *     The status code 1000 is used when calling the {@link WebSocket#sendClose(int, String)} method. See
+     *     list of common web socket status codes
+     *     <a href='https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent/code'>here</a>.
+     * </p>
+     * @param reason Custom reason text.
+     */
     public void close(String reason) {
         attemptConnect.set(false);
         webSocket.sendClose(1000, reason);
     }
 
+    /**
+     * @param connectCallback A {@link Runnable} invoked when web socket connects successfully.
+     */
     public void setConnectCallback(Runnable connectCallback) {
         this.connectCallback = connectCallback;
     }
 
+    /**
+     * @param disconnectCallback A {@link Runnable} invoked when web socket disconnects, either
+     *                           when closed explicitly, or if remote peer goes away.
+     */
     public void setDisconnectCallback(Runnable disconnectCallback) {
         this.disconnectCallback = disconnectCallback;
     }

services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/websocket/WebSocket.java

@@ -8,35 +8,20 @@
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.node.JsonNodeType;
-import org.epics.vtype.VType;
 import org.springframework.web.socket.TextMessage;
 import org.springframework.web.socket.WebSocketSession;
 
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+/**
+ * Utility class for handling web socket messages. In the context of the save-and-restore service,
+ * only messages from server are expected. Client messages are logged, but do not invoke any behavior.
+ */
 @SuppressWarnings("nls")
 public class WebSocket {
-    /**
-     * Time when web socket was created
-     */
-    private final long created = System.currentTimeMillis();
-
-    /**
-     * Track when the last message was received by web client
-     */
-    private volatile long lastClientMessage = 0;
-
-    /**
-     * Track when the last message was sent to web client
-     */
-    private volatile long lastMessageSent = 0;
 
     /**
      * Is the queue full?
@@ -54,9 +39,8 @@ public class WebSocket {
 
     private static final String EXIT_MESSAGE = "EXIT";
 
-    private volatile WebSocketSession session;
-    private volatile String id;
-
+    private final WebSocketSession session;
+    private final String id;
 
     private final Logger logger = Logger.getLogger(WebSocket.class.getName());
 
@@ -74,7 +58,6 @@ public WebSocket(ObjectMapper objectMapper, WebSocketSession webSocketSession) {
         writeThread.setName("Web Socket Write Thread " + this.id);
         writeThread.setDaemon(true);
         writeThread.start();
-        trackClientUpdate();
     }
 
     /**
@@ -87,34 +70,6 @@ public String getId() {
             return id;
     }
 
-    /**
-     * @return Timestamp (ms since epoch) when socket was created
-     */
-    public long getCreateTime() {
-        return created;
-    }
-
-    /**
-     * @return Timestamp (ms since epoch) of last client message
-     */
-    public long getLastClientMessage() {
-        return lastClientMessage;
-    }
-
-    /**
-     * @return Timestamp (ms since epoch) of last message sent to client
-     */
-    public long getLastMessageSent() {
-        return lastMessageSent;
-    }
-
-    /**
-     * @return Number of queued messages
-     */
-    public int getQueuedMessageCount() {
-        return writeQueue.size();
-    }
-
     /**
      * @param message Potentially long message
      * @return Message shorted to 200 chars
@@ -162,7 +117,6 @@ private void writeQueuedMessages() {
                     if (!safeSession.isOpen())
                         throw new Exception("Session closed");
                     safeSession.sendMessage(new TextMessage(message));
-                    lastMessageSent = System.currentTimeMillis();
                 } catch (final Exception ex) {
                     logger.log(Level.WARNING, ex, () -> "Cannot write '" + shorten(message) + "' for " + id);
 
@@ -182,10 +136,6 @@ private void writeQueuedMessages() {
         }
     }
 
-    public void trackClientUpdate() {
-        lastClientMessage = System.currentTimeMillis();
-    }
-
     /**
      * Called when client sends a general message
      *
@@ -197,12 +147,7 @@ public void handleTextMessage(TextMessage message) throws Exception {
         if (node.isMissingNode())
             throw new Exception("Missing 'type' in " + shorten(message.getPayload()));
         final String type = node.asText();
-
-        switch (type) {
-            case "monitor":
-            default:
-                throw new Exception("Unknown message type: " + shorten(message.getPayload()));
-        }
+        logger.log(Level.INFO, "Client message type: " + type);
     }
 
     /**
@@ -224,24 +169,5 @@ public void dispose() {
             logger.log(Level.WARNING, "Error disposing " + getId(), ex);
         }
         logger.log(Level.INFO, () -> "Web socket " + session.getId() + " closed");
-        lastClientMessage = 0;
-    }
-
-
-    private void write(String message, JsonNode json) throws Exception {
-        JsonNode n = json.path("pv");
-        if (n.isMissingNode())
-            throw new Exception("Missing 'pv' in " + shorten(message));
-        final String pv_name = n.asText();
-
-        n = json.path("value");
-        if (n.isMissingNode())
-            throw new Exception("Missing 'value' in " + shorten(message));
-        final Object value;
-        if (n.getNodeType() == JsonNodeType.NUMBER)
-            value = n.asDouble();
-        else
-            value = n.asText();
-
     }
 }

services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/websocket/WebSocketHandler.java

@@ -145,8 +145,6 @@ protected void handlePongMessage(@NonNull WebSocketSession session, @NonNull Pon
         if (webSocketOptional.isEmpty()) {
             return; // Should only happen in case of timing issues?
         }
-        webSocketOptional.get().trackClientUpdate();
-
     }
 
     /**