Skip to content
This repository has been archived by the owner on Apr 20, 2021. It is now read-only.

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 authored and indradhanush committed Jan 24, 2018
1 parent d090e66 commit 29d6b95
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 29d6b95

Please sign in to comment.