Task: Add IfAbsent to LinkPhysicalAddress

treeverse · Feb 20, 2024 · 625c07a · 625c07a
1 parent b9ddb4f
commit 625c07a
Show file tree

Hide file tree

Showing 19 changed files with 155 additions and 10 deletions.
diff --git a/api/swagger.yml b/api/swagger.yml
@@ -1299,6 +1299,9 @@ components:
         force:
           type: boolean
           default: false
+        if_absent:
+          type: boolean
+          default: false
       required:
         - staging
         - checksum

diff --git a/clients/java-legacy/api/openapi.yaml b/clients/java-legacy/api/openapi.yaml
diff --git a/clients/java-legacy/docs/StagingMetadata.md b/clients/java-legacy/docs/StagingMetadata.md
diff --git a/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/StagingMetadata.java b/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/StagingMetadata.java
diff --git a/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/StagingMetadataTest.java b/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/StagingMetadataTest.java
diff --git a/clients/java/api/openapi.yaml b/clients/java/api/openapi.yaml
diff --git a/clients/java/docs/StagingMetadata.md b/clients/java/docs/StagingMetadata.md
diff --git a/clients/java/src/main/java/io/lakefs/clients/sdk/model/StagingMetadata.java b/clients/java/src/main/java/io/lakefs/clients/sdk/model/StagingMetadata.java
diff --git a/clients/java/src/test/java/io/lakefs/clients/sdk/model/StagingMetadataTest.java b/clients/java/src/test/java/io/lakefs/clients/sdk/model/StagingMetadataTest.java
diff --git a/clients/python-legacy/docs/StagingApi.md b/clients/python-legacy/docs/StagingApi.md
diff --git a/clients/python-legacy/docs/StagingMetadata.md b/clients/python-legacy/docs/StagingMetadata.md
diff --git a/clients/python-legacy/lakefs_client/model/staging_metadata.py b/clients/python-legacy/lakefs_client/model/staging_metadata.py
diff --git a/clients/python/docs/StagingMetadata.md b/clients/python/docs/StagingMetadata.md
diff --git a/clients/python/lakefs_sdk/models/staging_metadata.py b/clients/python/lakefs_sdk/models/staging_metadata.py
diff --git a/clients/python/test/test_staging_metadata.py b/clients/python/test/test_staging_metadata.py
diff --git a/docs/assets/js/swagger.yml b/docs/assets/js/swagger.yml
@@ -1299,6 +1299,9 @@ components:
         force:
           type: boolean
           default: false
+        if_absent:
+          type: boolean
+          default: false
       required:
         - staging
         - checksum

diff --git a/pkg/api/controller.go b/pkg/api/controller.go
@@ -683,8 +683,7 @@ func (c *Controller) LinkPhysicalAddress(w http.ResponseWriter, r *http.Request,
 		entryBuilder.Metadata(body.UserMetadata.AdditionalProperties)
 	}
 	entry := entryBuilder.Build()
-
-	err = c.Catalog.CreateEntry(ctx, repo.Name, branch, entry, graveler.WithForce(swag.BoolValue(body.Force)))
+	err = c.Catalog.CreateEntry(ctx, repo.Name, branch, entry, graveler.WithForce(swag.BoolValue(body.Force)), graveler.WithIfAbsent(swag.BoolValue(body.IfAbsent)))
 	if c.handleAPIError(ctx, w, r, err) {
 		return
 	}

diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go
@@ -3007,6 +3007,50 @@ func TestController_LinkPhysicalAddressHandler(t *testing.T) {
 		}
 	})
 
+	t.Run("link physical address twice if absent", func(t *testing.T) {
+		linkResp, err := clt.GetPhysicalAddressWithResponse(ctx, repo, "main", &apigen.GetPhysicalAddressParams{Path: "foo/bar2"})
+		verifyResponseOK(t, linkResp, err)
+		if linkResp.JSON200 == nil {
+			t.Fatalf("GetPhysicalAddress non 200 response - status code %d", linkResp.StatusCode())
+		}
+		const expectedSizeBytes = 38
+		ifAbsent := false
+		resp, err := clt.LinkPhysicalAddressWithResponse(ctx, repo, "main", &apigen.LinkPhysicalAddressParams{
+			Path: "foo/bar2",
+		}, apigen.LinkPhysicalAddressJSONRequestBody{
+			Checksum:  "afb0689fe58b82c5f762991453edbbec",
+			SizeBytes: expectedSizeBytes,
+			Staging: apigen.StagingLocation{
+				PhysicalAddress: linkResp.JSON200.PhysicalAddress,
+			},
+			IfAbsent: &ifAbsent,
+		})
+		verifyResponseOK(t, resp, err)
+
+		// Try again with ifAbsent == true and expect failure
+		linkResp, err = clt.GetPhysicalAddressWithResponse(ctx, repo, "main", &apigen.GetPhysicalAddressParams{Path: "foo/bar2"})
+		verifyResponseOK(t, linkResp, err)
+		if linkResp.JSON200 == nil {
+			t.Fatalf("GetPhysicalAddress non 200 response - status code %d", linkResp.StatusCode())
+		}
+		ifAbsent = true
+		resp, err = clt.LinkPhysicalAddressWithResponse(ctx, repo, "main", &apigen.LinkPhysicalAddressParams{
+			Path: "foo/bar2",
+		}, apigen.LinkPhysicalAddressJSONRequestBody{
+			Checksum:  "afb0689fe58b82c5f762991453edbbec",
+			SizeBytes: expectedSizeBytes,
+			Staging: apigen.StagingLocation{
+				PhysicalAddress: linkResp.JSON200.PhysicalAddress,
+			},
+			IfAbsent: &ifAbsent,
+		})
+		testutil.Must(t, err)
+		expectedStatusCode := http.StatusPreconditionFailed
+		if resp.HTTPResponse.StatusCode != expectedStatusCode {
+			t.Fatalf("LinkPhysicalAddress status code: %d, expected: %d", resp.HTTPResponse.StatusCode, expectedStatusCode)
+		}
+	})
+
 	t.Run("link physical address without getting it first", func(t *testing.T) {
 		const expectedSizeBytes = 38
 		resp, err := clt.LinkPhysicalAddressWithResponse(ctx, repo, "main", &apigen.LinkPhysicalAddressParams{

diff --git a/pkg/graveler/graveler.go b/pkg/graveler/graveler.go
@@ -1711,7 +1711,10 @@ func (g *Graveler) Set(ctx context.Context, repository *RepositoryRecord, branch
 
 		// verify the key not found
 		_, err := g.Get(ctx, repository, Ref(branchID), key)
-		if err == nil || !errors.Is(err, ErrNotFound) {
+		if err == nil { // Entry found, return precondition failed
+			err = ErrPreconditionFailed
+		}
+		if !errors.Is(err, ErrNotFound) {
 			return err
 		}