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 2021/01/05 07:35:44 UTC

[GitHub] [servicecomb-service-center] robotLJW opened a new pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

robotLJW opened a new pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809


   Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it).  Trivial changes like typos do not require a JIRA issue.  Your pull request should address just this issue, without pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `go build` `go test` `go fmt` `go vet` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
    - [ ] Never comment source code, delete it.
    - [ ] UT should has "context, subject, expected result" result as test case name, when you call t.Run().
   ---
   


----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
little-cui commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r555738570



##########
File path: server/service/rbac/rbac_test.go
##########
@@ -176,8 +176,7 @@ func TestInitRBAC(t *testing.T) {
 	})
 
 	t.Run("delete the new role", func(t *testing.T) {
-		r, err := dao.DeleteRole(context.Background(), "tester")
+		_, err := dao.DeleteRole(context.Background(), "tester")

Review comment:
       为什么删掉？

##########
File path: server/service/instance_test.go
##########
@@ -485,13 +485,6 @@ var _ = Describe("'Instance' service", func() {
 				Expect(err).To(BeNil())
 				Expect(resp.Response.GetCode()).To(Equal(pb.ErrInvalidParams))
 
-				By("serviceId does not exist")

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

[GitHub] [servicecomb-service-center] robotLJW commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r556223711



##########
File path: server/service/rbac/rbac_test.go
##########
@@ -176,8 +176,7 @@ func TestInitRBAC(t *testing.T) {
 	})
 
 	t.Run("delete the new role", func(t *testing.T) {
-		r, err := dao.DeleteRole(context.Background(), "tester")
+		_, err := dao.DeleteRole(context.Background(), "tester")

Review comment:
       这个我修改了，我发现之前role那块的delete操作最后的判断存在问题，不应该使用count!=0来判断，应该使用resp.Succeeded




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r555035887



##########
File path: datasource/mongo/ms.go
##########
@@ -1570,23 +1570,31 @@ func (ds *DataSource) GetInstance(ctx context.Context, request *discovery.GetOne
 	}
 
 	domainProject := util.ParseDomainProject(ctx)
-	services, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))
+	services, exist, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))

Review comment:
       不存在是一种err
   不是一个bool
   




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r556348569



##########
File path: datasource/mongo/ms_test.go
##########
@@ -175,7 +175,7 @@ func TestService_Register(t *testing.T) {
 		})
 		assert.NotNil(t, resp)
 		assert.NoError(t, err)
-		assert.Equal(t, pb.ErrServiceAlreadyExists, resp.Response.GetCode())
+		assert.Equal(t, pb.ResponseSuccess, resp.Response.GetCode())

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

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r556349697



##########
File path: server/service/rbac/rbac_test.go
##########
@@ -178,6 +177,6 @@ func TestInitRBAC(t *testing.T) {
 	t.Run("delete the new role", func(t *testing.T) {
 		r, err := dao.DeleteRole(context.Background(), "tester")
 		assert.NoError(t, err)
-		assert.Equal(t, false, r)
+		assert.Equal(t, true, r)

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

[GitHub] [servicecomb-service-center] robotLJW commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r556373943



##########
File path: datasource/mongo/ms_test.go
##########
@@ -175,7 +175,7 @@ func TestService_Register(t *testing.T) {
 		})
 		assert.NotNil(t, resp)
 		assert.NoError(t, err)
-		assert.Equal(t, pb.ErrServiceAlreadyExists, resp.Response.GetCode())
+		assert.Equal(t, pb.ResponseSuccess, resp.Response.GetCode())

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

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r555035035



##########
File path: datasource/mongo/ms.go
##########
@@ -1570,23 +1570,31 @@ func (ds *DataSource) GetInstance(ctx context.Context, request *discovery.GetOne
 	}
 
 	domainProject := util.ParseDomainProject(ctx)
-	services, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))
+	services, exist, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))
 	if err != nil {
 		log.Error(fmt.Sprintf("get instance failed %s", findFlag()), err)
 		return &discovery.GetOneInstanceResponse{
 			Response: discovery.CreateResponse(discovery.ErrInternal, err.Error()),
 		}, err
 	}
