fix issue openpitrix#75 openpitrix#81 openpitrix#82 openpitrix#69 ope…

…npitrix#84

Makefile

@@ -18,8 +18,9 @@ ETCD_TEST:=NOTIFICATION_ETCD_UNIT_TEST=1 NOTIFICATION_ETCD_ENDPOINTS=127.0.0.1:1
 
 DOCKER_TAGS=latest
 BUILDER_IMAGE=openpitrix/notification-builder:v1.0.0
-#RUN_IN_DOCKER:=docker run -it -v $(GOPATH)/pkg/mod:/go/pkg/mod -v `pwd`:/go/src/$(TRAG.Gopkg) -v `pwd`/tmp/cache:/root/.cache/go-build  -w /go/src/$(TRAG.Gopkg) -e GOBIN=/go/src/$(TRAG.Gopkg)/tmp/bin -e USER_ID=`id -u` -e GROUP_ID=`id -g` $(BUILDER_IMAGE)
-RUN_IN_DOCKER:=docker run -it -v `pwd`:/go/src/$(TRAG.Gopkg) -v `pwd`/tmp/cache:/root/.cache/go-build  -w /go/src/$(TRAG.Gopkg) -e GOBIN=/go/src/$(TRAG.Gopkg)/tmp/bin -e USER_ID=`id -u` -e GROUP_ID=`id -g` $(BUILDER_IMAGE)
+#use local go mod lib
+RUN_IN_DOCKER:=docker run -it -v $(GOPATH)/pkg/mod:/go/pkg/mod -v `pwd`:/go/src/$(TRAG.Gopkg) -v `pwd`/tmp/cache:/root/.cache/go-build  -w /go/src/$(TRAG.Gopkg) -e GOBIN=/go/src/$(TRAG.Gopkg)/tmp/bin -e USER_ID=`id -u` -e GROUP_ID=`id -g` $(BUILDER_IMAGE)
+#RUN_IN_DOCKER:=docker run -it -v `pwd`:/go/src/$(TRAG.Gopkg) -v `pwd`/tmp/cache:/root/.cache/go-build  -w /go/src/$(TRAG.Gopkg) -e GOBIN=/go/src/$(TRAG.Gopkg)/tmp/bin -e USER_ID=`id -u` -e GROUP_ID=`id -g` $(BUILDER_IMAGE)
 
 define get_diff_files
     $(eval DIFF_FILES=$(shell git diff --name-only --diff-filter=ad | grep -e "^(cmd|pkg)/.+\.go"))

api/notification.proto

