From 5dd27ca00efd068c134a4917fb63a14470c25fe2 Mon Sep 17 00:00:00 2001
From: "Ariel Shaqed (Scolnicov)" <ariels@treeverse.io>
Date: Wed, 30 Oct 2024 10:34:08 +0200
Subject: [PATCH] [CR] Sleep only on S3 in multipart test, and fix some
 comments

---
 esti/multipart_test.go              |  9 ++++++---
 esti/system_test.go                 | 15 ++++++++++++---
 pkg/api/controller.go               |  6 +++---
 pkg/gateway/operations/putobject.go |  1 -
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/esti/multipart_test.go b/esti/multipart_test.go
index 4dfb079270b..62a375f5975 100644
--- a/esti/multipart_test.go
+++ b/esti/multipart_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/thanhpk/randstr"
 	"github.com/treeverse/lakefs/pkg/api/apigen"
+	"github.com/treeverse/lakefs/pkg/block"
 	"github.com/treeverse/lakefs/pkg/logging"
 )
 
@@ -53,9 +54,11 @@ func TestMultipartUpload(t *testing.T) {
 
 	completedParts := uploadMultipartParts(t, ctx, logger, resp, parts, 0)
 
-	// On S3 the object should have Last-Modified time at around time of MPU creation.
-	// Ensure lakeFS fails the test if it fakes it by using the current time.
-	time.Sleep(2 * timeResolution)
+	if isBlockstoreType(block.BlockstoreTypeS3) == nil {
+		// Object should have Last-Modified time at around time of MPU creation.  Ensure
+		// lakeFS fails the test if it fakes it by using the current time.
+		time.Sleep(2 * timeResolution)
+	}
 
 	completeResponse, err := uploadMultipartComplete(ctx, svc, resp, completedParts)
 	require.NoError(t, err, "failed to complete multipart upload")
diff --git a/esti/system_test.go b/esti/system_test.go
index 8ceb57c855f..acb79a9a5bd 100644
--- a/esti/system_test.go
+++ b/esti/system_test.go
@@ -337,11 +337,20 @@ func listRepositories(t *testing.T, ctx context.Context) []apigen.Repository {
 	return listedRepos
 }
 
+// isBlockstoreType returns nil if the blockstore type is one of requiredTypes, or the actual
+// type of the blockstore.
+func isBlockstoreType(requiredTypes ...string) *string {
+	blockstoreType := viper.GetString(config.BlockstoreTypeKey)
+	if slices.Contains(requiredTypes, blockstoreType) {
+		return nil
+	}
+	return &blockstoreType
+}
+
 // requireBlockstoreType Skips test if blockstore type doesn't match the required type
 func requireBlockstoreType(t testing.TB, requiredTypes ...string) {
-	blockstoreType := viper.GetString(config.BlockstoreTypeKey)
-	if !slices.Contains(requiredTypes, blockstoreType) {
-		t.Skipf("Required blockstore types: %v, got: %s", requiredTypes, blockstoreType)
+	if blockstoreType := isBlockstoreType(requiredTypes...); blockstoreType != nil {
+		t.Skipf("Required blockstore types: %v, got: %s", requiredTypes, *blockstoreType)
 	}
 }
 
diff --git a/pkg/api/controller.go b/pkg/api/controller.go
index 4381ef89659..9e29817418f 100644
--- a/pkg/api/controller.go
+++ b/pkg/api/controller.go
@@ -384,10 +384,10 @@ func (c *Controller) CompletePresignMultipartUpload(w http.ResponseWriter, r *ht
 
 	writeTime := time.Now()
 	checksum := httputil.StripQuotesAndSpaces(mpuResp.ETag)
+	// Anything else can be _really_ wrong when the storage layer assigns the time of MPU
+	// creation.  For instance, the S3 block adapter makes sure to return an MTime from
+	// headObject to ensure that we do have a time here.
 	if mpuResp.MTime != nil {
-		// Anything else can be _really_ wrong when the storage layer assigns the time
-		// of MPU creation.  For instance, the S3 block adapter takes sure to return an
-		// MTime from headObject to ensure that we do have a time here.
 		writeTime = *mpuResp.MTime
 	}
 	entryBuilder := catalog.NewDBEntryBuilder().
diff --git a/pkg/gateway/operations/putobject.go b/pkg/gateway/operations/putobject.go
index 2ef5fffa5d9..f54f601ead2 100644
--- a/pkg/gateway/operations/putobject.go
+++ b/pkg/gateway/operations/putobject.go
@@ -309,7 +309,6 @@ func handlePut(w http.ResponseWriter, req *http.Request, o *PathOperation) {
 	// write metadata
 	metadata := amzMetaAsMetadata(req)
 	contentType := req.Header.Get("Content-Type")
-	// BUG(ariels): Read MTime from upload!
 	err = o.finishUpload(req, nil, blob.Checksum, blob.PhysicalAddress, blob.Size, true, metadata, contentType)
 	if errors.Is(err, graveler.ErrWriteToProtectedBranch) {
 		_ = o.EncodeError(w, req, err, gatewayErrors.Codes.ToAPIErr(gatewayErrors.ErrWriteToProtectedBranch))