tianxiaoliang commented on a change in pull request #698:
URL: 
https://github.com/apache/servicecomb-service-center/pull/698#discussion_r495547579



##########
File path: server/service/ms/datasource.go
##########
@@ -18,14 +18,14 @@ package ms
 import (
        "context"
        pb "github.com/apache/servicecomb-service-center/pkg/registry"
-       "github.com/apache/servicecomb-service-center/server/plugin/registry"
 )
 
 type DataSource interface {
-       RegisterService(ctx context.Context, service *pb.CreateServiceRequest) 
(*registry.PluginResponse, error)
-       GetService(ctx context.Context, service *pb.GetServiceRequest)
-       UpdateService(ctx context.Context, service 
*pb.UpdateServicePropsRequest)
-       UnregisterService(ctx context.Context, service *pb.DeleteServiceRequest)
+       RegisterService(ctx context.Context, request *pb.CreateServiceRequest) 
(*pb.CreateServiceResponse, error)
+       GetServices(ctx context.Context, request *pb.GetServicesRequest) 
(*pb.GetServicesResponse, error)
+       GetService(ctx context.Context, request *pb.GetServiceRequest) 
(*pb.GetServiceResponse, error)
+       UpdateService(ctx context.Context, request 
*pb.UpdateServicePropsRequest)

Review comment:
       UpdateService,UnregisterService should return response

##########
File path: server/rest/controller/v4/microservice_controller.go
##########
@@ -120,6 +120,7 @@ func (s *MicroServiceService) Unregister(w 
http.ResponseWriter, r *http.Request)
 func (s *MicroServiceService) GetServices(w http.ResponseWriter, r 
*http.Request) {
        request := &pb.GetServicesRequest{}
        resp, _ := core.ServiceAPI.GetServices(r.Context(), request)

Review comment:
       check error

##########
File path: server/service/ms/etcd/etcd.go
##########
@@ -57,78 +52,94 @@ func (ds *DataSource) initialize() error {
 }
 
 // RegisterService() implement:
-// 1. capsule request to etcd kv format
-// 2. invoke registry interface to store service information to etcd cluster
-// Attention: parameters validation && response check must be checked outside 
the method
+// 1. request validation
+// 2. capsule request to etcd kv format
+// 3. invoke etcd client to store data
+// 4. check etcd-client response && construct createServiceResponse
 func (ds *DataSource) RegisterService(ctx context.Context, request 
*pb.CreateServiceRequest) (
-       *registry.PluginResponse, error) {
+       *pb.CreateServiceResponse, error) {
+       if request == nil || request.Service == nil {
+               log.Errorf(nil, "create micro-service failed: request body is 
empty")
+               return &pb.CreateServiceResponse{
+                       Response: proto.CreateResponse(scerr.ErrInvalidParams, 
"Request body is empty"),
+               }, nil
+       }
+
+       // start to store service to etcd
        remoteIP := util.GetIPFromContext(ctx)
        serviceBody := request.Service
        serviceFlag := util.StringJoin([]string{
                serviceBody.Environment, serviceBody.AppId, 
serviceBody.ServiceName, serviceBody.Version}, "/")
 
-       serviceUtil.SetServiceDefaultValue(serviceBody)
-
-       domainProject := util.ParseDomainProject(ctx)
+       // request validation
+       err := service.Validate(request)

Review comment:
       validation should be common logic for any storage




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to