Skip to content

Commit

Permalink
schema/labels: consistently order LabelsFromMap
Browse files Browse the repository at this point in the history
Prior to this change, the slice of labels produced by LabelsFromMap
could have any arbitrary ordering.

See appc/docker2aci#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.
  • Loading branch information
euank committed Jun 12, 2017
1 parent 846ceef commit de6562a
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 3 deletions.
11 changes: 8 additions & 3 deletions schema/types/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down
66 changes: 66 additions & 0 deletions schema/types/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package types

import (
"encoding/json"
"errors"
"reflect"
"strings"
"testing"
)
Expand Down Expand Up @@ -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)
}
}
}

0 comments on commit de6562a

Please sign in to comment.