Merge pull request #1759 from bdon/graph-map

Minor refactor of Graph; replace uses of Graph.All (slice) with Graph.Map (map)

api_test.go

@@ -68,7 +68,7 @@ func TestGetInfo(t *testing.T) {
 
 	srv := &Server{runtime: runtime}
 
-	initialImages, err := srv.runtime.graph.All()
+	initialImages, err := srv.runtime.graph.Map()
 	if err != nil {
 		t.Fatal(err)
 	}

graph.go

@@ -274,30 +274,19 @@ func (graph *Graph) Delete(name string) error {
 
 // Map returns a list of all images in the graph, addressable by ID.
 func (graph *Graph) Map() (map[string]*Image, error) {
-	// FIXME: this should replace All()
-	all, err := graph.All()
+	images := make(map[string]*Image)
+	err := graph.walkAll(func(image *Image) {
+		images[image.ID] = image
+	})
 	if err != nil {
 		return nil, err
 	}
-	images := make(map[string]*Image, len(all))
-	for _, image := range all {
-		images[image.ID] = image
-	}
 	return images, nil
 }
 
-// All returns a list of all images in the graph.
-func (graph *Graph) All() ([]*Image, error) {
-	var images []*Image
-	err := graph.WalkAll(func(image *Image) {
-		images = append(images, image)
-	})
-	return images, err
-}
-
-// WalkAll iterates over each image in the graph, and passes it to a handler.
+// walkAll iterates over each image in the graph, and passes it to a handler.
 // The walking order is undetermined.
-func (graph *Graph) WalkAll(handler func(*Image)) error {
+func (graph *Graph) walkAll(handler func(*Image)) error {
 	files, err := ioutil.ReadDir(graph.Root)
 	if err != nil {
 		return err
@@ -319,7 +308,7 @@ func (graph *Graph) WalkAll(handler func(*Image)) error {
 // If an image has no children, it will not have an entry in the table.
 func (graph *Graph) ByParent() (map[string][]*Image, error) {
 	byParent := make(map[string][]*Image)
-	err := graph.WalkAll(func(image *Image) {
+	err := graph.walkAll(func(image *Image) {
 		parent, err := graph.Get(image.Parent)
 		if err != nil {
 			return
@@ -341,7 +330,7 @@ func (graph *Graph) Heads() (map[string]*Image, error) {
 	if err != nil {
 		return nil, err
 	}
-	err = graph.WalkAll(func(image *Image) {
+	err = graph.walkAll(func(image *Image) {
 		// If it's not in the byParent lookup table, then
 		// it's not a parent -> so it's a head!
 		if _, exists := byParent[image.ID]; !exists {

graph_test.go

@@ -20,11 +20,11 @@ func TestInit(t *testing.T) {
 	if _, err := os.Stat(graph.Root); err != nil {
 		t.Fatal(err)
 	}
-	// All() should be empty
-	if l, err := graph.All(); err != nil {
+	// Map() should be empty
+	if l, err := graph.Map(); err != nil {
 		t.Fatal(err)
 	} else if len(l) != 0 {
-		t.Fatalf("List() should return %d, not %d", 0, len(l))
+		t.Fatalf("len(Map()) should return %d, not %d", 0, len(l))
 	}
 }
 
@@ -76,11 +76,15 @@ func TestGraphCreate(t *testing.T) {
 	if image.DockerVersion != VERSION {
 		t.Fatalf("Wrong docker_version: should be '%s', not '%s'", VERSION, image.DockerVersion)
 	}
-	if images, err := graph.All(); err != nil {
+	images, err := graph.Map()
+	if err != nil {
 		t.Fatal(err)
 	} else if l := len(images); l != 1 {
 		t.Fatalf("Wrong number of images. Should be %d, not %d", 1, l)
 	}
+	if images[image.ID] == nil {
+		t.Fatalf("Could not find image with id %s", image.ID)
+	}
 }
 
 func TestRegister(t *testing.T) {
@@ -99,7 +103,7 @@ func TestRegister(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if images, err := graph.All(); err != nil {
+	if images, err := graph.Map(); err != nil {
 		t.Fatal(err)
 	} else if l := len(images); l != 1 {
 		t.Fatalf("Wrong number of images. Should be %d, not %d", 1, l)
@@ -274,7 +278,7 @@ func TestByParent(t *testing.T) {
 }
 
 func assertNImages(graph *Graph, t *testing.T, n int) {
-	if images, err := graph.All(); err != nil {
+	if images, err := graph.Map(); err != nil {
 		t.Fatal(err)
 	} else if actualN := len(images); actualN != n {
 		t.Fatalf("Expected %d images, found %d", n, actualN)

runtime_test.go

@@ -50,7 +50,7 @@ func cleanup(runtime *Runtime) error {
 		container.Kill()
 		runtime.Destroy(container)
 	}
-	images, err := runtime.graph.All()
+	images, err := runtime.graph.Map()
 	if err != nil {
 		return err
 	}
@@ -123,13 +123,13 @@ func init() {
 // FIXME: test that ImagePull(json=true) send correct json output
 
 func GetTestImage(runtime *Runtime) *Image {
-	imgs, err := runtime.graph.All()
+	imgs, err := runtime.graph.Map()
 	if err != nil {
 		panic(err)
 	}
-	for i := range imgs {
-		if imgs[i].ID == unitTestImageID {
-			return imgs[i]
+	for _, image := range imgs {
+		if image.ID == unitTestImageID {
+			return image
 		}
 	}
 	panic(fmt.Errorf("Test image %v not found", unitTestImageID))

server.go

@@ -157,7 +157,7 @@ func (srv *Server) ImageInsert(name, url, path string, out io.Writer, sf *utils.
 }
 
 func (srv *Server) ImagesViz(out io.Writer) error {
-	images, _ := srv.runtime.graph.All()
+	images, _ := srv.runtime.graph.Map()
 	if images == nil {
 		return nil
 	}
@@ -248,7 +248,7 @@ func (srv *Server) Images(all bool, filter string) ([]APIImages, error) {
 }
 
 func (srv *Server) DockerInfo() *APIInfo {
-	images, _ := srv.runtime.graph.All()
+	images, _ := srv.runtime.graph.Map()
 	var imgcount int
 	if images == nil {
 		imgcount = 0
@@ -1110,7 +1110,7 @@ func (srv *Server) ImageDelete(name string, autoPrune bool) ([]APIRmi, error) {
 func (srv *Server) ImageGetCached(imgID string, config *Config) (*Image, error) {
 
 	// Retrieve all images
-	images, err := srv.runtime.graph.All()
+	images, err := srv.runtime.graph.Map()
 	if err != nil {
 		return nil, err
 	}