diff --git a/api/v6/container.go b/api/v6/container.go index 5fd0dc1543..46d7ccb862 100644 --- a/api/v6/container.go +++ b/api/v6/container.go @@ -28,52 +28,6 @@ type Container struct { // NewContainer creates a Container given a list of images and the current image func NewContainer(name string, images []image.Info, currentImage image.Info, tagPattern policy.Pattern, fields []string) (Container, error) { - sorted := update.SortImages(images, tagPattern) - - // All images - imagesCount := len(sorted) - imagesErr := "" - if sorted == nil { - imagesErr = registry.ErrNoImageData.Error() - } - var newImages update.SortedImageInfos - for _, img := range sorted { - if tagPattern.Newer(&img, ¤tImage) { - newImages = append(newImages, img) - } - } - newImagesCount := len(newImages) - - // Filtered images (which respects sorting) - filteredImages := update.SortedImageInfos(update.FilterImages(sorted, tagPattern)) - filteredImagesCount := len(filteredImages) - var newFilteredImages update.SortedImageInfos - for _, img := range filteredImages { - if tagPattern.Newer(&img, ¤tImage) { - newFilteredImages = append(newFilteredImages, img) - } - } - newFilteredImagesCount := len(newFilteredImages) - latestFiltered, _ := filteredImages.Latest() - - container := Container{ - Name: name, - Current: currentImage, - LatestFiltered: latestFiltered, - - Available: sorted, - AvailableError: imagesErr, - AvailableImagesCount: imagesCount, - NewAvailableImagesCount: newImagesCount, - FilteredImagesCount: filteredImagesCount, - NewFilteredImagesCount: newFilteredImagesCount, - } - return filterContainerFields(container, fields) -} - -// filterContainerFields returns a new container with only the fields specified. If not fields are specified, -// a list of default fields is used. -func filterContainerFields(container Container, fields []string) (Container, error) { // Default fields if len(fields) == 0 { fields = []string{ @@ -90,29 +44,109 @@ func filterContainerFields(container Container, fields []string) (Container, err } var c Container + + // The following machinery attempts to minimise the number of + // filters (`O(n)`) and sorts (`O(n log n)`), by memoising and + // sharing intermediate results. + + var ( + sortedImages update.SortedImageInfos + filteredImages []image.Info + sortedFilteredImages update.SortedImageInfos + ) + + getSortedImages := func() update.SortedImageInfos { + if sortedImages == nil { + sortedImages = update.SortImages(images, tagPattern) + } + return sortedImages + } + + getFilteredImages := func() []image.Info { + if filteredImages == nil { + filteredImages = update.FilterImages(images, tagPattern) + } + return filteredImages + } + + getSortedFilteredImages := func() update.SortedImageInfos { + if sortedFilteredImages == nil { + sortedFilteredImages = update.SortImages(getFilteredImages(), tagPattern) + } + return sortedFilteredImages + } + + assignFields := []func(){} + for _, field := range fields { switch field { + // these first few rely only on the inputs case "Name": - c.Name = container.Name + c.Name = name case "Current": - c.Current = container.Current - case "LatestFiltered": - c.LatestFiltered = container.LatestFiltered - case "Available": - c.Available = container.Available + c.Current = currentImage case "AvailableError": - c.AvailableError = container.AvailableError + if images == nil { + c.AvailableError = registry.ErrNoImageData.Error() + } case "AvailableImagesCount": - c.AvailableImagesCount = container.AvailableImagesCount + c.AvailableImagesCount = len(images) + + case "Available": // needs sorted images + assignFields = append(assignFields, func() { + c.Available = getSortedImages() + }) + // now that we know we need to sort all the images anyway, + // the fastest way to get sorted, filtered images will be + // to filter the already sorted images + getSortedFilteredImages = func() update.SortedImageInfos { + return update.FilterImages(getSortedImages(), tagPattern) + } + getFilteredImages = func() []image.Info { + return []image.Info(getSortedFilteredImages()) + } + + case "LatestFiltered": // needs sorted, filtered images + assignFields = append(assignFields, func() { + latest, _ := getSortedFilteredImages().Latest() + c.LatestFiltered = latest + }) + case "NewAvailableImagesCount": - c.NewAvailableImagesCount = container.NewAvailableImagesCount - case "FilteredImagesCount": - c.FilteredImagesCount = container.FilteredImagesCount - case "NewFilteredImagesCount": - c.NewFilteredImagesCount = container.NewFilteredImagesCount + assignFields = append(assignFields, func() { + newImagesCount := 0 + for _, img := range getSortedImages() { + if !tagPattern.Newer(&img, ¤tImage) { + break + } + newImagesCount++ + } + c.NewAvailableImagesCount = newImagesCount + }) + + case "FilteredImagesCount": // needs filtered tags + assignFields = append(assignFields, func() { + c.FilteredImagesCount = len(getFilteredImages()) + }) + case "NewFilteredImagesCount": // needs filtered images + assignFields = append(assignFields, func() { + newFilteredImagesCount := 0 + for _, img := range getSortedFilteredImages() { + if !tagPattern.Newer(&img, ¤tImage) { + break + } + newFilteredImagesCount++ + } + c.NewFilteredImagesCount = newFilteredImagesCount + }) default: return c, errors.Errorf("%s is an invalid field", field) } } + + for _, fn := range assignFields { + fn() + } + return c, nil } diff --git a/api/v6/container_test.go b/api/v6/container_test.go index cb77e388ca..fb1be24dd6 100644 --- a/api/v6/container_test.go +++ b/api/v6/container_test.go @@ -1,7 +1,6 @@ package v6 import ( - "reflect" "testing" "github.com/stretchr/testify/assert" @@ -71,81 +70,66 @@ func TestNewContainer(t *testing.T) { }, wantErr: false, }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := NewContainer(tt.args.name, tt.args.images, tt.args.currentImage, tt.args.tagPattern, tt.args.fields) - assert.Equal(t, tt.wantErr, err != nil) - assert.Equal(t, tt.want, got) - }) - } -} - -func TestFilterContainerFields(t *testing.T) { - testContainer := Container{ - Name: "test", - Current: image.Info{ImageID: "123"}, - LatestFiltered: image.Info{ImageID: "123"}, - Available: []image.Info{{ImageID: "123"}}, - AvailableError: "test", - AvailableImagesCount: 1, - NewAvailableImagesCount: 2, - FilteredImagesCount: 3, - NewFilteredImagesCount: 4, - } - - type args struct { - container Container - fields []string - } - tests := []struct { - name string - args args - want Container - wantErr bool - }{ { - name: "Default fields", + name: "Require only some calculations", args: args{ - container: testContainer, + name: "container-some", + images: []image.Info{currentSemver, newSemver, oldSemver, testImage}, + currentImage: currentSemver, + tagPattern: policy.NewPattern("semver:*"), + fields: []string{"Name", "NewFilteredImagesCount"}, // but not, e.g., "FilteredImagesCount" + }, + want: Container{ + Name: "container-some", + NewFilteredImagesCount: 1, }, - want: testContainer, - wantErr: false, }, { - name: "FilterImages", + name: "Fields in one order", args: args{ - container: testContainer, - fields: []string{"Name", "Available", "NewAvailableImagesCount", "NewFilteredImagesCount"}, + name: "container-ordered1", + images: []image.Info{currentSemver, newSemver, oldSemver, testImage}, + currentImage: currentSemver, + tagPattern: policy.NewPattern("semver:*"), + fields: []string{"Name", + "AvailableImagesCount", "Available", // these two both depend on the same intermediate result + "LatestFiltered", "FilteredImagesCount", // these two both depend on another intermediate result + }, }, want: Container{ - Name: "test", - Available: []image.Info{{ImageID: "123"}}, - NewAvailableImagesCount: 2, - NewFilteredImagesCount: 4, + Name: "container-ordered1", + Available: []image.Info{newSemver, currentSemver, oldSemver, testImage}, + AvailableImagesCount: 4, + LatestFiltered: newSemver, + FilteredImagesCount: 3, }, - wantErr: false, }, { - name: "Invalid field", + name: "Fields in another order", args: args{ - container: testContainer, - fields: []string{"Invalid"}, + name: "container-ordered2", + images: []image.Info{currentSemver, newSemver, oldSemver, testImage}, + currentImage: currentSemver, + tagPattern: policy.NewPattern("semver:*"), + fields: []string{"Name", + "Available", "AvailableImagesCount", // these two latter depend on the same intermediate result, as above + "FilteredImagesCount", "LatestFiltered", // as above, similarly + }, + }, + want: Container{ + Name: "container-ordered2", + Available: []image.Info{newSemver, currentSemver, oldSemver, testImage}, + AvailableImagesCount: 4, + LatestFiltered: newSemver, + FilteredImagesCount: 3, }, - want: Container{}, - wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := filterContainerFields(tt.args.container, tt.args.fields) - if (err != nil) != tt.wantErr { - t.Errorf("FilterContainerFields() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("FilterContainerFields() = %v, want %v", got, tt.want) - } + got, err := NewContainer(tt.args.name, tt.args.images, tt.args.currentImage, tt.args.tagPattern, tt.args.fields) + assert.Equal(t, tt.wantErr, err != nil) + assert.Equal(t, tt.want, got) }) } }