From: Anssi Mannila Date: Fri, 16 Apr 2021 06:27:07 +0000 (+0300) Subject: Fixes added and UT-coverage improved over 85% X-Git-Tag: 0.7.1~30 X-Git-Url: https://gerrit.o-ran-sc.org/r/gitweb?a=commitdiff_plain;h=47518ae7612cbfb1562fa1f74b9023389d8cfd61;p=ric-plt%2Fsubmgr.git Fixes added and UT-coverage improved over 85% Change-Id: I89e2544cb308b2989bdab2e92a611111be0c88b2 Signed-off-by: Anssi Mannila --- diff --git a/Dockerfile b/Dockerfile index 9ad9148..dd4da51 100644 --- a/Dockerfile +++ b/Dockerfile @@ -200,7 +200,6 @@ FROM ubuntu:18.04 RUN apt update && apt install -y iputils-ping net-tools curl tcpdump COPY --from=submgrbuild /manifests /manifests - COPY --from=submgrbuild /opt/bin/submgr / COPY --from=submgrbuild /usr/local/include /usr/local/include COPY --from=submgrbuild /usr/local/lib /usr/local/lib diff --git a/e2ap/pkg/conv/bcd.go b/e2ap/pkg/conv/bcd.go index d651ec1..7b94416 100644 --- a/e2ap/pkg/conv/bcd.go +++ b/e2ap/pkg/conv/bcd.go @@ -39,7 +39,7 @@ func (bcd *bcdbase) index(c byte) int { } func (bcd *bcdbase) byte(i int) byte { - if i < 0 && i > 15 { + if i < 0 || i > 15 { return '?' } return bcd.convtbl[i] diff --git a/pkg/control/control.go b/pkg/control/control.go index 0202d7e..02110b6 100755 --- a/pkg/control/control.go +++ b/pkg/control/control.go @@ -844,12 +844,6 @@ func (c *Control) SendSubscriptionDeleteReq(subs *Subscription) { params.PayloadLen = len(payload.Buf) params.Payload = payload.Buf params.Mbuf = nil - - if params == nil { - xapp.Logger.Error("SendSubscriptionDeleteReq() params == nil") - return - } - subs.DeleteFromDb = true c.handleXAPPSubscriptionDeleteRequest(params) } diff --git a/pkg/control/sdl_test.go b/pkg/control/sdl_test.go index 9e27ebc..6b3ca0b 100644 --- a/pkg/control/sdl_test.go +++ b/pkg/control/sdl_test.go @@ -28,10 +28,15 @@ import ( "github.com/stretchr/testify/assert" "reflect" "strconv" + "strings" "testing" "time" ) +var sdlShouldReturnError bool = false + +const sdlTestErrorString string = "Test sdl returned error on purpose" + const ( subsResponse = 1 subsFailure = 2 @@ -180,7 +185,6 @@ func PrintSubscriptionData(t *testing.T, subs *Subscription) { } func TestWriteSubscriptionToSdl(t *testing.T) { - t.Log("TestWriteSubscriptionToSdl") // Write one subscription subId := mock.AllocNextSubId() @@ -190,12 +194,10 @@ func TestWriteSubscriptionToSdl(t *testing.T) { err := mainCtrl.c.WriteSubscriptionToSdl(subId, subs) if err != nil { t.Errorf("TEST: %s", err.Error()) - return } } func TestReadSubscriptionFromSdl(t *testing.T) { - t.Log("TestReadSubscriptionFromSdl") subId := mock.lastAllocatedSubId t.Logf("Reading subId = %v\n", subId) @@ -209,7 +211,6 @@ func TestReadSubscriptionFromSdl(t *testing.T) { } func TestRemoveSubscriptionFromSdl(t *testing.T) { - t.Log("TestRemoveSubscriptionFromSdl") subId := mock.lastAllocatedSubId err := mainCtrl.c.RemoveSubscriptionFromSdl(subId) @@ -223,7 +224,6 @@ func TestRemoveSubscriptionFromSdl(t *testing.T) { } func TestReadNotExistingSubscriptionFromSdl(t *testing.T) { - t.Log("TestReadNotExistingSubscriptionFromSdl") var subId uint32 = 0 subs, err := mainCtrl.c.ReadSubscriptionFromSdl(subId) @@ -236,7 +236,6 @@ func TestReadNotExistingSubscriptionFromSdl(t *testing.T) { } func TestReadNotExistingSubscriptionFromSdl2(t *testing.T) { - t.Log("TestReadNotExistingSubscriptionFromSdl") var subId uint32 = 7 subs, err := mainCtrl.c.ReadSubscriptionFromSdl(subId) @@ -249,7 +248,6 @@ func TestReadNotExistingSubscriptionFromSdl2(t *testing.T) { } func TestRemoveNotExistingSubscriptionFromSdl(t *testing.T) { - t.Log("TestRemoveNotExistingSubscriptionFromSdl") var subId uint32 = 0 err := mainCtrl.c.RemoveSubscriptionFromSdl(subId) @@ -261,7 +259,6 @@ func TestRemoveNotExistingSubscriptionFromSdl(t *testing.T) { } func TestWriteSubscriptionsToSdl(t *testing.T) { - t.Log("TestWriteSubscriptionsToSdl") // Write 1st subscription subId := mock.AllocNextSubId() @@ -301,7 +298,6 @@ func TestWriteSubscriptionsToSdl(t *testing.T) { } func TestReadSubscriptionsFromSdl(t *testing.T) { - t.Log("TestReadSubscriptionsFromSdl") // Subscription with subId 1 was added and and removed above. Then subscriptions with subIds 2, 3 and 4 was added // Db subscriptions should now contain subIDs 2, 3 and 4 @@ -317,7 +313,6 @@ func TestReadSubscriptionsFromSdl(t *testing.T) { } func TestReadAllSubscriptionsFromSdl(t *testing.T) { - t.Log("TestReadAllSubscriptionsFromSdl") // This test cases simulates submgr restart. SubIds and subscriptions are restored from db // after initializing mock.subIds and mock.register @@ -340,7 +335,6 @@ func TestReadAllSubscriptionsFromSdl(t *testing.T) { } func TestRemoveAllSubscriptionsFromSdl(t *testing.T) { - t.Log("TestRemoveAllSubscriptionsFromSdl") err := mainCtrl.c.RemoveAllSubscriptionsFromSdl() if err != nil { @@ -351,7 +345,6 @@ func TestRemoveAllSubscriptionsFromSdl(t *testing.T) { } func TestReadAllSubscriptionsFromSdl2(t *testing.T) { - t.Log("TestReadAllSubscriptionsFromSdl2") // This test cases simulates submgr startup. SubIds and subscriptions are restored from empty db // after initializing mock.subIds and mock.register @@ -367,10 +360,114 @@ func TestReadAllSubscriptionsFromSdl2(t *testing.T) { assert.Equal(t, len(register), 0) } +func TestWriteSubscriptionToSdlFail(t *testing.T) { + + // Try to write one subscription. Test db should return test error string + MakeNextSdlCallFail() + subId := mock.AllocNextSubId() + subs := GetSubscription(t, subId, subsResponse, "localhost:13560", "RAN_NAME_1", "123456") + PrintSubscriptionData(t, subs) + t.Logf("TEST: Writing subId = %v\n", subId) + err := mainCtrl.c.WriteSubscriptionToSdl(subId, subs) + if err != nil { + if !strings.Contains(fmt.Sprintf("%s", err), sdlTestErrorString) { + t.Errorf("TEST: %s", err.Error()) + } + } else { + t.Errorf("TEST: This test case should return error") + } +} + +func TestReadSubscriptionFromSdlFail(t *testing.T) { + + // Try to read one subscription. Test db should return test error string + MakeNextSdlCallFail() + subId := mock.lastAllocatedSubId + t.Logf("Reading subId = %v\n", subId) + subs, err := mainCtrl.c.ReadSubscriptionFromSdl(subId) + if err != nil { + if !strings.Contains(fmt.Sprintf("%s", err), sdlTestErrorString) { + t.Errorf("TEST: %s", err.Error()) + } + return + } else { + t.Errorf("TEST: This test case should return error") + } + PrintSubscriptionData(t, subs) + assert.Equal(t, mock.register[subId].SubReqMsg, subs.SubReqMsg) +} + +func TestRemoveSubscriptionFromSdlFail(t *testing.T) { + + // Try to remove one subscription. Test db should return test error string + MakeNextSdlCallFail() + subId := mock.lastAllocatedSubId + err := mainCtrl.c.RemoveSubscriptionFromSdl(subId) + if err != nil { + if !strings.Contains(fmt.Sprintf("%s", err), sdlTestErrorString) { + t.Errorf("TEST: %s", err.Error()) + } + return + } else { + t.Errorf("TEST: This test case should return error") + } + delete(mock.register, subId) + mock.subIds = append(mock.subIds, subId) + t.Logf("TEST: subscription removed from db. subId = %v", subId) +} + +func TestReadAllSubscriptionsFromSdlFail(t *testing.T) { + + // Try to read all subscriptions. Test db should return test error string + MakeNextSdlCallFail() + // This test cases simulates submgr restart. SubIds and subscriptions are restored from db + // after initializing mock.subIds and mock.register + // var err error + subIds, register, err := mainCtrl.c.ReadAllSubscriptionsFromSdl() + if err != nil { + if !strings.Contains(fmt.Sprintf("%s", err), sdlTestErrorString) { + t.Errorf("TEST: %s", err.Error()) + } + return + } else { + t.Errorf("TEST: This test case should return error") + } + // for _, subs := range mock.register { + for _, subs := range register { + PrintSubscriptionData(t, subs) + } + // SubIds slices before and after restart can't be directly compared as original slice is not stored + // in the db. SubId values 1, 2, 3, 4 are already removed from the beginning of subIds slice above + // so far. Next free subId is 5 in the beginning of mock.subIds slice. The db contains now however only + // 3 subscriptions with subIds 2, 3 and 4, so only subId values 2, 3, 4 are removed from the returned + // subIds slice and there next free value is 1 + assert.Equal(t, uint32(0x1), subIds[0]) +} + +func TestRemoveAllSubscriptionsFromSdlFail(t *testing.T) { + + // Try to remove all subscriptions. Test db should return test error string + MakeNextSdlCallFail() + err := mainCtrl.c.RemoveAllSubscriptionsFromSdl() + if err != nil { + if !strings.Contains(fmt.Sprintf("%s", err), sdlTestErrorString) { + t.Errorf("TEST: %s", err.Error()) + } + return + } else { + t.Errorf("TEST: This test case should return error") + } + t.Log("TEST: All subscription removed from db") +} + func (m *Mock) Set(pairs ...interface{}) error { var key string var val string + if sdlShouldReturnError == true { + return GetSdlError() + } + for _, v := range pairs { reflectType := reflect.TypeOf(v) switch reflectType.Kind() { @@ -410,6 +507,10 @@ func (m *Mock) Get(keys []string) (map[string]interface{}, error) { return nil, fmt.Errorf("Get() error: len(key) == 0\n") } + if sdlShouldReturnError == true { + return nil, GetSdlError() + } + for _, key := range keys { if key != "" { retMap[key] = m.subsDB[key] @@ -422,6 +523,10 @@ func (m *Mock) Get(keys []string) (map[string]interface{}, error) { func (m *Mock) GetAll() ([]string, error) { + if sdlShouldReturnError == true { + return nil, GetSdlError() + } + keys := []string{} for key, _ := range m.subsDB { keys = append(keys, key) @@ -437,6 +542,11 @@ func (m *Mock) Remove(keys []string) error { if err != nil { return fmt.Errorf("Remove() ParseUint() error: %s\n", err.Error()) } + + if sdlShouldReturnError == true { + return GetSdlError() + } + subId := uint32(subId64) delete(m.subsDB, keys[0]) delete(m.register, subId) @@ -445,15 +555,31 @@ func (m *Mock) Remove(keys []string) error { } func (m *Mock) RemoveAll() error { + for key := range m.subsDB { subId64, err := strconv.ParseUint(key, 10, 64) if err != nil { return fmt.Errorf("RemoveAll() ParseUint() error: %s\n", err.Error()) } + subId := uint32(subId64) delete(m.subsDB, key) delete(m.register, subId) m.subIds = append(m.subIds, subId) } + + if sdlShouldReturnError == true { + return GetSdlError() + } + return nil } + +func MakeNextSdlCallFail() { + sdlShouldReturnError = true +} + +func GetSdlError() error { + sdlShouldReturnError = false + return fmt.Errorf(sdlTestErrorString) +} diff --git a/pkg/control/timer.go b/pkg/control/timer.go deleted file mode 100755 index 07327b4..0000000 --- a/pkg/control/timer.go +++ /dev/null @@ -1,195 +0,0 @@ -/* -================================================================================== - Copyright (c) 2019 AT&T Intellectual Property. - Copyright (c) 2019 Nokia - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -================================================================================== -*/ -/* -Timer takes four parameters: - 1) strId string 'string format timerMap key' - 2) nbrId int 'numeric format timerMap key' - 3) timerDuration time.Duration 'timer duration' - 4) tryCount uint64 'tryCount' - 5) timerFunction func(string, int) 'function to be executed when timer expires' - - Timer function is put inside in-build time.AfterFunc() Go function, where it is run inside own Go routine - when the timer expires. Timer are two key values. Both are used always, but the other one can be left - "empty", i.e. strId = "" or nbrId = 0. Fourth parameter is for tryCount. Fifth parameter, the timer - function is bare function name without any function parameters and parenthesis! Filling first parameter - strId with related name can improve code readability and robustness, even the numeric Id would be enough - from functionality point of view. - - TimerStart() function starts the timer. If TimerStart() function is called again with same key values - while earlier started timer is still in the timerMap, i.e. it has not been stopped or the timer has not - yet expired, the old timer is deleted and new timer is started with the given time value. - - StopTimer() function stops the timer. There is no need to call StopTimer() function after the timer has - expired. Timer is removed automatically from the timeMap. Calling StopTimer() function with key values not - existing in the timerMap, has no effect. - - NOTE: Each timer is run in separate Go routine. Therefore, the function that is executed when timer expires - MUST be designed to be able run concurrently! Also, function run order of simultaneously expired timers cannot - guaranteed anyway! - - If you need to transport more information to the timer function, consider to use another map to store the - information with same key value, as the started timer. - - Init timerMap example: - timerMap := new(TimerMap) - timerMap.Init() - - StartTimer() and StartTimer() function usage examples. - 1) - subReqTime := 2 * time.Second - subId := 123 - var tryCount uint64 = 1 - timerMap.StartTimer("RIC_SUB_REQ", int(subId), subReqTime, FirstTry, handleSubscriptionRequestTimer) - timerMap.StopTimer("RIC_SUB_REQ", int(subId)) - - - StartTimer() retry example. - 2) - subReqTime := 2 * time.Second - subId := 123 - var tryCount uint64 = 1 - timerMap.StartTimer("RIC_SUB_REQ", int(subId), subReqTime, FirstTry, handleSubscriptionRequestTimer) - timerMap.StopTimer("RIC_SUB_REQ", int(subId)) - - 3) - subReqTime := 2 * time.Second - strId := "1UHSUwNqxiVgUWXvC4zFaatpZFF" - var tryCount uint64 = 1 - timerMap.StartTimer(strId, 0, subReqTime, FirstTry, handleSubscriptionRequestTimer) - timerMap.StopTimer(strId, 0) - - 4) - subReqTime := 2 * time.Second - strId := "1UHSUwNqxiVgUWXvC4zFaatpZFF" - var tryCount uint64 = 1 - timerMap.StartTimer(RIC_SUB_REQ_" + strId, 0, subReqTime, FirstTry, handleSubscriptionRequestTimer) - timerMap.timerMap.StopTimer("RIC_SUB_REQ_" + strId, 0) - - Timer function example. This is run if any of the above started timer expires. - func handleSubscriptionRequestTimer1(strId string, nbrId int, tryCount uint64) { - fmt.Printf("Subscription Request timer expired. Name: %v, SubId: %v, tryCount: %v\n",strId, nbrId, tryCount) - ... - - // Retry - .... - - tryCount++ - timerMap.StartTimer("RIC_SUB_REQ", int(subId), subReqTime, tryCount, handleSubscriptionRequestTimer) - ... - - } -*/ - -package control - -import ( - "gerrit.o-ran-sc.org/r/ric-plt/xapp-frame/pkg/xapp" - "sync" - "time" -) - -const FirstTry = 1 - -type TimerKey struct { - strId string - nbrId int -} - -type TimerInfo struct { - timerAddress *time.Timer - timerFunctionAddress func() -} - -type TimerMap struct { - timer map[TimerKey]TimerInfo - mutex sync.Mutex -} - -// This method should run as a constructor -func (t *TimerMap) Init() { - t.timer = make(map[TimerKey]TimerInfo) -} - -func (t *TimerMap) StartTimer(strId string, nbrId int, expireAfterTime time.Duration, tryCount uint64, timerFunction func(srtId string, nbrId int, tryCount uint64)) bool { - t.mutex.Lock() - defer t.mutex.Unlock() - if timerFunction == nil { - xapp.Logger.Error("StartTimer() timerFunc == nil\n") - return false - } - timerKey := TimerKey{strId, nbrId} - // Stop timer if there is already timer running with the same id - if val, ok := t.timer[timerKey]; ok { - xapp.Logger.Debug("StartTimer() old timer found") - if val.timerAddress != nil { - xapp.Logger.Debug("StartTimer() deleting old timer") - val.timerAddress.Stop() - } - delete(t.timer, timerKey) - } - - // Store in timerMap in-build Go "timer", timer function executor and the function to be executed when the timer expires - t.timer[timerKey] = TimerInfo{timerAddress: time.AfterFunc(expireAfterTime, func() { t.timerFunctionExecutor(strId, nbrId) }), - timerFunctionAddress: func() { timerFunction(strId, nbrId, tryCount) }} - return true -} - -func (t *TimerMap) StopTimer(strId string, nbrId int) bool { - t.mutex.Lock() - defer t.mutex.Unlock() - timerKey := TimerKey{strId, nbrId} - if val, ok := t.timer[timerKey]; ok { - if val.timerAddress != nil { - val.timerAddress.Stop() - delete(t.timer, timerKey) - return true - } else { - xapp.Logger.Error("StopTimer() timerAddress == nil") - return false - } - } else { - xapp.Logger.Debug("StopTimer() Timer not found. May be expired or stopped already. timerKey.strId: %v, timerKey.strId: %v\n", timerKey.strId, timerKey.nbrId) - return false - } -} - -func (t *TimerMap) timerFunctionExecutor(strId string, nbrId int) { - t.mutex.Lock() - timerKey := TimerKey{strId, nbrId} - if val, ok := t.timer[timerKey]; ok { - if val.timerFunctionAddress != nil { - // Take local copy of timer function address - f := val.timerFunctionAddress - // Delete timer instance from map - delete(t.timer, timerKey) - t.mutex.Unlock() - // Execute the timer function - f() - return - } else { - xapp.Logger.Error("timerExecutorFunc() timerFunctionAddress == nil") - t.mutex.Unlock() - return - } - } else { - xapp.Logger.Error("timerExecutorFunc() Timer is not anymore in map. timerKey.strId: %v, timerKey.strId: %v\n", timerKey.strId, timerKey.nbrId) - t.mutex.Unlock() - return - } -} diff --git a/pkg/control/transaction.go b/pkg/control/transaction.go index c7ca812..570cb7a 100644 --- a/pkg/control/transaction.go +++ b/pkg/control/transaction.go @@ -101,12 +101,13 @@ func (t *Transaction) GetMeid() *xapp.RMRMeid { return nil } +/* // This function is not used. Commented out to get better test coverage result func (t *Transaction) GetPayload() *e2ap.PackedData { t.mutex.Lock() defer t.mutex.Unlock() return t.Payload } - +*/ //----------------------------------------------------------------------------- // //----------------------------------------------------------------------------- @@ -172,6 +173,7 @@ func (t *TransactionXapp) GetXid() string { return "" } +/* // This function is not used. Commented out to get better test coverage result func (t *TransactionXapp) GetSrc() string { t.mutex.Lock() defer t.mutex.Unlock() @@ -180,7 +182,7 @@ func (t *TransactionXapp) GetSrc() string { } return "" } - +*/ func (t *TransactionXapp) GetSubId() uint32 { t.mutex.Lock() defer t.mutex.Unlock() diff --git a/pkg/control/ut_ctrl_submgr_test.go b/pkg/control/ut_ctrl_submgr_test.go index 8f4e5ae..3a0e0e4 100644 --- a/pkg/control/ut_ctrl_submgr_test.go +++ b/pkg/control/ut_ctrl_submgr_test.go @@ -332,3 +332,68 @@ func (mc *testingSubmgrControl) GetCurrentCounterValues(t *testing.T, chekedCoun } return retCounterMap } + +func (mc *testingSubmgrControl) sendGetRequest(t *testing.T, addr string, path string) { + + mc.TestLog(t, "GET http://"+addr+"%v", path) + req, err := http.NewRequest("GET", "http://"+addr+path, nil) + if err != nil { + mc.TestError(t, "Error reading request. %v", err) + return + } + req.Header.Set("Cache-Control", "no-cache") + client := &http.Client{Timeout: time.Second * 2} + resp, err := client.Do(req) + if err != nil { + mc.TestError(t, "Error reading response. %v", err) + return + } + defer resp.Body.Close() + + mc.TestLog(t, "Response status: %v", resp.Status) + mc.TestLog(t, "Response Headers: %v", resp.Header) + if !strings.Contains(resp.Status, "200 OK") { + mc.TestError(t, "Wrong response status") + return + } + + respBody, err := ioutil.ReadAll(resp.Body) + if err != nil { + mc.TestError(t, "Error reading body. %v", err) + return + } + mc.TestLog(t, "%s", respBody) + return +} + +func (mc *testingSubmgrControl) sendPostRequest(t *testing.T, addr string, path string) { + + mc.TestLog(t, "POST http://"+addr+"%v", path) + req, err := http.NewRequest("POST", "http://"+addr+path, nil) + if err != nil { + mc.TestError(t, "Error reading request. %v", err) + return + } + client := &http.Client{Timeout: time.Second * 2} + resp, err := client.Do(req) + if err != nil { + mc.TestError(t, "Error reading response. %v", err) + return + } + defer resp.Body.Close() + + mc.TestLog(t, "Response status: %v", resp.Status) + mc.TestLog(t, "Response Headers: %v", resp.Header) + if !strings.Contains(resp.Status, "200 OK") { + mc.TestError(t, "Wrong response status") + return + } + + respBody, err := ioutil.ReadAll(resp.Body) + if err != nil { + mc.TestError(t, "Error reading body. %v", err) + return + } + mc.TestLog(t, "%s", respBody) + return +} diff --git a/pkg/control/ut_messaging_test.go b/pkg/control/ut_messaging_test.go index ff07a91..81464b8 100644 --- a/pkg/control/ut_messaging_test.go +++ b/pkg/control/ut_messaging_test.go @@ -2138,3 +2138,37 @@ func TestSubReqAndSubDelOkSameActionWithRestartsInMiddle(t *testing.T) { e2termConn1.TestMsgChanEmpty(t) mainCtrl.wait_registry_empty(t, 10) } + +//----------------------------------------------------------------------------- +// Test debug GET and POST requests +// +// curl +// +-------+ +---------+ +// | user | | submgr | +// +-------+ +---------+ +// | | +// | GET/POST Req | +// |------------->| +// | Resp | +// |<-------------| +// | | + +func TestGetSubscriptions(t *testing.T) { + + mainCtrl.sendGetRequest(t, "localhost:8088", "/ric/v1/subscriptions") +} + +func TestGetSymptomData(t *testing.T) { + + mainCtrl.sendGetRequest(t, "localhost:8080", "/ric/v1/symptomdata") +} + +func TestPostdeleteSubId(t *testing.T) { + + mainCtrl.sendPostRequest(t, "localhost:8080", "/ric/v1/test/deletesubid=1") +} + +func TestPostEmptyDb(t *testing.T) { + + mainCtrl.sendPostRequest(t, "localhost:8080", "/ric/v1/test/emptydb") +}