Merge pull request #3019 from greatroar/refactor-decodeindex

Refactor index decoding

internal/checker/checker.go

@@ -117,12 +117,16 @@ func (c *Checker) LoadIndex(ctx context.Context) (hints []error, errs []error) {
 			debug.Log("worker got file %v", fi.ID.Str())
 			var err error
 			var idx *repository.Index
-			idx, buf, err = repository.LoadIndexWithDecoder(ctx, c.repo, buf[:0], fi.ID, repository.DecodeIndex)
+			oldFormat := false
-			if errors.Cause(err) == repository.ErrOldIndexFormat {
+
+			buf, err = c.repo.LoadAndDecrypt(ctx, buf[:0], restic.IndexFile, fi.ID)
+			if err == nil {
+				idx, oldFormat, err = repository.DecodeIndex(buf)
+			}
+
+			if oldFormat {
 				debug.Log("index %v has old format", fi.ID.Str())
 				hints = append(hints, ErrOldIndexFormat{fi.ID})
-
-				idx, buf, err = repository.LoadIndexWithDecoder(ctx, c.repo, buf[:0], fi.ID, repository.DecodeOldIndex)
 			}
 
 			err = errors.Wrapf(err, "error loading index %v", fi.ID.Str())

internal/repository/index.go

@@ -515,18 +515,12 @@ func (idx *Index) merge(idx2 *Index) error {
 // isErrOldIndex returns true if the error may be caused by an old index
 // format.
 func isErrOldIndex(err error) bool {
-	if e, ok := err.(*json.UnmarshalTypeError); ok && e.Value == "array" {
+	e, ok := err.(*json.UnmarshalTypeError)
-		return true
+	return ok && e.Value == "array"
-	}
-
-	return false
 }
 
-// ErrOldIndexFormat means an index with the old format was detected.
+// DecodeIndex unserializes an index from buf.
-var ErrOldIndexFormat = errors.New("index has old format")
+func DecodeIndex(buf []byte) (idx *Index, oldFormat bool, err error) {
-
-// DecodeIndex loads and unserializes an index from rd.
-func DecodeIndex(buf []byte) (idx *Index, err error) {
 	debug.Log("Start decoding index")
 	idxJSON := &jsonIndex{}
 
@@ -536,10 +530,11 @@ func DecodeIndex(buf []byte) (idx *Index, err error) {
 
 		if isErrOldIndex(err) {
 			debug.Log("index is probably old format, trying that")
-			err = ErrOldIndexFormat
+			idx, err = decodeOldIndex(buf)
+			return idx, err == nil, err
 		}
 
-		return nil, errors.Wrap(err, "Decode")
+		return nil, false, errors.Wrap(err, "DecodeIndex")
 	}
 
 	idx = NewIndex()
@@ -571,11 +566,11 @@ func DecodeIndex(buf []byte) (idx *Index, err error) {
 	idx.final = true
 
 	debug.Log("done")
-	return idx, nil
+	return idx, false, nil
 }
 
 // DecodeOldIndex loads and unserializes an index in the old format from rd.
-func DecodeOldIndex(buf []byte) (idx *Index, err error) {
+func decodeOldIndex(buf []byte) (idx *Index, err error) {
 	debug.Log("Start decoding old index")
 	list := []*packJSON{}
 
@@ -615,23 +610,3 @@ func DecodeOldIndex(buf []byte) (idx *Index, err error) {
 	debug.Log("done")
 	return idx, nil
 }
-
-// LoadIndexWithDecoder loads the index and decodes it with fn.
-func LoadIndexWithDecoder(ctx context.Context, repo restic.Repository, buf []byte, id restic.ID, fn func([]byte) (*Index, error)) (*Index, []byte, error) {
-	debug.Log("Loading index %v", id)
-
-	buf, err := repo.LoadAndDecrypt(ctx, buf[:0], restic.IndexFile, id)
-	if err != nil {
-		return nil, buf[:0], err
-	}
-
-	idx, err := fn(buf)
-	if err != nil {
-		debug.Log("error while decoding index %v: %v", id, err)
-		return nil, buf[:0], err
-	}
-
-	idx.ids = append(idx.ids, id)
-
-	return idx, buf, nil
-}

internal/repository/index_test.go

@@ -56,10 +56,11 @@ func TestIndexSerialize(t *testing.T) {
 	err := idx.Encode(wr)
 	rtest.OK(t, err)
 
-	idx2, err := repository.DecodeIndex(wr.Bytes())
+	idx2, oldFormat, err := repository.DecodeIndex(wr.Bytes())
 	rtest.OK(t, err)
 	rtest.Assert(t, idx2 != nil,
 		"nil returned for decoded index")
+	rtest.Assert(t, !oldFormat, "new index format recognized as old format")
 
 	wr2 := bytes.NewBuffer(nil)
 	err = idx2.Encode(wr2)
@@ -135,12 +136,13 @@ func TestIndexSerialize(t *testing.T) {
 	rtest.OK(t, err)
 	rtest.Equals(t, restic.IDs{id}, ids)
 
-	idx3, err := repository.DecodeIndex(wr3.Bytes())
+	idx3, oldFormat, err := repository.DecodeIndex(wr3.Bytes())
 	rtest.OK(t, err)
 	rtest.Assert(t, idx3 != nil,
 		"nil returned for decoded index")
 	rtest.Assert(t, idx3.Final(),
 		"decoded index is not final")
+	rtest.Assert(t, !oldFormat, "new index format recognized as old format")
 
 	// all new blobs must be in the index
 	for _, testBlob := range newtests {
@@ -285,8 +287,9 @@ var exampleLookupTest = struct {
 func TestIndexUnserialize(t *testing.T) {
 	oldIdx := restic.IDs{restic.TestParseID("ed54ae36197f4745ebc4b54d10e0f623eaaaedd03013eb7ae90df881b7781452")}
 
-	idx, err := repository.DecodeIndex(docExample)
+	idx, oldFormat, err := repository.DecodeIndex(docExample)
 	rtest.OK(t, err)
+	rtest.Assert(t, !oldFormat, "new index format recognized as old format")
 
 	for _, test := range exampleTests {
 		list := idx.Lookup(test.id, test.tpe, nil)
@@ -338,7 +341,7 @@ func BenchmarkDecodeIndex(b *testing.B) {
 	b.ResetTimer()
 
 	for i := 0; i < b.N; i++ {
-		_, err := repository.DecodeIndex(benchmarkIndexJSON)
+		_, _, err := repository.DecodeIndex(benchmarkIndexJSON)
 		rtest.OK(b, err)
 	}
 }
@@ -349,15 +352,16 @@ func BenchmarkDecodeIndexParallel(b *testing.B) {
 
 	b.RunParallel(func(pb *testing.PB) {
 		for pb.Next() {
-			_, err := repository.DecodeIndex(benchmarkIndexJSON)
+			_, _, err := repository.DecodeIndex(benchmarkIndexJSON)
 			rtest.OK(b, err)
 		}
 	})
 }
 
 func TestIndexUnserializeOld(t *testing.T) {
-	idx, err := repository.DecodeOldIndex(docOldExample)
+	idx, oldFormat, err := repository.DecodeIndex(docOldExample)
 	rtest.OK(t, err)
+	rtest.Assert(t, oldFormat, "old index format recognized as new format")
 
 	for _, test := range exampleTests {
 		list := idx.Lookup(test.id, test.tpe, nil)

internal/repository/repository.go

@@ -457,14 +457,13 @@ func (r *Repository) LoadIndex(ctx context.Context) error {
 		var buf []byte
 		for fi := range ch {
 			var err error
-			var idx *Index
+			buf, err = r.LoadAndDecrypt(ctx, buf[:0], restic.IndexFile, fi.ID)
-			idx, buf, err = LoadIndexWithDecoder(ctx, r, buf[:0], fi.ID, DecodeIndex)
-			if err != nil && errors.Cause(err) == ErrOldIndexFormat {
-				idx, buf, err = LoadIndexWithDecoder(ctx, r, buf[:0], fi.ID, DecodeOldIndex)
-			}
-
 			if err != nil {
-				return errors.Wrap(err, fmt.Sprintf("unable to load index %v", fi.ID.Str()))
+				return errors.Wrapf(err, "unable to load index %s", fi.ID.Str())
+			}
+			idx, _, err := DecodeIndex(buf)
+			if err != nil {
+				return errors.Wrapf(err, "unable to decode index %s", fi.ID.Str())
 			}
 
 			select {
@@ -576,18 +575,16 @@ func (r *Repository) PrepareCache(indexIDs restic.IDSet) error {
 
 // LoadIndex loads the index id from backend and returns it.
 func LoadIndex(ctx context.Context, repo restic.Repository, id restic.ID) (*Index, error) {
-	idx, _, err := LoadIndexWithDecoder(ctx, repo, nil, id, DecodeIndex)
+	buf, err := repo.LoadAndDecrypt(ctx, nil, restic.IndexFile, id)
-	if err == nil {
+	if err != nil {
-		return idx, nil
+		return nil, err
 	}
 
-	if errors.Cause(err) == ErrOldIndexFormat {
+	idx, oldFormat, err := DecodeIndex(buf)
+	if oldFormat {
 		fmt.Fprintf(os.Stderr, "index %v has old format\n", id.Str())
-		idx, _, err := LoadIndexWithDecoder(ctx, repo, nil, id, DecodeOldIndex)
-		return idx, err
 	}
-
+	return idx, err
-	return nil, err
 }
 
 // SearchKey finds a key with the supplied password, afterwards the config is