You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/12/11 03:18:08 UTC

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #782: [SCB-2094] Supplemenet UT of instance and problem fix

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



##########
File path: datasource/mongo/ms.go
##########
@@ -20,20 +20,25 @@ package mongo
 import (
 	"context"
 	"fmt"
+	"reflect"
+	"regexp"
 	"strconv"
+	"strings"
 	"time"
 
+	pb "github.com/go-chassis/cari/discovery"

Review comment:
       不要用pb了,没意义

##########
File path: datasource/mongo/ms.go
##########
@@ -1464,44 +1469,51 @@ func (ds *DataSource) GetInstance(ctx context.Context, request *pb.GetOneInstanc
 				fmt.Sprintf("Provider[%s] does not exist.", request.ProviderServiceId)),
 		}, nil
 	}
+	findFlag := fmt.Sprintf("Consumer[%s][%s/%s/%s/%s] find provider[%s][%s/%s/%s/%s] instance[%s]",
+		request.ConsumerServiceId, service.ServiceInfo.Environment, service.ServiceInfo.AppId, service.ServiceInfo.ServiceName, service.ServiceInfo.Version,
+		provider.ServiceInfo.ServiceId, provider.ServiceInfo.Environment, provider.ServiceInfo.AppId, provider.ServiceInfo.ServiceName, provider.ServiceInfo.Version,
+		request.ProviderInstanceId)
 
-	findFlag := func() string {
-		return fmt.Sprintf("Consumer[%s][%s/%s/%s/%s] find provider[%s][%s/%s/%s/%s] instance[%s]",
-			request.ConsumerServiceId, service.ServiceInfo.Environment, service.ServiceInfo.AppId, service.ServiceInfo.ServiceName, service.ServiceInfo.Version,
-			provider.ServiceInfo.ServiceId, provider.ServiceInfo.Environment, provider.ServiceInfo.AppId, provider.ServiceInfo.ServiceName, provider.ServiceInfo.Version,
-			request.ProviderInstanceId)
-	}
-
-	domain := util.ParseDomain(ctx)
-	project := util.ParseProject(ctx)
-	filter = bson.M{
-		ColumnDomain:  domain,
-		ColumnProject: project,
-		StringBuilder([]string{ColumnInstanceInfo, ColumnServiceID}): request.ProviderServiceId}
-	findOneRes, err := client.GetMongoClient().FindOne(ctx, CollectionInstance, filter)
+	domainProject := util.ParseDomainProject(ctx)
+	services, err := findServices(ctx, pb.MicroServiceToKey(domainProject, provider.ServiceInfo))
 	if err != nil {
-		mes := fmt.Errorf("%s failed, provider instance does not exist", findFlag())
-		log.Error("FindInstances.GetWithProviderID failed", err)
+		log.Error(fmt.Sprintf("GetInstance.Get failed %s failed", findFlag), err)
 		return &pb.GetOneInstanceResponse{
-			Response: pb.CreateResponse(pb.ErrInstanceNotExists, mes.Error()),
+			Response: pb.CreateResponse(pb.ErrInternal, err.Error()),
+		}, err
+	}
+	if services == nil {
+		mes := fmt.Errorf("%s failed, provider does not exist", findFlag)
+		log.Error("GetInstance.Get failed", mes)
+		return &pb.GetOneInstanceResponse{
+			Response: pb.CreateResponse(pb.ErrServiceNotExists, mes.Error()),
 		}, nil
 	}
-	var instance Instance
-	err = findOneRes.Decode(&instance)
-	if err != nil {
-		log.Error(fmt.Sprintf("FindInstances.GetWithProviderID failed %s failed", findFlag()), err)
+	serviceIDs := filterServiceIDs(ctx, request.ConsumerServiceId, request.Tags, services)
+	if len(serviceIDs) == 0 {
+		mes := fmt.Errorf("%s failed, provider instance does not exist", findFlag)

Review comment:
       你这行错误和上面一模一样

##########
File path: datasource/mongo/ms.go
##########
@@ -1464,44 +1469,51 @@ func (ds *DataSource) GetInstance(ctx context.Context, request *pb.GetOneInstanc
 				fmt.Sprintf("Provider[%s] does not exist.", request.ProviderServiceId)),
 		}, nil
 	}
