Repository: appc/spec
Pull request #689 (2017-06-12 20:21:25)

schema/labels: consistently order LabelsFromMap
Prior to this change, the slice of labels produced by LabelsFromMap
could have any arbitrary ordering.

See https://github.com/appc/docker2aci/issues/245 for one of the issues
this causes in practice.

I also renamed the type 'labels' to 'labelsSlice' since, as it was, it
was being constantly shadowed.

This is not accompanied with any spec change since labels in the appc
spec are already a slice, and thus already have a consistent ordering.
It's unfortunate that a round trip of ToMap/FromMap won't be consistent,
but I don't think any callers actually do that currently.


Patch:
diff --git a/schema/types/labels.go b/schema/types/labels.go
index 89fa428..25931f9 100644
--- a/schema/types/labels.go
+++ b/schema/types/labels.go
@@ -28,7 +28,11 @@ var ValidOSArch = map[string][]string{
 
 type Labels []Label
 
-type labels Labels
+type labelsSlice Labels
+
+func (l labelsSlice) Len() int           { return len(l) }
+func (l labelsSlice) Swap(i, j int)      { l[i], l[j] = l[j], l[i] }
+func (l labelsSlice) Less(i, j int) bool { return l[i].Name < l[j].Name }
 
 type Label struct {
 	Name  ACIdentifier `json:"name"`
@@ -97,11 +101,11 @@ func (l Labels) MarshalJSON() ([]byte, error) {
 	if err := l.assertValid(); err != nil {
 		return nil, err
 	}
-	return json.Marshal(labels(l))
+	return json.Marshal(labelsSlice(l))
 }
 
 func (l *Labels) UnmarshalJSON(data []byte) error {
-	var jl labels
+	var jl labelsSlice
 	if err := json.Unmarshal(data, &jl); err != nil {
 		return err
 	}
@@ -141,6 +145,7 @@ func LabelsFromMap(labelsMap map[ACIdentifier]string) (Labels, error) {
 	if err := labels.assertValid(); err != nil {
 		return nil, err
 	}
+	sort.Sort(labelsSlice(labels))
 	return labels, nil
 }
 


Test patch:
diff --git a/schema/types/labels_test.go b/schema/types/labels_test.go
index 31da1d8..82a1122 100644
--- a/schema/types/labels_test.go
+++ b/schema/types/labels_test.go
@@ -16,6 +16,8 @@ package types
 
 import (
 	"encoding/json"
+	"errors"
+	"reflect"
 	"strings"
 	"testing"
 )
@@ -221,3 +223,67 @@ func TestLabels(t *testing.T) {
 		}
 	}
 }
+
+func TestLabelsFromMap(t *testing.T) {
+	tests := []struct {
+		in          map[ACIdentifier]string
+		expectedOut Labels
+		expectedErr error
+	}{
+		{
+			in: map[ACIdentifier]string{
+				"foo": "bar",
+				"bar": "baz",
+				"baz": "foo",
+			},
+			expectedOut: []Label{
+				Label{
+					Name:  "bar",
+					Value: "baz",
+				},
+				Label{
+					Name:  "baz",
+					Value: "foo",
+				},
+				Label{
+					Name:  "foo",
+					Value: "bar",
+				},
+			},
+		},
+		{
+			in: map[ACIdentifier]string{
+				"foo": "",
+			},
+			expectedOut: []Label{
+				Label{
+					Name:  "foo",
+					Value: "",
+				},
+			},
+		},
+		{
+			in: map[ACIdentifier]string{
+				"name": "foo",
+			},
+			expectedErr: errors.New(`invalid label name: "name"`),
+		},
+	}
+
+	for i, test := range tests {
+		out, err := LabelsFromMap(test.in)
+		if err != nil {
+			if err.Error() != test.expectedErr.Error() {
+				t.Errorf("case %d: expected %v = %v", i, err, test.expectedErr)
+			}
+			continue
+		}
+		if test.expectedErr != nil {
+			t.Errorf("case %d: expected error %v, but got none", i, test.expectedErr)
+			continue
+		}
+		if !reflect.DeepEqual(test.expectedOut, out) {
+			t.Errorf("case %d: expected %v = %v", i, out, test.expectedOut)
+		}
+	}
+}