From 3a6927daf4bd5a68a0b3b53e51179e02e9046853 Mon Sep 17 00:00:00 2001
From: Nathan Hertz <nhertz@nrao.edu>
Date: Fri, 5 Feb 2021 18:27:01 -0500
Subject: [PATCH] Preparation for submitting the download capability from the
 front end

---
 .../app/workspaces/workspaces.component.html  |  2 +-
 .../app/workspaces/workspaces.component.ts    | 56 ++++++++-------
 ...e_ssa_6391_add_download_capability_and_.py | 20 +++---
 .../workspaces/test/test_capability_steps.py  | 12 ++--
 .../workspaces/workspaces/capability/enums.py |  3 +-
 .../workspaces/capability/helpers.py          | 67 ++++++++++++++----
 .../capability/test/test_helpers.py           | 68 +++++++++++++++++++
 7 files changed, 172 insertions(+), 56 deletions(-)
 create mode 100644 shared/workspaces/workspaces/capability/test/test_helpers.py

diff --git a/apps/web/src/app/workspaces/workspaces.component.html b/apps/web/src/app/workspaces/workspaces.component.html
index 800ce5e42..83247bb52 100644
--- a/apps/web/src/app/workspaces/workspaces.component.html
+++ b/apps/web/src/app/workspaces/workspaces.component.html
@@ -1,7 +1,7 @@
 <div class="container-fluid">
   <div class="row py-2">
     <div class="col">
-      <button type="button" class="btn btn-primary" (click)="nullButtonOnClick()">Launch null capability [null -g]</button>
+      <button type="button" class="btn btn-primary" (click)="downloadButtonOnClick()">Launch null capability [null -g]</button>
     </div>
   </div>
 </div>
