Repository: seqsense/aws-iot-device-sdk-go
Pull request #130 (2020-07-21 11:31:04)

shadow: fix error message type
Error code is int on Thing Shadow.
(It's string on Jobs.)


Patch:
diff --git a/examples/shadow/main.go b/examples/shadow/main.go
index ec0a947..6c459df 100644
--- a/examples/shadow/main.go
+++ b/examples/shadow/main.go
@@ -81,40 +81,45 @@ func main() {
 	s.OnError(func(err error) {
 		fmt.Printf("async error: %v\n", err)
 	})
+	s.OnDelta(func(delta map[string]interface{}) {
+		fmt.Printf("delta: %+v\n", delta)
+	})
 	cli.Handle(s)
 
 	fmt.Print("> update desire\n")
-	doc, err := s.Desire(ctx, sampleState{Value: 1})
+	doc, err := s.Desire(ctx, sampleState{Value: 1, Struct: sampleStruct{Values: []int{1, 2}}})
 	if err != nil {
 		panic(err)
 	}
-	fmt.Printf("document: %v\n", doc)
+	fmt.Printf("document: %+v\n", doc)
 
 	fmt.Print("> update report\n")
-	doc, err = s.Report(ctx, sampleState{Value: 2})
+	doc, err = s.Report(ctx, sampleState{Value: 2, Struct: sampleStruct{Values: []int{1, 2}}})
 	if err != nil {
 		panic(err)
 	}
-	fmt.Printf("document: %v\n", doc)
+	fmt.Printf("document: %+v\n", doc)
 
 	fmt.Print("> get document\n")
 	doc, err = s.Get(ctx)
 	if err != nil {
 		panic(err)
 	}
-	fmt.Printf("document: %v\n", doc)
+	fmt.Printf("document: %+v\n", doc)
 
 	fmt.Print("> delete\n")
 	err = s.Delete(ctx)
 	if err != nil {
 		panic(err)
 	}
-	fmt.Printf("document: %v\n", s.Document())
+	fmt.Printf("document: %+v\n", s.Document())
+}
+
+type sampleStruct struct {
+	Values []int
 }
 
 type sampleState struct {
 	Value  int
-	Struct struct {
-		NestedValue int
-	}
+	Struct sampleStruct
 }
diff --git a/shadow/copy.go b/shadow/copy.go
new file mode 100644
index 0000000..d64237c
--- /dev/null
+++ b/shadow/copy.go
@@ -0,0 +1,59 @@
+// Copyright 2020 SEQSENSE, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package shadow
+
+// cloneState creates deepcopy of thing state.
+func cloneState(s map[string]interface{}) map[string]interface{} {
+	c := map[string]interface{}{}
+	for k, v := range s {
+		switch vv := v.(type) {
+		case map[string]interface{}:
+			c[k] = cloneState(vv)
+		case []interface{}:
+			c[k] = cloneStateSlice(vv)
+		default:
+			c[k] = vv
+		}
+	}
+	return c
+}
+
+// cloneStateSlice creates deepcopy of thing state slice.
+func cloneStateSlice(s []interface{}) []interface{} {
+	c := []interface{}{}
+	for _, v := range s {
+		switch vv := v.(type) {
+		case map[string]interface{}:
+			c = append(c, cloneState(vv))
+		case []interface{}:
+			c = append(c, cloneStateSlice(vv))
+		default:
+			c = append(c, vv)
+		}
+	}
+	return c
+}
+
+// clone returns deepcopy of the document.
+func (s *ThingDocument) clone() *ThingDocument {
+	if s == nil {
+		return nil
+	}
+	c := *s
+	c.State.Desired = cloneState(s.State.Desired)
+	c.State.Reported = cloneState(s.State.Reported)
+	c.State.Delta = cloneState(s.State.Delta)
+	return &c
+}
diff --git a/shadow/shadow.go b/shadow/shadow.go
index c22c288..dfa3c2a 100644
--- a/shadow/shadow.go
+++ b/shadow/shadow.go
@@ -83,10 +83,12 @@ func (s *shadow) getAccepted(msg *mqtt.Message) {
 		s.handleError(err)
 		return
 	}
+	s.mu.Lock()
 	s.doc = doc
+	s.mu.Unlock()
 	s.handleResponse(doc)
 
-	s.handleDelta(s.doc.State.Delta)
+	s.handleDelta(doc.State.Delta)
 }
 
 func (s *shadow) rejected(msg *mqtt.Message) {
@@ -104,7 +106,10 @@ func (s *shadow) updateAccepted(msg *mqtt.Message) {
 		s.handleError(err)
 		return
 	}
-	if err := s.doc.update(doc); err != nil {
+	s.mu.Lock()
+	err := s.doc.update(doc)
+	s.mu.Unlock()
+	if err != nil {
 		s.handleError(err)
 		return
 	}
@@ -117,9 +122,12 @@ func (s *shadow) updateDelta(msg *mqtt.Message) {
 		s.handleError(err)
 		return
 	}
+	s.mu.Lock()
 	ok := s.doc.updateDelta(state)
+	delta := cloneState(s.doc.State.Delta)
+	s.mu.Unlock()
 	if ok {
-		s.handleDelta(s.doc.State.Delta)
+		s.handleDelta(delta)
 	}
 }
 
@@ -129,7 +137,9 @@ func (s *shadow) deleteAccepted(msg *mqtt.Message) {
 		s.handleError(err)
 		return
 	}
+	s.mu.Lock()
 	s.doc = nil
+	s.mu.Unlock()
 	s.handleResponse(doc)
 }
 
