Skip to content

Commit

Permalink
feat(storage): autodetect narrow down text/plain contentype (#3022)
Browse files Browse the repository at this point in the history
* feat(storage): autodetect narrow down text/plain contentype based on extension

- Add content type detection for JavaScript (text/javascript)
- Add content type detection for CSS (text/css)
- Add content type detection for SQL (application/x-sql)
- Default to text/plain for unrecognized text files like .go

* fix: tests for cross platform run

* chore: reuse upload object method

---------

Co-authored-by: Qiao Han <[email protected]>
  • Loading branch information
avallete and sweatybridge authored Jan 9, 2025
1 parent b0c7523 commit 3429667
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 15 deletions.
6 changes: 3 additions & 3 deletions internal/storage/cp/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func Run(ctx context.Context, src, dst string, recursive bool, maxJobs uint, fsy
if recursive {
return UploadStorageObjectAll(ctx, api, dstParsed.Path, localPath, maxJobs, fsys, opts...)
}
return api.UploadObject(ctx, dstParsed.Path, src, fsys, opts...)
return api.UploadObject(ctx, dstParsed.Path, src, utils.NewRootFS(fsys), opts...)
} else if strings.EqualFold(srcParsed.Scheme, client.STORAGE_SCHEME) && strings.EqualFold(dstParsed.Scheme, client.STORAGE_SCHEME) {
return errors.New("Copying between buckets is not supported")
}
Expand Down Expand Up @@ -149,7 +149,7 @@ func UploadStorageObjectAll(ctx context.Context, api storage.StorageAPI, remoteP
}
fmt.Fprintln(os.Stderr, "Uploading:", filePath, "=>", dstPath)
job := func() error {
err := api.UploadObject(ctx, dstPath, filePath, fsys, opts...)
err := api.UploadObject(ctx, dstPath, filePath, utils.NewRootFS(fsys), opts...)
if err != nil && strings.Contains(err.Error(), `"error":"Bucket not found"`) {
// Retry after creating bucket
if bucket, prefix := client.SplitBucketPrefix(dstPath); len(prefix) > 0 {
Expand All @@ -162,7 +162,7 @@ func UploadStorageObjectAll(ctx context.Context, api storage.StorageAPI, remoteP
if _, err := api.CreateBucket(ctx, body); err != nil {
return err
}
err = api.UploadObject(ctx, dstPath, filePath, fsys, opts...)
err = api.UploadObject(ctx, dstPath, filePath, utils.NewRootFS(fsys), opts...)
}
}
return err
Expand Down
14 changes: 3 additions & 11 deletions pkg/storage/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,9 @@ func (s *StorageAPI) UpsertObjects(ctx context.Context, bucketConfig config.Buck
}
fmt.Fprintln(os.Stderr, "Uploading:", filePath, "=>", dstPath)
job := func() error {
f, err := fsys.Open(filePath)
if err != nil {
return errors.Errorf("failed to open file: %w", err)
}
defer f.Close()
fo, err := ParseFileOptions(f)
if err != nil {
return err
}
fo.Overwrite = true
return s.UploadObjectStream(ctx, dstPath, f, *fo)
return s.UploadObject(ctx, dstPath, filePath, fsys, func(fo *FileOptions) {
fo.Overwrite = true
})
}
return jq.Put(job)
}
Expand Down
11 changes: 10 additions & 1 deletion pkg/storage/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import (
"context"
"io"
"io/fs"
"mime"
"net/http"
"os"
"path"
"path/filepath"
"strings"

"github.com/go-errors/errors"
Expand Down Expand Up @@ -88,7 +90,7 @@ func ParseFileOptions(f fs.File, opts ...func(*FileOptions)) (*FileOptions, erro
return fo, nil
}

func (s *StorageAPI) UploadObject(ctx context.Context, remotePath, localPath string, fsys afero.Fs, opts ...func(*FileOptions)) error {
func (s *StorageAPI) UploadObject(ctx context.Context, remotePath, localPath string, fsys fs.FS, opts ...func(*FileOptions)) error {
f, err := fsys.Open(localPath)
if err != nil {
return errors.Errorf("failed to open file: %w", err)
Expand All @@ -98,6 +100,13 @@ func (s *StorageAPI) UploadObject(ctx context.Context, remotePath, localPath str
if err != nil {
return err
}
// For text/plain content types, we try to determine a more specific type
// based on the file extension, as the initial detection might be too generic
if strings.Contains(fo.ContentType, "text/plain") {
if extensionType := mime.TypeByExtension(filepath.Ext(localPath)); extensionType != "" {
fo.ContentType = extensionType
}
}
return s.UploadObjectStream(ctx, remotePath, f, *fo)
}

Expand Down
92 changes: 92 additions & 0 deletions pkg/storage/objects_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package storage

import (
"context"
"mime"
"net/http"
"testing"
fs "testing/fstest"

"github.com/h2non/gock"
"github.com/stretchr/testify/assert"
"github.com/supabase/cli/internal/testing/apitest"
"github.com/supabase/cli/pkg/fetcher"
)

var mockApi = StorageAPI{Fetcher: fetcher.NewFetcher(
"http://127.0.0.1",
)}

func TestParseFileOptionsContentTypeDetection(t *testing.T) {
tests := []struct {
name string
content []byte
filename string
opts []func(*FileOptions)
wantMimeType string
wantCacheCtrl string
}{
{
name: "detects PNG image",
content: []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A}, // PNG header
filename: "test.image",
wantMimeType: "image/png",
wantCacheCtrl: "max-age=3600",
},
{
name: "detects JavaScript file",
content: []byte("const hello = () => console.log('Hello, World!');"),
filename: "script.js",
wantMimeType: mime.TypeByExtension(".js"),
wantCacheCtrl: "max-age=3600",
},
{
name: "detects CSS file",
content: []byte(".header { color: #333; font-size: 16px; }"),
filename: "styles.css",
wantMimeType: mime.TypeByExtension(".css"),
wantCacheCtrl: "max-age=3600",
},
{
name: "detects SQL file",
content: []byte("SELECT * FROM users WHERE id = 1;"),
filename: "query.sql",
wantMimeType: mime.TypeByExtension(".sql"),
wantCacheCtrl: "max-age=3600",
},
{
name: "use text/plain as fallback for unrecognized extensions",
content: []byte("const hello = () => console.log('Hello, World!');"),
filename: "main.nonexistent",
wantMimeType: "text/plain; charset=utf-8",
wantCacheCtrl: "max-age=3600",
},
{
name: "respects custom content type",
content: []byte("const hello = () => console.log('Hello, World!');"),
filename: "custom.js",
wantMimeType: "application/custom",
wantCacheCtrl: "max-age=3600",
opts: []func(*FileOptions){func(fo *FileOptions) { fo.ContentType = "application/custom" }},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Create a temporary file with test content
fsys := fs.MapFS{tt.filename: &fs.MapFile{Data: tt.content}}
// Setup mock api
defer gock.OffAll()
gock.New("http://127.0.0.1").
Post("/storage/v1/object/"+tt.filename).
MatchHeader("Content-Type", tt.wantMimeType).
MatchHeader("Cache-Control", tt.wantCacheCtrl).
Reply(http.StatusOK)
// Parse options
err := mockApi.UploadObject(context.Background(), tt.filename, tt.filename, fsys, tt.opts...)
// Assert results
assert.NoError(t, err)
assert.Empty(t, apitest.ListUnmatchedRequests())
})
}
}

0 comments on commit 3429667

Please sign in to comment.