fix(gateway): use resource type provided in request instead of detecting

- gateway will check for resource type of request instead of always detecting the resource type again - adds a new resource type FILE to gateway protocol which indicates that the gateway should determine the resource type based on the resource name BREAKING CHANGE: - gateway protocol resource type default value is now FILE instead of BPMN
camunda · Nov 2, 2018 · 9fbaccb · 9fbaccb
1 parent c096298
commit 9fbaccb
Show file tree

Hide file tree

Showing 11 changed files with 266 additions and 182 deletions.
diff --git a/clients/go/commands/deploy_command.go b/clients/go/commands/deploy_command.go
@@ -5,7 +5,6 @@ import (
 	"github.com/zeebe-io/zeebe/clients/go/pb"
 	"io/ioutil"
 	"log"
-	"strings"
 	"time"
 )
 
@@ -16,17 +15,12 @@ type DeployCommand struct {
 }
 
 func (cmd *DeployCommand) AddResourceFile(path string) *DeployCommand {
-	workflowType := pb.WorkflowRequestObject_BPMN
-	if strings.HasSuffix(path, ".yaml") || strings.HasSuffix(path, ".yml") {
-		workflowType = pb.WorkflowRequestObject_YAML
-	}
-
 	b, err := ioutil.ReadFile(path)
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	cmd.request.Workflows = append(cmd.request.Workflows, &pb.WorkflowRequestObject{Definition: b, Name: path, Type: workflowType})
+	cmd.request.Workflows = append(cmd.request.Workflows, &pb.WorkflowRequestObject{Definition: b, Name: path, Type: pb.WorkflowRequestObject_FILE})
 	return cmd
 }
 

diff --git a/clients/go/commands/deploy_command_test.go b/clients/go/commands/deploy_command_test.go
@@ -26,17 +26,17 @@ func TestDeployCommand(t *testing.T) {
 		Workflows: []*pb.WorkflowRequestObject{
 			{
 				Name:       demoName,
-				Type:       pb.WorkflowRequestObject_BPMN,
+				Type:       pb.WorkflowRequestObject_FILE,
 				Definition: demoBytes,
 			},
 			{
 				Name:       anotherName,
-				Type:       pb.WorkflowRequestObject_BPMN,
+				Type:       pb.WorkflowRequestObject_FILE,
 				Definition: anotherBytes,
 			},
 			{
 				Name:       yamlName,
-				Type:       pb.WorkflowRequestObject_YAML,
+				Type:       pb.WorkflowRequestObject_FILE,
 				Definition: yamlBytes,
 			},
 		},

diff --git a/clients/go/mock_pb/mock_gateway.go b/clients/go/mock_pb/mock_gateway.go
diff --git a/clients/go/pb/gateway.pb.go b/clients/go/pb/gateway.pb.go
diff --git a/clients/java/src/main/java/io/zeebe/client/impl/workflow/DeployWorkflowCommandImpl.java b/clients/java/src/main/java/io/zeebe/client/impl/workflow/DeployWorkflowCommandImpl.java
@@ -56,7 +56,7 @@ public DeployWorkflowCommandBuilderStep2 addResourceBytes(
 
     request.addWorkflows(
         WorkflowRequestObject.newBuilder()
-            .setType(getResourceType(resourceName))
+            .setType(WorkflowRequestObject.ResourceType.FILE)
             .setName(resourceName)
             .setDefinition(ByteString.copyFrom(resource)));
 
@@ -134,19 +134,6 @@ public DeployWorkflowCommandBuilderStep2 addWorkflowModel(
     return addResourceBytes(outStream.toByteArray(), resourceName);
   }
 
-  private WorkflowRequestObject.ResourceType getResourceType(String resourceName) {
-    resourceName = resourceName.toLowerCase();
-
-    if (resourceName.endsWith(".yaml")) {
-      return WorkflowRequestObject.ResourceType.YAML;
-    } else if (resourceName.endsWith(".bpmn") || resourceName.endsWith(".bpmn20.xml")) {
-      return WorkflowRequestObject.ResourceType.BPMN;
-    } else {
-      throw new RuntimeException(
-          String.format("Cannot resolve type of resource '%s'.", resourceName));
-    }
-  }
-
   @Override
   public ZeebeFuture<DeploymentEvent> send() {
     final ZeebeClientFutureImpl<DeploymentEvent, DeployWorkflowResponse> future =

diff --git a/clients/java/src/test/java/io/zeebe/client/workflow/DeployWorkflowTest.java b/clients/java/src/test/java/io/zeebe/client/workflow/DeployWorkflowTest.java
@@ -68,7 +68,7 @@ public void shouldDeployWorkflowFromFile() {
 
     final DeployWorkflowRequest request = gatewayService.getLastRequest();
     final WorkflowRequestObject workflow = request.getWorkflows(0);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.BPMN);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(getBytes(BPMN_1_FILENAME));
   }
@@ -84,7 +84,7 @@ public void shouldDeployWorkflowFromClasspath() {
     // then
     final DeployWorkflowRequest request = gatewayService.getLastRequest();
     final WorkflowRequestObject workflow = request.getWorkflows(0);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.BPMN);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(getBytes(BPMN_1_FILENAME));
   }
@@ -106,7 +106,7 @@ public void shouldDeployWorkflowFromInputStream() {
     // then
     final DeployWorkflowRequest request = gatewayService.getLastRequest();
     final WorkflowRequestObject workflow = request.getWorkflows(0);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.BPMN);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(getBytes(BPMN_1_FILENAME));
   }
