From f8e6153904a5c4ed1dcf0053d1e1ada101339d7b Mon Sep 17 00:00:00 2001
From: Ahmet Alp Balkan <ahmetb@google.com>
Date: Thu, 14 Jan 2021 14:17:58 -0800
Subject: [PATCH] provide env ordering via "order" key

Introducing an optional "order" key to the env object.

The sorting takes a look at if "order" field is specified or not. If not,
the key is prompted last. If specified, it sorts them by the value of the
"order" field.

The order is still non-deterministic if env objects don't specify an
"order" value. (i.e. this patch still doesn't preserve map order).

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
---
 README.md                           |  7 ++++
 cmd/cloudshell_open/appfile.go      | 52 ++++++++++++++++++++++++-----
 cmd/cloudshell_open/appfile_test.go | 22 ++++++++++++
 3 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/README.md b/README.md
index b3584c1f..f3c52dd8 100644
--- a/README.md
+++ b/README.md
@@ -63,6 +63,10 @@ For example, a fully populated `app.json` file looks like this:
         },
         "APP_SECRET": {
             "generator": "secret"
+        },
+        "ORDERED_ENV": {
+            "description": "control the order env variables are prompted",
+            "order": 100
         }
     },
     "options": {
@@ -115,6 +119,9 @@ Reference:
   - `required`, _(optional, default: `true`)_ indicates if they user must provide
     a value for this variable.
   - `generator`, _(optional)_ use a generator for the value, currently only support `secret`
+  - `order`, _(optional)_ if specified, used to indicate the order in which the
+    variable is prompted to the user. If some variables specify this and some
+    don't, then the unspecified ones are prompted last.
 - `options`: _(optional)_ Options when deploying the service
   - `allow-unauthenticated`: _(optional, default: `true`)_ allow unauthenticated requests
   - `memory`: _(optional)_ memory for each instance
diff --git a/cmd/cloudshell_open/appfile.go b/cmd/cloudshell_open/appfile.go
index 949d80ee..52d90de2 100644
--- a/cmd/cloudshell_open/appfile.go
+++ b/cmd/cloudshell_open/appfile.go
@@ -22,6 +22,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"sort"
 
 	"github.com/fatih/color"
 
@@ -33,6 +34,7 @@ type env struct {
 	Value       string `json:"value"`
 	Required    *bool  `json:"required"`
 	Generator   string `json:"generator"`
+	Order       *int   `json:"order"`
 }
 
 type options struct {
@@ -191,7 +193,6 @@ func promptOrGenerateEnvs(list map[string]env) ([]string, error) {
 }
 
 func generateEnvs(keys []string) ([]string, error) {
-
 	for i, key := range keys {
 		resp, err := rand64String()
 		if err != nil {
@@ -203,15 +204,50 @@ func generateEnvs(keys []string) ([]string, error) {
 	return keys, nil
 }
 
-func promptEnv(list map[string]env) ([]string, error) {
-	// TODO(ahmetb): remove these defers and make customizations at the
-	// individual prompt-level once survey lib allows non-global settings.
+type envKeyValuePair struct {
+	k string
+	v env
+}
+
+type envKeyValuePairs []envKeyValuePair
+
+func (e envKeyValuePairs) Len() int { return len(e) }
+
+func (e envKeyValuePairs) Swap(i, j int) {
+	e[i], e[j] = e[j], e[i]
+}
+
+func (e envKeyValuePairs) Less(i, j int) bool {
+	// if env.Order is unspecified, it should appear less.
+	// otherwise, less values show earlier.
+	if e[i].v.Order == nil {
+		return false
+	}
+	if e[j].v.Order == nil {
+		return true
+	}
+	return *e[i].v.Order < *e[j].v.Order
+}
 
+func sortedEnvs(envs map[string]env) []string {
+	var v envKeyValuePairs
+	for key, value := range envs {
+		v = append(v, envKeyValuePair{key, value})
+	}
+	sort.Sort(v)
+	var keys []string
+	for _, vv := range v {
+		keys = append(keys, vv.k)
+	}
+	return keys
+}
+
+func promptEnv(list map[string]env) ([]string, error) {
 	var out []string
-	// TODO(ahmetb): we should ideally use an ordered map structure for Env
-	// field and prompt the questions as they appear in the app.json file as
-	// opposed to random order we do here.
-	for k, e := range list {
+	sortedKeys := sortedEnvs(list)
+
+	for _, k := range sortedKeys {
+		e := list[k]
 		var resp string
 
 		if err := survey.AskOne(&survey.Input{
diff --git a/cmd/cloudshell_open/appfile_test.go b/cmd/cloudshell_open/appfile_test.go
index 0d7bdac8..afc5bcea 100644
--- a/cmd/cloudshell_open/appfile_test.go
+++ b/cmd/cloudshell_open/appfile_test.go
@@ -233,3 +233,25 @@ func TestGetAppFile(t *testing.T) {
 		t.Fatalf("wrong parsed value: got=%#v, expected=%#v", v, expected)
 	}
 }
+
+func Test_sortedEnvs(t *testing.T) {
+	envs := map[string]env{
+		"NIL_ORDER": {},
+		"ORDER_100": {Order: mkInt(100)},
+		"ORDER_0":   {Order: mkInt(0)},
+		"ORDER_-10": {Order: mkInt(-10)},
+		"ORDER_50":  {Order: mkInt(50)},
+	}
+	got := sortedEnvs(envs)
+	expected := []string{
+		"ORDER_-10", "ORDER_0", "ORDER_50", "ORDER_100", "NIL_ORDER",
+	}
+
+	if !reflect.DeepEqual(got, expected) {
+		t.Fatalf("sorted envs in wrong order: expected:%v\ngot=%v", expected, got)
+	}
+}
+
+func mkInt(i int) *int {
+	return &i
+}