+	if services == nil && exist {
+		if exist {
+			return &discovery.GetOneInstanceResponse{
+				Response: discovery.CreateResponse(discovery.ResponseSuccess, "Get instance successfully."),
+				Instance: nil,
+			}, nil
+		} else {

Review comment:
       1586永远跑不到




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r553703554



##########
File path: scripts/ut_test_in_docker.sh
##########
@@ -67,11 +67,23 @@ fi
 
 export TEST_MODE=mongo
 [ $? == 0 ] && ut_for_dir datasource/mongo
-# 由於mongo接口未全部實現先注釋
-#[ $? == 0 ] && ut_for_dir pkg
-#[ $? == 0 ] && ut_for_dir server
-#[ $? == 0 ] && ut_for_dir scctl
-#[ $? == 0 ] && ut_for_dir syncer
+[ $? == 0 ] && ut_for_dir server/alarm
+[ $? == 0 ] && ut_for_dir server/bootstrap
+[ $? == 0 ] && ut_for_dir server/command
+[ $? == 0 ] && ut_for_dir server/config

Review comment:
       以后我加个包，那ut还跑不了了，这种维护成本也是有的。目标是：保证开发做自己的代码，不要因为增个小目录，然后他不知道这有个维护列表，导致忘记加




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] robotLJW commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r555716440



##########
File path: datasource/mongo/ms.go
##########
@@ -1570,23 +1570,31 @@ func (ds *DataSource) GetInstance(ctx context.Context, request *discovery.GetOne
 	}
 
 	domainProject := util.ParseDomainProject(ctx)
-	services, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))
+	services, exist, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))

Review comment:
       这个我修改了exist命名，修改为existExcludeVesion，并在上面加了这个字段的解释




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r553704208



##########
File path: datasource/mongo/role.go
##########
@@ -20,29 +20,120 @@ package mongo
 import (
 	"context"
 
+	"go.mongodb.org/mongo-driver/bson"
+
+	"github.com/apache/servicecomb-service-center/datasource"
+	"github.com/apache/servicecomb-service-center/datasource/mongo/client"
+	"github.com/apache/servicecomb-service-center/pkg/log"
 	"github.com/apache/servicecomb-service-center/pkg/rbacframe"
+	"github.com/apache/servicecomb-service-center/pkg/util"
 )
 
 func (ds *DataSource) CreateRole(ctx context.Context, r *rbacframe.Role) error {
+	exist, err := ds.RoleExist(ctx, r.Name)
+	if err != nil {
+		log.Error("failed to query role", err)
+		return err
+	}
+	if exist {
+		return datasource.ErrRoleDuplicated
+	}
+	r.ID = util.GenerateUUID()
+	_, err = client.GetMongoClient().Insert(ctx, CollectionRole, r)
+	if err != nil {
+		if client.IsDuplicateKey(err) {
+			return datasource.ErrRoleDuplicated
+		}
+		return err
+	}
+	log.Info("create new role: " + r.ID)
 	return nil
 }
 
 func (ds *DataSource) RoleExist(ctx context.Context, name string) (bool, error) {
-	return false, nil
+	filter := bson.M{
+		ColumnName: name,
+	}
+	count, err := client.GetMongoClient().Count(ctx, CollectionRole, filter)
+	if err != nil {
+		return false, err
+	}
+	if count == 0 {
+		return false, nil
+	}
+	return true, nil
 }
 
 func (ds *DataSource) GetRole(ctx context.Context, name string) (*rbacframe.Role, error) {
-	return &rbacframe.Role{}, nil
+	filter := bson.M{
+		ColumnName: name,
+	}
+	result, err := client.GetMongoClient().FindOne(ctx, CollectionRole, filter)
+	if err != nil {
+		return nil, err
+	}
+	if result.Err() != nil {

Review comment:
       要把不存在的错误分辨出来，这样http才能反馈404状态




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] robotLJW commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r557005991



##########
File path: datasource/mongo/ms.go
##########
@@ -1570,23 +1570,31 @@ func (ds *DataSource) GetInstance(ctx context.Context, request *discovery.GetOne
 	}
 
 	domainProject := util.ParseDomainProject(ctx)
-	services, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))
+	services, exist, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))

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

[GitHub] [servicecomb-service-center] robotLJW commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r555713898



