From 0a417fe0bb24514984be16d1577c4426c8cdda31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 12 Oct 2022 03:20:31 +0200 Subject: [PATCH] Use generics in readAllLayerStores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- check.go | 20 +++---- check_test.go | 4 +- store.go | 151 ++++++++++++++++++++++++-------------------------- 3 files changed, 83 insertions(+), 92 deletions(-) diff --git a/check.go b/check.go index c9e0f00ac7..c18e8e7f08 100644 --- a/check.go +++ b/check.go @@ -160,10 +160,10 @@ func (s *store) Check(options *CheckOptions) (CheckReport, error) { // Walk the list of layer stores, looking at each layer that we didn't see in a // previously-visited store. - if errored, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if _, _, err := readAllLayerStores(s, func(store roLayerStore) (struct{}, bool, error) { layers, err := store.Layers() if err != nil { - return true, err + return struct{}{}, true, err } isReadWrite := roLayerStoreIsReallyReadWrite(store) readWriteDesc := "" @@ -337,7 +337,7 @@ func (s *store) Check(options *CheckOptions) (CheckReport, error) { // At this point we're out of things that we can be sure will work in read-only // stores, so skip the rest for any stores that aren't also read-write stores. if !isReadWrite { - return false, nil + return struct{}{}, false, nil } // Content and mount checks are also things that we can only be sure will work in // read-write stores. @@ -439,8 +439,8 @@ func (s *store) Check(options *CheckOptions) (CheckReport, error) { err := fmt.Errorf("%slayer %s: %w", readWriteDesc, parent, ErrLayerMissing) report.Layers[id] = append(report.Layers[id], err) } - return false, nil - }); errored { + return struct{}{}, false, nil + }); err != nil { return CheckReport{}, err } @@ -630,13 +630,13 @@ func (s *store) Check(options *CheckOptions) (CheckReport, error) { // Now go back through all of the layer stores, and flag any layers which don't belong // to an image or a container, and has been around longer than we can reasonably expect // such a layer to be present before a corresponding image record is added. - if errored, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if _, _, err := readAllLayerStores(s, func(store roLayerStore) (struct{}, bool, error) { if isReadWrite := roLayerStoreIsReallyReadWrite(store); !isReadWrite { - return false, nil + return struct{}{}, false, nil } layers, err := store.Layers() if err != nil { - return true, err + return struct{}{}, true, err } for _, layer := range layers { maximumAge := defaultMaximumUnreferencedLayerAge @@ -654,8 +654,8 @@ func (s *store) Check(options *CheckOptions) (CheckReport, error) { } } } - return false, nil - }); errored { + return struct{}{}, false, nil + }); err != nil { return CheckReport{}, err } diff --git a/check_test.go b/check_test.go index 2920d9f266..69a08a2c0e 100644 --- a/check_test.go +++ b/check_test.go @@ -80,11 +80,11 @@ func TestCheckDetectWriteable(t *testing.T) { require.NoError(t, err, "unexpected error initializing test store") s, ok := stoar.(*store) require.True(t, ok, "unexpected error making type assertion") - done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + _, done, err := readAllLayerStores(s, func(store roLayerStore) (struct{}, bool, error) { if roLayerStoreIsReallyReadWrite(store) { // implicitly checking that the type assertion in this function doesn't panic sawRWlayers = true } - return false, nil + return struct{}{}, false, nil }) assert.False(t, done, "unexpected error from readAllLayerStores") assert.NoError(t, err, "unexpected error from readAllLayerStores") diff --git a/store.go b/store.go index 4d7e9be5d6..07de3cf81a 100644 --- a/store.go +++ b/store.go @@ -1149,38 +1149,39 @@ func (s *store) allLayerStores() ([]roLayerStore, error) { // readAllLayerStores processes allLayerStores() in order: // It locks the store for reading, checks for updates, and calls // -// (done, err) := fn(store) +// (data, done, err) := fn(store) // // until the callback returns done == true, and returns the data from the callback. // -// If reading any layer store fails, it immediately returns (true, err). +// If reading any layer store fails, it immediately returns ({}, true, err). // -// If all layer stores are processed without setting done == true, it returns (false, nil). +// If all layer stores are processed without setting done == true, it returns ({}, false, nil). // // Typical usage: // -// var res T = failureValue -// if done, err := s.readAllLayerStores(store, func(…) { +// if res, done, err := s.readAllLayerStores(store, func(…) { // … // }; done { // return res, err // } -func (s *store) readAllLayerStores(fn func(store roLayerStore) (bool, error)) (bool, error) { +func readAllLayerStores[T any](s *store, fn func(store roLayerStore) (T, bool, error)) (T, bool, error) { + var zeroRes T // A zero value of T + layerStores, err := s.allLayerStores() if err != nil { - return true, err + return zeroRes, true, err } for _, s := range layerStores { store := s if err := store.startReading(); err != nil { - return true, err + return zeroRes, true, err } defer store.stopReading() - if done, err := fn(store); done { - return true, err + if res, done, err := fn(store); done { + return res, true, err } } - return false, nil + return zeroRes, false, nil } // writeToLayerStore is a helper for working with store.getLayerStore(): @@ -1828,19 +1829,17 @@ func (s *store) SetMetadata(id, metadata string) error { } func (s *store) Metadata(id string) (string, error) { - var res string - - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if res, done, err := readAllLayerStores(s, func(store roLayerStore) (string, bool, error) { if store.Exists(id) { - var err error - res, err = store.Metadata(id) - return true, err + res, err := store.Metadata(id) + return res, true, err } - return false, nil + return "", false, nil }); done { return res, err } + var res string if done, err := s.readAllImageStores(func(store roImageStore) (bool, error) { if store.Exists(id) { var err error @@ -1937,17 +1936,15 @@ func (s *store) ImageBigData(id, key string) ([]byte, error) { // named data associated with an layer. func (s *store) ListLayerBigData(id string) ([]string, error) { foundLayer := false - var res []string - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if res, done, err := readAllLayerStores(s, func(store roLayerStore) ([]string, bool, error) { data, err := store.BigDataNames(id) if err == nil { - res = data - return true, nil + return data, true, nil } if store.Exists(id) { foundLayer = true } - return false, nil + return nil, false, nil }); done { return res, err } @@ -1961,17 +1958,15 @@ func (s *store) ListLayerBigData(id string) ([]string, error) { // associated with a layer. func (s *store) LayerBigData(id, key string) (io.ReadCloser, error) { foundLayer := false - var res io.ReadCloser - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if res, done, err := readAllLayerStores(s, func(store roLayerStore) (io.ReadCloser, bool, error) { data, err := store.BigData(id, key) if err == nil { - res = data - return true, nil + return data, true, nil } if store.Exists(id) { foundLayer = true } - return false, nil + return nil, false, nil }); done { return res, err } @@ -2204,18 +2199,20 @@ func (s *store) SetContainerBigData(id, key string, data []byte) error { } func (s *store) Exists(id string) bool { - var res = false - - if done, _ := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + found, _, err := readAllLayerStores(s, func(store roLayerStore) (bool, bool, error) { if store.Exists(id) { - res = true - return true, nil + return true, true, nil } - return false, nil - }); done { - return res + return false, false, nil + }) + if err != nil { + return false + } + if found { + return true } + var res = false if done, _ := s.readAllImageStores(func(store roImageStore) (bool, error) { if store.Exists(id) { res = true @@ -2345,18 +2342,16 @@ func (s *store) updateNames(id string, names []string, op updateNameOperation) e } func (s *store) Names(id string) ([]string, error) { - var res []string - - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if res, done, err := readAllLayerStores(s, func(store roLayerStore) ([]string, bool, error) { if l, err := store.Get(id); l != nil && err == nil { - res = l.Names - return true, nil + return l.Names, true, nil } - return false, nil + return nil, false, nil }); done { return res, err } + var res []string if done, err := s.readAllImageStores(func(store roImageStore) (bool, error) { if i, err := store.Get(id); i != nil && err == nil { res = i.Names @@ -2378,18 +2373,16 @@ func (s *store) Names(id string) ([]string, error) { } func (s *store) Lookup(name string) (string, error) { - var res string - - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if res, done, err := readAllLayerStores(s, func(store roLayerStore) (string, bool, error) { if l, err := store.Get(name); l != nil && err == nil { - res = l.ID - return true, nil + return l.ID, true, nil } - return false, nil + return "", false, nil }); done { return res, err } + var res string if done, err := s.readAllImageStores(func(store roImageStore) (bool, error) { if i, err := store.Get(name); i != nil && err == nil { res = i.ID @@ -2782,14 +2775,12 @@ func (s *store) Unmount(id string, force bool) (bool, error) { } func (s *store) Changes(from, to string) ([]archive.Change, error) { - var res []archive.Change - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if res, done, err := readAllLayerStores(s, func(store roLayerStore) ([]archive.Change, bool, error) { if store.Exists(to) { - var err error - res, err = store.Changes(from, to) - return true, err + res, err := store.Changes(from, to) + return res, true, err } - return false, nil + return nil, false, nil }); done { return res, err } @@ -2797,16 +2788,17 @@ func (s *store) Changes(from, to string) ([]archive.Change, error) { } func (s *store) DiffSize(from, to string) (int64, error) { - var res int64 = -1 - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if res, done, err := readAllLayerStores(s, func(store roLayerStore) (int64, bool, error) { if store.Exists(to) { - var err error - res, err = store.DiffSize(from, to) - return true, err + res, err := store.DiffSize(from, to) + return res, true, err } - return false, nil + return -1, false, nil }); done { - return res, err + if err != nil { + return -1, err + } + return res, nil } return -1, ErrLayerUnknown } @@ -2895,16 +2887,16 @@ func (s *store) ApplyDiff(to string, diff io.Reader) (int64, error) { func (s *store) layersByMappedDigest(m func(roLayerStore, digest.Digest) ([]Layer, error), d digest.Digest) ([]Layer, error) { var layers []Layer - if _, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if _, _, err := readAllLayerStores(s, func(store roLayerStore) (struct{}, bool, error) { storeLayers, err := m(store, d) if err != nil { if !errors.Is(err, ErrLayerUnknown) { - return true, err + return struct{}{}, true, err } - return false, nil + return struct{}{}, false, nil } layers = append(layers, storeLayers...) - return false, nil + return struct{}{}, false, nil }); err != nil { return nil, err } @@ -2929,16 +2921,17 @@ func (s *store) LayersByUncompressedDigest(d digest.Digest) ([]Layer, error) { } func (s *store) LayerSize(id string) (int64, error) { - var res int64 = -1 - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if res, done, err := readAllLayerStores(s, func(store roLayerStore) (int64, bool, error) { if store.Exists(id) { - var err error - res, err = store.Size(id) - return true, err + res, err := store.Size(id) + return res, true, err } - return false, nil + return -1, false, nil }); done { - return res, err + if err != nil { + return -1, err + } + return res, nil } return -1, ErrLayerUnknown } @@ -2983,13 +2976,13 @@ func (s *store) ContainerParentOwners(id string) ([]int, []int, error) { func (s *store) Layers() ([]Layer, error) { var layers []Layer - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if _, done, err := readAllLayerStores(s, func(store roLayerStore) (struct{}, bool, error) { storeLayers, err := store.Layers() if err != nil { - return true, err + return struct{}{}, true, err } layers = append(layers, storeLayers...) - return false, nil + return struct{}{}, false, nil }); done { return nil, err } @@ -3021,14 +3014,12 @@ func (s *store) Containers() ([]Container, error) { } func (s *store) Layer(id string) (*Layer, error) { - var res *Layer - if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { + if res, done, err := readAllLayerStores(s, func(store roLayerStore) (*Layer, bool, error) { layer, err := store.Get(id) if err == nil { - res = layer - return true, nil + return layer, true, nil } - return false, nil + return nil, false, nil }); done { return res, err }