Refactor ExtractZip

It appears to be more flexible to call the function with a reader rather
than expect the function to return a writer.

Author: code-asher

storage/local.go

@@ -33,13 +33,19 @@ func (s *Local) AddExtension(ctx context.Context, manifest *VSIXManifest, vsix [
 	// Extract the zip to the correct path.
 	identity := manifest.Metadata.Identity
 	dir := filepath.Join(s.extdir, identity.Publisher, identity.ID, identity.Version)
-	err := ExtractZip(vsix, func(name string) (io.WriteCloser, error) {
+	err := ExtractZip(vsix, func(name string, r io.Reader) error {
 		path := filepath.Join(dir, name)
 		err := os.MkdirAll(filepath.Dir(path), 0o755)
 		if err != nil {
-			return nil, err
+			return err
 		}
-		return os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644)
+		w, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644)
+		if err != nil {
+			return err
+		}
+		defer w.Close()
+		_, err = io.Copy(w, r)
+		return err
 	})
 	if err != nil {
 		return "", err

storage/zip.go

@@ -48,26 +48,17 @@ func GetZipFileReader(rawZip []byte, filename string) (io.ReadCloser, error) {
 	return reader, nil
 }
 
-// ExtractZip extracts a zip's files to the specified directory.  The writer is
-// expected to create any necessary directories and return a writer for writing
-// a file.
-func ExtractZip(rawZip []byte, writer func(name string) (io.WriteCloser, error)) error {
+// ExtractZip applies a function with a reader for every file in the zip.  If
+// the function returns an error the walk is aborted.
+func ExtractZip(rawZip []byte, fn func(name string, reader io.Reader) error) error {
 	_, err := WalkZip(rawZip, func(zf *zip.File) (stop bool, err error) {
 		if !zf.FileInfo().IsDir() {
-			dst, err := writer(zf.Name)
-			if err != nil {
-				return false, err
-			}
-			defer dst.Close()
-			src, err := zf.Open()
-			if err != nil {
-				return false, err
-			}
-			defer src.Close()
-			_, err = io.Copy(dst, src)
+			zr, err := zf.Open()
 			if err != nil {
 				return false, err
 			}
+			defer zr.Close()
+			return false, fn(zf.Name, zr)
 		}
 		return false, nil
 	})

storage/zip_test.go

@@ -16,6 +16,7 @@ var files = []struct {
 	{"alpha.txt", "Alpha content."},
 	{"beta.txt", "Beta content."},
 	{"charlie.txt", "Charlie content."},
+	{"delta/delta.txt", "Delta content."},
 }
 
 func createZip() ([]byte, error) {
@@ -55,32 +56,26 @@ func TestGetZipFileReader(t *testing.T) {
 	require.Error(t, err)
 }
 
-type nopCloser struct {
-	io.Writer
-}
-
-func (nopCloser) Close() error { return nil }
-
 func TestExtract(t *testing.T) {
 	t.Parallel()
 
 	buffer, err := createZip()
 	require.NoError(t, err)
 
 	t.Run("Error", func(t *testing.T) {
-		err := ExtractZip(buffer, func(name string) (io.WriteCloser, error) {
-			return nil, errors.New("error")
+		err := ExtractZip(buffer, func(name string, reader io.Reader) error {
+			return errors.New("error")
 		})
 		require.Error(t, err)
 	})
 
 	t.Run("OK", func(t *testing.T) {
 		called := []string{}
-		err := ExtractZip(buffer, func(name string) (io.WriteCloser, error) {
+		err := ExtractZip(buffer, func(name string, reader io.Reader) error {
 			called = append(called, name)
-			return nopCloser{io.Discard}, nil
+			return nil
 		})
 		require.NoError(t, err)
-		require.Equal(t, []string{"alpha.txt", "beta.txt", "charlie.txt"}, called)
+		require.Equal(t, []string{"alpha.txt", "beta.txt", "charlie.txt", "delta/delta.txt"}, called)
 	})
 }