From 44628d6c294871b57e4262597b100f1517cffa1c Mon Sep 17 00:00:00 2001 From: jaegonkim Date: Sun, 7 Apr 2019 10:30:32 +0900 Subject: [PATCH] [ONOS-7732] Automating switch workflow - checking workflow definitition Change-Id: I66b3bcd43377869b82be5bb7a446152857344355 --- .../api/WorkflowDefinitionException.java | 54 ++++++ .../workflow/api/WorkflowService.java | 8 + .../workflow/cli/WorkFlowTestCommand.java | 49 ++++-- .../workflow/cli/WorkFlowTestCompleter.java | 5 +- .../workflow/impl/WorkflowManager.java | 166 ++++++++++++++++-- .../workflow/impl/example/SampleWorkflow.java | 28 +-- 6 files changed, 265 insertions(+), 45 deletions(-) create mode 100644 apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowDefinitionException.java diff --git a/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowDefinitionException.java b/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowDefinitionException.java new file mode 100644 index 0000000000..fab72e9acd --- /dev/null +++ b/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowDefinitionException.java @@ -0,0 +1,54 @@ +/* + * Copyright 2019-present Open Networking Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.onosproject.workflow.api; + + +import java.net.URI; +import java.util.List; + +/** + * Workflow Definition exception class. + */ +public class WorkflowDefinitionException extends WorkflowException { + + private URI workflowId; + private List errorMsgs; + + /** + * Default Constructor for Workflow Definition Exception. + * + * @param msg exception message + */ + public WorkflowDefinitionException(String msg) { + + super(msg); + } + + /** + * Constructor for Workflow Definition Exception. + * + * @param workflowId id of workflow + * @param errorMsgs error message for json data model + */ + public WorkflowDefinitionException(URI workflowId, List errorMsgs) { + super("Invalid workflow definition: " + + " workflow: " + workflowId.toString() + + ", errors: " + errorMsgs); + this.workflowId = workflowId; + this.errorMsgs = errorMsgs; + } +} diff --git a/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowService.java b/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowService.java index b11e83592b..f47578d392 100644 --- a/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowService.java +++ b/apps/workflow/api/src/main/java/org/onosproject/workflow/api/WorkflowService.java @@ -23,6 +23,14 @@ import com.fasterxml.jackson.databind.JsonNode; */ public interface WorkflowService { + + /** + * Registers workflow. + * @param workflow registering workflow + * @throws WorkflowException workflow exception + */ + void register(Workflow workflow) throws WorkflowException; + /** * Creates workplace. * @param wpDesc workplace description diff --git a/apps/workflow/app/src/main/java/org/onosproject/workflow/cli/WorkFlowTestCommand.java b/apps/workflow/app/src/main/java/org/onosproject/workflow/cli/WorkFlowTestCommand.java index 381b2d5370..3069be6335 100644 --- a/apps/workflow/app/src/main/java/org/onosproject/workflow/cli/WorkFlowTestCommand.java +++ b/apps/workflow/app/src/main/java/org/onosproject/workflow/cli/WorkFlowTestCommand.java @@ -23,9 +23,13 @@ import org.apache.karaf.shell.api.action.Completion; import org.apache.karaf.shell.api.action.lifecycle.Service; import org.onosproject.cli.AbstractShellCommand; import org.onosproject.workflow.api.DefaultWorkflowDescription; +import org.onosproject.workflow.api.ImmutableListWorkflow; +import org.onosproject.workflow.api.Workflow; import org.onosproject.workflow.api.WorkflowException; import org.onosproject.workflow.api.WorkflowService; +import org.onosproject.workflow.impl.example.SampleWorkflow; +import java.net.URI; import java.util.Arrays; import java.util.Objects; @@ -37,16 +41,19 @@ import java.util.Objects; public class WorkFlowTestCommand extends AbstractShellCommand { static final String INVOKE_SAMPLE = "invoke-sample"; - static final String EXCEPTION_SAMPLE = "exception-sample"; + static final String INVOKE_INVALID_DATAMODEL_TYPE = "invoke-invalid-datamodel-type"; + static final String DEFINE_INVALID_WORKFLOW = "define-invalid-workflow"; @Argument(index = 0, name = "test-name", - description = "Test name (" + INVOKE_SAMPLE + " | " + EXCEPTION_SAMPLE + ")", + description = "Test name (" + INVOKE_SAMPLE + + " | " + INVOKE_INVALID_DATAMODEL_TYPE + ")", required = true) @Completion(WorkFlowTestCompleter.class) private String testName = null; @Argument(index = 1, name = "arg1", - description = "number of test for (" + INVOKE_SAMPLE + " | " + EXCEPTION_SAMPLE + ")", + description = "number of test for (" + INVOKE_SAMPLE + + " | " + INVOKE_INVALID_DATAMODEL_TYPE + ")", required = false) private String arg1 = null; @@ -78,9 +85,9 @@ public class WorkFlowTestCommand extends AbstractShellCommand { invokeSampleTest(num); break; - case EXCEPTION_SAMPLE: + case INVOKE_INVALID_DATAMODEL_TYPE: if (Objects.isNull(arg1)) { - error("arg1 is required for test " + EXCEPTION_SAMPLE); + error("arg1 is required for test " + INVOKE_INVALID_DATAMODEL_TYPE); return; } int count; @@ -94,7 +101,11 @@ public class WorkFlowTestCommand extends AbstractShellCommand { return; } - invokeExceptionTest(count); + invokeInvalidDatamodelTypeTest(count); + break; + + case DEFINE_INVALID_WORKFLOW: + defineInvalidWorkflow(); break; default: @@ -117,14 +128,14 @@ public class WorkFlowTestCommand extends AbstractShellCommand { } /** - * Workflow datatype exception test. + * Workflow datatmodel type exception test. * * @param num the number of workflow to test */ - private void invokeExceptionTest(int num) { + private void invokeInvalidDatamodelTypeTest(int num) { for (int i = 0; i <= num; i++) { String wpName = "test-" + i; - invoke("sample.workflow-3", wpName); + invoke("sample.workflow-invalid-datamodel-type", wpName); } } @@ -150,7 +161,25 @@ public class WorkFlowTestCommand extends AbstractShellCommand { .build(); service.invokeWorkflow(wfDesc); } catch (WorkflowException e) { - error(e.getMessage() + "trace: " + Arrays.asList(e.getStackTrace())); + error(e.getMessage() + ", trace: " + Arrays.asList(e.getStackTrace())); } } + + private void defineInvalidWorkflow() { + + WorkflowService service = get(WorkflowService.class); + + try { + URI uri = URI.create("sample.workflow-invalid-datamodel-type"); + Workflow workflow = ImmutableListWorkflow.builder() + .id(uri) + .chain(SampleWorkflow.SampleWorklet5.class.getName()) + .chain(SampleWorkflow.SampleWorklet6.class.getName()) + .build(); + service.register(workflow); + } catch (WorkflowException e) { + error(e.getMessage() + ", trace: " + Arrays.asList(e.getStackTrace())); + } + } + } diff --git a/apps/workflow/app/src/main/java/org/onosproject/workflow/cli/WorkFlowTestCompleter.java b/apps/workflow/app/src/main/java/org/onosproject/workflow/cli/WorkFlowTestCompleter.java index c08a701db3..df82ad0ad9 100644 --- a/apps/workflow/app/src/main/java/org/onosproject/workflow/cli/WorkFlowTestCompleter.java +++ b/apps/workflow/app/src/main/java/org/onosproject/workflow/cli/WorkFlowTestCompleter.java @@ -21,7 +21,8 @@ import org.onosproject.cli.AbstractChoicesCompleter; import java.util.List; -import static org.onosproject.workflow.cli.WorkFlowTestCommand.EXCEPTION_SAMPLE; +import static org.onosproject.workflow.cli.WorkFlowTestCommand.DEFINE_INVALID_WORKFLOW; +import static org.onosproject.workflow.cli.WorkFlowTestCommand.INVOKE_INVALID_DATAMODEL_TYPE; import static org.onosproject.workflow.cli.WorkFlowTestCommand.INVOKE_SAMPLE; /** @@ -31,6 +32,6 @@ import static org.onosproject.workflow.cli.WorkFlowTestCommand.INVOKE_SAMPLE; public class WorkFlowTestCompleter extends AbstractChoicesCompleter { @Override protected List choices() { - return ImmutableList.of(INVOKE_SAMPLE, EXCEPTION_SAMPLE); + return ImmutableList.of(INVOKE_SAMPLE, INVOKE_INVALID_DATAMODEL_TYPE, DEFINE_INVALID_WORKFLOW); } } 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 ca0570f616..b1acf7f51f 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 @@ -21,8 +21,10 @@ 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 com.google.common.base.MoreObjects; import org.onosproject.net.config.NetworkConfigRegistry; import org.onosproject.net.config.NetworkConfigService; +import org.onosproject.workflow.api.WorkflowDefinitionException; import org.onosproject.workflow.api.WorkflowService; import org.onosproject.workflow.api.WorkflowExecutionService; import org.onosproject.workflow.api.WorkplaceStore; @@ -49,7 +51,9 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Field; import java.net.URI; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.regex.Matcher; @@ -94,6 +98,12 @@ public class WorkflowManager implements WorkflowService { log.info("Stopped"); } + @Override + public void register(Workflow workflow) throws WorkflowException { + checkWorkflow(workflow); + workflowStore.register(workflow); + } + @Override public void createWorkplace(WorkplaceDescription wpDesc) throws WorkflowException { log.info("createWorkplace: {}", wpDesc); @@ -142,7 +152,7 @@ public class WorkflowManager implements WorkflowService { throw new WorkflowException("Invalid Workflow"); } - checkWorkflowSchema(workflow, worklowDescJson); + checkWorkflowDataModelSchema(workflow, worklowDescJson); Workflow wfCreationWf = workflowStore.get(URI.create(WorkplaceWorkflow.WF_CREATE_WORKFLOW)); if (Objects.isNull(wfCreationWf)) { @@ -154,13 +164,129 @@ public class WorkflowManager implements WorkflowService { } /** - * Checks if the type of worklet is same as that of wfdesc Json. + * Checks the validity of workflow definition. + * @param workflow workflow to be checked + * @throws WorkflowException workflow exception + */ + private void checkWorkflow(Workflow workflow) throws WorkflowException { + + Map descMap = new HashMap<>(); + + List errors = new ArrayList<>(); + + for (String workletType : workflow.getWorkletTypeList()) { + + Worklet worklet = workflow.getWorkletInstance(workletType); + if (Worklet.Common.COMPLETED.equals(worklet) || Worklet.Common.INIT.equals(worklet)) { + continue; + } + + Class cls = worklet.getClass(); + for (Field field : cls.getDeclaredFields()) { + + if (field.isSynthetic()) { + continue; + } + + for (Annotation annotation : field.getAnnotations()) { + + if (!(annotation instanceof JsonDataModel)) { + continue; + } + + JsonDataModel jsonDataModel = (JsonDataModel) annotation; + Matcher matcher = Pattern.compile("(\\w+)").matcher(jsonDataModel.path()); + if (!matcher.find()) { + throw new WorkflowException( + "Invalid Json Data Model Path(" + jsonDataModel.path() + ") in " + worklet.tag()); + } + String path = matcher.group(1); + + WorkletDataModelFieldDesc desc = + new WorkletDataModelFieldDesc(workletType, path, field.getType(), jsonDataModel.optional()); + + WorkletDataModelFieldDesc existing = descMap.get(path); + + if (Objects.isNull(existing)) { + descMap.put(path, desc); + } else { + if (!desc.hasSameAttributes(existing)) { + errors.add("" + desc + " is conflicted with " + existing + " in workflow " + workflow.id()); + } + } + } + } + } + + if (!errors.isEmpty()) { + throw new WorkflowDefinitionException(workflow.id(), errors); + } + } + + /** + * Description of worklet data model field. + */ + private static class WorkletDataModelFieldDesc { + + private final String workletType; + + private final String path; + + private final Class type; + + private final boolean optional; + + /** + * Constructor of worklet data model field description. + * @param workletType worklet type + * @param path path of data model + * @param type type of data model + * @param optional optional + */ + public WorkletDataModelFieldDesc(String workletType, String path, Class type, boolean optional) { + this.workletType = workletType; + this.path = path; + this.type = type; + this.optional = optional; + } + + /** + * Checks the attributes of worklet data model field. + * @param desc worklet data model description + * @return true means that this worklet data model field description has same attributes with desc + */ + public boolean hasSameAttributes(WorkletDataModelFieldDesc desc) { + + if (!Objects.equals(type, desc.type)) { + return false; + } + + if (!Objects.equals(optional, desc.optional)) { + return false; + } + + return true; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(getClass()) + .add("worklet", workletType) + .add("path", path) + .add("type", type) + .add("optional", optional) + .toString(); + } + } + + /** + * Checks the schema of workflow data. * * @param workflow workflow * @param worklowDescJson jsonNode * @throws WorkflowException workflow exception */ - private void checkWorkflowSchema(Workflow workflow, JsonNode worklowDescJson) throws WorkflowException { + private void checkWorkflowDataModelSchema(Workflow workflow, JsonNode worklowDescJson) throws WorkflowException { List errors = new ArrayList<>(); @@ -186,23 +312,25 @@ public class WorkflowManager implements WorkflowService { 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(" + jsonDataModel.path() + ") in " + worklet.tag()); - } - String path = matcher.group(1); - - Optional optError = - getJsonNodeDataError(dataNode, worklet, field, path, jsonDataModel.optional()); - - if (optError.isPresent()) { - errors.add(optError.get()); - } + if (!(annotation instanceof JsonDataModel)) { + continue; } + + JsonDataModel jsonDataModel = (JsonDataModel) annotation; + Matcher matcher = Pattern.compile("(\\w+)").matcher(jsonDataModel.path()); + if (!matcher.find()) { + throw new WorkflowException( + "Invalid Json Data Model Path(" + jsonDataModel.path() + ") in " + worklet.tag()); + } + String path = matcher.group(1); + + Optional optError = + getJsonNodeDataError(dataNode, worklet, field, path, jsonDataModel.optional()); + + if (optError.isPresent()) { + errors.add(optError.get()); + } + } } } diff --git a/apps/workflow/app/src/main/java/org/onosproject/workflow/impl/example/SampleWorkflow.java b/apps/workflow/app/src/main/java/org/onosproject/workflow/impl/example/SampleWorkflow.java index 2a9b9f6025..8652c397e3 100644 --- a/apps/workflow/app/src/main/java/org/onosproject/workflow/impl/example/SampleWorkflow.java +++ b/apps/workflow/app/src/main/java/org/onosproject/workflow/impl/example/SampleWorkflow.java @@ -27,9 +27,8 @@ import org.onosproject.workflow.api.JsonDataModelTree; import org.onosproject.workflow.api.Workflow; import org.onosproject.workflow.api.WorkflowContext; import org.onosproject.workflow.api.WorkflowException; -import org.onosproject.workflow.api.WorkflowExecutionService; +import org.onosproject.workflow.api.WorkflowService; import org.onosproject.workflow.api.WorkflowStore; -import org.onosproject.workflow.api.WorkplaceStore; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; @@ -52,10 +51,7 @@ public class SampleWorkflow { protected WorkflowStore workflowStore; @Reference(cardinality = ReferenceCardinality.MANDATORY) - protected WorkplaceStore workplaceStore; - - @Reference(cardinality = ReferenceCardinality.MANDATORY) - protected WorkflowExecutionService workflowExecutionService; + protected WorkflowService workflowService; @Reference(cardinality = ReferenceCardinality.MANDATORY) protected DeviceService deviceService; @@ -65,8 +61,12 @@ public class SampleWorkflow { public void activate() { log.info("Activated"); - registerWorkflows(); - + try { + registerWorkflows(); + } catch (WorkflowException e) { + log.error("exception: " + e); + e.printStackTrace(); + } } @Deactivate @@ -77,7 +77,7 @@ public class SampleWorkflow { /** * Registers example workflows. */ - private void registerWorkflows() { + private void registerWorkflows() throws WorkflowException { // registering class-loader workflowStore.registerLocal(this.getClass().getClassLoader()); @@ -91,7 +91,7 @@ public class SampleWorkflow { .chain(SampleWorklet4.class.getName()) .chain(SampleWorklet5.class.getName()) .build(); - workflowStore.register(workflow); + workflowService.register(workflow); // registering new workflow definition uri = URI.create("sample.workflow-1"); @@ -103,7 +103,7 @@ public class SampleWorkflow { .chain(SampleWorklet4.class.getName()) .chain(SampleWorklet5.class.getName()) .build(); - workflowStore.register(workflow); + workflowService.register(workflow); // registering new workflow definition uri = URI.create("sample.workflow-2"); @@ -115,15 +115,15 @@ public class SampleWorkflow { .chain(SampleWorklet4.class.getName()) .chain(SampleWorklet5.class.getName()) .build(); - workflowStore.register(workflow); + workflowService.register(workflow); // registering new workflow definition - uri = URI.create("sample.workflow-3"); + uri = URI.create("sample.workflow-invalid-datamodel-type"); workflow = ImmutableListWorkflow.builder() .id(uri) .chain(SampleWorklet6.class.getName()) .build(); - workflowStore.register(workflow); + workflowService.register(workflow); }