Skip to content

Commit

Permalink
Return ygot strcut from app module processGet() functions (sonic-net#72)
Browse files Browse the repository at this point in the history 
* Return ygot strcut from app module processGet() functions

* Updated the appInterface method processGet() to accept a
  TranslibFmtType argument indicating whether the response data should
  be returned as IETF json bytes or a ygot.GoStruct object. Translib
  subscription infra will use TranslibFmtType=ygot. The translib.Get()
  API will continue to use TranslibFmtType=ietf_json.

* Modified processGet() method signature in all existing app modules.
  All of them will call the common generateGetResponse() utility func
  which builds the response data as per TranslibFmtType input.
  Removed all direct dumpIetfJson() calls from app modules.

* GetResponse struct contains both []byte and ygot.GoStruct fields.
  One of them will be populated based on the TranslibFmtType input.

* Added TranslibFmtType field to GetRequest also. This can be used in
  future to support protocol specific response encodings like protobuf
  encoding.

Signed-off-by: Sachin Holla <sachin.holla@broadcom.com>

* Minor code cleanup to address review comments

---------

Signed-off-by: Sachin Holla <sachin.holla@broadcom.com>
  • Loading branch information
@sachinholla
sachinholla authored May 18, 2023
1 parent 80df301 commit 72e2cec
Show file tree
Hide file tree
Showing 13 changed files with 107 additions and 134 deletions.
9 changes: 2 additions & 7 deletions translib/acl_app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (app *AclApp) processDelete(d *db.DB) (SetResponse, error) {
return resp, err
}

func (app *AclApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
func (app *AclApp) processGet(dbs [db.MaxDB]*db.DB, fmtType TranslibFmtType) (GetResponse, error) {
var err error
var payload []byte

Expand All @@ -204,12 +204,7 @@ func (app *AclApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
return GetResponse{Payload: payload, ErrSrc: AppErr}, err
}

payload, err = generateGetResponsePayload(app.pathInfo.Path, (*app.ygotRoot).(*ocbinds.Device), app.ygotTarget)
if err != nil {
return GetResponse{Payload: payload, ErrSrc: AppErr}, err
}

return GetResponse{Payload: payload}, err
return generateGetResponse(app.pathInfo.Path, app.ygotRoot, fmtType)
}

func (app *AclApp) processAction(dbs [db.MaxDB]*db.DB) (ActionResponse, error) {
Expand Down
2 changes: 1 addition & 1 deletion translib/api_tests_app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (app *apiTests) processDelete(d *db.DB) (SetResponse, error) {
return app.processSet()
}

func (app *apiTests) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
func (app *apiTests) processGet(dbs [db.MaxDB]*db.DB, fmtType TranslibFmtType) (GetResponse, error) {
var gr GetResponse
err := app.getError()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion translib/app_interface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type appInterface interface {
processUpdate(d *db.DB) (SetResponse, error)
processReplace(d *db.DB) (SetResponse, error)
processDelete(d *db.DB) (SetResponse, error)
processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error)
processGet(dbs [db.MaxDB]*db.DB, fmtType TranslibFmtType) (GetResponse, error)
processAction(dbs [db.MaxDB]*db.DB) (ActionResponse, error)
processSubscribe(req processSubRequest) (processSubResponse, error)
}
Expand Down
34 changes: 21 additions & 13 deletions translib/app_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package translib
import (
"reflect"
"strings"

"github.com/Azure/sonic-mgmt-common/translib/db"
"github.com/Azure/sonic-mgmt-common/translib/ocbinds"
"github.com/Azure/sonic-mgmt-common/translib/tlerr"
Expand Down Expand Up @@ -67,16 +68,19 @@ func getYangPathFromYgotStruct(s ygot.GoStruct, yangPathPrefix string, appModule
return ""
}

func generateGetResponsePayload(targetUri string, deviceObj *ocbinds.Device, ygotTarget *interface{}) ([]byte, error) {
func generateGetResponse(targetUri string, root *ygot.GoStruct, fmtType TranslibFmtType) (GetResponse, error) {
var err error
var payload []byte
var resp GetResponse

if root == nil {
return resp, tlerr.InvalidArgs("ygotRoot not specified")
}
if len(targetUri) == 0 {
return payload, tlerr.InvalidArgs("GetResponse failed as target Uri is not valid")
return resp, tlerr.InvalidArgs("GetResponse failed as target Uri is not valid")
}
path, err := ygot.StringToPath(targetUri, ygot.StructuredPath, ygot.StringSlicePath)
path, err := ygot.StringToPath(targetUri, ygot.StructuredPath)
if err != nil {
return payload, tlerr.InvalidArgs("URI to path conversion failed: %v", err)
return resp, tlerr.InvalidArgs("URI to path conversion failed: %v", err)
}

// Get current node (corresponds to ygotTarget) and its parent node
Expand All @@ -92,21 +96,21 @@ func generateGetResponsePayload(targetUri string, deviceObj *ocbinds.Device, ygo
parentPath.Elem = append(parentPath.Elem, pathList[i])
}
}
parentNodeList, err := ytypes.GetNode(ygSchema.RootSchema(), deviceObj, parentPath)
parentNodeList, err := ytypes.GetNode(ygSchema.RootSchema(), *root, parentPath)
if err != nil {
return payload, err
return resp, err
}
if len(parentNodeList) == 0 {
return payload, tlerr.InvalidArgs("Invalid URI: %s", targetUri)
return resp, tlerr.InvalidArgs("Invalid URI: %s", targetUri)
}
parentNode := parentNodeList[0].Data

currentNodeList, err := ytypes.GetNode(ygSchema.RootSchema(), deviceObj, path, &(ytypes.GetPartialKeyMatch{}))
currentNodeList, err := ytypes.GetNode(ygSchema.RootSchema(), *root, path, &(ytypes.GetPartialKeyMatch{}))
if err != nil {
return payload, err
return resp, err
}
if len(currentNodeList) == 0 {
return payload, tlerr.NotFound("Resource not found")
return resp, tlerr.NotFound("Resource not found")
}
//currentNode := currentNodeList[0].Data
currentNodeYangName := currentNodeList[0].Schema.Name
Expand Down Expand Up @@ -137,9 +141,13 @@ func generateGetResponsePayload(targetUri string, deviceObj *ocbinds.Device, ygo
}
}

payload, err = dumpIetfJson(parentCloneObj, true)
if fmtType == TRANSLIB_FMT_YGOT {
resp.ValueTree = parentCloneObj
} else {
resp.Payload, err = dumpIetfJson(parentCloneObj, true)
}

return payload, err
return resp, err
}

func getTargetNodeYangSchema(targetUri string, deviceObj *ocbinds.Device) (*yang.Entry, error) {
Expand Down
36 changes: 29 additions & 7 deletions translib/app_utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,37 @@ func TestMain(m *testing.M) {

func processGetRequest(url string, expectedRespJson string, errorCase bool) func(*testing.T) {
return func(t *testing.T) {
response, err := Get(GetRequest{Path: url})
if err != nil && !errorCase {
t.Fatalf("Error %v received for Url: %s", err, url)
}
t.Run("ietf_json", func(t *testing.T) {
verifyGet(t, GetRequest{Path: url}, expectedRespJson, errorCase)
})
t.Run("ygot", func(t *testing.T) {
verifyGet(t, GetRequest{Path: url, FmtType: TRANSLIB_FMT_YGOT}, expectedRespJson, errorCase)
})
}
}

respJson := response.Payload
if string(respJson) != expectedRespJson {
t.Fatalf("Response for Url: %s received is not expected:\n%s", url, string(respJson))
func verifyGet(t *testing.T, req GetRequest, expJson string, expError bool) {
t.Helper()
response, err := Get(req)
if expError {
if err == nil {
t.Fatalf("GET %s did not return an error", req.Path)
}
return
}
var respJson []byte
if req.FmtType == TRANSLIB_FMT_YGOT && response.ValueTree != nil {
respJson, err = dumpIetfJson(response.ValueTree, true)
if err != nil {
t.Fatalf("GET %s returned invalid YGOT. error=%v", req.Path, err)
}
} else if req.FmtType == TRANSLIB_FMT_IETF_JSON {
respJson = response.Payload
}
if j := string(respJson); j != expJson {
t.Errorf("GET %s returned invalid response", req.Path)
t.Errorf("Expected: %s", expJson)
t.Fatalf("Received: %s", j)
}
}

Expand Down
8 changes: 2 additions & 6 deletions translib/common_app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func (app *CommonApp) processDelete(d *db.DB) (SetResponse, error) {
return resp, err
}

func (app *CommonApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
func (app *CommonApp) processGet(dbs [db.MaxDB]*db.DB, fmtType TranslibFmtType) (GetResponse, error) {
var err error
var payload []byte
var resPayload []byte
Expand Down Expand Up @@ -277,11 +277,7 @@ func (app *CommonApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
}
}
if resYgot != nil {
resPayload, err = generateGetResponsePayload(app.pathInfo.Path, resYgot.(*ocbinds.Device), app.ygotTarget)
if err != nil {
log.Warning("generateGetResponsePayload() couldn't generate payload.")
resPayload = payload
}
return generateGetResponse(app.pathInfo.Path, &resYgot, fmtType)
} else {
resPayload = payload
}
Expand Down
48 changes: 9 additions & 39 deletions translib/intf_app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func (app *IntfApp) processDelete(d *db.DB) (SetResponse, error) {
}

/* Note : Registration already happened, followed by filling the internal DS and filling the JSON */
func (app *IntfApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
func (app *IntfApp) processGet(dbs [db.MaxDB]*db.DB, fmtType TranslibFmtType) (GetResponse, error) {

var err error
var payload []byte
Expand Down Expand Up @@ -339,12 +339,8 @@ func (app *IntfApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
return GetResponse{Payload: payload, ErrSrc: AppErr}, e
}

payload, err = dumpIetfJson(oc_val, false)
if err == nil {
return GetResponse{Payload: payload}, err
} else {
return GetResponse{Payload: payload, ErrSrc: AppErr}, err
}
ifInfo.State = oc_val
continue
}

/* Filling the counter Info to internal DS */
Expand All @@ -365,12 +361,8 @@ func (app *IntfApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
return GetResponse{Payload: payload, ErrSrc: AppErr}, e
}

payload, err = dumpIetfJson(counter_val, false)
if err == nil {
return GetResponse{Payload: payload}, err
} else {
return GetResponse{Payload: payload, ErrSrc: AppErr}, err
}
ifInfo.State = &ocbinds.OpenconfigInterfaces_Interfaces_Interface_State{Counters: counter_val}
continue
}

/* Filling Interface IP info to internal DS */
Expand All @@ -385,28 +377,10 @@ func (app *IntfApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
ygot.BuildEmptyTree(ifInfo.State)
}
app.convertInternalToOCIntfInfo(&ifKey, ifInfo)
if *app.ygotTarget == ifInfo {
payload, err = dumpIetfJson(intfObj, false)
} else {
dummyifInfo := &ocbinds.OpenconfigInterfaces_Interfaces_Interface{}
counter_all_val := &ocbinds.OpenconfigInterfaces_Interfaces_Interface_State{}
if *app.ygotTarget == ifInfo.Config {
dummyifInfo.Config = ifInfo.Config
payload, err = dumpIetfJson(dummyifInfo, false)
} else if *app.ygotTarget == ifInfo.State {
dummyifInfo.State = ifInfo.State
payload, err = dumpIetfJson(dummyifInfo, false)
} else if *app.ygotTarget == ifInfo.State.Counters {
counter_all_val.Counters = ifInfo.State.Counters
payload, err = dumpIetfJson(counter_all_val, false)
} else {
log.Info("Not supported get type!")
err = errors.New("Requested get-type not supported!")
}
}
}
}
return GetResponse{Payload: payload}, err

return generateGetResponse(pathInfo.Path, app.ygotRoot, fmtType)
}

/* Get all Interfaces */
Expand Down Expand Up @@ -442,13 +416,9 @@ func (app *IntfApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
ygot.BuildEmptyTree(ifInfo)
app.convertInternalToOCIntfInfo(&ifKey, ifInfo)
}
if *app.ygotTarget == intfObj {
payload, err = dumpIetfJson((*app.ygotRoot).(*ocbinds.Device), true)
} else {
log.Error("Wrong request!")
}
}
return GetResponse{Payload: payload}, err

return generateGetResponse(pathInfo.Path, app.ygotRoot, fmtType)
}

func (app *IntfApp) processAction(dbs [db.MaxDB]*db.DB) (ActionResponse, error) {
Expand Down
24 changes: 6 additions & 18 deletions translib/lldp_app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,18 @@ func (app *lldpApp) processDelete(d *db.DB) (SetResponse, error) {
return resp, err
}

func (app *lldpApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
func (app *lldpApp) processGet(dbs [db.MaxDB]*db.DB, fmtType TranslibFmtType) (GetResponse, error) {
var err error
var payload []byte

app.appDb = dbs[db.ApplDB]
lldpIntfObj := app.getAppRootObject()

targetUriPath, err := getYangPathFromUri(app.path.Path)
if err != nil {
return GetResponse{}, err
}

log.Info("lldp processGet")
log.Info("targetUriPath: ", targetUriPath)

Expand All @@ -205,12 +209,6 @@ func (app *lldpApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
}
ygot.BuildEmptyTree(oneIfInfo)
app.getLldpNeighInfoFromInternalMap(&ifname, oneIfInfo)
if *app.ygotTarget == lldpIntfObj.Interfaces {
payload, err = dumpIetfJson(lldpIntfObj, true)
} else {
log.Info("Wrong request!")
}

}
} else if targetUriPath == "/openconfig-lldp:lldp/interfaces/interface" {
intfObj := lldpIntfObj.Interfaces
Expand All @@ -222,23 +220,13 @@ func (app *lldpApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
ifInfo := intfObj.Interface[ifname]
ygot.BuildEmptyTree(ifInfo)
app.getLldpNeighInfoFromInternalMap(&ifname, ifInfo)

if *app.ygotTarget == intfObj.Interface[ifname] {
payload, err = dumpIetfJson(intfObj, true)
if err != nil {
log.Info("Creation of subinterface subtree failed!")
return GetResponse{Payload: payload, ErrSrc: AppErr}, err
}
} else {
log.Info("Wrong request!")
}
}
} else {
log.Info("No data")
}
}

return GetResponse{Payload:payload}, err
return generateGetResponse(app.path.Path, app.ygotRoot, fmtType)
}

func (app *lldpApp) processAction(dbs [db.MaxDB]*db.DB) (ActionResponse, error) {
Expand Down
Loading

0 comments on commit 72e2cec

Please sign in to comment.