Merge pull request #689 from euank/order-labels

schema/labels: consistently order LabelsFromMap

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
 }
 

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)
+		}
+	}
+}