##########
File path: datasource/mongo/role.go
##########
@@ -20,29 +20,120 @@ package mongo
 import (
 	"context"
 
+	"go.mongodb.org/mongo-driver/bson"
+
+	"github.com/apache/servicecomb-service-center/datasource"
+	"github.com/apache/servicecomb-service-center/datasource/mongo/client"
+	"github.com/apache/servicecomb-service-center/pkg/log"
 	"github.com/apache/servicecomb-service-center/pkg/rbacframe"
+	"github.com/apache/servicecomb-service-center/pkg/util"
 )
 
 func (ds *DataSource) CreateRole(ctx context.Context, r *rbacframe.Role) error {
+	exist, err := ds.RoleExist(ctx, r.Name)
+	if err != nil {
+		log.Error("failed to query role", err)
+		return err
+	}
+	if exist {
+		return datasource.ErrRoleDuplicated
+	}
+	r.ID = util.GenerateUUID()
+	_, err = client.GetMongoClient().Insert(ctx, CollectionRole, r)
+	if err != nil {
+		if client.IsDuplicateKey(err) {
+			return datasource.ErrRoleDuplicated
+		}
+		return err
+	}
+	log.Info("create new role: " + r.ID)
 	return nil
 }
 
 func (ds *DataSource) RoleExist(ctx context.Context, name string) (bool, error) {
-	return false, nil
+	filter := bson.M{
+		ColumnName: name,
+	}
+	count, err := client.GetMongoClient().Count(ctx, CollectionRole, filter)
+	if err != nil {
+		return false, err
+	}
+	if count == 0 {
+		return false, nil
+	}
+	return true, nil
 }
 
 func (ds *DataSource) GetRole(ctx context.Context, name string) (*rbacframe.Role, error) {
-	return &rbacframe.Role{}, nil
+	filter := bson.M{
+		ColumnName: name,
+	}
+	result, err := client.GetMongoClient().FindOne(ctx, CollectionRole, filter)
+	if err != nil {
+		return nil, err
+	}
+	if result.Err() != nil {

Review comment:
       已经修改，返回client.ErrNoDocuments
   
   




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] robotLJW commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r555740065



##########
File path: server/service/instance_test.go
##########
@@ -485,13 +485,6 @@ var _ = Describe("'Instance' service", func() {
 				Expect(err).To(BeNil())
 				Expect(resp.Response.GetCode()).To(Equal(pb.ErrInvalidParams))
 
-				By("serviceId does not exist")

Review comment:
       这个删除是因为 mongo就使用instanceid去查询了，会返回成功




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r555439359



##########
File path: datasource/mongo/role.go
##########
@@ -20,29 +20,120 @@ package mongo
 import (
 	"context"
 
+	"go.mongodb.org/mongo-driver/bson"
+
+	"github.com/apache/servicecomb-service-center/datasource"
+	"github.com/apache/servicecomb-service-center/datasource/mongo/client"
+	"github.com/apache/servicecomb-service-center/pkg/log"
 	"github.com/apache/servicecomb-service-center/pkg/rbacframe"
+	"github.com/apache/servicecomb-service-center/pkg/util"
 )
 
 func (ds *DataSource) CreateRole(ctx context.Context, r *rbacframe.Role) error {
+	exist, err := ds.RoleExist(ctx, r.Name)
+	if err != nil {
+		log.Error("failed to query role", err)
+		return err
+	}
+	if exist {
+		return datasource.ErrRoleDuplicated
+	}
+	r.ID = util.GenerateUUID()
+	_, err = client.GetMongoClient().Insert(ctx, CollectionRole, r)
+	if err != nil {
+		if client.IsDuplicateKey(err) {
+			return datasource.ErrRoleDuplicated
+		}
+		return err
+	}
+	log.Info("create new role: " + r.ID)
 	return nil
 }
 
 func (ds *DataSource) RoleExist(ctx context.Context, name string) (bool, error) {
-	return false, nil
+	filter := bson.M{
+		ColumnName: name,
+	}
+	count, err := client.GetMongoClient().Count(ctx, CollectionRole, filter)
+	if err != nil {
+		return false, err
+	}
+	if count == 0 {
+		return false, nil
+	}
+	return true, nil
 }
 
 func (ds *DataSource) GetRole(ctx context.Context, name string) (*rbacframe.Role, error) {
-	return &rbacframe.Role{}, nil
+	filter := bson.M{
+		ColumnName: name,
+	}
+	result, err := client.GetMongoClient().FindOne(ctx, CollectionRole, filter)
+	if err != nil {
+		return nil, err
+	}
+	if result.Err() != nil {

Review comment:
       这个还没改，别resolve




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] robotLJW commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r556375261



##########
File path: server/service/rbac/rbac_test.go
##########
@@ -178,6 +177,6 @@ func TestInitRBAC(t *testing.T) {
 	t.Run("delete the new role", func(t *testing.T) {
 		r, err := dao.DeleteRole(context.Background(), "tester")
 		assert.NoError(t, err)
-		assert.Equal(t, false, r)
+		assert.Equal(t, true, r)

Review comment:
       之前role删除判断错误，之前使用count!=0，当删除成功count也是0，存在问题




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] robotLJW closed pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW closed pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809


   


----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] robotLJW commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r557005924



##########
File path: datasource/mongo/ms.go
##########
@@ -2010,19 +2018,26 @@ func (ds *DataSource) findSharedServiceInstance(ctx context.Context, request *di
 	findFlag := func() string {
 		return fmt.Sprintf("find shared provider[%s/%s/%s/%s]", provider.Environment, provider.AppId, provider.ServiceName, provider.Version)
 	}
-	services, err := findServices(ctx, provider)
+	services, exist, err := findServices(ctx, provider)

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

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r555036521



##########
File path: datasource/mongo/ms.go
##########
@@ -2010,19 +2018,26 @@ func (ds *DataSource) findSharedServiceInstance(ctx context.Context, request *di
 	findFlag := func() string {
 		return fmt.Sprintf("find shared provider[%s/%s/%s/%s]", provider.Environment, provider.AppId, provider.ServiceName, provider.Version)
 	}
-	services, err := findServices(ctx, provider)
+	services, exist, err := findServices(ctx, provider)

Review comment:
       同类问题

##########
File path: datasource/mongo/ms.go
##########
@@ -2411,21 +2450,34 @@ func findServices(ctx context.Context, key *discovery.MicroServiceKey) ([]*Servi
 			StringBuilder([]string{ColumnServiceInfo, ColumnEnv}):         key.Environment,
 			StringBuilder([]string{ColumnServiceInfo, ColumnAppID}):       key.AppId,
 			StringBuilder([]string{ColumnServiceInfo, ColumnServiceName}): key.ServiceName,
-			StringBuilder([]string{ColumnServiceInfo, ColumnVersion}):     bson.M{"$gte": start, "$lte": end}}
-		return servicesFilter(ctx, filter)
+			StringBuilder([]string{ColumnServiceInfo, ColumnAlias}):       key.Alias,
+			StringBuilder([]string{ColumnServiceInfo, ColumnVersion}):     bson.M{"$gte": start, "$lte": end},
+		}
+		services, err = servicesFilter(ctx, filter)
 	default:
 		filter := bson.M{
 			ColumnDomain:  tenant[0],
 			ColumnProject: tenant[1],
 			StringBuilder([]string{ColumnServiceInfo, ColumnEnv}):         key.Environment,
 			StringBuilder([]string{ColumnServiceInfo, ColumnAppID}):       key.AppId,
 			StringBuilder([]string{ColumnServiceInfo, ColumnServiceName}): key.ServiceName,
-			StringBuilder([]string{ColumnServiceInfo, ColumnVersion}):     key.Version}
-		return servicesFilter(ctx, filter)
+			StringBuilder([]string{ColumnServiceInfo, ColumnAlias}):       key.Alias,
+			StringBuilder([]string{ColumnServiceInfo, ColumnVersion}):     key.Version,
+		}
+		services, err = servicesFilter(ctx, filter)
+		// If not found by version, exist is set to false
+		if len(services) == 0 {
+			exist = false

Review comment:
       这是一种err，不可以接受bool方案

##########
File path: datasource/mongo/ms.go
##########
@@ -2010,19 +2018,26 @@ func (ds *DataSource) findSharedServiceInstance(ctx context.Context, request *di
 	findFlag := func() string {
 		return fmt.Sprintf("find shared provider[%s/%s/%s/%s]", provider.Environment, provider.AppId, provider.ServiceName, provider.Version)
 	}
-	services, err := findServices(ctx, provider)
+	services, exist, err := findServices(ctx, provider)
 	if err != nil {
 		log.Error(fmt.Sprintf("find shared service instance failed %s", findFlag()), err)
 		return &discovery.FindInstancesResponse{
 			Response: discovery.CreateResponse(discovery.ErrInternal, err.Error()),
 		}, err
 	}
 	if services == nil {
-		mes := fmt.Errorf("%s failed, provider does not exist", findFlag())
-		log.Error("find shared service instance failed", mes)
-		return &discovery.FindInstancesResponse{
-			Response: discovery.CreateResponse(discovery.ErrServiceNotExists, mes.Error()),
-		}, nil
+		if exist {

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

[GitHub] [servicecomb-service-center] tianxiaoliang merged pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang merged pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809


   


----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] robotLJW commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
robotLJW commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r555715393



##########
File path: datasource/mongo/ms.go
##########
@@ -1570,23 +1570,31 @@ func (ds *DataSource) GetInstance(ctx context.Context, request *discovery.GetOne
 	}
 
 	domainProject := util.ParseDomainProject(ctx)
-	services, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))
+	services, exist, err := findServices(ctx, discovery.MicroServiceToKey(domainProject, provider.ServiceInfo))

Review comment:
       这个是这样的，就是原先逻辑排查版本去查询服务，若存在应该需要返回成功。所以加了一个bool




----------------------------------------------------------------
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

[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #809: 【SCB-2094】Fix: open Mongo's pkg, server and syncer test cases

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #809:
URL: https://github.com/apache/servicecomb-service-center/pull/809#discussion_r553702746



##########
File path: datasource/mongo/account.go
##########
@@ -59,7 +59,7 @@ func (ds *DataSource) CreateAccount(ctx context.Context, a *rbacframe.Account) e
 
 func (ds *DataSource) AccountExist(ctx context.Context, key string) (bool, error) {
 	filter := bson.M{
-		AccountName: key,
+		ColumnName: key,

Review comment:
       这个列名，似乎不易读了因为我会理解为：列名。而不是列-名

##########
File path: datasource/mongo/account.go
##########
@@ -85,15 +85,18 @@ func (ds *DataSource) GetAccount(ctx context.Context, key string) (*rbacframe.Ac
 	var account rbacframe.Account
 	err = result.Decode(&account)
 	if err != nil {
-		log.Error("Decode account failed: ", err)
+		log.Error("decode account failed: ", err)
 		return nil, err
 	}
 	return &account, nil
 }
 
 func (ds *DataSource) ListAccount(ctx context.Context, key string) ([]*rbacframe.Account, int64, error) {
 	filter := bson.M{
-		AccountName: bson.M{"$regex": key},
+		ColumnName: bson.M{"$regex": key},
+	}
+	if key == "" || len(key) == 0 {

Review comment:
       不需要2个条件，一个就够

##########
File path: datasource/mongo/account.go
##########
@@ -59,7 +59,7 @@ func (ds *DataSource) CreateAccount(ctx context.Context, a *rbacframe.Account) e
 
 func (ds *DataSource) AccountExist(ctx context.Context, key string) (bool, error) {
 	filter := bson.M{
-		AccountName: key,
+		ColumnName: key,

Review comment:
       ColumnAccountName更佳

##########
File path: datasource/mongo/account.go
##########
@@ -129,11 +133,11 @@ func (ds *DataSource) DeleteAccount(ctx context.Context, key string) (bool, erro
 
 func (ds *DataSource) UpdateAccount(ctx context.Context, key string, account *rbacframe.Account) error {
 	filter := bson.M{
-		AccountName: key,
+		ColumnName: key,
 	}
 	update := bson.M{
-		"$set": bson.M{AccountID: account.ID, AccountPassword: account.Name, AccountRole: account.Roles, AccountTokenExpirationTime: account.TokenExpirationTime,
-			AccountCurrentPassword: account.CurrentPassword, AccountStatus: account.Status,
+		"$set": bson.M{ColumnID: account.ID, ColumnName: account.Name, ColumnPassword: account.Password, ColumnRole: account.Roles, ColumnTokenExpirationTime: account.TokenExpirationTime,

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