Merge pull request #795 in YUNIONIO/onecloud from ~ZHOUYOUSONG/onecloud:bugfix/yousong-appsrv to release/2.4.0

* commit 'dfbd27fd3a9518176c9a547f0a51fd584c83374c': appsrv: remove debug code appsrv: use copy() for copying slice appsrv: rewrite TestRadixMatchParams() appsrv: fix TestWorkerManagerError() appsrv: fix fmt verb appsrv: use RWMutex for rootLock cronman: 改正app context key appname拼写
2026-10-12 23:38:30 +08:00 · 2018-12-20 10:28:29 +08:00
parent 2c5f7270b0 dfbd27fd3a
commit 4d727c4348
6 changed files with 102 additions and 66 deletions
@@ -26,7 +26,7 @@ type Application struct {
 	session           *SWorkerManager
 	systemSession     *SWorkerManager
 	roots             map[string]*RadixNode
-	rootLock          *sync.Mutex
+	rootLock          *sync.RWMutex
 	connMax           int
 	idleTimeout       time.Duration
 	readTimeout       time.Duration
@@ -54,7 +54,7 @@ func NewApplication(name string, connMax int) *Application {
 		session:           NewWorkerManager("HttpRequestWorkerManager", connMax, DEFAULT_BACKLOG),
 		systemSession:     NewWorkerManager("InternalHttpRequestWorkerManager", 1, DEFAULT_BACKLOG),
 		roots:             make(map[string]*RadixNode),
-		rootLock:          &sync.Mutex{},
+		rootLock:          &sync.RWMutex{},
 		idleTimeout:       DEFAULT_IDLE_TIMEOUT,
 		readTimeout:       DEFAULT_READ_TIMEOUT,
 		readHeaderTimeout: DEFAULT_READ_HEADER_TIMEOUT,
@@ -85,21 +85,19 @@ func (app *Application) GetName() string {
 	return app.name
 }

-func (app *Application) getRootLocked(method string) *RadixNode {
-	app.rootLock.Lock()
-	defer app.rootLock.Unlock()
-	if _, ok := app.roots[method]; !ok {
-		app.roots[method] = NewRadix()
-	}
-	return app.roots[method]
-}
-
 func (app *Application) getRoot(method string) *RadixNode {
+	app.rootLock.RLock()
 	if v, ok := app.roots[method]; ok {
+		app.rootLock.RUnlock()
 		return v
-	} else {
-		return app.getRootLocked(method)
 	}
+	app.rootLock.RUnlock()
+
+	v := NewRadix()
+	app.rootLock.Lock()
+	app.roots[method] = v
+	app.rootLock.Unlock()
+	return v
 }

 func (app *Application) AddReverseProxyHandler(prefix string, ef *proxy.SEndpointFactory) {
@@ -114,7 +112,6 @@ func (app *Application) AddHandler(method string, prefix string, handler func(co
 }

 func (app *Application) AddHandler2(method string, prefix string, handler func(context.Context, http.ResponseWriter, *http.Request), metadata map[string]interface{}, name string, tags map[string]string) {
-	log.Debugf("%s - %s", method, prefix)
 	segs := SplitPath(prefix)
 	hi := newHandlerInfo(method, segs, handler, metadata, name, tags)
 	app.AddHandler3(hi)
@@ -63,9 +63,7 @@ func (r *RadixNode) add(path []string, segments []string, data interface{}) erro
 		} else {
 			r.data = data
 			r.fullPath = make([]string, len(path))
-			for i := 0; i < len(path); i += 1 {
-				r.fullPath[i] = path[i]
-			}
+			copy(r.fullPath, path)
 			return nil
 		}
 	}
@@ -1,6 +1,7 @@
 package appsrv

 import (
+	"strings"
 	"testing"
 )

@@ -59,23 +60,60 @@ func TestRadixNode(t *testing.T) {
 	}
 }

-func TestParams(t *testing.T) {
+func TestRadixMatchParams(t *testing.T) {
 	r := NewRadix()
 	r.Add([]string{"POST", "clouds", "<cls_action>"}, "classAction")
 	r.Add([]string{"POST", "clouds", "<resid>", "sync"}, "objectSyncAction")
 	r.Add([]string{"POST", "clouds", "<resid>", "<obj_action>"}, "objectAction")
-	params := make(map[string]string)
-	ret := r.Match([]string{"POST", "clouds", "myid", "sync"}, params)
-	t.Logf("match: %s", ret)
-	t.Logf("params: %s", params)
-
-	params = make(map[string]string)
-	ret = r.Match([]string{"POST", "clouds", "myid", "start"}, params)
-	t.Logf("match: %s", ret)
-	t.Logf("params: %s", params)
-
-	params = make(map[string]string)
-	ret = r.Match([]string{"POST", "clouds", "sync"}, params)
-	t.Logf("match: %s", ret)
-	t.Logf("params: %s", params)
+	cases := []struct {
+		in        []string
+		out       interface{}
+		outParams map[string]string
+	}{
+		{
+			in:  []string{"POST", "clouds", "myid", "sync"},
+			out: "objectSyncAction",
+			outParams: map[string]string{
+				"<resid>": "myid",
+			},
+		},
+		{
+			in:  []string{"POST", "clouds", "myid", "start"},
+			out: "objectAction",
+			outParams: map[string]string{
+				"<resid>":      "myid",
+				"<obj_action>": "start",
+			},
+		},
+		{
+			in:  []string{"POST", "clouds", "start"},
+			out: "classAction",
+			outParams: map[string]string{
+				"<cls_action>": "start",
+			},
+		},
+	}
+	for _, c := range cases {
+		t.Run(strings.Join(c.in, "_"), func(t *testing.T) {
+			gotParams := map[string]string{}
+			got := r.Match(c.in, gotParams)
+			if got != c.out {
+				t.Fatalf("want %s, got %s", c.out, c.out)
+			}
+			if len(gotParams) != len(c.outParams) {
+				t.Fatalf("params length mismatch\nwant %#v\ngot %#v",
+					c.outParams, gotParams,
+				)
+			}
+			for k, v := range gotParams {
+				v1 := c.outParams[k]
+				if v == v1 {
+					continue
+				}
+				t.Fatalf("params key %s has mismatch value\nwant %#v\ngot %#v",
+					k, c.outParams, gotParams,
+				)
+			}
+		})
+	}
 }
@@ -160,7 +160,7 @@ func (wm *SWorkerManager) Run(task func(), worker chan *SWorker, onErr func(erro
 	if ret {
 		wm.schedule()
 	} else {
-		log.Warningf("[%] BUSY fail to push task, queue is FULL")
+		log.Warningf("queue full, task dropped")
 	}
 	return ret
 }
@@ -247,19 +247,3 @@ func WorkerStatsHandler(ctx context.Context, w http.ResponseWriter, r *http.Requ
 	result.Add(jsonutils.Marshal(&stats), "workers")
 	fmt.Fprintf(w, result.String())
 }
-
-func WaitChannel(ch chan interface{}) interface{} {
-	var ret interface{}
-	stop := false
-	for !stop {
-		select {
-		case c, more := <-ch:
-			if more {
-				ret = c
-			} else {
-				stop = true
-			}
-		}
-	}
-	return ret
-}
@@ -1,6 +1,7 @@
 package appsrv

 import (
+	"sync"
 	"testing"
 	"time"
 )
@@ -27,21 +28,39 @@ func TestWorkerManager(t *testing.T) {

 func TestWorkerManagerError(t *testing.T) {
 	wm := NewWorkerManager("testwm", 2, 10)
-	err := make(chan interface{})
-	wm.Run(func() {
-		panic("Panic inside worker")
-	}, nil, err)
-	e := WaitChannel(err)
-	if e == nil {
-		t.Error("Panic not captured")
+	errCbFactory := func(wg *sync.WaitGroup, errMark *bool) func(error) {
+		return func(error) {
+			defer wg.Done()
+			if errMark != nil && !*errMark {
+				*errMark = true
+			}
+		}
 	}
-	err = make(chan interface{})
-	wm.Run(func() {
-		time.Sleep(1 * time.Second)
-	}, nil, err)
-	e = WaitChannel(err)
-	if e != nil {
-		t.Error("Should no error")
-	}
-
+	t.Run("normal", func(t *testing.T) {
+		wg := &sync.WaitGroup{}
+		errMark := false
+		errCb := errCbFactory(wg, &errMark)
+		wg.Add(1)
+		wm.Run(func() {
+			defer wg.Done()
+		}, nil, errCb)
+		wg.Wait()
+		if errMark {
+			t.Errorf("should be normal")
+		}
+	})
+	t.Run("panic", func(t *testing.T) {
+		wg := &sync.WaitGroup{}
+		errMark := false
+		errCb := errCbFactory(wg, &errMark)
+		wg.Add(2) // 1 for errCb
+		wm.Run(func() {
+			defer wg.Done()
+			panic("panic inside worker")
+		}, nil, errCb)
+		wg.Wait()
+		if !errMark {
+			t.Errorf("expecting error")
+		}
+	})
 }
@@ -196,7 +196,7 @@ func (job *SCronJob) runJob(isStart bool) {

 	log.Debugf("Cron job: %s started", job.Name)
 	ctx := context.Background()
-	ctx = context.WithValue(ctx, appctx.APP_CONTEXT_KEY_APPNAME, "Region-Corn-Service")
+	ctx = context.WithValue(ctx, appctx.APP_CONTEXT_KEY_APPNAME, "Region-Cron-Service")
 	userCred := auth.AdminCredential()
 	job.job(ctx, userCred, isStart)
 }