From 3a32b587922eca87d1ab23689f407b75a8603ccb Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 13 Sep 2016 07:01:31 +0000 Subject: [PATCH] Fix broken JSON support in cli/command/formatter How to test: $ docker ps --format '{{json .}}' $ docker network ls --format '{{json .}}' $ docker volume ls --format '{{json .}}' Signed-off-by: Akihiro Suda --- cli/command/formatter/container.go | 4 ++ cli/command/formatter/container_test.go | 47 ++++++++++++++++++ cli/command/formatter/network.go | 4 ++ cli/command/formatter/network_test.go | 46 +++++++++++++++++ cli/command/formatter/reflect.go | 65 ++++++++++++++++++++++++ cli/command/formatter/reflect_test.go | 66 +++++++++++++++++++++++++ cli/command/formatter/service.go | 4 ++ cli/command/formatter/volume.go | 4 ++ cli/command/formatter/volume_test.go | 45 +++++++++++++++++ cli/command/service/inspect_test.go | 47 +++++++++++++++--- pkg/testutil/assert/assert.go | 9 ++++ 11 files changed, 334 insertions(+), 7 deletions(-) create mode 100644 cli/command/formatter/reflect.go create mode 100644 cli/command/formatter/reflect_test.go diff --git a/cli/command/formatter/container.go b/cli/command/formatter/container.go index ceef75890f..094bc85447 100644 --- a/cli/command/formatter/container.go +++ b/cli/command/formatter/container.go @@ -79,6 +79,10 @@ type containerContext struct { c types.Container } +func (c *containerContext) MarshalJSON() ([]byte, error) { + return marshalJSON(c) +} + func (c *containerContext) ID() string { c.AddHeader(containerIDHeader) if c.trunc { diff --git a/cli/command/formatter/container_test.go b/cli/command/formatter/container_test.go index 1ef48ae2d2..4b520f94ba 100644 --- a/cli/command/formatter/container_test.go +++ b/cli/command/formatter/container_test.go @@ -2,6 +2,7 @@ package formatter import ( "bytes" + "encoding/json" "fmt" "strings" "testing" @@ -323,3 +324,49 @@ func TestContainerContextWriteWithNoContainers(t *testing.T) { out.Reset() } } + +func TestContainerContextWriteJSON(t *testing.T) { + unix := time.Now().Add(-65 * time.Second).Unix() + containers := []types.Container{ + {ID: "containerID1", Names: []string{"/foobar_baz"}, Image: "ubuntu", Created: unix}, + {ID: "containerID2", Names: []string{"/foobar_bar"}, Image: "ubuntu", Created: unix}, + } + expectedCreated := time.Unix(unix, 0).String() + expectedJSONs := []map[string]interface{}{ + {"Command": "\"\"", "CreatedAt": expectedCreated, "ID": "containerID1", "Image": "ubuntu", "Labels": "", "LocalVolumes": "0", "Mounts": "", "Names": "foobar_baz", "Ports": "", "RunningFor": "About a minute", "Size": "0 B", "Status": ""}, + {"Command": "\"\"", "CreatedAt": expectedCreated, "ID": "containerID2", "Image": "ubuntu", "Labels": "", "LocalVolumes": "0", "Mounts": "", "Names": "foobar_bar", "Ports": "", "RunningFor": "About a minute", "Size": "0 B", "Status": ""}, + } + out := bytes.NewBufferString("") + err := ContainerWrite(Context{Format: "{{json .}}", Output: out}, containers) + if err != nil { + t.Fatal(err) + } + for i, line := range strings.Split(strings.TrimSpace(out.String()), "\n") { + t.Logf("Output: line %d: %s", i, line) + var m map[string]interface{} + if err := json.Unmarshal([]byte(line), &m); err != nil { + t.Fatal(err) + } + assert.DeepEqual(t, m, expectedJSONs[i]) + } +} + +func TestContainerContextWriteJSONField(t *testing.T) { + containers := []types.Container{ + {ID: "containerID1", Names: []string{"/foobar_baz"}, Image: "ubuntu"}, + {ID: "containerID2", Names: []string{"/foobar_bar"}, Image: "ubuntu"}, + } + out := bytes.NewBufferString("") + err := ContainerWrite(Context{Format: "{{json .ID}}", Output: out}, containers) + if err != nil { + t.Fatal(err) + } + for i, line := range strings.Split(strings.TrimSpace(out.String()), "\n") { + t.Logf("Output: line %d: %s", i, line) + var s string + if err := json.Unmarshal([]byte(line), &s); err != nil { + t.Fatal(err) + } + assert.Equal(t, s, containers[i].ID) + } +} diff --git a/cli/command/formatter/network.go b/cli/command/formatter/network.go index d808fdc22d..7fbad7d2ab 100644 --- a/cli/command/formatter/network.go +++ b/cli/command/formatter/network.go @@ -53,6 +53,10 @@ type networkContext struct { n types.NetworkResource } +func (c *networkContext) MarshalJSON() ([]byte, error) { + return marshalJSON(c) +} + func (c *networkContext) ID() string { c.AddHeader(networkIDHeader) if c.trunc { diff --git a/cli/command/formatter/network_test.go b/cli/command/formatter/network_test.go index 28f078548f..b40a534eed 100644 --- a/cli/command/formatter/network_test.go +++ b/cli/command/formatter/network_test.go @@ -2,6 +2,7 @@ package formatter import ( "bytes" + "encoding/json" "strings" "testing" @@ -160,3 +161,48 @@ foobar_bar } } } + +func TestNetworkContextWriteJSON(t *testing.T) { + networks := []types.NetworkResource{ + {ID: "networkID1", Name: "foobar_baz"}, + {ID: "networkID2", Name: "foobar_bar"}, + } + expectedJSONs := []map[string]interface{}{ + {"Driver": "", "ID": "networkID1", "IPv6": "false", "Internal": "false", "Labels": "", "Name": "foobar_baz", "Scope": ""}, + {"Driver": "", "ID": "networkID2", "IPv6": "false", "Internal": "false", "Labels": "", "Name": "foobar_bar", "Scope": ""}, + } + + out := bytes.NewBufferString("") + err := NetworkWrite(Context{Format: "{{json .}}", Output: out}, networks) + if err != nil { + t.Fatal(err) + } + for i, line := range strings.Split(strings.TrimSpace(out.String()), "\n") { + t.Logf("Output: line %d: %s", i, line) + var m map[string]interface{} + if err := json.Unmarshal([]byte(line), &m); err != nil { + t.Fatal(err) + } + assert.DeepEqual(t, m, expectedJSONs[i]) + } +} + +func TestNetworkContextWriteJSONField(t *testing.T) { + networks := []types.NetworkResource{ + {ID: "networkID1", Name: "foobar_baz"}, + {ID: "networkID2", Name: "foobar_bar"}, + } + out := bytes.NewBufferString("") + err := NetworkWrite(Context{Format: "{{json .ID}}", Output: out}, networks) + if err != nil { + t.Fatal(err) + } + for i, line := range strings.Split(strings.TrimSpace(out.String()), "\n") { + t.Logf("Output: line %d: %s", i, line) + var s string + if err := json.Unmarshal([]byte(line), &s); err != nil { + t.Fatal(err) + } + assert.Equal(t, s, networks[i].ID) + } +} diff --git a/cli/command/formatter/reflect.go b/cli/command/formatter/reflect.go new file mode 100644 index 0000000000..d1d8737d21 --- /dev/null +++ b/cli/command/formatter/reflect.go @@ -0,0 +1,65 @@ +package formatter + +import ( + "encoding/json" + "fmt" + "reflect" + "unicode" +) + +func marshalJSON(x interface{}) ([]byte, error) { + m, err := marshalMap(x) + if err != nil { + return nil, err + } + return json.Marshal(m) +} + +// marshalMap marshals x to map[string]interface{} +func marshalMap(x interface{}) (map[string]interface{}, error) { + val := reflect.ValueOf(x) + if val.Kind() != reflect.Ptr { + return nil, fmt.Errorf("expected a pointer to a struct, got %v", val.Kind()) + } + if val.IsNil() { + return nil, fmt.Errorf("expxected a pointer to a struct, got nil pointer") + } + valElem := val.Elem() + if valElem.Kind() != reflect.Struct { + return nil, fmt.Errorf("expected a pointer to a struct, got a pointer to %v", valElem.Kind()) + } + typ := val.Type() + m := make(map[string]interface{}) + for i := 0; i < val.NumMethod(); i++ { + k, v, err := marshalForMethod(typ.Method(i), val.Method(i)) + if err != nil { + return nil, err + } + if k != "" { + m[k] = v + } + } + return m, nil +} + +var unmarshallableNames = map[string]struct{}{"FullHeader": {}} + +// marshalForMethod returns the map key and the map value for marshalling the method. +// It returns ("", nil, nil) for valid but non-marshallable parameter. (e.g. "unexportedFunc()") +func marshalForMethod(typ reflect.Method, val reflect.Value) (string, interface{}, error) { + if val.Kind() != reflect.Func { + return "", nil, fmt.Errorf("expected func, got %v", val.Kind()) + } + name, numIn, numOut := typ.Name, val.Type().NumIn(), val.Type().NumOut() + _, blackListed := unmarshallableNames[name] + // FIXME: In text/template, (numOut == 2) is marshallable, + // if the type of the second param is error. + marshallable := unicode.IsUpper(rune(name[0])) && !blackListed && + numIn == 0 && numOut == 1 + if !marshallable { + return "", nil, nil + } + result := val.Call(make([]reflect.Value, numIn)) + intf := result[0].Interface() + return name, intf, nil +} diff --git a/cli/command/formatter/reflect_test.go b/cli/command/formatter/reflect_test.go new file mode 100644 index 0000000000..e547b18411 --- /dev/null +++ b/cli/command/formatter/reflect_test.go @@ -0,0 +1,66 @@ +package formatter + +import ( + "reflect" + "testing" +) + +type dummy struct { +} + +func (d *dummy) Func1() string { + return "Func1" +} + +func (d *dummy) func2() string { + return "func2(should not be marshalled)" +} + +func (d *dummy) Func3() (string, int) { + return "Func3(should not be marshalled)", -42 +} + +func (d *dummy) Func4() int { + return 4 +} + +type dummyType string + +func (d *dummy) Func5() dummyType { + return dummyType("Func5") +} + +func (d *dummy) FullHeader() string { + return "FullHeader(should not be marshalled)" +} + +var dummyExpected = map[string]interface{}{ + "Func1": "Func1", + "Func4": 4, + "Func5": dummyType("Func5"), +} + +func TestMarshalMap(t *testing.T) { + d := dummy{} + m, err := marshalMap(&d) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(dummyExpected, m) { + t.Fatalf("expected %+v, got %+v", + dummyExpected, m) + } +} + +func TestMarshalMapBad(t *testing.T) { + if _, err := marshalMap(nil); err == nil { + t.Fatal("expected an error (argument is nil)") + } + if _, err := marshalMap(dummy{}); err == nil { + t.Fatal("expected an error (argument is non-pointer)") + } + x := 42 + if _, err := marshalMap(&x); err == nil { + t.Fatal("expected an error (argument is a pointer to non-struct)") + } +} diff --git a/cli/command/formatter/service.go b/cli/command/formatter/service.go index a92326e75d..71ee4d656a 100644 --- a/cli/command/formatter/service.go +++ b/cli/command/formatter/service.go @@ -139,6 +139,10 @@ type serviceInspectContext struct { subContext } +func (ctx *serviceInspectContext) MarshalJSON() ([]byte, error) { + return marshalJSON(ctx) +} + func (ctx *serviceInspectContext) ID() string { return ctx.Service.ID } diff --git a/cli/command/formatter/volume.go b/cli/command/formatter/volume.go index 7bc3537573..b76c8ba039 100644 --- a/cli/command/formatter/volume.go +++ b/cli/command/formatter/volume.go @@ -52,6 +52,10 @@ type volumeContext struct { v types.Volume } +func (c *volumeContext) MarshalJSON() ([]byte, error) { + return marshalJSON(c) +} + func (c *volumeContext) Name() string { c.AddHeader(nameHeader) return c.v.Name diff --git a/cli/command/formatter/volume_test.go b/cli/command/formatter/volume_test.go index 8c715b3438..4e1c8d3ab9 100644 --- a/cli/command/formatter/volume_test.go +++ b/cli/command/formatter/volume_test.go @@ -2,6 +2,7 @@ package formatter import ( "bytes" + "encoding/json" "strings" "testing" @@ -142,3 +143,47 @@ foobar_bar } } } + +func TestVolumeContextWriteJSON(t *testing.T) { + volumes := []*types.Volume{ + {Driver: "foo", Name: "foobar_baz"}, + {Driver: "bar", Name: "foobar_bar"}, + } + expectedJSONs := []map[string]interface{}{ + {"Driver": "foo", "Labels": "", "Links": "N/A", "Mountpoint": "", "Name": "foobar_baz", "Scope": "", "Size": "N/A"}, + {"Driver": "bar", "Labels": "", "Links": "N/A", "Mountpoint": "", "Name": "foobar_bar", "Scope": "", "Size": "N/A"}, + } + out := bytes.NewBufferString("") + err := VolumeWrite(Context{Format: "{{json .}}", Output: out}, volumes) + if err != nil { + t.Fatal(err) + } + for i, line := range strings.Split(strings.TrimSpace(out.String()), "\n") { + t.Logf("Output: line %d: %s", i, line) + var m map[string]interface{} + if err := json.Unmarshal([]byte(line), &m); err != nil { + t.Fatal(err) + } + assert.DeepEqual(t, m, expectedJSONs[i]) + } +} + +func TestVolumeContextWriteJSONField(t *testing.T) { + volumes := []*types.Volume{ + {Driver: "foo", Name: "foobar_baz"}, + {Driver: "bar", Name: "foobar_bar"}, + } + out := bytes.NewBufferString("") + err := VolumeWrite(Context{Format: "{{json .Name}}", Output: out}, volumes) + if err != nil { + t.Fatal(err) + } + for i, line := range strings.Split(strings.TrimSpace(out.String()), "\n") { + t.Logf("Output: line %d: %s", i, line) + var s string + if err := json.Unmarshal([]byte(line), &s); err != nil { + t.Fatal(err) + } + assert.Equal(t, s, volumes[i].Name) + } +} diff --git a/cli/command/service/inspect_test.go b/cli/command/service/inspect_test.go index 8e73a70efa..04a65080c7 100644 --- a/cli/command/service/inspect_test.go +++ b/cli/command/service/inspect_test.go @@ -2,15 +2,17 @@ package service import ( "bytes" + "encoding/json" "strings" "testing" "time" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/cli/command/formatter" + "github.com/docker/docker/pkg/testutil/assert" ) -func TestPrettyPrintWithNoUpdateConfig(t *testing.T) { +func formatServiceInspect(t *testing.T, format formatter.Format, now time.Time) string { b := new(bytes.Buffer) endpointSpec := &swarm.EndpointSpec{ @@ -29,8 +31,8 @@ func TestPrettyPrintWithNoUpdateConfig(t *testing.T) { ID: "de179gar9d0o7ltdybungplod", Meta: swarm.Meta{ Version: swarm.Version{Index: 315}, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + CreatedAt: now, + UpdatedAt: now, }, Spec: swarm.ServiceSpec{ Annotations: swarm.Annotations{ @@ -73,14 +75,14 @@ func TestPrettyPrintWithNoUpdateConfig(t *testing.T) { }, }, UpdateStatus: swarm.UpdateStatus{ - StartedAt: time.Now(), - CompletedAt: time.Now(), + StartedAt: now, + CompletedAt: now, }, } ctx := formatter.Context{ Output: b, - Format: formatter.NewServiceFormat("pretty"), + Format: format, } err := formatter.ServiceInspectWrite(ctx, []string{"de179gar9d0o7ltdybungplod"}, func(ref string) (interface{}, []byte, error) { @@ -89,8 +91,39 @@ func TestPrettyPrintWithNoUpdateConfig(t *testing.T) { if err != nil { t.Fatal(err) } + return b.String() +} - if strings.Contains(b.String(), "UpdateStatus") { +func TestPrettyPrintWithNoUpdateConfig(t *testing.T) { + s := formatServiceInspect(t, formatter.NewServiceFormat("pretty"), time.Now()) + if strings.Contains(s, "UpdateStatus") { t.Fatal("Pretty print failed before parsing UpdateStatus") } } + +func TestJSONFormatWithNoUpdateConfig(t *testing.T) { + now := time.Now() + // s1: [{"ID":..}] + // s2: {"ID":..} + s1 := formatServiceInspect(t, formatter.NewServiceFormat(""), now) + t.Log("// s1") + t.Logf("%s", s1) + s2 := formatServiceInspect(t, formatter.NewServiceFormat("{{json .}}"), now) + t.Log("// s2") + t.Logf("%s", s2) + var m1Wrap []map[string]interface{} + if err := json.Unmarshal([]byte(s1), &m1Wrap); err != nil { + t.Fatal(err) + } + if len(m1Wrap) != 1 { + t.Fatalf("strange s1=%s", s1) + } + m1 := m1Wrap[0] + t.Logf("m1=%+v", m1) + var m2 map[string]interface{} + if err := json.Unmarshal([]byte(s2), &m2); err != nil { + t.Fatal(err) + } + t.Logf("m2=%+v", m2) + assert.DeepEqual(t, m2, m1) +} diff --git a/pkg/testutil/assert/assert.go b/pkg/testutil/assert/assert.go index f6ee8d75f0..6de0ef6779 100644 --- a/pkg/testutil/assert/assert.go +++ b/pkg/testutil/assert/assert.go @@ -4,6 +4,7 @@ package assert import ( "fmt" "path/filepath" + "reflect" "runtime" "strings" ) @@ -44,6 +45,14 @@ func NilError(t TestingT, err error) { } } +// DeepEqual compare the actual value to the expected value and fails the test if +// they are not "deeply equal". +func DeepEqual(t TestingT, actual, expected interface{}) { + if !reflect.DeepEqual(actual, expected) { + fatal(t, "Expected '%v' (%T) got '%v' (%T)", expected, expected, actual, actual) + } +} + // Error asserts that error is not nil, and contains the expected text, // otherwise it fails the test. func Error(t TestingT, err error, contains string) {