8000 address PR comments · coder/coder@33860ae · GitHub
[go: up one dir, main page]
Skip to content

Commit 33860ae

Browse files
johnstcnjohnstcn
committed
address PR comments
1 parent 804d9e8 commit 33860ae

File tree

8 files changed

+37
-30
lines changed

8 files changed

+37
-30
lines changed

‎fileszip/fileszip.go renamed to ‎archive/archive.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package fileszip
1+
package archive
22

33
import (
44
"archive/tar"
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111
)
1212

13+
// CreateTarFromZip converts the given zipReader to a tar archive.
1314
func CreateTarFromZip(zipReader *zip.Reader, maxSize int64) ([]byte, error) {
1415
var tarBuffer bytes.Buffer
1516
err := writeTarArchive(&tarBuffer, zipReader, maxSize)
@@ -60,16 +61,18 @@ func processFileInZipArchive(file *zip.File, tarWriter *tar.Writer, maxSize int6
6061
return err
6162
}
6263

64+
// CreateZipFromTar converts the given tarReader to a zip archive.
6365
func CreateZipFromTar(tarReader *tar.Reader, maxSize int64) ([]byte, error) {
6466
var zipBuffer bytes.Buffer
65-
err := WriteZipArchive(&zipBuffer, tarReader, maxSize)
67+
err := WriteZip(&zipBuffer, tarReader, maxSize)
6668
if err != nil {
6769
return nil, err
6870
}
6971
return zipBuffer.Bytes(), nil
7072
}
7173

72-
func WriteZipArchive(w io.Writer, tarReader *tar.Reader, maxSize int64) error {
74+
// WriteZip writes the given tarReader to w.
75+
func WriteZip(w io.Writer, tarReader *tar.Reader, maxSize int64) error {
7376
zipWriter := zip.NewWriter(w)
7477
defer zipWriter.Close()
7578

‎fileszip/fileszip_test.go renamed to ‎archive/archive_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package fileszip_test
1+
package archive_test
22

33
import (
44
"archive/tar"
@@ -15,8 +15,8 @@ import (
1515
"github.com/stretchr/testify/assert"
1616
"github.com/stretchr/testify/require"
1717

18-
"github.com/coder/coder/v2/fileszip"
19-
"github.com/coder/coder/v2/fileszip/filesziptest"
18+
"github.com/coder/coder/v2/archive"
19+
"github.com/coder/coder/v2/archive/archivetest"
2020
"github.com/coder/coder/v2/testutil"
2121
)
2222

@@ -28,17 +28,17 @@ func TestCreateTarFromZip(t *testing.T) {
2828

2929
// Read a zip file we prepared earlier
3030
ctx := testutil.Context(t, testutil< 10000 /span>.WaitShort)
31-
zipBytes := filesziptest.TestZipFileBytes()
31+
zipBytes := archivetest.TestZipFileBytes()
3232
// Assert invariant
33-
filesziptest.AssertSampleZipFile(t, zipBytes)
33+
archivetest.AssertSampleZipFile(t, zipBytes)
3434

3535
zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes)))
3636
require.NoError(t, err, "failed to parse sample zip file")
3737

38-
tarBytes, err := fileszip.CreateTarFromZip(zr, 10240)
38+
tarBytes, err := archive.CreateTarFromZip(zr, int64(len(zipBytes)))
3939
require.NoError(t, err, "failed to convert zip to tar")
4040

41-
filesziptest.AssertSampleTarFile(t, tarBytes)
41+
archivetest.AssertSampleTarFile(t, tarBytes)
4242

4343
tempDir := t.TempDir()
4444
tempFilePath := filepath.Join(tempDir, "test.tar")
@@ -57,13 +57,13 @@ func TestCreateZipFromTar(t *testing.T) {
5757
}
5858
t.Run("OK", func(t *testing.T) {
5959
t.Parallel()
60-
tarBytes := filesziptest.TestTarFileBytes()
60+
tarBytes := archivetest.TestTarFileBytes()
6161

6262
tr := tar.NewReader(bytes.NewReader(tarBytes))
63-
zipBytes, err := fileszip.CreateZipFromTar(tr, 10240)
63+
zipBytes, err := archive.CreateZipFromTar(tr, int64(len(tarBytes)))
6464
require.NoError(t, err)
6565

66-
filesziptest.AssertSampleZipFile(t, zipBytes)
66+
archivetest.AssertSampleZipFile(t, zipBytes)
6767

6868
tempDir := t.TempDir()
6969
tempFilePath := filepath.Join(tempDir, "test.zip")
@@ -95,7 +95,7 @@ func TestCreateZipFromTar(t *testing.T) {
9595

9696
// When: we convert this to a zip
9797
tr := tar.NewReader(&tarBytes)
98-
zipBytes, err := fileszip.CreateZipFromTar(tr, 10240)
98+
zipBytes, err := archive.CreateZipFromTar(tr, int64(tarBytes.Len()))
9999
require.NoError(t, err)
100100

101101
// Then: the resulting zip should contain a corresponding directory
@@ -129,7 +129,7 @@ func assertExtractedFiles(t *testing.T, dir string, checkModePerm bool) {
129129
if checkModePerm {
130130
assert.Equal(t, fs.ModePerm&0o755, stat.Mode().Perm(), "expected mode 0755 on directory")
131131
}
132-
assert.Equal(t, filesziptest.ArchiveRefTime(t).UTC(), stat.ModTime().UTC(), "unexpected modtime of %q", path)
132+
assert.Equal(t, archivetest.ArchiveRefTime(t).UTC(), stat.ModTime().UTC(), "unexpected modtime of %q", path)
133133
case "/test/hello.txt":
134134
stat, err := os.Stat(path)
135135
assert.NoError(t, err, "failed to stat path %q", path)

‎fileszip/filesziptest/filesziptest.go renamed to ‎archive/archivetest/archivetest.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package filesziptest
1+
package archivetest
22

33
import (
44
"archive/tar"
@@ -19,14 +19,17 @@ var testTarFileBytes []byte
1919
//go:embed testdata/test.zip
2020
var testZipFileBytes []byte
2121

22+
// TestTarFileBytes returns the content of testdata/test.tar
2223
func TestTarFileBytes() []byte {
2324
return append([]byte{}, testTarFileBytes...)
2425
}
2526

27+
// TestZipFileBytes returns the content of testdata/test.zip
2628
func TestZipFileBytes() []byte {
2729
return append([]byte{}, testZipFileBytes...)
2830
}
2931

32+
// AssertSampleTarfile compares the content of tarBytes against testdata/test.tar.
3033
func AssertSampleTarFile(t *testing.T, tarBytes []byte) {
3134
t.Helper()
3235

@@ -68,6 +71,7 @@ func AssertSampleTarFile(t *testing.T, tarBytes []byte) {
6871
}
6972
}
7073

74+
// AssertSampleZipFile compares the content of zipBytes against testdata/test.zip.
7175
func AssertSampleZipFile(t *testing.T, zipBytes []byte) {
7276
t.Helper()
7377

‎fileszip/filesziptest/testdata/test.tar renamed to ‎archive/archivetest/testdata/test.tar

File renamed without changes.

‎fileszip/filesziptest/testdata/test.zip renamed to ‎archive/archivetest/testdata/test.zip

File renamed without changes.

‎cli/templatepull_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ import (
1313
"github.com/google/uuid"
1414
"github.com/stretchr/testify/require"
1515

16+
"github.com/coder/coder/v2/archive"
1617
"github.com/coder/coder/v2/cli/clitest"
1718
"github.com/coder/coder/v2/coderd/coderdtest"
1819
"github.com/coder/coder/v2/coderd/rbac"
19-
"github.com/coder/coder/v2/fileszip"
2020
"github.com/coder/coder/v2/provisioner/echo"
2121
"github.com/coder/coder/v2/provisionersdk"
2222
"github.com/coder/coder/v2/provisionersdk/proto"
@@ -95,7 +95,7 @@ func TestTemplatePull_Stdout(t *testing.T) {
9595

9696
// Verify .zip format
9797
tarReader := tar.NewReader(bytes.NewReader(expected))
98-
expectedZip, err := fileszip.CreateZipFromTar(tarReader, int64(len(expected)))
98+
expectedZip, err := archive.CreateZipFromTar(tarReader, int64(len(expected)))
9999
require.NoError(t, err)
100100

101101
inv, root = clitest.New(t, "templates", "pull", "--zip", template.Name)

‎coderd/files.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ import (
1616
"github.com/google/uuid"
1717

1818
"cdr.dev/slog"
19+
"github.com/coder/coder/v2/archive"
1920
"github.com/coder/coder/v2/coderd/database"
2021
"github.com/coder/coder/v2/coderd/database/dbtime"
2122
"github.com/coder/coder/v2/coderd/httpapi"
2223
"github.com/coder/coder/v2/coderd/httpmw"
2324
"github.com/coder/coder/v2/codersdk"
24-
"github.com/coder/coder/v2/fileszip"
2525
)
2626

2727
const (
@@ -76,7 +76,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
7676
return
7777
}
7878

79-
data, err = fileszip.CreateTarFromZip(zipReader, httpFileMaxBytes)
79+
data, err = archive.CreateTarFromZip(zipReader, httpFileMaxBytes)
8080
if err != nil {
8181
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
8282
Message: "Internal error processing .zip archive.",
@@ -182,7 +182,7 @@ func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) {
182182

183183
rw.Header().Set("Content-Type", codersdk.ContentTypeZip)
184184
rw.WriteHeader(http.StatusOK)
185-
err = fileszip.WriteZipArchive(rw, tar.NewReader(bytes.NewReader(file.Data)), httpFileMaxBytes)
185+
err = archive.WriteZip(rw, tar.NewReader(bytes.NewReader(file.Data)), httpFileMaxBytes)
186186
if err != nil {
187187
api.Logger.Error(ctx, "invalid .zip archive", slog.F("file_id", fileID), slog.F("mimetype", file.Mimetype), slog.Error(err))
188188
}

‎coderd/files_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ import (
1010
"github.com/google/uuid"
1111
"github.com/stretchr/testify/require"
1212

13+
"github.com/coder/coder/v2/archive"
14+
"github.com/coder/coder/v2/archive/archivetest"
1315
"github.com/coder/coder/v2/coderd/coderdtest"
1416
"github.com/coder/coder/v2/codersdk"
15-
"github.com/coder/coder/v2/fileszip"
16-
"github.com/coder/coder/v2/fileszip/filesziptest"
1717
"github.com/coder/coder/v2/testutil"
1818
)
1919

@@ -84,7 +84,7 @@ func TestDownload(t *testing.T) {
8484
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
8585
defer cancel()
8686

87-
tarball := filesziptest.TestTarFileBytes()
87+
tarball := archivetest.TestTarFileBytes()
8888

8989
// when
9090
resp, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(tarball))
@@ -96,7 +96,7 @@ func TestDownload(t *testing.T) {
9696
require.Len(t, data, len(tarball))
9797
require.Equal(t, codersdk.ContentTypeTar, contentType)
9898
require.Equal(t, tarball, data)
99-
filesziptest.AssertSampleTarFile(t, data)
99+
archivetest.AssertSampleTarFile(t, data)
100100
})
101101

102102
t.Run("InsertZip_DownloadTar", func(t *testing.T) {
@@ -105,7 +105,7 @@ func TestDownload(t *testing.T) {
105105
_ = coderdtest.CreateFirstUser(t, client)
106106

107107
// given
108-
zipContent := filesziptest.TestZipFileBytes()
108+
zipContent := archivetest.TestZipFileBytes()
109109

110110
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
111111
defer cancel()
@@ -121,7 +121,7 @@ func TestDownload(t *testing.T) {
121121

122122
// Note: creating a zip from a tar will result in some loss of information
123123
// as zip files do not store UNIX user:group data.
124-
filesziptest.AssertSampleTarFile(t, data)
124+
archivetest.AssertSampleTarFile(t, data)
125125
})
126126

127127
t.Run("InsertTar_DownloadZip", func(t *testing.T) {
@@ -130,10 +130,10 @@ func TestDownload(t *testing.T) {
130130
_ = coderdtest.CreateFirstUser(t, client)
131131

132132
// given
133-
tarball := filesziptest.TestTarFileBytes()
133+
tarball := archivetest.TestTarFileBytes()
134134

135135
tarReader := tar.NewReader(bytes.NewReader(tarball))
136-
expectedZip, err := fileszip.CreateZipFromTar(tarReader, 10240)
136+
expectedZip, err := archive.CreateZipFromTar(tarReader, 10240)
137137
require.NoError(t, err)
138138

139139
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
@@ -148,6 +148,6 @@ func TestDownload(t *testing.T) {
148148
// then
149149
require.Equal(t, codersdk.ContentTypeZip, contentType)
150150
require.Equal(t, expectedZip, data)
151-
filesziptest.AssertSampleZipFile(t, data)
151+
archivetest.AssertSampleZipFile(t, data)
152152
})
153153
}

0 commit comments

Comments
 (0)
0