Workhorse: improve doc comments in internal/upload

workhorse/internal/upload/accelerate.go

@@ -17,6 +17,11 @@ type MultipartClaims struct {
 	jwt.StandardClaims
 }
 
+// Multipart is a request middleware. If the request has a MIME multipart
+// request body, the middleware will iterate through the multipart parts.
+// When it finds a file part (filename != ""), the middleware will save
+// the file contents to a temporary location and replace the file part
+// with a reference to the temporary location.
 func Multipart(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
 	return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		s := &SavedFileTracker{Request: r}

workhorse/internal/upload/body_uploader.go

@@ -16,16 +16,23 @@ type PreAuthorizer interface {
 	PreAuthorizeHandler(next api.HandleFunc, suffix string) http.Handler
 }
 
-// Verifier allows to check an upload before sending it to rails
+// Verifier is an optional pluggable behavior for upload paths. If
+// Verify() returns an error, Workhorse will return an error response to
+// the client instead of propagating the request to Rails. The motivating
+// use case is Git LFS, where Workhorse checks the size and SHA256
+// checksum of the uploaded file.
 type Verifier interface {
-	// Verify can abort the upload returning an error
+	// Verify can abort the upload by returning an error
 	Verify(handler *filestore.FileHandler) error
 }
 
-// Preparer allows to customize RequestBody configuration
+// Preparer is a pluggable behavior that interprets a Rails API response
+// and either tells Workhorse how to handle the upload, via the
+// SaveFileOpts and Verifier, or it rejects the request by returning a
+// non-nil error. Its intended use is to make sure the upload gets stored
+// in the right location: either a local directory, or one of several
+// supported object storage backends.
 type Preparer interface {
-	// Prepare converts api.Response into a *SaveFileOpts, it can optionally return an Verifier that will be
-	// invoked after the real upload, before the finalization with rails
 	Prepare(a *api.Response) (*filestore.SaveFileOpts, Verifier, error)
 }
 
@@ -36,9 +43,9 @@ func (s *DefaultPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, Ver
 	return opts, nil, err
 }
 
-// RequestBody is an http.Handler that perform a pre authorization call to rails before hijacking the request body and
-// uploading it.
-// Providing an Preparer allows to customize the upload process
+// RequestBody is a request middleware. It will store the request body to
+// a location by determined an api.Response value. It then forwards the
+// request to gitlab-rails without the original request body.
 func RequestBody(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
 	return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		opts, verifier, err := p.Prepare(a)

workhorse/internal/upload/skip_rails_authorizer.go

@@ -6,15 +6,15 @@ import (
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
 )
 
-// SkipRailsAuthorizer implements a fake PreAuthorizer that do not calls rails API and
-// authorize each call as a local only upload to TempPath
+// SkipRailsAuthorizer implements a fake PreAuthorizer that does not call
+// the gitlab-rails API. It must be fast because it gets called on each
+// request proxied to Rails.
 type SkipRailsAuthorizer struct {
-	// TempPath is the temporary path for a local only upload
+	// TempPath is a directory where workhorse can store files that can later
+	// be accessed by gitlab-rails.
 	TempPath string
 }
 
-// PreAuthorizeHandler implements PreAuthorizer. It always grant the upload.
-// The fake API response contains only TempPath
 func (l *SkipRailsAuthorizer) PreAuthorizeHandler(next api.HandleFunc, _ string) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		next(w, r, &api.Response{TempPath: l.TempPath})

workhorse/internal/upload/uploads.go

@@ -15,7 +15,8 @@ import (
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/zipartifacts"
 )
 
-// These methods are allowed to have thread-unsafe implementations.
+// MultipartFormProcessor abstracts away implementation differences
+// between generic MIME multipart file uploads and CI artifact uploads.
 type MultipartFormProcessor interface {
 	ProcessFile(ctx context.Context, formName string, file *filestore.FileHandler, writer *multipart.Writer) error
 	ProcessField(ctx context.Context, formName string, writer *multipart.Writer) error
@@ -24,6 +25,9 @@ type MultipartFormProcessor interface {
 	Count() int
 }
 
+// InterceptMultipartFiles is the core of the implementation of
+// Multipart. Because it is also used for CI artifact uploads it is a
+// public function.
 func InterceptMultipartFiles(w http.ResponseWriter, r *http.Request, h http.Handler, preauth *api.Response, filter MultipartFormProcessor, opts *filestore.SaveFileOpts) {
 	var body bytes.Buffer
 	writer := multipart.NewWriter(&body)