@@ -219,7 +229,10 @@ func (s *shadow) Report(ctx context.Context, state interface{}) (*ThingDocument,
 	case res := <-ch:
 		switch r := res.(type) {
 		case *thingDocumentRaw:
-			return s.doc, nil
+			s.mu.Lock()
+			doc := s.doc.clone()
+			s.mu.Unlock()
+			return doc, nil
 		case *ErrorResponse:
 			return nil, r
 		default:
@@ -267,7 +280,10 @@ func (s *shadow) Desire(ctx context.Context, state interface{}) (*ThingDocument,
 	case res := <-ch:
 		switch r := res.(type) {
 		case *thingDocumentRaw:
-			return s.doc, nil
+			s.mu.Lock()
+			doc := s.doc.clone()
+			s.mu.Unlock()
+			return doc, nil
 		case *ErrorResponse:
 			return nil, r
 		default:
@@ -362,7 +378,9 @@ func (s *shadow) Delete(ctx context.Context) error {
 }
 
 func (s *shadow) Document() *ThingDocument {
-	return s.doc
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.doc.clone()
 }
 
 func (s *shadow) OnDelta(cb func(delta map[string]interface{})) {
diff --git a/shadow/state.go b/shadow/state.go
index e2e21af..ee3db8e 100644
--- a/shadow/state.go
+++ b/shadow/state.go
@@ -15,7 +15,7 @@ type simpleRequest struct {
 
 // ErrorResponse represents error response from AWS IoT.
 type ErrorResponse struct {
-	Code        string `json:"code"`
+	Code        int    `json:"code"`
 	Message     string `json:"message"`
 	Timestamp   int64  `json:"timestamp"`
 	ClientToken string `json:"clientToken"`
@@ -23,7 +23,7 @@ type ErrorResponse struct {
 
 // Error implements error interface.
 func (e *ErrorResponse) Error() string {
-	return fmt.Sprintf("%s (%s): %s", e.Code, e.ClientToken, e.Message)
+	return fmt.Sprintf("%d (%s): %s", e.Code, e.ClientToken, e.Message)
 }
 
 // ThingState represents Thing Shadow State.


Test patch:
diff --git a/shadow/copy_test.go b/shadow/copy_test.go
new file mode 100644
index 0000000..b29bb3a
--- /dev/null
+++ b/shadow/copy_test.go
@@ -0,0 +1,103 @@
+// Copyright 2020 SEQSENSE, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package shadow
+
+import (
+	"bytes"
+	"encoding/json"
+	"testing"
+)
+
+func TestClone(t *testing.T) {
+	testCases := map[string]struct {
+		doc    *ThingDocument
+		mutate func(*ThingDocument)
+	}{
+		"Base": {
+			doc: &ThingDocument{Version: 10},
+			mutate: func(doc *ThingDocument) {
+				doc.Version = 1
+			},
+		},
+		"Map": {
+			doc: &ThingDocument{
+				State: ThingState{
+					Desired: map[string]interface{}{
+						"key": "value",
+					},
+				},
+			},
+			mutate: func(doc *ThingDocument) {
+				doc.State.Desired["key"] = "value2"
+			},
+		},
+		"MapInMap": {
+			doc: &ThingDocument{
+				State: ThingState{
+					Desired: map[string]interface{}{
+						"key": map[string]interface{}{
+							"childKey": "value",
+						},
+					},
+				},
+			},
+			mutate: func(doc *ThingDocument) {
+				doc.State.Desired["key"].(map[string]interface{})["childKey"] = "value2"
+			},
+		},
+		"SliceInMap": {
+			doc: &ThingDocument{
+				State: ThingState{
+					Desired: map[string]interface{}{
+						"key": []interface{}{
+							"value1",
+							"value2",
+						},
+					},
+				},
+			},
+			mutate: func(doc *ThingDocument) {
+				doc.State.Desired["key"].([]interface{})[0] = "value1b"
+			},
+		},
+		"Nil": {
+			doc:    nil,
+			mutate: func(doc *ThingDocument) {},
+		},
+	}
+	for name, testCase := range testCases {
+		testCase := testCase
+		t.Run(name, func(t *testing.T) {
+			originalJSON, err := json.Marshal(testCase.doc)
+			if err != nil {
+				t.Fatal(err)
+			}
+			cloned := testCase.doc.clone()
+			testCase.mutate(cloned)
+
+			resultJSON, err := json.Marshal(testCase.doc)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !bytes.Equal(originalJSON, resultJSON) {
+				t.Errorf(
+					"Original document is mutated by updating cloned one\nexpected: %s\ngot: %s",
+					originalJSON, resultJSON,
+				)
+			}
+		})
+	}
+}
diff --git a/shadow/shadow_test.go b/shadow/shadow_test.go
index 9790d5a..40302f2 100644
--- a/shadow/shadow_test.go
+++ b/shadow/shadow_test.go
@@ -366,12 +366,12 @@ func TestGet(t *testing.T) {
 		},
 		"Error": {
 			response: &ErrorResponse{
-				Code:    "Failed",
+				Code:    400,
 				Message: "Reason",
 			},
 			responseTopic: "get/rejected",
 			err: &ErrorResponse{
-				Code:    "Failed",
+				Code:    400,
 				Message: "Reason",
 			},
 		},
@@ -460,12 +460,12 @@ func TestDesire(t *testing.T) {
 		"Error": {
 			input: map[string]interface{}{"key": "value"},
 			response: &ErrorResponse{
-				Code:    "Failed",
+				Code:    400,
 				Message: "Reason",
 			},
 			responseTopic: "update/rejected",
 			err: &ErrorResponse{
-				Code:    "Failed",
+				Code:    400,
 				Message: "Reason",
 			},
 		},
@@ -554,12 +554,12 @@ func TestReport(t *testing.T) {
 		"Error": {
 			input: map[string]interface{}{"key": "value"},
 			response: &ErrorResponse{
-				Code:    "Failed",
+				Code:    400,
 				Message: "Reason",
 			},
 			responseTopic: "update/rejected",
 			err: &ErrorResponse{
-				Code:    "Failed",
+				Code:    400,
 				Message: "Reason",
 			},
 		},
@@ -631,12 +631,12 @@ func TestDelete(t *testing.T) {
 		},
 		"Error": {
 			response: &ErrorResponse{
-				Code:    "Failed",
+				Code:    400,
 				Message: "Reason",
 			},
 			responseTopic: "delete/rejected",
 			err: &ErrorResponse{
-				Code:    "Failed",
+				Code:    400,
 				Message: "Reason",
 			},
 		},
diff --git a/shadow/state_test.go b/shadow/state_test.go
index 722efea..5a8152e 100644
--- a/shadow/state_test.go
+++ b/shadow/state_test.go
@@ -3,6 +3,7 @@ package shadow
 import (
 	"encoding/json"
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -73,3 +74,17 @@ func TestThingDocument_update(t *testing.T) {
 		)
 	}
 }
+
+func TestErrorResponse(t *testing.T) {
+	err := &ErrorResponse{
+		Code:    100,
+		Message: "error message",
+	}
+	errStr := err.Error()
+	if !strings.Contains(errStr, "100") {
+		t.Error("Error string should contain error code")
+	}
+	if !strings.Contains(errStr, "error message") {
+		t.Error("Error string should contain error message")
+	}
+}