internal/relui, internal/workflow: add parameter type support

Previously, it was only possible to create workflow parameters with the implicit "string" type hard-coded. Some workflows we're creating either require or will benefit from more flexibility in parameter types (slices of strings, single-line vs multi-line strings, and so on). It was also not yet possible to associate metadata to parameters (such as documentation, example values). This change implements that flexibility for workflow parameters, and uses it to better document the existing Twitter workflows. The next change will add a workflow that uses new slice types. For simplicity and clarity reasons, all parameter information is contained in one place in the workflow.Parameter struct, including some fields that control the HTML presentation of said parameters, instead of trying to factor out HTML bits into the relui package and creating a bridge between the two. Also type check in more stages of the workflow processing. For golang/go#47405. Fixes golang/go#51191. Change-Id: Ia805b3b355e65fcbf2397ad21800da448ccb826a Reviewed-on: https://go-review.googlesource.com/c/build/+/404454 Reviewed-by: Dmitri Shuralyov <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
golang · May 11, 2022 · fb638e1 · fb638e1
1 parent 5d48fa5
commit fb638e1
Show file tree

Hide file tree

Showing 10 changed files with 292 additions and 65 deletions.
diff --git a/internal/relui/listener.go b/internal/relui/listener.go
@@ -60,7 +60,7 @@ func (l *PGListener) TaskStateChanged(workflowID uuid.UUID, taskName string, sta
 }
 
 // WorkflowStarted persists a new workflow execution in the database.
