Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: propagate Subscriber deletion to Device Groups #299

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions configapi/api_sub_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"strings"

"github.com/gin-gonic/gin"
"github.com/omec-project/openapi/models"
Expand Down Expand Up @@ -268,9 +269,12 @@ func GetSubscribers(c *gin.Context) {
logger.WebUILog.Infoln("Get All Subscribers List")

var subsList []configmodels.SubsListIE
subsList = make([]configmodels.SubsListIE, 0)
amDataList, errGetMany := dbadapter.CommonDBClient.RestfulAPIGetMany(amDataColl, bson.M{})
if errGetMany != nil {
logger.DbLog.Warnln(errGetMany)
logger.DbLog.Errorw("failed to retrieve subscribers list", "error", errGetMany)
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to retrieve subscribers list"})
return
}
for _, amData := range amDataList {

Expand Down Expand Up @@ -483,17 +487,61 @@ func DeleteSubscriberByID(c *gin.Context) {

ueId := c.Param("ueId")

c.JSON(http.StatusNoContent, gin.H{})
imsi := strings.TrimPrefix(ueId, "imsi-")
err := updateSubscriberInDeviceGroups(imsi)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "error deleting subscriber"})
return
}

msg := configmodels.ConfigMessage{
MsgType: configmodels.Sub_data,
MsgMethod: configmodels.Delete_op,
Imsi: ueId,
}
configChannel <- &msg
c.JSON(http.StatusNoContent, gin.H{})
logger.WebUILog.Infoln("Delete Subscriber Data complete")
}

func updateSubscriberInDeviceGroups(imsi string) error {
filterByImsi := bson.M{
"imsis": imsi,
Comment on lines +508 to +509
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to validate the existence of imsi here.

}
rawDeviceGroups, err := dbadapter.CommonDBClient.RestfulAPIGetMany(devGroupDataColl, filterByImsi)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In RestfulAPIGetMany var resultArray []map[string]interface{} initial value is nil. According to cur, err := collection.Find(ctx, filter), if filter does not match it can stay as nil and it can return rawDeviceGroups as nil and err as nil.
In line 509, as we only check the err is nil or not, execution continue.
On line 513, iterating over rawDeviceGroups which is nil may cause panic.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial value of var resultArray []map[string]interface{} is not nil but []. Actually, there is a test (TestSubscriberDeleteNoDeviceGroup) where the IMSI to be deleted does not belong to any device group and it does not panic. From the debugger: []map[string]interface {}=[]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Please discard this comment.

if err != nil {
logger.DbLog.Errorf("failed to fetch device groups: %v", err)
return err
}
var deviceGroupUpdateMessages []configmodels.ConfigMessage
for _, rawDeviceGroup := range rawDeviceGroups {
var deviceGroup configmodels.DeviceGroups
if err = json.Unmarshal(configmodels.MapToByte(rawDeviceGroup), &deviceGroup); err != nil {
logger.DbLog.Errorf("error unmarshaling device group: %v", err)
return err
}
filteredImsis := []string{}
for _, currImsi := range deviceGroup.Imsis {
if currImsi != imsi {
filteredImsis = append(filteredImsis, currImsi)
}
}
deviceGroup.Imsis = filteredImsis
deviceGroupUpdateMessage := configmodels.ConfigMessage{
MsgType: configmodels.Device_group,
MsgMethod: configmodels.Post_op,
DevGroupName: deviceGroup.DeviceGroupName,
DevGroup: &deviceGroup,
}
deviceGroupUpdateMessages = append(deviceGroupUpdateMessages, deviceGroupUpdateMessage)
}
for _, msg := range deviceGroupUpdateMessages {

This comment was marked as outdated.

configChannel <- &msg
logger.WebUILog.Infof("device group [%v] update sent to config channel", msg.DevGroupName)
}
return nil
}

func GetRegisteredUEContext(c *gin.Context) {
setCorsHeader(c)

Expand Down
Loading
Loading