+	findFlag := fmt.Sprintf("Consumer[%s][%s/%s/%s/%s] find provider[%s][%s/%s/%s/%s] instance[%s]",
+		request.ConsumerServiceId, service.ServiceInfo.Environment, service.ServiceInfo.AppId, service.ServiceInfo.ServiceName, service.ServiceInfo.Version,
+		provider.ServiceInfo.ServiceId, provider.ServiceInfo.Environment, provider.ServiceInfo.AppId, provider.ServiceInfo.ServiceName, provider.ServiceInfo.Version,
+		request.ProviderInstanceId)
 
-	findFlag := func() string {
-		return fmt.Sprintf("Consumer[%s][%s/%s/%s/%s] find provider[%s][%s/%s/%s/%s] instance[%s]",
-			request.ConsumerServiceId, service.ServiceInfo.Environment, service.ServiceInfo.AppId, service.ServiceInfo.ServiceName, service.ServiceInfo.Version,
-			provider.ServiceInfo.ServiceId, provider.ServiceInfo.Environment, provider.ServiceInfo.AppId, provider.ServiceInfo.ServiceName, provider.ServiceInfo.Version,
-			request.ProviderInstanceId)
-	}
-
-	domain := util.ParseDomain(ctx)
-	project := util.ParseProject(ctx)
-	filter = bson.M{
-		ColumnDomain:  domain,
-		ColumnProject: project,
-		StringBuilder([]string{ColumnInstanceInfo, ColumnServiceID}): request.ProviderServiceId}
-	findOneRes, err := client.GetMongoClient().FindOne(ctx, CollectionInstance, filter)
+	domainProject := util.ParseDomainProject(ctx)
+	services, err := findServices(ctx, pb.MicroServiceToKey(domainProject, provider.ServiceInfo))
 	if err != nil {
-		mes := fmt.Errorf("%s failed, provider instance does not exist", findFlag())
-		log.Error("FindInstances.GetWithProviderID failed", err)
+		log.Error(fmt.Sprintf("GetInstance.Get failed %s failed", findFlag), err)
 		return &pb.GetOneInstanceResponse{
-			Response: pb.CreateResponse(pb.ErrInstanceNotExists, mes.Error()),
+			Response: pb.CreateResponse(pb.ErrInternal, err.Error()),
+		}, err
+	}
+	if services == nil {
+		mes := fmt.Errorf("%s failed, provider does not exist", findFlag)
+		log.Error("GetInstance.Get failed", mes)
+		return &pb.GetOneInstanceResponse{
+			Response: pb.CreateResponse(pb.ErrServiceNotExists, mes.Error()),
 		}, nil
 	}
-	var instance Instance
-	err = findOneRes.Decode(&instance)
-	if err != nil {
-		log.Error(fmt.Sprintf("FindInstances.GetWithProviderID failed %s failed", findFlag()), err)
+	serviceIDs := filterServiceIDs(ctx, request.ConsumerServiceId, request.Tags, services)
+	if len(serviceIDs) == 0 {
+		mes := fmt.Errorf("%s failed, provider instance does not exist", findFlag)
+		log.Error("GetInstances.GetWithProviderID failed", mes)

Review comment:
       go 的日志要开头小写,另外不要再用这种内部code来描述日志信息了,我觉得非常不合理,不是开发者,根本理解不了

##########
File path: datasource/mongo/ms.go
##########
@@ -1464,44 +1469,51 @@ func (ds *DataSource) GetInstance(ctx context.Context, request *pb.GetOneInstanc
 				fmt.Sprintf("Provider[%s] does not exist.", request.ProviderServiceId)),
 		}, nil
 	}