diff --git a/apps/web/src/app/workspaces/workspaces.component.ts b/apps/web/src/app/workspaces/workspaces.component.ts
index 1d58b5a51..c7ef91ffc 100644
--- a/apps/web/src/app/workspaces/workspaces.component.ts
+++ b/apps/web/src/app/workspaces/workspaces.component.ts
@@ -1,7 +1,7 @@
-import {Component, OnInit} from "@angular/core";
-import {CapabilityLauncherService} from "./services/capability-launcher.service";
-import {CapabilityRequest} from "./model/capability-request";
-import {CapabilityExecution} from "./model/capability-execution";
+import { Component, OnInit } from "@angular/core";
+import { CapabilityLauncherService } from "./services/capability-launcher.service";
+import { CapabilityRequest } from "./model/capability-request";
+import { CapabilityExecution } from "./model/capability-execution";
 
 @Component({
   selector: "app-workspaces",
@@ -19,29 +19,33 @@ export class WorkspacesComponent implements OnInit {
   /**
    * OnClick method that creates a capability request for the null capability and submits it
    */
-  nullButtonOnClick(): void {
+  downloadButtonOnClick(): void {
     // Create capability request
-    // TODO: Change this to request download capability execution
-    this.capabilityLauncher.createRequest("null", "-g").subscribe(
-      (requestResponse) => {
-        this.capabilityRequests.push(requestResponse);
-        if (requestResponse.id) {
-          // Capability request created; ID found
-          const requestId = requestResponse.id;
-          // Submit capability request
-          this.capabilityLauncher.submit(requestId).subscribe(
-            (submitResponse) => {
-              this.capabilityExecutions.push(submitResponse);
-            },
-            (error) => {
-              console.log(error);
-            }
-          );
+    this.capabilityLauncher
+      .createRequest(
+        "test_download",
+        "shared/workspaces/test/test_data/spool/724126739/17A-109.sb33151331.eb33786546.57892.65940042824"
+      )
+      .subscribe(
+        (requestResponse) => {
+          this.capabilityRequests.push(requestResponse);
+          if (requestResponse.id) {
+            // Capability request created; ID found
+            const requestId = requestResponse.id;
+            // Submit capability request
+            this.capabilityLauncher.submit(requestId).subscribe(
+              (submitResponse) => {
+                this.capabilityExecutions.push(submitResponse);
+              },
+              (error) => {
+                console.log(error);
+              }
+            );
+          }
+        },
+        (error) => {
+          console.log(error);
         }
-      },
-      (error) => {
-        console.log(error);
-      }
-    );
+      );
   }
 }
diff --git a/schema/versions/e50583fc2a8e_ssa_6391_add_download_capability_and_.py b/schema/versions/e50583fc2a8e_ssa_6391_add_download_capability_and_.py
index e77646f73..57ab8c9ea 100644
--- a/schema/versions/e50583fc2a8e_ssa_6391_add_download_capability_and_.py
+++ b/schema/versions/e50583fc2a8e_ssa_6391_add_download_capability_and_.py
@@ -1,4 +1,4 @@
-"""SSA-6391: add download capability and workflow
+"""SSA-6391: add test_download capability and workflow
 
 Revision ID: e50583fc2a8e
 Revises: 21dc67216082
@@ -18,25 +18,25 @@ depends_on = None
 def upgrade():
     op.execute(
         "INSERT INTO capabilities (capability_name, capability_steps, max_jobs) "
-        "VALUES ('download', 'await-product\nprepare-and-run-workflow download\nawait-workflow', 2)"
+        "VALUES ('test_download', 'prepare-and-run-workflow deliver -r,-l\nawait-workflow', 2)"
     )
-    op.execute("INSERT INTO workflows (workflow_name) VALUES ('download')")
+    op.execute("INSERT INTO workflows (workflow_name) VALUES ('test_download')")
     op.execute(
         "INSERT INTO workflow_templates (workflow_name, filename, content) "
-        "VALUES ('download', 'download.condor', E'executable = download.sh\narguments = {{arguments}}"
-        "\nerror = download.err\nlog = download.log\n\n\nqueue')"
+        "VALUES ('test_download', 'test_download.condor', E'executable = test_download.sh\narguments = {{arguments}}"
+        "\nerror = test_download.err\nlog = test_download.log\n\n\nqueue')"
     )
     op.execute(
         "INSERT INTO workflow_templates (workflow_name, filename, content) VALUES "
-        "('download', 'download.sh', E'#!/bin/sh\n\ndatafetcher --product-locator $1\ndeliver -r .\n')"
+        "('test_download', 'test_download.sh', E'#!/bin/sh\n\ndatafetcher --product-locator $1\ndeliver -r .\n')"
     )
     op.execute(
         "INSERT INTO workflow_templates (Workflow_name, filename, content)"
-        "VALUES ('download', 'download.dag', E'JOB download download.condor')"
+        "VALUES ('test_download', 'test_download.dag', E'JOB test_download test_download.condor')"
     )
 
 
 def downgrade():
-    op.execute("DELETE FROM capabilities WHERE capability_name = 'download'")
-    op.execute("DELETE FROM workflows WHERE workflow_name = 'download'")
-    op.execute("DELETE FROM workflow_templates WHERE workflow_name = 'download'")
+    op.execute("DELETE FROM capabilities WHERE capability_name = 'test_download'")
+    op.execute("DELETE FROM workflows WHERE workflow_name = 'test_download'")
+    op.execute("DELETE FROM workflow_templates WHERE workflow_name = 'test_download'")
diff --git a/shared/workspaces/test/test_capability_steps.py b/shared/workspaces/test/test_capability_steps.py
index 1ab24abe3..f758654c7 100644
--- a/shared/workspaces/test/test_capability_steps.py
+++ b/shared/workspaces/test/test_capability_steps.py
@@ -13,15 +13,15 @@ def test_parse_workflow():
     """
     Tests that CapabilityStep parsing works as intended
     """
-    step = CapabilityStep.from_str("prepare-and-run-workflow foo bar")
+    step = CapabilityStep.from_str("prepare-and-run-workflow foo [bar]")
     assert isinstance(step, PrepareAndRunWorkflow)
-    step = CapabilityStep.from_str("await-qa foo bar")
+    step = CapabilityStep.from_str("await-qa foo [bar]")
     assert isinstance(step, AwaitQa)
-    step = CapabilityStep.from_str("await-workflow foo bar")
+    step = CapabilityStep.from_str("await-workflow foo [bar]")
     assert isinstance(step, AwaitWorkflow)
-    step = CapabilityStep.from_str("await-product foo bar")
+    step = CapabilityStep.from_str("await-product foo [bar]")
     assert isinstance(step, AwaitProduct)
-    step = CapabilityStep.from_str("await-parameter foo bar")
+    step = CapabilityStep.from_str("await-parameter foo [bar]")
     assert isinstance(step, AwaitParameter)
-    step = CapabilityStep.from_str("await-large-alloc-approval foo bar")
+    step = CapabilityStep.from_str("await-large-alloc-approval foo [bar]")
     assert isinstance(step, AwaitLargeAllocationApproval)
diff --git a/shared/workspaces/workspaces/capability/enums.py b/shared/workspaces/workspaces/capability/enums.py
index 7244de245..35427ed25 100644
--- a/shared/workspaces/workspaces/capability/enums.py
+++ b/shared/workspaces/workspaces/capability/enums.py
@@ -14,6 +14,7 @@ class CapabilityStepType(Enum):
     AwaitProduct = 3
     AwaitParameter = 4
     AwaitLargeAllocApproval = 5
+    Invalid = -1
 
     @classmethod
     def from_string(cls, string: str) -> CapabilityStepType:
@@ -31,7 +32,7 @@ class CapabilityStepType(Enum):
             "await-parameter": cls.AwaitParameter,
             "await-large-alloc-approval": cls.AwaitLargeAllocApproval,
         }
-        return strings[string]
+        return strings.get(string, cls.Invalid)
 
 
 class CapabilityEventType(Enum):
diff --git a/shared/workspaces/workspaces/capability/helpers.py b/shared/workspaces/workspaces/capability/helpers.py
index 13b42bc83..8f4e74ee3 100644
--- a/shared/workspaces/workspaces/capability/helpers.py
+++ b/shared/workspaces/workspaces/capability/helpers.py
@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import json
+import re
 from typing import Iterator, List, Optional
 
 from workspaces.capability.enums import CapabilityStepType
@@ -13,6 +14,10 @@ from workspaces.capability.schema_interfaces import CapabilityExecutionIF
 from workspaces.capability.services.interfaces import CapabilityEngineIF
 
 
+class MalformedCapabilityStep(ValueError):
+    pass
+
+
 class CapabilityStep(CapabilityStepIF):
     """
     Class that represents a step in a capability sequence. A step is of a certain type and
@@ -31,25 +36,63 @@ class CapabilityStep(CapabilityStepIF):
         self.step_value = step_value
         self.step_args = step_args
 
+    @staticmethod
+    def parse_args(args_string: str) -> Optional[List[str]]:
+        """
+        Parse a string of the form [arg-1, arg_2, -a 3], etc. into a list of those args
+
+        :param args_string: Args as a string
+        :return: List of given args or None if no args given
+        """
+        args_string = args_string.replace("[", "").replace("]", "")
+        return args_string.split(", ") if args_string else None
+
     @classmethod
-    def from_str(cls, step_string: str):
+    def from_str(cls, step_string: str) -> CapabilityStep:
         """
         Create CapabilityStep from string containing space-separated type, value, and args
 
         :param step_string: String of capability step, e.g. "PrepareAndRunWorkflow null"
         :return: CapabilityStep of given string
         """
-        step_list = step_string.split(" ")
-        step_type = CapabilityStepType.from_string(step_list[0])
-        if step_type is CapabilityStepType.PrepareAndRunWorkflow:
-            step_value = step_list[1]
-            step_args = step_list[2] if len(step_list) == 3 else None
+        # === Regexes ===
+        # Matches a string with letters, digits, underscores, and hyphens, labeled steptype
+        # Ex: prepare-and-run-workflow, await-qa
+        r_step_type = r"(?P<steptype>[\w-]+)"
+        # Matches a string with letters, digits, underscores, and these specials: [./:], labeled stepval
+        # Ex: cal://alma/..., workflow_name, null
+        r_step_val = r"(?P<stepval>[\w\-./:]+)"
+        # Matches a string surrounded by [], with 0 or more of the same string as above (plus spaces), separated by
+        # ", " inside them
+        # Ex: [-f, -l path/to/location, arg_value], [arg], []
+        r_step_args = r"(?P<stepargs>\[(?:[\w\-./: ]+(?:, )?)*\])"
+        # Matches a string taking the form of a full capability step
+        # Ex: prepare-and-run-workflow null [-g], prepare-and-run-workflow download, await-qa
+        r_step = rf"^{r_step_type} ?{r_step_val}? ?{r_step_args}?$"
+
+        if match := re.search(r_step, step_string):
+            groups = match.groupdict()
+            step_type = groups.get("steptype", None)
+            step_value = groups.get("stepval", None)
+            step_args = groups.get("stepargs", None)
+            if step_type:
+                # Translate step_type from str to CapabilityStepType
+                step_type = CapabilityStepType.from_string(step_type)
+
+                if step_type is CapabilityStepType.Invalid:
+                    raise MalformedCapabilityStep(f"Step type {step_type} invalid.")
+            else:
+                raise MalformedCapabilityStep(
+                    f"Capability step {step_string} is malformed. Step type not found when it is required."
+                )
+            if step_args:
+                step_args = cls.parse_args(step_args)
+            return cls.TYPES[step_type](step_type, step_value, step_args)
         else:
-            # Any other step type
-            step_value = step_list[1] if len(step_list) == 2 else None
-            step_args = None
-
-        return cls.TYPES[step_type](step_type, step_value, step_args)
+            # Step string not well-formatted
+            raise MalformedCapabilityStep(
+                f"Capability step {step_string} is malformed."
+            )
 
     def __str__(self):
         string = f"{self.step_type.name}"
@@ -122,7 +165,7 @@ class PrepareAndRunWorkflow(CapabilityStep):
             # DO NOT TAKE THIS OUT! Python will yell at you.
             parameters = json.dumps(parameters)
             workflow_args = json.loads(parameters)
-        engine.submit_workflow_request(execution.id, workflow_name, workflow_args, files)
+        engine.submit_workflow_request(workflow_name, workflow_args, files)
 
 
 class AwaitQa(CapabilityStep):
diff --git a/shared/workspaces/workspaces/capability/test/test_helpers.py b/shared/workspaces/workspaces/capability/test/test_helpers.py
new file mode 100644
index 000000000..38aafc6f2
--- /dev/null
+++ b/shared/workspaces/workspaces/capability/test/test_helpers.py
@@ -0,0 +1,68 @@
+import pytest
+
+from workspaces.capability.enums import CapabilityStepType
+from workspaces.capability.helpers import CapabilityStep, MalformedCapabilityStep
+
+
+def test_capability_step_from_str():
+    """
+    Tests that a capability step can be correctly parsed from a string
+    """
+    step_with_val_and_args = "prepare-and-run-workflow deliver [-r, -l ., shared/workspaces/test/test_data/spool/724126739/17A-109.sb33151331.eb33786546.57892.65940042824]"
+    step_with_val_and_one_arg = "prepare-and-run-workflow null [-g]"
+    step_with_val_and_empty_args = "prepare-and-run-workflow workflow []"
+    step_with_val = "prepare-and-run-workflow name_of_workflow"
+    step_only_type = "await-qa"
+
+    # Step of the form "step-type step-value [step_arg1, ...]"
+    step_with_val_and_args = CapabilityStep.from_str(step_with_val_and_args)
+    assert step_with_val_and_args.step_type is CapabilityStepType.PrepareAndRunWorkflow
+    assert step_with_val_and_args.step_value == "deliver"
+    assert step_with_val_and_args.step_args == [
+        "-r",
+        "-l .",
+        "shared/workspaces/test/test_data/spool/724126739/17A-109.sb33151331.eb33786546.57892.65940042824",
+    ]
+
+    # Step of the form "step-type step-value [step_arg]"
+    step_with_val_and_one_arg = CapabilityStep.from_str(step_with_val_and_one_arg)
+    assert (
+        step_with_val_and_one_arg.step_type is CapabilityStepType.PrepareAndRunWorkflow
+    )
+    assert step_with_val_and_one_arg.step_value == "null"
+    assert step_with_val_and_one_arg.step_args == ["-g"]
+
+    # Step of the form "step-type step-value []"
+    step_with_val_and_empty_args = CapabilityStep.from_str(step_with_val_and_empty_args)
+    assert (
+        step_with_val_and_empty_args.step_type
+        is CapabilityStepType.PrepareAndRunWorkflow
+    )
+    assert step_with_val_and_empty_args.step_value == "workflow"
+    assert step_with_val_and_empty_args.step_args is None
+
+    # Step of the form "step-type step-value"
+    step_with_val = CapabilityStep.from_str(step_with_val)
+    assert step_with_val.step_type is CapabilityStepType.PrepareAndRunWorkflow
+    assert step_with_val.step_value == "name_of_workflow"
+    assert step_with_val.step_args is None
+
+    # Step of the form "step-type"
+    step_only_type = CapabilityStep.from_str(step_only_type)
+    assert step_only_type.step_type is CapabilityStepType.AwaitQA
+    assert step_only_type.step_value is None
+    assert step_only_type.step_args is None
+
+
+def test_capability_step_from_str_error():
+    """
+    Tests that an invalid capability step strings will correctly throw an error
+    """
+    step_empty = ""
+    step_bad_format = "!step.type step$value arg1 arg2 arg3"
+    step_invalid_type = "invalid-step-type step-value [arg1, arg2]"
+
+    with pytest.raises(MalformedCapabilityStep):
+        CapabilityStep.from_str(step_empty)
+        CapabilityStep.from_str(step_bad_format)
+        CapabilityStep.from_str(step_invalid_type)
-- 
GitLab