From b972510912bfb3a1c7b45f233b38ffac80ed01c3 Mon Sep 17 00:00:00 2001
From: Shawn <xiaoliang.tian@gmail.com>
Date: Thu, 17 Jan 2019 20:43:25 +0800
Subject: [PATCH] fix critical bug: miss used index (#34)

---
 client.go      | 22 ++--------------------
 client_test.go |  2 +-
 util.go        | 24 ++++++++++++++++++++++++
 util_test.go   | 43 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+), 21 deletions(-)
 create mode 100644 util_test.go

diff --git a/client.go b/client.go
index 58447f6..90cf9a8 100755
--- a/client.go
+++ b/client.go
@@ -528,7 +528,6 @@ func (c *RegistryClient) BatchFindInstances(consumerID string, keys []*proto.Fin
 	if err != nil {
 		return nil, NewJSONException(err, string(rBody))
 	}
-	openlogging.Debug("request uri:" + url)
 	openlogging.Debug("request body:" + string(rBody))
 	resp, err := c.HTTPDo("POST", url, http.Header{"X-ConsumerId": []string{consumerID}}, rBody)
 	if err != nil {
@@ -538,31 +537,14 @@ func (c *RegistryClient) BatchFindInstances(consumerID string, keys []*proto.Fin
 		return nil, fmt.Errorf("BatchFindInstances failed, response is empty")
 	}
 	body := httputil.ReadBody(resp)
+	openlogging.Debug("response body:" + string(body))
 	if resp.StatusCode == 200 {
-		instanceMap := make(map[string][]*proto.MicroServiceInstance, 0)
 		var response proto.BatchFindInstancesResponse
 		err = json.Unmarshal(body, &response)
 		if err != nil {
 			return nil, NewJSONException(err, string(body))
 		}
-		if response.Services != nil {
-			for i, result := range response.Services.Updated {
-				if len(result.Instances) == 0 {
-					continue
-				}
-				for _, instance := range result.Instances {
-					instance.ServiceName = keys[i].Service.ServiceName
-					instance.App = keys[i].Service.AppId
-					instances, ok := instanceMap[instance.ServiceName]
-					if !ok {
-						instances = make([]*proto.MicroServiceInstance, 0)
-						instanceMap[instance.ServiceName] = instances
-					}
-					instanceMap[instance.ServiceName] = append(instances, instance)
-				}
-
-			}
-		}
+		instanceMap := RegroupInstances(keys, response)
 		return instanceMap, nil
 	}
 	return nil, fmt.Errorf("batch find failed, status %d, body %s", resp.StatusCode, body)
diff --git a/client_test.go b/client_test.go
index 16f469b..cc8706e 100755
--- a/client_test.go
+++ b/client_test.go
@@ -314,7 +314,7 @@ func TestRegistryClient_FindMicroServiceInstances(t *testing.T) {
 	t.Log(instances)
 	assert.NoError(t, err)
 
-	fs = []*proto.FindService{f, f2}
+	fs = []*proto.FindService{f2, f}
 	instances, err = registryClient.BatchFindInstances(sid, fs)
 	t.Log(instances)
 	assert.NoError(t, err)
diff --git a/util.go b/util.go
index 2ce5a3e..65ae50d 100644
--- a/util.go
+++ b/util.go
@@ -5,6 +5,7 @@ import (
 	"log"
 	"net/url"
 	"time"
+	"github.com/go-chassis/go-sc-client/proto"
 )
 
 func getBackOff(backoffType string) backoff.BackOff {
@@ -39,3 +40,26 @@ func getProtocolMap(eps []string) map[string]string {
 	}
 	return m
 }
+
+func RegroupInstances(keys []*proto.FindService,response proto.BatchFindInstancesResponse) map[string][]*proto.MicroServiceInstance{
+	instanceMap := make(map[string][]*proto.MicroServiceInstance, 0)
+	if response.Services != nil {
+		for _, result := range response.Services.Updated {
+			if len(result.Instances) == 0 {
+				continue
+			}
+			for _, instance := range result.Instances {
+				instance.ServiceName = keys[result.Index].Service.ServiceName
+				instance.App = keys[result.Index].Service.AppId
+				instances, ok := instanceMap[instance.ServiceName]
+				if !ok {
+					instances = make([]*proto.MicroServiceInstance, 0)
+					instanceMap[instance.ServiceName] = instances
+				}
+				instanceMap[instance.ServiceName] = append(instances, instance)
+			}
+
+		}
+	}
+	return instanceMap
+}
diff --git a/util_test.go b/util_test.go
new file mode 100644
index 0000000..37a47e2
--- /dev/null
+++ b/util_test.go
@@ -0,0 +1,43 @@
+package client_test
+
+import (
+	"github.com/go-chassis/go-sc-client"
+	"github.com/go-chassis/go-sc-client/proto"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestRegroupInstances(t *testing.T) {
+	keys := []*proto.FindService{
+		{
+			Service: &proto.MicroServiceKey{
+				ServiceName: "Service1",
+			},
+		},
+		{
+			Service: &proto.MicroServiceKey{
+				ServiceName: "Service2",
+			},
+		},
+		{
+			Service: &proto.MicroServiceKey{
+				ServiceName: "Service3",
+			},
+		},
+	}
+	resp := proto.BatchFindInstancesResponse{
+		Services: &proto.BatchFindResult{
+			Updated: []*proto.FindResult{
+				{Index: 2,
+					Instances: []*proto.MicroServiceInstance{{
+						InstanceId: "1",
+					}}},
+			},
+		},
+	}
+	m := client.RegroupInstances(keys, resp)
+	t.Log(m)
+	assert.Equal(t, 1, len(m["Service3"]))
+	assert.Equal(t, 0, len(m["Service1"]))
+	assert.Equal(t, 0, len(m["Service2"]))
+}