@@ -123,7 +123,7 @@ public void shouldDeployWorkflowFromBytes() {
     // then
     final DeployWorkflowRequest request = gatewayService.getLastRequest();
     final WorkflowRequestObject workflow = request.getWorkflows(0);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.BPMN);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(getBytes(BPMN_1_FILENAME));
   }
@@ -145,7 +145,7 @@ public void shouldDeployWorkflowFromString() {
     // then
     final DeployWorkflowRequest request = gatewayService.getLastRequest();
     final WorkflowRequestObject workflow = request.getWorkflows(0);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.BPMN);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(getBytes(BPMN_1_FILENAME));
   }
@@ -162,7 +162,7 @@ public void shouldDeployWorkflowFromUtf8String() {
     // then
     final DeployWorkflowRequest request = gatewayService.getLastRequest();
     final WorkflowRequestObject workflow = request.getWorkflows(0);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.BPMN);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(getBytes(BPMN_1_FILENAME));
   }
@@ -189,7 +189,7 @@ public void shouldDeployWorkflowFromWorkflowModel() {
     // then
     final DeployWorkflowRequest request = gatewayService.getLastRequest();
     final WorkflowRequestObject workflow = request.getWorkflows(0);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.BPMN);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(expectedBytes);
   }
@@ -234,17 +234,17 @@ public void shouldDeployMultipleWorkflows() {
     assertThat(request.getWorkflowsList()).hasSize(3);
 
     WorkflowRequestObject workflow = request.getWorkflows(0);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.BPMN);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename1);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(getBytes(BPMN_1_FILENAME));
 
     workflow = request.getWorkflows(1);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.BPMN);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename2);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(getBytes(BPMN_2_FILENAME));
 
     workflow = request.getWorkflows(2);
-    assertThat(workflow.getType()).isEqualTo(ResourceType.YAML);
+    assertThat(workflow.getType()).isEqualTo(ResourceType.FILE);
     assertThat(workflow.getName()).isEqualTo(filename3);
     assertThat(workflow.getDefinition().toByteArray()).isEqualTo(getBytes(YAML_FILENAME));
   }

