[ONOS-7732] Automating switch workflow: data model schema checking bug fix
Change-Id: Ia724c1c660969ab9fcf6ae5c18e8e81abc45ccbe
diff --git a/apps/workflow/api/src/main/java/org/onosproject/workflow/api/ImmutableListWorkflow.java b/apps/workflow/api/src/main/java/org/onosproject/workflow/api/ImmutableListWorkflow.java
index 02a7836..ec2be65 100644
--- a/apps/workflow/api/src/main/java/org/onosproject/workflow/api/ImmutableListWorkflow.java
+++ b/apps/workflow/api/src/main/java/org/onosproject/workflow/api/ImmutableListWorkflow.java
@@ -143,6 +143,10 @@
@Override
public Worklet getWorkletInstance(String workletType) throws WorkflowException {
+ if (Worklet.Common.INIT.tag().equals(workletType)) {
+ return Worklet.Common.INIT;
+ }
+
if (Worklet.Common.COMPLETED.tag().equals(workletType)) {
return Worklet.Common.COMPLETED;
}
diff --git a/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowDataModelException.java b/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowDataModelException.java
index 9227092..7d17529 100644
--- a/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowDataModelException.java
+++ b/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowDataModelException.java
@@ -17,19 +17,22 @@
package org.onosproject.workflow.api;
-import java.util.Map;
+import com.fasterxml.jackson.databind.JsonNode;
+
+import java.net.URI;
+import java.util.List;
/**
* Workflow DataModel exception class.
*/
public class WorkflowDataModelException extends WorkflowException {
- private String workflowName;
- private Map<String, Map<String, String>> errorListMap;
-
+ private URI workflowId;
+ private JsonNode parameterJson;
+ private List<String> errorMsgs;
/**
- * Constructor for Workflow DataModel Exception.
+ * Default Constructor for Workflow DataModel Exception.
*
* @param msg exception message
*/
@@ -41,22 +44,17 @@
/**
* Constructor for Workflow DataModel Exception.
*
- * @param msg exception message
- * @param workflowName workflow name
- * @param errorListMap throwable to deliver
+ * @param workflowId id of workflow
+ * @param parameterJson paramter json data model
+ * @param errorMsgs error message for json data model
*/
- public WorkflowDataModelException(String msg, String workflowName, Map<String, Map<String, String>> errorListMap) {
- super(msg);
- this.workflowName = workflowName;
- this.errorListMap = errorListMap;
+ public WorkflowDataModelException(URI workflowId, JsonNode parameterJson, List<String> errorMsgs) {
+ super("Invalid workflow data model: " +
+ " workflow: " + workflowId.toString() +
+ ", parameter json: " + parameterJson.toString() +
+ ", errors: " + errorMsgs);
+ this.workflowId = workflowId;
+ this.parameterJson = parameterJson;
+ this.errorMsgs = errorMsgs;
}
-
- @Override
- public String toString() {
- return "WorkflowDataModelException{" +
- "workflowName='" + workflowName + '\'' +
- ", errorListMap=" + errorListMap.toString() +
- '}';
- }
-
}
diff --git a/apps/workflow/app/src/main/java/org/onosproject/workflow/impl/WorkflowManager.java b/apps/workflow/app/src/main/java/org/onosproject/workflow/impl/WorkflowManager.java
index 8eb7c0c..ca0570f 100644
--- a/apps/workflow/app/src/main/java/org/onosproject/workflow/impl/WorkflowManager.java
+++ b/apps/workflow/app/src/main/java/org/onosproject/workflow/impl/WorkflowManager.java
@@ -17,8 +17,10 @@
import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import com.fasterxml.jackson.databind.node.JsonNodeType;
+import com.fasterxml.jackson.databind.node.MissingNode;
import org.onosproject.net.config.NetworkConfigRegistry;
import org.onosproject.net.config.NetworkConfigService;
import org.onosproject.workflow.api.WorkflowService;
@@ -46,10 +48,10 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.net.URI;
-import java.util.Map;
-import java.util.HashMap;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Objects;
-import java.util.Arrays;
+import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -140,9 +142,7 @@
throw new WorkflowException("Invalid Workflow");
}
- if (!checkWorkflowSchema(workflow, worklowDescJson)) {
- throw new WorkflowException("Invalid Workflow " + worklowDescJson.get("id").asText());
- }
+ checkWorkflowSchema(workflow, worklowDescJson);
Workflow wfCreationWf = workflowStore.get(URI.create(WorkplaceWorkflow.WF_CREATE_WORKFLOW));
if (Objects.isNull(wfCreationWf)) {
@@ -157,92 +157,119 @@
* Checks if the type of worklet is same as that of wfdesc Json.
*
* @param workflow workflow
- * @param jsonNode jsonNode
+ * @param worklowDescJson jsonNode
* @throws WorkflowException workflow exception
*/
+ private void checkWorkflowSchema(Workflow workflow, JsonNode worklowDescJson) throws WorkflowException {
- private boolean checkWorkflowSchema(Workflow workflow, JsonNode jsonNode) throws WorkflowException {
+ List<String> errors = new ArrayList<>();
- Map<String, Map<String, String>> workletDataTypeMap = new HashMap<>();
+ JsonNode dataNode = worklowDescJson.get("data");
+ if (Objects.isNull(dataNode) || dataNode instanceof MissingNode) {
+ errors.add("workflow description json does not have 'data'");
+ throw new WorkflowDataModelException(workflow.id(), worklowDescJson, errors);
+ }
+
for (String workletType : workflow.getWorkletTypeList()) {
- Map<String, String> jsonDataModelMap = new HashMap<>();
- if (Objects.equals(workletType, Worklet.Common.INIT.tag())
- || (Objects.equals(workletType, Worklet.Common.COMPLETED.tag()))) {
+
+ Worklet worklet = workflow.getWorkletInstance(workletType);
+ if (Worklet.Common.COMPLETED.equals(worklet) || Worklet.Common.INIT.equals(worklet)) {
continue;
}
- Worklet worklet = workflow.getWorkletInstance(workletType);
+
Class cls = worklet.getClass();
for (Field field : cls.getDeclaredFields()) {
+
if (field.isSynthetic()) {
continue;
}
- Annotation[] annotations = field.getAnnotations();
- for (Annotation annotation : annotations) {
+
+ for (Annotation annotation : field.getAnnotations()) {
+
if (annotation instanceof JsonDataModel) {
+
JsonDataModel jsonDataModel = (JsonDataModel) annotation;
Matcher matcher = Pattern.compile("(\\w+)").matcher(jsonDataModel.path());
if (!matcher.find()) {
- throw new WorkflowException("Invalid Json Data Model Path");
+ throw new WorkflowException(
+ "Invalid Json Data Model Path(" + jsonDataModel.path() + ") in " + worklet.tag());
}
String path = matcher.group(1);
- if (checkJsonNodeDataType(jsonNode, field, path)) {
- jsonDataModelMap.put(path, field.getType().getName());
+
+ Optional<String> optError =
+ getJsonNodeDataError(dataNode, worklet, field, path, jsonDataModel.optional());
+
+ if (optError.isPresent()) {
+ errors.add(optError.get());
}
}
}
}
- if (!jsonDataModelMap.isEmpty()) {
- workletDataTypeMap.put(worklet.tag(), jsonDataModelMap);
- }
+ }
+ if (!errors.isEmpty()) {
+ throw new WorkflowDataModelException(workflow.id(), worklowDescJson, errors);
}
- if (!workletDataTypeMap.isEmpty()) {
- throw new WorkflowDataModelException("invalid workflow ", workflow.id().toString(), workletDataTypeMap);
- }
- return true;
}
+ private Optional<String> getJsonNodeDataError(
+ JsonNode dataNode, Worklet worklet, Field field, String path, boolean isOptional) throws WorkflowException {
- private boolean checkJsonNodeDataType(JsonNode jsonNode, Field field, String path) throws WorkflowException {
- if (!Objects.nonNull(jsonNode.get("data")) && !Objects.nonNull(jsonNode.get("data").get(path))) {
- throw new WorkflowException("Invalid Json");
- }
- JsonNodeType jsonNodeType = jsonNode.get("data").get(path).getNodeType();
- if (jsonNodeType != null) {
- switch (jsonNodeType) {
- case NUMBER:
- if (!(field.getType().isAssignableFrom(Integer.class))) {
- return true;
- }
- break;
- case STRING:
- if (!(field.getType().isAssignableFrom(String.class))) {
- return true;
- }
- break;
- case OBJECT:
- if (!(field.getType().isAssignableFrom(Objects.class))) {
- return true;
- }
- break;
- case BOOLEAN:
- if (!(field.getType().isAssignableFrom(Boolean.class))) {
- return true;
- }
- break;
- case ARRAY:
- if (!(field.getType().isAssignableFrom(Arrays.class))) {
- return true;
- }
- break;
- default:
- return true;
+ // Checking the existence of path in dataNode
+ JsonNode pathNode = dataNode.get(path);
+ if (Objects.isNull(pathNode) || pathNode instanceof MissingNode) {
+
+ if (isOptional) {
+ return Optional.empty();
+
+ } else {
+ return Optional.of("data doesn't have '" + path + "' in worklet<" + worklet.tag() + ">");
}
- } else {
- return false;
-
}
- return false;
+
+ // Checking the type of path
+ JsonNodeType type = pathNode.getNodeType();
+
+ if (Objects.isNull(type)) {
+ throw new WorkflowException("Invalid type for " + pathNode);
+ }
+
+ switch (type) {
+ case NUMBER:
+ if (!(field.getType().isAssignableFrom(Integer.class))) {
+ return Optional.of("'" + path + "<NUMBER>' cannot be assigned to " +
+ field.getName() + "<" + field.getType() + "> in worklet<" + worklet.tag() + ">");
+ }
+ break;
+ case STRING:
+ if (!(field.getType().isAssignableFrom(String.class))) {
+ return Optional.of("'" + path + "<STRING>' cannot be assigned to " +
+ field.getName() + "<" + field.getType() + "> in worklet<" + worklet.tag() + ">");
+ }
+ break;
+ case BOOLEAN:
+ if (!(field.getType().isAssignableFrom(Boolean.class))) {
+ return Optional.of("'" + path + "<BOOLEAN>' cannot be assigned to " +
+ field.getName() + "<" + field.getType() + "> in worklet<" + worklet.tag() + ">");
+ }
+ break;
+ case OBJECT:
+ if (!(field.getType().isAssignableFrom(JsonNode.class))) {
+ return Optional.of("'" + path + "<OBJECT>' cannot be assigned to " +
+ field.getName() + "<" + field.getType() + "> in worklet<" + worklet.tag() + ">");
+ }
+ break;
+ case ARRAY:
+ if (!(field.getType().isAssignableFrom(ArrayNode.class))) {
+ return Optional.of("'" + path + "<ARRAY>' cannot be assigned to " +
+ field.getName() + "<" + field.getType() + "> in worklet<" + worklet.tag() + ">");
+ }
+ break;
+ default:
+ return Optional.of("'" + path + "<" + type + ">' is not supported");
+ }
+
+ return Optional.empty();
}
@Override
diff --git a/apps/workflow/ofoverlay/app/src/main/java/org/onosproject/ofoverlay/impl/Ovs.java b/apps/workflow/ofoverlay/app/src/main/java/org/onosproject/ofoverlay/impl/Ovs.java
index ef75806..319bb92 100644
--- a/apps/workflow/ofoverlay/app/src/main/java/org/onosproject/ofoverlay/impl/Ovs.java
+++ b/apps/workflow/ofoverlay/app/src/main/java/org/onosproject/ofoverlay/impl/Ovs.java
@@ -792,7 +792,7 @@
@JsonDataModel(path = MODEL_OVS_DATAPATH_TYPE)
String strOvsDatapath;
- @JsonDataModel(path = MODEL_OF_DEVID_UNDERLAY_BRIDGE)
+ @JsonDataModel(path = MODEL_OF_DEVID_UNDERLAY_BRIDGE, optional = true)
String strOfDevIdUnderlay;
@Override