Split extract requests into game and music contents

- Add Analyzer interface to handle game and music zips differently,
  and add implementations for both. Logic previously in extractAndUploadOne
  was moved into the GameAnalyzer implementation.
- Observe query param "contents". If present and equal to "music", use
  MusicAnalyzer, otherwise use default behavior.
- Change some structs, namely removing ResourceSpec, which fragmented
  concerns across domains a bit. Logic which affected outgoing headers was
  consolidated under the GameAnalyzer implementation.
- Add optional Metadata field to API result JSON.
- Tests: Replaced assert.NoError() with require.NoError(). The latter stops
  a test immediatly if the assertion fails, which prevents useless and
  noisy panic traces in test output. No dependencies added, already part
  of the testify module.
- Removed unnecessary struct literal type names when they can be inferred,
  per linter.

The current implementation of MusicAnalyzer is not efficient with memory.
I left a TODO to address this once we confirm it works.

go.mod

@@ -3,6 +3,7 @@ module github.com/itchio/zipserver
 go 1.17
 
 require (
+	github.com/dhowden/tag v0.0.0-20220618230019-adf36e896086
 	github.com/go-errors/errors v1.4.2
 	github.com/stretchr/testify v1.7.0
 	golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8

go.sum

@@ -42,6 +42,10 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk
 github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
 github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/dhowden/itl v0.0.0-20170329215456-9fbe21093131/go.mod h1:eVWQJVQ67aMvYhpkDwaH2Goy2vo6v8JCMfGXfQ9sPtw=
+github.com/dhowden/plist v0.0.0-20141002110153-5db6e0d9931a/go.mod h1:sLjdR6uwx3L6/Py8F+QgAfeiuY87xuYGwCDqRFrvCzw=
+github.com/dhowden/tag v0.0.0-20220618230019-adf36e896086 h1:ORubSQoKnncsBnR4zD9CuYFJCPOCuSNEpWEZrDdBXkc=
+github.com/dhowden/tag v0.0.0-20220618230019-adf36e896086/go.mod h1:Z3Lomva4pyMWYezjMAU5QWRh0p1VvO4199OHlFnyKkM=
 github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
 github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
 github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98=

zipserver/analyzer.go

@@ -0,0 +1,25 @@
+package zipserver
+
+import (
+	"errors"
+	"io"
+)
+
+// ErrSkipped is non-critical and indicates that analysis
+// chose to ignore a file. The file should not be uploaded.
+var ErrSkipped = errors.New("skipped file")
+
+// Analyzer analyzes individual files in a zip archive.
+// Behavior may change based on the intended workload.
+type Analyzer interface {
+	// Analyze should return info about the contained file.
+	// It should return ErrSkipped if a file was ignored.
+	Analyze(r io.Reader, key string) (AnalyzeResult, error)
+}
+
+type AnalyzeResult struct {
+	Key             string // Analysis may choose a new key/filename
+	Metadata        interface{}
+	ContentType     string
+	ContentEncoding string
+}

zipserver/archive.go

@@ -1,7 +1,6 @@
 package zipserver
 
 import (
-	"bytes"
 	"context"
 	"crypto/md5"
 	"encoding/hex"
@@ -40,8 +39,9 @@ type Archiver struct {
 
 // ExtractedFile represents a file extracted from a .zip into a GCS bucket
 type ExtractedFile struct {
-	Key  string
-	Size uint64
+	Key      string
+	Size     uint64
+	Metadata interface{} `json:",omitempty"`
 }
 
 // NewArchiver creates a new archiver from the given config
@@ -130,15 +130,16 @@ type UploadFileTask struct {
 
 // UploadFileResult is successful is Error is nil - in that case, it contains the
 // GCS key the file was uploaded under, and the number of bytes written for that file.
+// An error causes the job to abort processing all further files in the archive.
 type UploadFileResult struct {
 	Error error
-	Key   string
-	Size  uint64
+	ExtractedFile
 }
 
 func uploadWorker(
 	ctx context.Context,
 	a *Archiver,
+	analyzer Analyzer,
 	tasks <-chan UploadFileTask,
 	results chan<- UploadFileResult,
 	done chan struct{},
@@ -150,16 +151,20 @@ func uploadWorker(
 		key := task.Key
 
 		ctx, cancel := context.WithTimeout(ctx, time.Duration(a.Config.FilePutTimeout))
-		resource, err := a.extractAndUploadOne(ctx, key, file)
+		info, err := a.extractAndUploadOne(ctx, key, file, analyzer)
 		cancel() // Free resources now instead of deferring till func returns
 
 		if err != nil {
+			if errors.Is(err, ErrSkipped) {
+				log.Printf("Skipping file: %s", key)
+				continue
+			}
 			log.Print("Failed sending " + key + ": " + err.Error())
-			results <- UploadFileResult{err, key, 0}
+			results <- UploadFileResult{Error: err}
 			return
 		}
 
-		results <- UploadFileResult{nil, resource.key, resource.size}
+		results <- UploadFileResult{ExtractedFile: info}
 	}
 }
 
@@ -168,6 +173,7 @@ func (a *Archiver) sendZipExtracted(
 	ctx context.Context,
 	prefix, fname string,
 	limits *ExtractLimits,
+	analyzer Analyzer,
 ) ([]ExtractedFile, error) {
 	zipReader, err := zip.OpenReader(fname)
 	if err != nil {
@@ -224,7 +230,7 @@ func (a *Archiver) sendZipExtracted(
 	defer cancel()
 
 	for i := 0; i < limits.ExtractionThreads; i++ {
-		go uploadWorker(ctx, a, tasks, results, done)
+		go uploadWorker(ctx, a, analyzer, tasks, results, done)
 	}
 
 	activeWorkers := limits.ExtractionThreads
@@ -253,7 +259,7 @@ func (a *Archiver) sendZipExtracted(
 				extractError = result.Error
 				cancel()
 			} else {
-				extractedFiles = append(extractedFiles, ExtractedFile{result.Key, result.Size})
+				extractedFiles = append(extractedFiles, result.ExtractedFile)
 				fileCount++
 			}
 		case <-done:
@@ -275,77 +281,54 @@ func (a *Archiver) sendZipExtracted(
 
 // sends an individual file from a zip
 // Caller should set the job timeout in ctx.
-func (a *Archiver) extractAndUploadOne(ctx context.Context, key string, file *zip.File) (*ResourceSpec, error) {
-	readerCloser, err := file.Open()
+func (a *Archiver) extractAndUploadOne(
+	ctx context.Context,
+	key string,
+	file *zip.File,
+	analyzer Analyzer,
+) (ExtractedFile, error) {
+	none := ExtractedFile{}
+
+	analyzerReader, err := file.Open()
 	if err != nil {
-		return nil, err
+		return none, err
 	}
-	defer readerCloser.Close()
-
-	var reader io.Reader = readerCloser
+	defer analyzerReader.Close()
 
-	resource := &ResourceSpec{
-		key: key,
+	info, err := analyzer.Analyze(analyzerReader, key)
+	if err != nil {
+		return none, err
 	}
 
-	// try determining MIME by extension
-	mimeType := mime.TypeByExtension(path.Ext(key))
-
-	var buffer bytes.Buffer
-	_, err = io.Copy(&buffer, io.LimitReader(reader, 512))
-
+	// Analysis may have called Read() but we cannot seek back, so open a new Reader with initialized cursor.
+	uploadReader, err := file.Open()
 	if err != nil {
-		return nil, errors.Wrap(err, 0)
+		return none, err
 	}
+	defer uploadReader.Close()
 
-	contentMimeType := http.DetectContentType(buffer.Bytes())
-	// join the bytes read and the original reader
-	reader = io.MultiReader(&buffer, reader)
-
-	if contentMimeType == "application/x-gzip" || contentMimeType == "application/gzip" {
-		resource.contentEncoding = "gzip"
+	log.Printf("Sending key=%q mime=%q encoding=%q", info.Key, info.ContentType, info.ContentEncoding)
 
-		// try to see if there's a real extension hidden beneath
-		if strings.HasSuffix(key, ".gz") {
-			realMimeType := mime.TypeByExtension(path.Ext(strings.TrimSuffix(key, ".gz")))
+	var size uint64 // Written to by limitedReader
+	limited := limitedReader(uploadReader, file.UncompressedSize64, &size)
 
-			if realMimeType != "" {
-				mimeType = realMimeType
-			}
-		}
-
-	} else if strings.HasSuffix(key, ".br") {
-		// there is no way to detect a brotli stream by content, so we assume if it ends if .br then it's brotli
-		// this path is used for Unity 2020 webgl games built with brotli compression
-		resource.contentEncoding = "br"
-		realMimeType := mime.TypeByExtension(path.Ext(strings.TrimSuffix(key, ".br")))
-
-		if realMimeType != "" {
-			mimeType = realMimeType
+	err = a.Storage.PutFileWithSetup(ctx, a.Bucket, info.Key, limited, func(r *http.Request) error {
+		r.Header.Set("X-Goog-Acl", "public-read")
+		r.Header.Set("Content-Type", info.ContentType)
+		if info.ContentEncoding != "" {
+			r.Header.Set("Content-Encoding", info.ContentEncoding)
 		}
-	} else if mimeType == "" {
-		// fall back to the extension detected from content, eg. someone uploaded a .png with wrong extension
-		mimeType = contentMimeType
-	}
-
-	if mimeType == "" {
-		// default mime type
-		mimeType = "application/octet-stream"
-	}
-	resource.contentType = mimeType
-
-	resource.applyRewriteRules()
-
-	log.Printf("Sending: %s", resource)
-
-	limited := limitedReader(reader, file.UncompressedSize64, &resource.size)
-
-	err = a.Storage.PutFileWithSetup(ctx, a.Bucket, resource.key, limited, resource.setupRequest)
+		return nil
+	})
 	if err != nil {
-		return resource, errors.Wrap(err, 0)
+		return none, errors.Wrap(err, 0)
 	}
 
-	return resource, nil
+	return ExtractedFile{
+		Key:      info.Key,
+		Size:     size,
+		Metadata: info.Metadata,
+	}, nil
 }
 
 // ExtractZip downloads the zip at `key` to a temporary file,
@@ -355,6 +338,7 @@ func (a *Archiver) ExtractZip(
 	ctx context.Context,
 	key, prefix string,
 	limits *ExtractLimits,
+	analyzer Analyzer,
 ) ([]ExtractedFile, error) {
 	fname, err := a.fetchZip(ctx, key)
 	if err != nil {
@@ -363,7 +347,7 @@ func (a *Archiver) ExtractZip(
 
 	defer os.Remove(fname)
 	prefix = path.Join(a.ExtractPrefix, prefix)
-	return a.sendZipExtracted(ctx, prefix, fname, limits)
+	return a.sendZipExtracted(ctx, prefix, fname, limits, analyzer)
 }
 
 // Caller should set the job timeout in ctx.
@@ -373,5 +357,6 @@ func (a *Archiver) UploadZipFromFile(
 	limits *ExtractLimits,
 ) ([]ExtractedFile, error) {
 	prefix = path.Join("_zipserver", prefix)
-	return a.sendZipExtracted(ctx, prefix, fname, limits)
+	// TODO: Add CLI option to choose game or music content.
+	return a.sendZipExtracted(ctx, prefix, fname, limits, &GameAnalyzer{})
 }