+	findFlag := fmt.Sprintf("Consumer[%s][%s/%s/%s/%s] find provider[%s][%s/%s/%s/%s] instance[%s]",
+		request.ConsumerServiceId, service.ServiceInfo.Environment, service.ServiceInfo.AppId, service.ServiceInfo.ServiceName, service.ServiceInfo.Version,
+		provider.ServiceInfo.ServiceId, provider.ServiceInfo.Environment, provider.ServiceInfo.AppId, provider.ServiceInfo.ServiceName, provider.ServiceInfo.Version,
+		request.ProviderInstanceId)
 
-	findFlag := func() string {
-		return fmt.Sprintf("Consumer[%s][%s/%s/%s/%s] find provider[%s][%s/%s/%s/%s] instance[%s]",
-			request.ConsumerServiceId, service.ServiceInfo.Environment, service.ServiceInfo.AppId, service.ServiceInfo.ServiceName, service.ServiceInfo.Version,
-			provider.ServiceInfo.ServiceId, provider.ServiceInfo.Environment, provider.ServiceInfo.AppId, provider.ServiceInfo.ServiceName, provider.ServiceInfo.Version,
-			request.ProviderInstanceId)
-	}
-
-	domain := util.ParseDomain(ctx)
-	project := util.ParseProject(ctx)
-	filter = bson.M{
-		ColumnDomain:  domain,
-		ColumnProject: project,
-		StringBuilder([]string{ColumnInstanceInfo, ColumnServiceID}): request.ProviderServiceId}
-	findOneRes, err := client.GetMongoClient().FindOne(ctx, CollectionInstance, filter)
+	domainProject := util.ParseDomainProject(ctx)
+	services, err := findServices(ctx, pb.MicroServiceToKey(domainProject, provider.ServiceInfo))
 	if err != nil {
-		mes := fmt.Errorf("%s failed, provider instance does not exist", findFlag())
-		log.Error("FindInstances.GetWithProviderID failed", err)
+		log.Error(fmt.Sprintf("GetInstance.Get failed %s failed", findFlag), err)
 		return &pb.GetOneInstanceResponse{
-			Response: pb.CreateResponse(pb.ErrInstanceNotExists, mes.Error()),
+			Response: pb.CreateResponse(pb.ErrInternal, err.Error()),
+		}, err
+	}
+	if services == nil {
+		mes := fmt.Errorf("%s failed, provider does not exist", findFlag)
+		log.Error("GetInstance.Get failed", mes)
+		return &pb.GetOneInstanceResponse{
+			Response: pb.CreateResponse(pb.ErrServiceNotExists, mes.Error()),
 		}, nil
 	}
-	var instance Instance
-	err = findOneRes.Decode(&instance)
-	if err != nil {
-		log.Error(fmt.Sprintf("FindInstances.GetWithProviderID failed %s failed", findFlag()), err)
+	serviceIDs := filterServiceIDs(ctx, request.ConsumerServiceId, request.Tags, services)
+	if len(serviceIDs) == 0 {
+		mes := fmt.Errorf("%s failed, provider instance does not exist", findFlag)
+		log.Error("GetInstances.GetWithProviderID failed", mes)
 		return &pb.GetOneInstanceResponse{
-			Response: pb.CreateResponse(pb.ErrInternal, err.Error()),
-		}, err
+			Response: pb.CreateResponse(pb.ErrServiceNotExists, mes.Error()),
+		}, nil
+	}
+	instances, err := findInstancesByServiceIDs(ctx, serviceIDs)
+	if len(instances) == 0 {
+		mes := fmt.Errorf("%s failed, provider instance does not exist", findFlag)

Review comment:
       一模一样的错误定义




----------------------------------------------------------------
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:
users@infra.apache.org