diff --git a/gateway-protocol/src/main/proto/gateway.proto b/gateway-protocol/src/main/proto/gateway.proto
@@ -31,8 +31,10 @@ message TopologyResponse {
 message WorkflowRequestObject {
   string name = 1;
   enum ResourceType {
-    BPMN = 0;
-    YAML = 1;
+    // FILE type means the gateway will try to detect the resource type using the file extension of the name
+    FILE = 0;
+    BPMN = 1;
+    YAML = 2;
   }
   ResourceType type = 2;
   bytes definition = 3;

diff --git a/gateway/src/main/java/io/zeebe/gateway/RequestMapper.java b/gateway/src/main/java/io/zeebe/gateway/RequestMapper.java
@@ -60,7 +60,8 @@ public static BrokerDeployWorkflowRequest toDeployWorkflowRequest(
     final BrokerDeployWorkflowRequest brokerRequest = new BrokerDeployWorkflowRequest();
 
     for (WorkflowRequestObject workflow : grpcRequest.getWorkflowsList()) {
-      brokerRequest.addResource(workflow.getDefinition().toByteArray(), workflow.getName());
+      brokerRequest.addResource(
+          workflow.getDefinition().toByteArray(), workflow.getName(), workflow.getType());
     }
 
     return brokerRequest;

diff --git a/gateway/src/main/java/io/zeebe/gateway/impl/broker/request/BrokerDeployWorkflowRequest.java b/gateway/src/main/java/io/zeebe/gateway/impl/broker/request/BrokerDeployWorkflowRequest.java
@@ -17,6 +17,7 @@
 
 import static io.zeebe.protocol.impl.record.value.deployment.ResourceType.getResourceType;
 
+import io.zeebe.gateway.protocol.GatewayOuterClass.WorkflowRequestObject.ResourceType;
 import io.zeebe.protocol.Protocol;
 import io.zeebe.protocol.clientapi.ValueType;
 import io.zeebe.protocol.impl.record.value.deployment.DeploymentRecord;
@@ -32,17 +33,30 @@ public BrokerDeployWorkflowRequest() {
     setPartitionId(Protocol.DEPLOYMENT_PARTITION);
   }
 
-  public BrokerDeployWorkflowRequest addResource(final byte[] resource, final String resourceName) {
+  public BrokerDeployWorkflowRequest addResource(
+      final byte[] resource, final String resourceName, ResourceType resourceType) {
     requestDto
         .resources()
         .add()
         .setResource(resource)
         .setResourceName(resourceName)
-        .setResourceType(getResourceType(resourceName));
+        .setResourceType(determineResourceType(resourceName, resourceType));
 
     return this;
   }
 
+  private io.zeebe.protocol.impl.record.value.deployment.ResourceType determineResourceType(
+      String resourceName, ResourceType resourceType) {
+    switch (resourceType) {
+      case BPMN:
+        return io.zeebe.protocol.impl.record.value.deployment.ResourceType.BPMN_XML;
+      case YAML:
+        return io.zeebe.protocol.impl.record.value.deployment.ResourceType.YAML_WORKFLOW;
+      default:
+        return getResourceType(resourceName);
+    }
+  }
+
   @Override
   public DeploymentRecord getRequestWriter() {
     return requestDto;

diff --git a/gateway/src/test/java/io/zeebe/gateway/api/deployment/DeployWorkflowStub.java b/gateway/src/test/java/io/zeebe/gateway/api/deployment/DeployWorkflowStub.java
@@ -25,6 +25,8 @@ public class DeployWorkflowStub
     implements RequestStub<BrokerDeployWorkflowRequest, BrokerResponse<DeploymentRecord>> {
 
   private static final long KEY = 123;
+  private static final long WORKFLOW_KEY = 456;
+  private static final int WORKFLOW_VERSION = 789;
 
   @Override
   public void registerWith(StubbedGateway gateway) {
@@ -35,6 +37,14 @@ protected long getKey() {
     return KEY;
   }
 
+  protected long getWorkflowKey() {
+    return WORKFLOW_KEY;
+  }
+
+  public int getWorkflowVersion() {
+    return WORKFLOW_VERSION;
+  }
+
   @Override
   public BrokerResponse<DeploymentRecord> handle(BrokerDeployWorkflowRequest request)
       throws Exception {
@@ -49,8 +59,8 @@ public BrokerResponse<DeploymentRecord> handle(BrokerDeployWorkflowRequest reque
                   .add()
                   .setBpmnProcessId(r.getResourceName())
                   .setResourceName(r.getResourceName())
-                  .setVersion(r.getResourceType().ordinal())
-                  .setKey(r.getResourceType().ordinal());
+                  .setVersion(WORKFLOW_VERSION)
+                  .setKey(WORKFLOW_KEY);
             });
     return new BrokerResponse<>(deploymentRecord, 0, KEY);
   }

diff --git a/gateway/src/test/java/io/zeebe/gateway/api/deployment/DeployWorkflowTest.java b/gateway/src/test/java/io/zeebe/gateway/api/deployment/DeployWorkflowTest.java
@@ -26,6 +26,8 @@
 import io.zeebe.gateway.protocol.GatewayOuterClass.WorkflowMetadata;
 import io.zeebe.gateway.protocol.GatewayOuterClass.WorkflowRequestObject.ResourceType;
 import io.zeebe.protocol.clientapi.ValueType;
+import io.zeebe.protocol.impl.record.value.deployment.DeploymentRecord;
+import io.zeebe.protocol.impl.record.value.deployment.DeploymentResource;
 import io.zeebe.protocol.intent.DeploymentIntent;
 import org.junit.Test;
 
@@ -37,15 +39,18 @@ public void shouldMapRequestAndResponse() {
     final DeployWorkflowStub stub = new DeployWorkflowStub();
     stub.registerWith(gateway);
 
+    final String bpmnName = "testProcess.bpmn";
+    final String yamlName = "testProcess.yaml";
+
     final Builder builder = DeployWorkflowRequest.newBuilder();
     builder
         .addWorkflowsBuilder()
-        .setName("testProcess.bpmn")
+        .setName(bpmnName)
         .setType(ResourceType.BPMN)
         .setDefinition(ByteString.copyFromUtf8("<xml/>"));
     builder
         .addWorkflowsBuilder()
-        .setName("testProcess.yaml")
+        .setName(yamlName)
         .setType(ResourceType.YAML)
         .setDefinition(ByteString.copyFromUtf8("yaml"));
 
@@ -59,19 +64,85 @@ public void shouldMapRequestAndResponse() {
     assertThat(response.getWorkflowsCount()).isEqualTo(2);
 
     WorkflowMetadata workflow = response.getWorkflows(0);
-    assertThat(workflow.getBpmnProcessId()).isEqualTo("testProcess.bpmn");
-    assertThat(workflow.getResourceName()).isEqualTo("testProcess.bpmn");
-    assertThat(workflow.getWorkflowKey()).isEqualTo(ResourceType.BPMN.ordinal());
-    assertThat(workflow.getVersion()).isEqualTo(ResourceType.BPMN.ordinal());
+    assertThat(workflow.getBpmnProcessId()).isEqualTo(bpmnName);
+    assertThat(workflow.getResourceName()).isEqualTo(bpmnName);
+    assertThat(workflow.getWorkflowKey()).isEqualTo(stub.getWorkflowKey());
+    assertThat(workflow.getVersion()).isEqualTo(stub.getWorkflowVersion());
 
     workflow = response.getWorkflows(1);
-    assertThat(workflow.getBpmnProcessId()).isEqualTo("testProcess.yaml");
-    assertThat(workflow.getResourceName()).isEqualTo("testProcess.yaml");
-    assertThat(workflow.getWorkflowKey()).isEqualTo(ResourceType.YAML.ordinal());
-    assertThat(workflow.getVersion()).isEqualTo(ResourceType.YAML.ordinal());
+    assertThat(workflow.getBpmnProcessId()).isEqualTo(yamlName);
+    assertThat(workflow.getResourceName()).isEqualTo(yamlName);
+    assertThat(workflow.getWorkflowKey()).isEqualTo(stub.getWorkflowKey());
+    assertThat(workflow.getVersion()).isEqualTo(stub.getWorkflowVersion());
 
     final BrokerDeployWorkflowRequest brokerRequest = gateway.getSingleBrokerRequest();
     assertThat(brokerRequest.getIntent()).isEqualTo(DeploymentIntent.CREATE);
     assertThat(brokerRequest.getValueType()).isEqualTo(ValueType.DEPLOYMENT);
   }
+
+  @Test
+  public void shouldDetermineResourceTypeBasedOnFileExtension() {
+    // given
+    final DeployWorkflowStub stub = new DeployWorkflowStub();
+    stub.registerWith(gateway);
+
+    final Builder builder = DeployWorkflowRequest.newBuilder();
+    builder
+        .addWorkflowsBuilder()
+        .setName("testProcess.bpmn")
+        .setDefinition(ByteString.copyFromUtf8("<xml/>"));
+    builder
+        .addWorkflowsBuilder()
+        .setName("testProcess.yaml")
+        .setDefinition(ByteString.copyFromUtf8("yaml"));
+
+    final DeployWorkflowRequest request = builder.build();
+
+    // when
+    client.deployWorkflow(request);
+
+    // then
+    final BrokerDeployWorkflowRequest brokerRequest = gateway.getSingleBrokerRequest();
+    final DeploymentRecord record = brokerRequest.getRequestWriter();
+
+    assertThat(record.resources())
+        .extracting(DeploymentResource::getResourceType)
+        .containsExactlyInAnyOrder(
+            io.zeebe.protocol.impl.record.value.deployment.ResourceType.BPMN_XML,
+            io.zeebe.protocol.impl.record.value.deployment.ResourceType.YAML_WORKFLOW);
+  }
+
+  @Test
+  public void shouldAcceptProvidedResourceTypes() {
+    // given
+    final DeployWorkflowStub stub = new DeployWorkflowStub();
+    stub.registerWith(gateway);
+
+    final Builder builder = DeployWorkflowRequest.newBuilder();
+    builder
+        .addWorkflowsBuilder()
+        .setName("testProcess.txt")
+        .setType(ResourceType.BPMN)
+        .setDefinition(ByteString.copyFromUtf8("<xml/>"));
+    builder
+        .addWorkflowsBuilder()
+        .setName("testProcess.txt")
+        .setType(ResourceType.YAML)
+        .setDefinition(ByteString.copyFromUtf8("yaml"));
+
+    final DeployWorkflowRequest request = builder.build();
+
+    // when
+    client.deployWorkflow(request);
+
+    // then
+    final BrokerDeployWorkflowRequest brokerRequest = gateway.getSingleBrokerRequest();
+    final DeploymentRecord record = brokerRequest.getRequestWriter();
+
+    assertThat(record.resources())
+        .extracting(DeploymentResource::getResourceType)
+        .containsExactlyInAnyOrder(
+            io.zeebe.protocol.impl.record.value.deployment.ResourceType.BPMN_XML,
+            io.zeebe.protocol.impl.record.value.deployment.ResourceType.YAML_WORKFLOW);
+  }
 }