From a7bfb2fb6f2e84e62c28dcff2c41bc2e8a2f9466 Mon Sep 17 00:00:00 2001
From: wyike <77846369+wangyikewxgm@users.noreply.github.com>
Date: Wed, 14 Apr 2021 15:48:23 +0800
Subject: [PATCH] refactor appRevison extract func to utils (#1481)

* refactor appRevison extract func to utils

* add some corener case check

* add more checks
---
 pkg/dsl/process/handle.go      | 11 +++-----
 pkg/dsl/process/handle_test.go | 26 -------------------
 pkg/oam/util/helper.go         | 15 +++++++++++
 pkg/oam/util/helper_test.go    | 47 ++++++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 33 deletions(-)

diff --git a/pkg/dsl/process/handle.go b/pkg/dsl/process/handle.go
index 0cef899fb14..cadae997a17 100644
--- a/pkg/dsl/process/handle.go
+++ b/pkg/dsl/process/handle.go
@@ -25,6 +25,7 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/oam-dev/kubevela/pkg/dsl/model"
+	"github.com/oam-dev/kubevela/pkg/oam/util"
 )
 
 const (
@@ -161,7 +162,9 @@ func (ctx *templateContext) BaseContextFile() string {
 	buff += fmt.Sprintf(ContextName+": \"%s\"\n", ctx.name)
 	buff += fmt.Sprintf(ContextAppName+": \"%s\"\n", ctx.appName)
 	buff += fmt.Sprintf(ContextAppRevision+": \"%s\"\n", ctx.appRevision)
-	buff += fmt.Sprintf(ContextAppRevisionNum+": %s\n", extractRevisionNum(ctx.appRevision))
+	// the appRevision is generated by vela,  the error always is nil, so ignore it
+	revNum, _ := util.ExtractRevisionNum(ctx.appRevision)
+	buff += fmt.Sprintf(ContextAppRevisionNum+": %d\n", revNum)
 	buff += fmt.Sprintf(ContextNamespace+": \"%s\"\n", ctx.namespace)
 
 	if ctx.base != nil {
@@ -257,9 +260,3 @@ func structMarshal(v string) string {
 	}
 	return fmt.Sprintf("{%s}", v)
 }
-
-func extractRevisionNum(appRevision string) string {
-	app := strings.Split(appRevision, "-")
-	vision := app[len(app)-1]
-	return strings.Replace(vision, "v", "", 1)
-}
diff --git a/pkg/dsl/process/handle_test.go b/pkg/dsl/process/handle_test.go
index b8eca3cf8eb..d3edf3ea59a 100644
--- a/pkg/dsl/process/handle_test.go
+++ b/pkg/dsl/process/handle_test.go
@@ -111,29 +111,3 @@ image: "myserver"
 	assert.Equal(t, nil, err)
 	assert.Equal(t, "{\"password\":\"123\"}", string(requiredSecrets))
 }
-
-func TestExtractRevisionNum(t *testing.T) {
-	testcases := []struct {
-		appRevision     string
-		wantRevisionNum string
-	}{{
-		appRevision:     "myapp-v1",
-		wantRevisionNum: "1",
-	}, {
-		appRevision:     "new-app-v2",
-		wantRevisionNum: "2",
-	}, {
-		appRevision:     "v1-v10",
-		wantRevisionNum: "10",
-	}, {
-		appRevision:     "v10-v1-v1",
-		wantRevisionNum: "1",
-	}, {
-		appRevision:     "myapp-v1-v2",
-		wantRevisionNum: "2",
-	}}
-
-	for _, tt := range testcases {
-		assert.Equal(t, tt.wantRevisionNum, extractRevisionNum(tt.appRevision))
-	}
-}
diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go
index b39acf7dbd4..6cd3ffa5153 100644
--- a/pkg/oam/util/helper.go
+++ b/pkg/oam/util/helper.go
@@ -24,6 +24,7 @@ import (
 	"hash/fnv"
 	"os"
 	"reflect"
+	"strconv"
 	"strings"
 	"time"
 
@@ -721,6 +722,20 @@ func ConvertComponentDef2WorkloadDef(dm discoverymapper.DiscoveryMapper, compone
 	return nil
 }
 
+// ExtractRevisionNum  extract revision number from appRevision name
+func ExtractRevisionNum(appRevision string) (int, error) {
+	splits := strings.Split(appRevision, "-")
+	// check some bad appRevision name, eg:v1, appv2
+	if len(splits) == 1 {
+		return 0, fmt.Errorf("bad revison name")
+	}
+	// check some bad appRevision name, eg:myapp-a1
+	if !strings.HasPrefix(splits[len(splits)-1], "v") {
+		return 0, fmt.Errorf("bad revison name")
+	}
+	return strconv.Atoi(strings.TrimPrefix(splits[len(splits)-1], "v"))
+}
+
 // Min for int
 func Min(a, b int) int {
 	if a < b {
diff --git a/pkg/oam/util/helper_test.go b/pkg/oam/util/helper_test.go
index 50c9f219880..8b4d73e999d 100644
--- a/pkg/oam/util/helper_test.go
+++ b/pkg/oam/util/helper_test.go
@@ -2054,3 +2054,50 @@ spec:
 	assert.Equal(t, expectWd.Spec.Status, wd.Spec.Status)
 	assert.Equal(t, expectWd.Spec.Schematic, wd.Spec.Schematic)
 }
+
+func TestExtractRevisionNum(t *testing.T) {
+	testcases := []struct {
+		appRevision     string
+		wantRevisionNum int
+		hasError        bool
+	}{{
+		appRevision:     "myapp-v1",
+		wantRevisionNum: 1,
+		hasError:        false,
+	}, {
+		appRevision:     "new-app-v2",
+		wantRevisionNum: 2,
+		hasError:        false,
+	}, {
+		appRevision:     "v1-v10",
+		wantRevisionNum: 10,
+		hasError:        false,
+	}, {
+		appRevision:     "v10-v1-v1",
+		wantRevisionNum: 1,
+		hasError:        false,
+	}, {
+		appRevision:     "myapp-v1-v2",
+		wantRevisionNum: 2,
+		hasError:        false,
+	}, {
+		appRevision:     "myapp-v1-vv",
+		wantRevisionNum: 0,
+		hasError:        true,
+	}, {
+		appRevision:     "v1",
+		wantRevisionNum: 0,
+		hasError:        true,
+	}, {
+		appRevision:     "myapp-a1",
+		wantRevisionNum: 0,
+		hasError:        true,
+	}}
+
+	for _, tt := range testcases {
+		revision, err := util.ExtractRevisionNum(tt.appRevision)
+		hasError := err != nil
+		assert.Equal(t, tt.wantRevisionNum, revision)
+		assert.Equal(t, tt.hasError, hasError)
+	}
+}