Disallow comparison data if read-back PV name is not set

Author: georgweiss

services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationController.java

@@ -17,6 +17,7 @@
  */
 package org.phoebus.service.saveandrestore.web.controllers;
 
+import org.phoebus.applications.saveandrestore.model.ConfigPv;
 import org.phoebus.applications.saveandrestore.model.Configuration;
 import org.phoebus.applications.saveandrestore.model.ConfigurationData;
 import org.phoebus.applications.saveandrestore.model.Node;
@@ -58,6 +59,12 @@ public class ConfigurationController extends BaseController {
     public Configuration createConfiguration(@RequestParam(value = "parentNodeId") String parentNodeId,
                                              @RequestBody Configuration configuration,
                                              Principal principal) {
+        // Validation: a ConfigPV cannot specify non-null Comparison unless read-back PV is set.
+        for(ConfigPv configPv : configuration.getConfigurationData().getPvList()){
+            if((configPv.getReadbackPvName() == null || configPv.getReadbackPvName().isEmpty()) && configPv.getComparison() != null){
+                throw new IllegalArgumentException("PV item \"" + configPv.getPvName() + "\" specifies non-null comparison, but read-back PV is not set");
+            }
+        }
         configuration.getConfigurationNode().setUserName(principal.getName());
         return nodeDAO.createConfiguration(parentNodeId, configuration);
     }

services/save-and-restore/src/test/java/org/phoebus/service/saveandrestore/web/controllers/ConfigurationControllerTest.java

@@ -22,9 +22,13 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.phoebus.applications.saveandrestore.model.Comparison;
+import org.phoebus.applications.saveandrestore.model.ConfigPv;
 import org.phoebus.applications.saveandrestore.model.Configuration;
+import org.phoebus.applications.saveandrestore.model.ConfigurationData;
 import org.phoebus.applications.saveandrestore.model.Node;
 import org.phoebus.applications.saveandrestore.model.NodeType;
+import org.phoebus.applications.saveandrestore.model.PvCompareMode;
 import org.phoebus.service.saveandrestore.persistence.dao.NodeDAO;
 import org.phoebus.service.saveandrestore.web.config.ControllersTestConfig;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -36,6 +40,8 @@
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
 
+import java.util.List;
+
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.when;
 import static org.phoebus.service.saveandrestore.web.controllers.BaseController.JSON;
@@ -155,6 +161,23 @@ public void testUpdateConfiguration() throws Exception {
 
         mockMvc.perform(request).andExpect(status().isUnauthorized()
         );
+    }
+
+    @Test
+    public void testCreateInvalidConfiguration() throws Exception {
+
+        reset(nodeDAO);
+
+        Configuration configuration = new Configuration();
+        configuration.setConfigurationNode(Node.builder().build());
+        ConfigurationData configurationData = new ConfigurationData();
+        configuration.setConfigurationData(configurationData);
+        configurationData.setPvList(List.of(ConfigPv.builder().pvName("foo").build(),
+                ConfigPv.builder().pvName("fooo").comparison(new Comparison(PvCompareMode.ABSOLUTE, 1.0)).build()));
+        MockHttpServletRequestBuilder request = put("/config?parentNodeId=a")
+                .header(HttpHeaders.AUTHORIZATION, adminAuthorization)
+                .contentType(JSON).content(objectMapper.writeValueAsString(configuration));
 
+        mockMvc.perform(request).andExpect(status().isBadRequest());
     }
 }