@@ -20,15 +20,13 @@ message CreateNotificationRequest {
 	google.protobuf.StringValue content = 3;
 	//notification short content
 	google.protobuf.StringValue short_content = 4;
-	//notification expired days,  0 is for never expired.
+	//notification expired days, 0 is for never expired
 	google.protobuf.UInt32Value expired_days = 5;
 	//notification owner
 	google.protobuf.StringValue owner = 6;
-	//the address to send the notification, json fmt, currently support 2 kinds types.
-	//1.key/value Type, key is the notification send Type,the value is the list of address,
-	//eg:{"email": ["[email protected]", "[email protected]"],"websocket": ["system", "user1"]}
-	//2.Array Type, eg:["adl-xxxx1", "adl-xxxx2"],
-	//the item is the address list id which is already created in advance.
+	//The addresses to receive the notification,json fmt, currently support 2 kinds fmt types.
+	//1.key/value Type, key is the notification send Type,the value is the list of address, eg:{"email":["[email protected]","[email protected]"],"websocket":["system", "user1"]}
+	//2.Array Type, eg:["adl-xxxx1","adl-xxxx2"], the item is the address list id which is already created in advance
 	google.protobuf.StringValue address_info = 7;
 	//the available start time to receive notification
 	google.protobuf.StringValue available_start_time = 8;
@@ -65,9 +63,9 @@ message Notification {
 	google.protobuf.Timestamp status_time = 9;
 	//the owner of the notification
 	google.protobuf.StringValue owner = 10;
-	//the address to send the notification, json fmt, currently support 2 kinds types.
-	//1.key/value Type, key is the notification send Type,the value is the list of address, eg:{"email": ["openpitrix@163.com", "openpitrix@163.com"],"websocket": ["system", "jo"]}
-	//2.Array Type, eg:["adl-xxxx1", "adl-xxxx2"], the item is the address list id which is already created in advance.
+	//The addresses to receive the notification,json fmt, currently support 2 kinds fmt types.
+	//1.key/value Type, key is the notification send Type,the value is the list of address, eg:{"email":["xxx1@163.com","xxx2@163.com"],"websocket":["system", "user1"]}
+	//2.Array Type, eg:["adl-xxxx1","adl-xxxx2"], the item is the address list id which is already created in advance
 	google.protobuf.StringValue address_info = 11;
 	//the available start time to receive notification
 	google.protobuf.StringValue available_start_time = 12;
@@ -177,7 +175,7 @@ message RetryTasksResponse {
 }
 
 message CreateAddressRequest {
-	//required, address details, could be email address for email, user id for websocket, mobile number for sms.
+	//required, address details, could be email address for email, user id for websocket, mobile number for sms
 	google.protobuf.StringValue address = 1;
 	//remarks for address
 	google.protobuf.StringValue remarks = 2;
@@ -197,7 +195,7 @@ message DescribeAddressesRequest {
 	repeated string address_id = 1;
 	//address list id
 	repeated string address_list_id = 2;
-	//address details, could be email address for email, user id for websocket, mobile number for sms.
+	//address details, could be email address for email, user id for websocket, mobile number for sms
 	repeated string address = 3;
 	//the notification type , eg:[email|websocket|sms|wechat]
 	repeated string notify_type = 4;
@@ -222,7 +220,7 @@ message Address {
 	google.protobuf.StringValue address_id = 1;
 	//address list id
 	google.protobuf.StringValue address_list_id = 2;
-	//address details, could be email address for email, user id for websocket, mobile number for sms.
+	//address details, could be email address for email, user id for websocket, mobile number for sms
 	google.protobuf.StringValue address = 3;
 	//remarks for address
 	google.protobuf.StringValue remarks = 4;
@@ -250,7 +248,7 @@ message DescribeAddressesResponse {
 message ModifyAddressRequest {
 	//required, address id
 	string address = 1;
-	//address details, could be email address for email, user id for websocket, mobile number for sms.
+	//address details, could be email address for email, user id for websocket, mobile number for sms
 	google.protobuf.StringValue address_detail = 2;
 	//remarks for address
 	google.protobuf.StringValue remarks = 3;
@@ -337,15 +335,15 @@ message DescribeAddressListResponse {
 }
 
 message ModifyAddressListRequest {
-	//required, address list id
+	//required,address list id
 	string addresslist = 1;
 	//address list name
 	google.protobuf.StringValue address_list_name = 2;
 	//the extra info of the address list
 	google.protobuf.StringValue extra = 3;
 	//address list status, eg:[active|disabled|deleted]
 	google.protobuf.StringValue status = 4;
-	//address ids of the address list
+	//address ids which the address list includes
 	repeated string address_id = 5;
 }
 
@@ -355,7 +353,7 @@ message ModifyAddressListResponse {
 }
 
 message DeleteAddressListRequest {
-	//required, address list id
+	//required,address list id
 	repeated string address_list_id = 1;
 }
 
@@ -431,7 +429,7 @@ message MessageDetail {
 
 
 service notification {
-	//Create one notification
+	//Create one notification.
 	rpc CreateNotification (CreateNotificationRequest) returns (CreateNotificationResponse) {
 		option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
 			summary: "Create one notification."
@@ -452,20 +450,20 @@ service notification {
 		};
 	}
 
-	//Retry notifications
+	//Retry notifications.
 	rpc RetryNotifications (RetryNotificationsRequest) returns (RetryNotificationsResponse) {
 		option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
-			summary: "Retry notifications"
+			summary: "Retry notifications."
 		};
 		option (google.api.http) = {
 			post: "/v1/notifications/retry"
 			body: "*"
 		};
 	}
-	//Describe tasks
+	//Describe tasks.
 	rpc DescribeTasks (DescribeTasksRequest) returns (DescribeTasksResponse) {
 		option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
-			summary: "Describe tasks"
+			summary: "Describe tasks."
 		};
 		option (google.api.http) = {
 			get: "/v1/tasks"
@@ -481,7 +479,7 @@ service notification {
 			body: "*"
 		};
 	}
-	//Create one address
+	//Create one address.
 	rpc CreateAddress (CreateAddressRequest) returns (CreateAddressResponse) {
 		option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
 			summary: "Create one address."
@@ -500,7 +498,7 @@ service notification {
 			get: "/v1/addresses"
 		};
 	}
-	//Modify one address
+	//Modify one address.
 	rpc ModifyAddress (ModifyAddressRequest) returns (ModifyAddressResponse) {
 		option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
 			summary: "Modify one address."
@@ -539,7 +537,7 @@ service notification {
 			get: "/v1/addresslists"
 		};
 	}
-	//Modify one address list
+	//Modify one address list.
 	rpc ModifyAddressList (ModifyAddressListRequest) returns (ModifyAddressListResponse) {
 		option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
 			summary: "Modify one address list."
@@ -549,7 +547,7 @@ service notification {
 			body: "*"
 		};
 	}
-	//Delete multiple address lists
+	//Delete multiple address lists.
 	rpc DeleteAddressList (DeleteAddressListRequest) returns (DeleteAddressListResponse) {
 		option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
 			summary: "Delete multiple address lists."
@@ -559,30 +557,30 @@ service notification {
 			body: "*"
 		};
 	}
-	//Set service configration
+	//Set service configration.
 	rpc SetServiceConfig (ServiceConfig) returns (SetServiceConfigResponse) {
 		option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
-			summary: "Set service configration"
+			summary: "Set service configration."
 		};
 		option (google.api.http) = {
 			post: "/v1/serviceconfigs"
 			body: "*"
 		};
 	}
-	//Get service configration
+	//Get service configration.
 	rpc GetServiceConfig (GetServiceConfigRequest) returns (ServiceConfig) {
 		option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
-			summary: "Get service configration"
+			summary: "Get service configration."
 		};
 		option (google.api.http) = {
 			get: "/v1/serviceconfigs"
 		};
 	}
 
-	//Validate email service
+	//Validate email service.
 	rpc ValidateEmailService(ServiceConfig)  returns (ValidateEmailServiceResponse){
 	option (grpc.gateway.protoc_gen_swagger.options.openapiv2_operation) = {
-			summary: "Validate email service"
+			summary: "Validate email service."
 		};
 	option (google.api.http) = {
 			post: "/v1/serviceconfigs/validation"

pkg/config/config.go

@@ -57,8 +57,8 @@ type Config struct {
 	}
 
 	Websocket struct {
-		//Service string `default:"op,ks"`
-		Service string `default:"none"`
+		Service string `default:"op,ks"`
+		//Service string `default:"none"`
 	}
 }
 

pkg/gerr/message.go

@@ -188,4 +188,9 @@ var (
 		en:   "illegal item in list [%s]",
 		zhCN: "列表[%s]中某些元素不合法",
 	}
+	ErrorExtraIsNotJsonFmt = ErrorMessage{
+		Name: "is_not_json_fmt",
+		en:   "is not json fmt [%s]",
+		zhCN: "不是JSON格式[%s]",
+	}
 )

pkg/services/notification/handler.go

@@ -400,7 +400,7 @@ func (s *Server) ModifyAddress(ctx context.Context, req *pb.ModifyAddressRequest
 
 func (s *Server) DeleteAddresses(ctx context.Context, req *pb.DeleteAddressesRequest) (*pb.DeleteAddressesResponse, error) {
 	addressIds := stringutil.SimplifyStringList(req.AddressId)
-	addressIds = removeDuplicatesAndEmpty(addressIds)
+	addressIds = stringutil.Unique(addressIds)
 	err := rs.DeleteAddresses(ctx, addressIds)
 	if err != nil {
 		logger.Errorf(ctx, "Failed to delete addresses [%+v], %+v.", addressIds, err)
@@ -413,21 +413,11 @@ func (s *Server) DeleteAddresses(ctx context.Context, req *pb.DeleteAddressesReq
 	}, nil
 
 }
-func removeDuplicatesAndEmpty(a []string) (ret []string) {
-	a_len := len(a)
-	for i := 0; i < a_len; i++ {
-		if (i > 0 && a[i-1] == a[i]) || len(a[i]) == 0 {
-			continue
-		}
-		ret = append(ret, a[i])
-	}
-	return
-}
 
 func (s *Server) CreateAddressList(ctx context.Context, req *pb.CreateAddressListRequest) (*pb.CreateAddressListResponse, error) {
 	addrList := models.NewAddressList(req)
 	addrIds := req.GetAddressId()
-	addrIds = removeDuplicatesAndEmpty(addrIds)
+	addrIds = stringutil.Unique(addrIds)
 	var addrListId string
 	var err error
 
@@ -510,7 +500,7 @@ func (s *Server) ModifyAddressList(ctx context.Context, req *pb.ModifyAddressLis
 
 func (s *Server) DeleteAddressList(ctx context.Context, req *pb.DeleteAddressListRequest) (*pb.DeleteAddressListResponse, error) {
 	addressListIds := stringutil.SimplifyStringList(req.AddressListId)
-	addressListIds = removeDuplicatesAndEmpty(addressListIds)
+	addressListIds = stringutil.Unique(addressListIds)
 	err := rs.DeleteAddressLists(ctx, addressListIds)
 	if err != nil {
 		logger.Errorf(ctx, "Failed to delete address lists [%+v], %+v.", addressListIds, err)

pkg/services/notification/resource_control/address.go

@@ -92,8 +92,8 @@ func DeleteAddresses(ctx context.Context, addressIds []string) error {
 	addrs, err := GetAddressesByIds(ctx, tx, addressIds)
 	if len(addrs) != len(addressIds) {
 		tx.Rollback()
-		logger.Errorf(ctx, "Failed to delete address [%s], address does not exits, %+v.", addressIds, err)
-		err := gerr.NewWithDetail(ctx, gerr.NotFound, err, gerr.ErrorNotExistItemInList, strings.Trim(fmt.Sprint(addressIds), "[]"))
+		logger.Errorf(ctx, "Failed to delete address[%s], some address to be deleted does not exits, %+v.", addressIds, err)
+		err = gerr.NewWithDetail(ctx, gerr.InvalidArgument, err, gerr.ErrorNotExistItemInList, addressIds)
 		return err
 	}
 

pkg/services/notification/resource_control/address_list.go

@@ -149,8 +149,8 @@ func DeleteAddressLists(ctx context.Context, addressListIds []string) error {
 	addrLists, err := GetAddressListsByIds(ctx, tx, addressListIds)
 	if len(addrLists) != len(addressListIds) {
 		tx.Rollback()
-		err := gerr.NewWithDetail(ctx, gerr.NotFound, err, gerr.ErrorNotExistItemInList, addressListIds)
-		logger.Errorf(ctx, "Failed to delete address_list [%s], address_list does not exits, %+v.", addressListIds, err)
+		logger.Errorf(ctx, "Failed to delete address_list[%s], some address to be deleted does not exits, %+v.", addressListIds, err)
+		err := gerr.NewWithDetail(ctx, gerr.InvalidArgument, err, gerr.ErrorNotExistItemInList, addressListIds)
 		return err
 	}
 
@@ -196,6 +196,22 @@ func GetAddressListsByIds(ctx context.Context, tx *gorm.DB, addressListIds []str
 	return addrLists, nil
 }
 
+func GetActiveAddressesListsByIds(ctx context.Context, addressListIds []string) ([]*models.AddressList, error) {
+	var addrLists []*models.AddressList
+	db := global.GetInstance().GetDB()
+
+	err := db.Table(models.TableAddressList).
+		Select("*").
+		Where(models.AddrLsColStatus+" in ( '"+constants.StatusActive+"' )").
+		Where(models.AddrLsColId+" in ( ? )", addressListIds).
+		Scan(&addrLists).Error
+	if err != nil {
+		logger.Errorf(ctx, "Failed to get active address list by ids[%+v], %+v.", addrLists, err)
+		return nil, err
+	}
+	return addrLists, nil
+}
+
 func GetDeletedAddressListsByIds(ctx context.Context, tx *gorm.DB, addressListIds []string) ([]*models.AddressList, error) {
 	var addrLists []*models.AddressList
 	err := tx.Table(models.TableAddressList).

pkg/services/notification/validation.go

@@ -16,6 +16,8 @@ import (
 	"openpitrix.io/notification/pkg/gerr"
 	"openpitrix.io/notification/pkg/models"
 	"openpitrix.io/notification/pkg/pb"
+	rs "openpitrix.io/notification/pkg/services/notification/resource_control"
+	"openpitrix.io/notification/pkg/util/stringutil"
 )
 
 func ValidateSetServiceConfigParams(ctx context.Context, req *pb.ServiceConfig) error {
@@ -50,8 +52,8 @@ func ValidateCreateNotificationParams(ctx context.Context, req *pb.CreateNotific
 	}
 	notification := models.NewNotification(req)
 
-	//2.1 validate extra
-	err = models.CheckExtra(ctx, notification.Extra)
+	//2.1 check extra is json str
+	err = stringutil.VerifyJSONString(notification.Extra)
 	if err != nil {
 		return err
 	}
@@ -67,7 +69,8 @@ func ValidateCreateNotificationParams(ctx context.Context, req *pb.CreateNotific
 		}
 	} else {
 		//3.2 check addressInfo format is like address_info = ["adl-xxxx1", "adl-xxxx2"]
-		//if needed, also check extra fmt eg:"{"ws_service": "ks","ws_message_type": "event"}"
+		//if address_info = ["adl-xxxx1", "adl-xxxx2"],check adl exists or not
+		//todo if needed, also check extra fmt eg:"{"ws_service": "ks","ws_message_type": "event"}"
 		err = validateAddressInfo4AddressListIds(ctx, notification)
 		if err != nil {
 			return err
@@ -100,6 +103,14 @@ func validateAvaiblableTime(ctx context.Context, req *pb.CreateNotificationReque
 func validateAddressInfo4AddressListIds(ctx context.Context, notification *models.Notification) error {
 	addressListIds, err := models.DecodeAddressListIds(notification.AddressInfo)
 	if err == nil {
+		//if address_info = ["adl-xxxx1", "adl-xxxx2"],check adl ids exists in DB or not
+		*addressListIds = stringutil.Unique(*addressListIds)
+		err = checkAddrListExistInDB(ctx, *addressListIds)
+		if err != nil {
+			logger.Errorf(ctx, "Failed to validate addressInfo, some address lists in address info[%s] do not exits: %+v", notification.AddressInfo, err)
+			return gerr.New(ctx, gerr.InvalidArgument, gerr.ErrorNotExistItemInList, notification.AddressInfo)
+		}
+
 		if len(*addressListIds) == 0 {
 			logger.Errorf(ctx, "Failed to validate addressInfo, address list id is blank: %+v", err)
 			return gerr.New(ctx, gerr.InvalidArgument, gerr.ErrorIllegalNotificationAddressList, notification.AddressInfo)
@@ -120,6 +131,14 @@ func validateAddressInfo4AddressListIds(ctx context.Context, notification *model
 	return nil
 }
 
+func checkAddrListExistInDB(ctx context.Context, addressListIds []string) error {
+	addrLists, err := rs.GetActiveAddressesListsByIds(ctx, addressListIds)
+	if len(addrLists) != len(addressListIds) {
+		return err
+	}
+	return nil
+}
+
 func validateAddressInfo4AddressMap(ctx context.Context, notification *models.Notification) error {
 	addrInfoMap, err := models.DecodeAddressInfo(notification.AddressInfo)
 	if err == nil {