-func (l *PGListener) WorkflowStarted(ctx context.Context, workflowID uuid.UUID, name string, params map[string]string) error {
+func (l *PGListener) WorkflowStarted(ctx context.Context, workflowID uuid.UUID, name string, params map[string]interface{}) error {
 	q := db.New(l.db)
 	m, err := json.Marshal(params)
 	if err != nil {

diff --git a/internal/relui/static/site.js b/internal/relui/static/site.js
@@ -17,3 +17,41 @@
 
   registerTaskListExpandListeners(".TaskList-expandableItem")
 })()
+
+// addSliceRow creates and appends a row to a slice parameter
+// for filling in an element.
+//
+// container is the parameter container element.
+// paramName is the parameter name.
+// element is the element tag to create, and inputType is its type attribute if element is "input".
+// paramExample is an example value for the parameter.
+addSliceRow = (container, paramName, element, inputType, paramExample) => {
+  /*
+    Create an input element, a button to remove it, group them in a "parameterRow" div:
+
+    <div class="NewWorkflow-parameterRow">
+      <input name="workflow.params.{{$p.Name}}" placeholder="{{paramExample}}" />
+      <button title="Remove this row from the slice." onclick="/ * Remove this row. * /">-</button>
+    </div>
+  */
+  const input = document.createElement(element)
+  input.name = "workflow.params." + paramName
+  if (element == "input") {
+    input.type = inputType
+  }
+  input.placeholder = paramExample
+  const removeButton = document.createElement("button")
+  removeButton.title = "Remove this row from the slice."
+  removeButton.addEventListener("click", (e) => {
+    e.preventDefault()
+    container.removeChild(div)
+  })
+  removeButton.appendChild(document.createTextNode("-"))
+  const div = document.createElement("div")
+  div.className = "NewWorkflow-parameterRow";
+  div.appendChild(input)
+  div.appendChild(removeButton)
+
+  // Add the "parameterRow" div to the document.
+  container.appendChild(div)
+}
diff --git a/internal/relui/static/styles.css b/internal/relui/static/styles.css
@@ -111,9 +111,24 @@ h6 {
   display: flex;
   gap: 0.5rem;
 }
-.NewWorkflow-parameter input {
+.NewWorkflow-parameter--slice {
+  flex-direction: column;
+}
+.NewWorkflow-parameterRow {
+  display: flex;
+  gap: 0.5rem;
+}
+.NewWorkflow-parameter--string input {
   flex-grow: 1;
 }
+.NewWorkflow-parameter--slice textarea {
+  font-family: inherit;
+  width: 100%;
+  height: 4rem;
+}
+.NewWorkflow-parameter--slice button {
+  font-size: 0.625rem;
+}
 .NewWorkflow-workflowCreate {
   padding-top: 0.5rem;
   border-top: 0.0625rem solid #d6d6d6;

diff --git a/internal/relui/templates/new_workflow.html b/internal/relui/templates/new_workflow.html
@@ -23,14 +23,30 @@ <h2>New Go Release</h2>
     {{if .Selected}}
       <form action="{{baseLink "/workflows/create"}}" method="post">
         <input type="hidden" id="workflow.name" name="workflow.name" value="{{$.Name}}" />
-        {{range $name := .Selected.ParameterNames}}
-          <div class="NewWorkflow-parameter">
-            <label for="workflow.params.{{$name}}">{{$name}}</label>
-            <input id="workflow.params.{{$name}}" name="workflow.params.{{$name}}" value="" />
-          </div>
+        {{range $_, $p := .Selected.Parameters}}
+          {{if eq $p.Type.String "string"}}
+            <div class="NewWorkflow-parameter NewWorkflow-parameter--string">
+              <label for="workflow.params.{{$p.Name}}" title="{{$p.Doc}}">{{$p.Name}}</label>
+              <input id="workflow.params.{{$p.Name}}" name="workflow.params.{{$p.Name}}"
+                {{- with $p.HTMLInputType}} type="{{.}}"{{end}}
+                {{- if $p.RequireNonZero}} required{{end}} placeholder="{{$p.Example}}" />
+            </div>
+          {{else if eq $p.Type.String "[]string"}}
+            <div class="NewWorkflow-parameter NewWorkflow-parameter--slice">
+              <div class="NewWorkflow-parameterRow">
+                <label title="{{$p.Doc}}">{{$p.Name}}</label>
+                <button title="Increment the slice length." onclick="event.preventDefault(); addSliceRow(this.parentElement.parentElement, '{{$p.Name}}', '{{$p.HTMLElement}}', '{{$p.HTMLInputType}}', '{{$p.Example}}');">+</button>
+              </div>
+            </div>
+          {{else}}
+            <div class="NewWorkflow-parameter">
+              <label title="{{$p.Doc}}">{{$p.Name}}</label>
+              <span>unsupported parameter type "{{$p.ParameterType}}"</span>
+            </div>
+          {{end}}
         {{end}}
         <div class="NewWorkflow-workflowCreate">
-          <input name="workflow.create" type="submit" value="Create" onclick="return confirm('This will create and immediately run this workflow.\n\nReady to proceed?')" />
+          <input name="workflow.create" type="submit" value="Create" onclick="return this.form.reportValidity() && confirm('This will create and immediately run this workflow.\n\nReady to proceed?')" />
         </div>
       </form>
     {{end}}

diff --git a/internal/relui/web.go b/internal/relui/web.go
@@ -220,14 +220,25 @@ func (s *Server) createWorkflowHandler(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
 		return
 	}
-	params := make(map[string]string)
-	for _, n := range d.ParameterNames() {
-		params[n] = r.FormValue(fmt.Sprintf("workflow.params.%s", n))
-
-		// TODO(go.dev/issue/51191): Create a better mechanism for storing parameter metadata.
-		requiredParam := !strings.HasSuffix(n, " (optional)")
-		if requiredParam && params[n] == "" {
-			http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
+	params := make(map[string]interface{})
+	for _, p := range d.Parameters() {
+		switch p.Type.String() {
+		case "string":
+			v := r.FormValue(fmt.Sprintf("workflow.params.%s", p.Name))
+			if p.RequireNonZero() && v == "" {
+				http.Error(w, fmt.Sprintf("parameter %q must have non-zero value", p.Name), http.StatusBadRequest)
+				return
+			}
+			params[p.Name] = v
+		case "[]string":
+			v := r.Form[fmt.Sprintf("workflow.params.%s", p.Name)]
+			if p.RequireNonZero() && len(v) == 0 {
+				http.Error(w, fmt.Sprintf("parameter %q must have non-zero value", p.Name), http.StatusBadRequest)
+				return
+			}
+			params[p.Name] = v
+		default:
+			http.Error(w, fmt.Sprintf("parameter %q has an unsupported type %q", p.Name, p.Type), http.StatusInternalServerError)
 			return
 		}
 	}

diff --git a/internal/relui/worker.go b/internal/relui/worker.go
@@ -22,7 +22,7 @@ import (
 type Listener interface {
 	workflow.Listener
 
-	WorkflowStarted(ctx context.Context, workflowID uuid.UUID, name string, params map[string]string) error
+	WorkflowStarted(ctx context.Context, workflowID uuid.UUID, name string, params map[string]interface{}) error
 	WorkflowFinished(ctx context.Context, workflowID uuid.UUID, outputs map[string]interface{}, err error) error
 }
 
@@ -84,7 +84,7 @@ func (w *Worker) run(wf *workflow.Workflow) error {
 }
 
 // StartWorkflow persists and starts running a workflow.
-func (w *Worker) StartWorkflow(ctx context.Context, name string, def *workflow.Definition, params map[string]string) (uuid.UUID, error) {
+func (w *Worker) StartWorkflow(ctx context.Context, name string, def *workflow.Definition, params map[string]interface{}) (uuid.UUID, error) {
 	wf, err := workflow.Start(def, params)
 	if err != nil {
 		return uuid.UUID{}, err

diff --git a/internal/relui/worker_test.go b/internal/relui/worker_test.go
@@ -34,7 +34,7 @@ func TestWorkerStartWorkflow(t *testing.T) {
 
 	wd := newTestEchoWorkflow()
 	dh.RegisterDefinition(t.Name(), wd)
-	params := map[string]string{"echo": "greetings"}
+	params := map[string]interface{}{"echo": "greetings"}
 
 	wg.Add(1)
 	wfid, err := w.StartWorkflow(ctx, t.Name(), wd, params)
@@ -217,7 +217,7 @@ func newTestEchoWorkflow() *workflow.Definition {
 	echo := func(ctx context.Context, arg string) (string, error) {
 		return arg, nil
 	}
-	wd.Output("echo", wd.Task("echo", echo, wd.Parameter("echo")))
+	wd.Output("echo", wd.Task("echo", echo, wd.Parameter(workflow.Parameter{Name: "echo"})))
 	return wd
 }
 
@@ -256,7 +256,7 @@ func (u *unimplementedListener) Logger(uuid.UUID, string) workflow.Logger {
 	return log.Default()
 }
 
-func (u *unimplementedListener) WorkflowStarted(context.Context, uuid.UUID, string, map[string]string) error {
+func (u *unimplementedListener) WorkflowStarted(context.Context, uuid.UUID, string, map[string]interface{}) error {
 	return errors.New("method WorkflowStarted not implemented")
 }
 

diff --git a/internal/relui/workflows.go b/internal/relui/workflows.go
@@ -58,32 +58,76 @@ func (h *DefinitionHolder) Definitions() map[string]*workflow.Definition {
 // RegisterTweetDefinitions registers workflow definitions involving tweeting
 // onto h, using e for the external service configuration.
 func RegisterTweetDefinitions(h *DefinitionHolder, e task.ExternalConfig) {
+	version := workflow.Parameter{
+		Name: "Version",
+		Doc: `Version is the Go version that has been released.
+
+The version string must use the same format as Go tags.`,
+	}
+	security := workflow.Parameter{
+		Name: "Security (optional)",
+		Doc: `Security is an optional sentence describing security fixes included in this release.
+
+The empty string means there are no security fixes to highlight.
+
+Past examples:
+• "Includes a security fix for crypto/tls (CVE-2021-34558)."
+• "Includes a security fix for the Wasm port (CVE-2021-38297)."
+• "Includes security fixes for encoding/pem (CVE-2022-24675), crypto/elliptic (CVE-2022-28327), crypto/x509 (CVE-2022-27536)."`,
+	}
+	announcement := workflow.Parameter{
+		Name:          "Announcement",
+		ParameterType: workflow.URL,
+		Doc: `Announcement is the announcement URL.
+
+It's applicable to all release types other than major
+(since major releases point to release notes instead).`,
+		Example: "https://groups.google.com/g/golang-announce/c/wB1fph5RpsE/m/ZGwOsStwAwAJ",
+	}
+
 	{
+		minorVersion := version
+		minorVersion.Example = "go1.18.2"
+		secondaryVersion := workflow.Parameter{
+			Name:    "SecondaryVersion",
+			Doc:     `SecondaryVersion is an older Go version that was also released.`,
+			Example: "go1.17.10",
+		}
+
 		wd := workflow.New()
 		wd.Output("TweetURL", wd.Task("tweet-minor", func(ctx *workflow.TaskContext, v1, v2, sec, ann string) (string, error) {
 			return task.TweetMinorRelease(ctx, task.ReleaseTweet{Version: v1, SecondaryVersion: v2, Security: sec, Announcement: ann}, e)
-		}, wd.Parameter("Version"), wd.Parameter("SecondaryVersion"), wd.Parameter("Security (optional)"), wd.Parameter("Announcement")))
+		}, wd.Parameter(minorVersion), wd.Parameter(secondaryVersion), wd.Parameter(security), wd.Parameter(announcement)))
 		h.RegisterDefinition("tweet-minor", wd)
 	}
 	{
+		betaVersion := version
+		betaVersion.Example = "go1.19beta1"
+
 		wd := workflow.New()
 		wd.Output("TweetURL", wd.Task("tweet-beta", func(ctx *workflow.TaskContext, v, sec, ann string) (string, error) {
 			return task.TweetBetaRelease(ctx, task.ReleaseTweet{Version: v, Security: sec, Announcement: ann}, e)
-		}, wd.Parameter("Version"), wd.Parameter("Security (optional)"), wd.Parameter("Announcement")))
+		}, wd.Parameter(betaVersion), wd.Parameter(security), wd.Parameter(announcement)))
 		h.RegisterDefinition("tweet-beta", wd)
 	}
 	{
+		rcVersion := version
+		rcVersion.Example = "go1.19rc1"
+
 		wd := workflow.New()
 		wd.Output("TweetURL", wd.Task("tweet-rc", func(ctx *workflow.TaskContext, v, sec, ann string) (string, error) {
 			return task.TweetRCRelease(ctx, task.ReleaseTweet{Version: v, Security: sec, Announcement: ann}, e)
-		}, wd.Parameter("Version"), wd.Parameter("Security (optional)"), wd.Parameter("Announcement")))
+		}, wd.Parameter(rcVersion), wd.Parameter(security), wd.Parameter(announcement)))
 		h.RegisterDefinition("tweet-rc", wd)
 	}
 	{
+		majorVersion := version
+		majorVersion.Example = "go1.19"
+
 		wd := workflow.New()
 		wd.Output("TweetURL", wd.Task("tweet-major", func(ctx *workflow.TaskContext, v, sec string) (string, error) {
 			return task.TweetMajorRelease(ctx, task.ReleaseTweet{Version: v, Security: sec}, e)
-		}, wd.Parameter("Version"), wd.Parameter("Security (optional)")))
+		}, wd.Parameter(majorVersion), wd.Parameter(security)))
 		h.RegisterDefinition("tweet-major", wd)
 	}
 }
@@ -92,8 +136,8 @@ func RegisterTweetDefinitions(h *DefinitionHolder, e task.ExternalConfig) {
 // development.
 func newEchoWorkflow() *workflow.Definition {
 	wd := workflow.New()
-	wd.Output("greeting", wd.Task("greeting", echo, wd.Parameter("greeting")))
-	wd.Output("farewell", wd.Task("farewell", echo, wd.Parameter("farewell")))
+	wd.Output("greeting", wd.Task("greeting", echo, wd.Parameter(workflow.Parameter{Name: "greeting"})))
+	wd.Output("farewell", wd.Task("farewell", echo, wd.Parameter(workflow.Parameter{Name: "farewell"})))
 	return wd
 }