From 88247ac0982da5a6c02fad44d0e5289cb99b88bf Mon Sep 17 00:00:00 2001 From: lynnro314 <64288262+lynnro314@users.noreply.github.com> Date: Wed, 3 Jan 2024 14:57:36 +0200 Subject: [PATCH] added optional allow empty commits (#7186) * added optional allow empty commits --- api/swagger.yml | 4 ++ clients/java-legacy/api/openapi.yaml | 5 ++ clients/java-legacy/docs/CommitCreation.md | 1 + .../clients/api/model/CommitCreation.java | 31 +++++++- .../clients/api/model/CommitCreationTest.java | 8 +++ clients/java/api/openapi.yaml | 5 ++ clients/java/docs/CommitCreation.md | 1 + .../clients/sdk/model/CommitCreation.java | 30 +++++++- .../clients/sdk/model/CommitCreationTest.java | 8 +++ clients/python-legacy/docs/CommitCreation.md | 1 + clients/python-legacy/docs/CommitsApi.md | 1 + clients/python-legacy/docs/ImportApi.md | 1 + .../lakefs_client/model/commit_creation.py | 4 ++ clients/python/docs/CommitCreation.md | 1 + .../lakefs_sdk/models/commit_creation.py | 4 +- clients/python/test/test_commit_creation.py | 1 + clients/python/test/test_import_creation.py | 2 + cmd/lakectl/cmd/commit.go | 10 ++- docs/assets/js/swagger.yml | 4 ++ docs/reference/cli.md | 1 + pkg/api/controller.go | 2 +- pkg/api/controller_test.go | 72 +++++++++---------- pkg/catalog/catalog.go | 11 +-- pkg/graveler/committed/manager.go | 4 +- pkg/graveler/graveler.go | 5 +- pkg/graveler/graveler_v2_test.go | 4 +- pkg/graveler/mock/graveler.go | 8 +-- pkg/graveler/testutil/fakes.go | 2 +- pkg/samplerepo/samplecontent.go | 2 +- 29 files changed, 173 insertions(+), 60 deletions(-) diff --git a/api/swagger.yml b/api/swagger.yml index 31c5690400e..a7b55c2c3aa 100644 --- a/api/swagger.yml +++ b/api/swagger.yml @@ -580,6 +580,10 @@ components: description: set date to override creation date in the commit (Unix Epoch in seconds) type: integer format: int64 + allow_empty: + description: sets whether a commit can contain no changes + type: boolean + default: false force: type: boolean default: false diff --git a/clients/java-legacy/api/openapi.yaml b/clients/java-legacy/api/openapi.yaml index c8a8aec7825..3626490d127 100644 --- a/clients/java-legacy/api/openapi.yaml +++ b/clients/java-legacy/api/openapi.yaml @@ -6851,6 +6851,7 @@ components: key: metadata force: false message: message + allow_empty: false properties: message: type: string @@ -6863,6 +6864,10 @@ components: in seconds) format: int64 type: integer + allow_empty: + default: false + description: sets whether a commit can contain no changes + type: boolean force: default: false type: boolean diff --git a/clients/java-legacy/docs/CommitCreation.md b/clients/java-legacy/docs/CommitCreation.md index 20bc5866b4e..be749dc88b7 100644 --- a/clients/java-legacy/docs/CommitCreation.md +++ b/clients/java-legacy/docs/CommitCreation.md @@ -10,6 +10,7 @@ Name | Type | Description | Notes **message** | **String** | | **metadata** | **Map<String, String>** | | [optional] **date** | **Long** | set date to override creation date in the commit (Unix Epoch in seconds) | [optional] +**allowEmpty** | **Boolean** | sets whether a commit can contain no changes | [optional] **force** | **Boolean** | | [optional] diff --git a/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/CommitCreation.java b/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/CommitCreation.java index f7399abaec2..dae22cf58f1 100644 --- a/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/CommitCreation.java +++ b/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/CommitCreation.java @@ -44,6 +44,10 @@ public class CommitCreation { @SerializedName(SERIALIZED_NAME_DATE) private Long date; + public static final String SERIALIZED_NAME_ALLOW_EMPTY = "allow_empty"; + @SerializedName(SERIALIZED_NAME_ALLOW_EMPTY) + private Boolean allowEmpty = false; + public static final String SERIALIZED_NAME_FORCE = "force"; @SerializedName(SERIALIZED_NAME_FORCE) private Boolean force = false; @@ -126,6 +130,29 @@ public void setDate(Long date) { } + public CommitCreation allowEmpty(Boolean allowEmpty) { + + this.allowEmpty = allowEmpty; + return this; + } + + /** + * sets whether a commit can contain no changes + * @return allowEmpty + **/ + @javax.annotation.Nullable + @ApiModelProperty(value = "sets whether a commit can contain no changes") + + public Boolean getAllowEmpty() { + return allowEmpty; + } + + + public void setAllowEmpty(Boolean allowEmpty) { + this.allowEmpty = allowEmpty; + } + + public CommitCreation force(Boolean force) { this.force = force; @@ -161,12 +188,13 @@ public boolean equals(Object o) { return Objects.equals(this.message, commitCreation.message) && Objects.equals(this.metadata, commitCreation.metadata) && Objects.equals(this.date, commitCreation.date) && + Objects.equals(this.allowEmpty, commitCreation.allowEmpty) && Objects.equals(this.force, commitCreation.force); } @Override public int hashCode() { - return Objects.hash(message, metadata, date, force); + return Objects.hash(message, metadata, date, allowEmpty, force); } @Override @@ -176,6 +204,7 @@ public String toString() { sb.append(" message: ").append(toIndentedString(message)).append("\n"); sb.append(" metadata: ").append(toIndentedString(metadata)).append("\n"); sb.append(" date: ").append(toIndentedString(date)).append("\n"); + sb.append(" allowEmpty: ").append(toIndentedString(allowEmpty)).append("\n"); sb.append(" force: ").append(toIndentedString(force)).append("\n"); sb.append("}"); return sb.toString(); diff --git a/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/CommitCreationTest.java b/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/CommitCreationTest.java index b7beac1db94..fb2a8071a6e 100644 --- a/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/CommitCreationTest.java +++ b/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/CommitCreationTest.java @@ -67,6 +67,14 @@ public void dateTest() { // TODO: test date } + /** + * Test the property 'allowEmpty' + */ + @Test + public void allowEmptyTest() { + // TODO: test allowEmpty + } + /** * Test the property 'force' */ diff --git a/clients/java/api/openapi.yaml b/clients/java/api/openapi.yaml index ce24f3a7b65..1702ecfe5a5 100644 --- a/clients/java/api/openapi.yaml +++ b/clients/java/api/openapi.yaml @@ -6825,6 +6825,7 @@ components: key: metadata force: false message: message + allow_empty: false properties: message: type: string @@ -6837,6 +6838,10 @@ components: in seconds) format: int64 type: integer + allow_empty: + default: false + description: sets whether a commit can contain no changes + type: boolean force: default: false type: boolean diff --git a/clients/java/docs/CommitCreation.md b/clients/java/docs/CommitCreation.md index d73afeff030..9d58c8a8ecd 100644 --- a/clients/java/docs/CommitCreation.md +++ b/clients/java/docs/CommitCreation.md @@ -10,6 +10,7 @@ |**message** | **String** | | | |**metadata** | **Map<String, String>** | | [optional] | |**date** | **Long** | set date to override creation date in the commit (Unix Epoch in seconds) | [optional] | +|**allowEmpty** | **Boolean** | sets whether a commit can contain no changes | [optional] | |**force** | **Boolean** | | [optional] | diff --git a/clients/java/src/main/java/io/lakefs/clients/sdk/model/CommitCreation.java b/clients/java/src/main/java/io/lakefs/clients/sdk/model/CommitCreation.java index aa251c174f9..2cd000671f7 100644 --- a/clients/java/src/main/java/io/lakefs/clients/sdk/model/CommitCreation.java +++ b/clients/java/src/main/java/io/lakefs/clients/sdk/model/CommitCreation.java @@ -66,6 +66,10 @@ public class CommitCreation { @SerializedName(SERIALIZED_NAME_DATE) private Long date; + public static final String SERIALIZED_NAME_ALLOW_EMPTY = "allow_empty"; + @SerializedName(SERIALIZED_NAME_ALLOW_EMPTY) + private Boolean allowEmpty = false; + public static final String SERIALIZED_NAME_FORCE = "force"; @SerializedName(SERIALIZED_NAME_FORCE) private Boolean force = false; @@ -144,6 +148,27 @@ public void setDate(Long date) { } + public CommitCreation allowEmpty(Boolean allowEmpty) { + + this.allowEmpty = allowEmpty; + return this; + } + + /** + * sets whether a commit can contain no changes + * @return allowEmpty + **/ + @javax.annotation.Nullable + public Boolean getAllowEmpty() { + return allowEmpty; + } + + + public void setAllowEmpty(Boolean allowEmpty) { + this.allowEmpty = allowEmpty; + } + + public CommitCreation force(Boolean force) { this.force = force; @@ -222,13 +247,14 @@ public boolean equals(Object o) { return Objects.equals(this.message, commitCreation.message) && Objects.equals(this.metadata, commitCreation.metadata) && Objects.equals(this.date, commitCreation.date) && + Objects.equals(this.allowEmpty, commitCreation.allowEmpty) && Objects.equals(this.force, commitCreation.force)&& Objects.equals(this.additionalProperties, commitCreation.additionalProperties); } @Override public int hashCode() { - return Objects.hash(message, metadata, date, force, additionalProperties); + return Objects.hash(message, metadata, date, allowEmpty, force, additionalProperties); } @Override @@ -238,6 +264,7 @@ public String toString() { sb.append(" message: ").append(toIndentedString(message)).append("\n"); sb.append(" metadata: ").append(toIndentedString(metadata)).append("\n"); sb.append(" date: ").append(toIndentedString(date)).append("\n"); + sb.append(" allowEmpty: ").append(toIndentedString(allowEmpty)).append("\n"); sb.append(" force: ").append(toIndentedString(force)).append("\n"); sb.append(" additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n"); sb.append("}"); @@ -265,6 +292,7 @@ private String toIndentedString(Object o) { openapiFields.add("message"); openapiFields.add("metadata"); openapiFields.add("date"); + openapiFields.add("allow_empty"); openapiFields.add("force"); // a set of required properties/fields (JSON key names) diff --git a/clients/java/src/test/java/io/lakefs/clients/sdk/model/CommitCreationTest.java b/clients/java/src/test/java/io/lakefs/clients/sdk/model/CommitCreationTest.java index 150b85eb8b9..192e213860f 100644 --- a/clients/java/src/test/java/io/lakefs/clients/sdk/model/CommitCreationTest.java +++ b/clients/java/src/test/java/io/lakefs/clients/sdk/model/CommitCreationTest.java @@ -63,6 +63,14 @@ public void dateTest() { // TODO: test date } + /** + * Test the property 'allowEmpty' + */ + @Test + public void allowEmptyTest() { + // TODO: test allowEmpty + } + /** * Test the property 'force' */ diff --git a/clients/python-legacy/docs/CommitCreation.md b/clients/python-legacy/docs/CommitCreation.md index 13041af35d2..8619ed6555d 100644 --- a/clients/python-legacy/docs/CommitCreation.md +++ b/clients/python-legacy/docs/CommitCreation.md @@ -7,6 +7,7 @@ Name | Type | Description | Notes **message** | **str** | | **metadata** | **{str: (str,)}** | | [optional] **date** | **int** | set date to override creation date in the commit (Unix Epoch in seconds) | [optional] +**allow_empty** | **bool** | sets whether a commit can contain no changes | [optional] if omitted the server will use the default value of False **force** | **bool** | | [optional] if omitted the server will use the default value of False **any string name** | **bool, date, datetime, dict, float, int, list, str, none_type** | any string name can be used but the value must be the correct type | [optional] diff --git a/clients/python-legacy/docs/CommitsApi.md b/clients/python-legacy/docs/CommitsApi.md index 74052e5570f..ff767aacf5e 100644 --- a/clients/python-legacy/docs/CommitsApi.md +++ b/clients/python-legacy/docs/CommitsApi.md @@ -81,6 +81,7 @@ with lakefs_client.ApiClient(configuration) as api_client: "key": "key_example", }, date=1, + allow_empty=False, force=False, ) # CommitCreation | source_metarange = "source_metarange_example" # str | The source metarange to commit. Branch must not have uncommitted changes. (optional) diff --git a/clients/python-legacy/docs/ImportApi.md b/clients/python-legacy/docs/ImportApi.md index e5e6c23c9bc..8e693aa4e4a 100644 --- a/clients/python-legacy/docs/ImportApi.md +++ b/clients/python-legacy/docs/ImportApi.md @@ -202,6 +202,7 @@ with lakefs_client.ApiClient(configuration) as api_client: "key": "key_example", }, date=1, + allow_empty=False, force=False, ), force=False, diff --git a/clients/python-legacy/lakefs_client/model/commit_creation.py b/clients/python-legacy/lakefs_client/model/commit_creation.py index 720cbb89188..75322f6b03d 100644 --- a/clients/python-legacy/lakefs_client/model/commit_creation.py +++ b/clients/python-legacy/lakefs_client/model/commit_creation.py @@ -85,6 +85,7 @@ def openapi_types(): 'message': (str,), # noqa: E501 'metadata': ({str: (str,)},), # noqa: E501 'date': (int,), # noqa: E501 + 'allow_empty': (bool,), # noqa: E501 'force': (bool,), # noqa: E501 } @@ -97,6 +98,7 @@ def discriminator(): 'message': 'message', # noqa: E501 'metadata': 'metadata', # noqa: E501 'date': 'date', # noqa: E501 + 'allow_empty': 'allow_empty', # noqa: E501 'force': 'force', # noqa: E501 } @@ -146,6 +148,7 @@ def _from_openapi_data(cls, message, *args, **kwargs): # noqa: E501 _visited_composed_classes = (Animal,) metadata ({str: (str,)}): [optional] # noqa: E501 date (int): set date to override creation date in the commit (Unix Epoch in seconds). [optional] # noqa: E501 + allow_empty (bool): sets whether a commit can contain no changes. [optional] if omitted the server will use the default value of False # noqa: E501 force (bool): [optional] if omitted the server will use the default value of False # noqa: E501 """ @@ -234,6 +237,7 @@ def __init__(self, message, *args, **kwargs): # noqa: E501 _visited_composed_classes = (Animal,) metadata ({str: (str,)}): [optional] # noqa: E501 date (int): set date to override creation date in the commit (Unix Epoch in seconds). [optional] # noqa: E501 + allow_empty (bool): sets whether a commit can contain no changes. [optional] if omitted the server will use the default value of False # noqa: E501 force (bool): [optional] if omitted the server will use the default value of False # noqa: E501 """ diff --git a/clients/python/docs/CommitCreation.md b/clients/python/docs/CommitCreation.md index 667836164c1..1c8ef0bebaa 100644 --- a/clients/python/docs/CommitCreation.md +++ b/clients/python/docs/CommitCreation.md @@ -8,6 +8,7 @@ Name | Type | Description | Notes **message** | **str** | | **metadata** | **Dict[str, str]** | | [optional] **var_date** | **int** | set date to override creation date in the commit (Unix Epoch in seconds) | [optional] +**allow_empty** | **bool** | sets whether a commit can contain no changes | [optional] [default to False] **force** | **bool** | | [optional] [default to False] ## Example diff --git a/clients/python/lakefs_sdk/models/commit_creation.py b/clients/python/lakefs_sdk/models/commit_creation.py index e120ca06001..86eb9ca4314 100644 --- a/clients/python/lakefs_sdk/models/commit_creation.py +++ b/clients/python/lakefs_sdk/models/commit_creation.py @@ -29,8 +29,9 @@ class CommitCreation(BaseModel): message: StrictStr = Field(...) metadata: Optional[Dict[str, StrictStr]] = None var_date: Optional[StrictInt] = Field(None, alias="date", description="set date to override creation date in the commit (Unix Epoch in seconds)") + allow_empty: Optional[StrictBool] = Field(False, description="sets whether a commit can contain no changes") force: Optional[StrictBool] = False - __properties = ["message", "metadata", "date", "force"] + __properties = ["message", "metadata", "date", "allow_empty", "force"] class Config: """Pydantic configuration""" @@ -71,6 +72,7 @@ def from_dict(cls, obj: dict) -> CommitCreation: "message": obj.get("message"), "metadata": obj.get("metadata"), "var_date": obj.get("date"), + "allow_empty": obj.get("allow_empty") if obj.get("allow_empty") is not None else False, "force": obj.get("force") if obj.get("force") is not None else False }) return _obj diff --git a/clients/python/test/test_commit_creation.py b/clients/python/test/test_commit_creation.py index 9828543a652..512383c21f9 100644 --- a/clients/python/test/test_commit_creation.py +++ b/clients/python/test/test_commit_creation.py @@ -44,6 +44,7 @@ def make_instance(self, include_optional): 'key' : '' }, var_date = 56, + allow_empty = True, force = True ) else : diff --git a/clients/python/test/test_import_creation.py b/clients/python/test/test_import_creation.py index b1c75497867..488fc51b890 100644 --- a/clients/python/test/test_import_creation.py +++ b/clients/python/test/test_import_creation.py @@ -51,6 +51,7 @@ def make_instance(self, include_optional): 'key' : '' }, date = 56, + allow_empty = True, force = True, ), force = True ) @@ -68,6 +69,7 @@ def make_instance(self, include_optional): 'key' : '' }, date = 56, + allow_empty = True, force = True, ), ) """ diff --git a/cmd/lakectl/cmd/commit.go b/cmd/lakectl/cmd/commit.go index 17727c0113a..22d884c0bca 100644 --- a/cmd/lakectl/cmd/commit.go +++ b/cmd/lakectl/cmd/commit.go @@ -14,6 +14,7 @@ var errInvalidKeyValueFormat = errors.New(`invalid key/value pair - should be se const ( dateFlagName = "epoch-time-seconds" + allowEmptyCommit = "allow-empty-commit" commitCreateTemplate = `Commit for branch "{{.Branch.Ref}}" completed. ID: {{.Commit.Id|yellow}} @@ -32,6 +33,7 @@ var commitCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { message, kvPairs := getCommitFlags(cmd) date := Must(cmd.Flags().GetInt64(dateFlagName)) + emptyCommitBool := Must(cmd.Flags().GetBool(allowEmptyCommit)) datePtr := &date if date < 0 { datePtr = nil @@ -46,9 +48,10 @@ var commitCmd = &cobra.Command{ } client := getClient() resp, err := client.CommitWithResponse(cmd.Context(), branchURI.Repository, branchURI.Ref, &apigen.CommitParams{}, apigen.CommitJSONRequestBody{ - Message: message, - Metadata: &metadata, - Date: datePtr, + Message: message, + Metadata: &metadata, + Date: datePtr, + AllowEmpty: &emptyCommitBool, }) DieOnErrorOrUnexpectedStatusCode(resp, err, http.StatusCreated) if resp.JSON201 == nil { @@ -66,6 +69,7 @@ var commitCmd = &cobra.Command{ //nolint:gochecknoinits func init() { commitCmd.Flags().Int64(dateFlagName, -1, "create commit with a custom unix epoch date in seconds") + commitCmd.Flags().Bool(allowEmptyCommit, false, "allow a commit with no changes") if err := commitCmd.Flags().MarkHidden(dateFlagName); err != nil { DieErr(err) } diff --git a/docs/assets/js/swagger.yml b/docs/assets/js/swagger.yml index 31c5690400e..a7b55c2c3aa 100644 --- a/docs/assets/js/swagger.yml +++ b/docs/assets/js/swagger.yml @@ -580,6 +580,10 @@ components: description: set date to override creation date in the commit (Unix Epoch in seconds) type: integer format: int64 + allow_empty: + description: sets whether a commit can contain no changes + type: boolean + default: false force: type: boolean default: false diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 7ca816b9f1a..c64e7feb288 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -1893,6 +1893,7 @@ lakectl commit [flags] {:.no_toc} ``` + --allow-empty-commit allow a commit with no changes --allow-empty-message allow an empty commit message -h, --help help for commit -m, --message string commit message diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 030299c6e32..678484f02d0 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -2520,7 +2520,7 @@ func (c *Controller) Commit(w http.ResponseWriter, r *http.Request, body apigen. metadata = body.Metadata.AdditionalProperties } - newCommit, err := c.Catalog.Commit(ctx, repository, branch, body.Message, user.Committer(), metadata, body.Date, params.SourceMetarange, graveler.WithForce(swag.BoolValue(body.Force))) + newCommit, err := c.Catalog.Commit(ctx, repository, branch, body.Message, user.Committer(), metadata, body.Date, params.SourceMetarange, swag.BoolValue(body.AllowEmpty), graveler.WithForce(swag.BoolValue(body.Force))) if c.handleAPIError(ctx, w, r, err) { return } diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 2d6459cf5e2..c7b4cb1fd98 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -230,7 +230,7 @@ func testCommitEntries(t *testing.T, ctx context.Context, cat *catalog.Catalog, }) testutil.MustDo(t, "create entry "+p, err) } - commit, err := cat.Commit(ctx, params.repo, params.branch, "commit"+params.commitName, params.user, nil, nil, nil) + commit, err := cat.Commit(ctx, params.repo, params.branch, "commit"+params.commitName, params.user, nil, nil, nil, false) testutil.MustDo(t, "commit", err) return commit.Reference } @@ -306,7 +306,7 @@ func TestController_LogCommitsHandler(t *testing.T) { p := prefix + n err := deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: p, PhysicalAddress: onBlock(deps, "bar"+n+"addr"), CreationDate: time.Now(), Size: int64(i) + 1, Checksum: "cksum" + n}) testutil.MustDo(t, "create entry "+p, err) - _, err = deps.catalog.Commit(ctx, repo, "main", "commit"+n, "some_user", nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "main", "commit"+n, "some_user", nil, nil, nil, false) testutil.MustDo(t, "commit "+p, err) } params := &apigen.LogCommitsParams{} @@ -349,7 +349,7 @@ func TestController_LogCommitsParallelHandler(t *testing.T) { p := prefix + n err := deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: p, PhysicalAddress: onBlock(deps, "bar"+n+"addr"), CreationDate: time.Now(), Size: int64(i) + 1, Checksum: "cksum" + n}) testutil.MustDo(t, "create entry "+p, err) - log, err := deps.catalog.Commit(ctx, repo, "main", t.Name()+" commit"+n, "some_user", nil, nil, nil) + log, err := deps.catalog.Commit(ctx, repo, "main", t.Name()+" commit"+n, "some_user", nil, nil, nil, false) testutil.MustDo(t, "commit "+p, err) if i%4 == 0 { commitsToLook[p] = log @@ -396,7 +396,7 @@ func TestController_LogCommitsPredefinedData(t *testing.T) { p := prefix + n err := deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: p, PhysicalAddress: onBlock(deps, "bar"+n+"addr"), CreationDate: time.Now(), Size: int64(i) + 1, Checksum: "cksum" + n}) testutil.MustDo(t, "create entry "+p, err) - commit, err := deps.catalog.Commit(ctx, repo, "main", "commit"+n, "some_user", nil, nil, nil) + commit, err := deps.catalog.Commit(ctx, repo, "main", "commit"+n, "some_user", nil, nil, nil, false) testutil.MustDo(t, "commit "+p, err) commits[i] = commit } @@ -746,7 +746,7 @@ func TestController_GetCommitHandler(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, "foo1", onBlock(deps, "foo1"), "main", false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar1", deps.catalog.CreateEntry(ctx, "foo1", "main", catalog.DBEntry{Path: "foo/bar1", PhysicalAddress: "bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"})) - commit1, err := deps.catalog.Commit(ctx, "foo1", "main", "some message", DefaultUserID, nil, nil, nil) + commit1, err := deps.catalog.Commit(ctx, "foo1", "main", "some message", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) reference1, err := deps.catalog.GetBranchReference(ctx, "foo1", "main") if err != nil { @@ -770,7 +770,7 @@ func TestController_GetCommitHandler(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repo, onBlock(deps, repo), "main", false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar1", deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "foo/bar1", PhysicalAddress: "bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"})) - commit1, err := deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil) + commit1, err := deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) reference1, err := deps.catalog.GetBranchReference(ctx, repo, "main") if err != nil { @@ -798,7 +798,7 @@ func TestController_GetCommitHandler(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repo, onBlock(deps, repo), "main", false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar1", deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "foo/bar1", PhysicalAddress: "bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"})) - commit1, err := deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil) + commit1, err := deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) _, err = deps.catalog.CreateTag(ctx, repo, "tag1", commit1.Reference) if err != nil { @@ -1280,7 +1280,7 @@ func TestController_ListBranchesHandler(t *testing.T) { // create the first "dummy" commit on main so that we can create branches from it testutil.Must(t, deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "a/b"})) - _, err = deps.catalog.Commit(ctx, repo, "main", "first commit", "test", nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "main", "first commit", "test", nil, nil, nil, false) testutil.Must(t, err) for i := 0; i < 7; i++ { @@ -1335,7 +1335,7 @@ func TestController_ListTagsHandler(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repo, onBlock(deps, "foo1"), "main", false) testutil.Must(t, err) testutil.Must(t, deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "obj1"})) - commitLog, err := deps.catalog.Commit(ctx, repo, "main", "first commit", "test", nil, nil, nil) + commitLog, err := deps.catalog.Commit(ctx, repo, "main", "first commit", "test", nil, nil, nil, false) testutil.Must(t, err) const createTagLen = 7 var createdTags []apigen.Ref @@ -1420,7 +1420,7 @@ func TestController_GetBranchHandler(t *testing.T) { t.Run("get default branch", func(t *testing.T) { // create the first "dummy" commit on main so that we can create branches from it testutil.Must(t, deps.catalog.CreateEntry(ctx, repo, testBranch, catalog.DBEntry{Path: "a/b"})) - _, err = deps.catalog.Commit(ctx, repo, testBranch, "first commit", "test", nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, testBranch, "first commit", "test", nil, nil, nil, false) testutil.Must(t, err) resp, err := clt.GetBranchWithResponse(ctx, repo, testBranch) @@ -1511,7 +1511,7 @@ func TestController_CreateBranchHandler(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repo, onBlock(deps, "foo1"), "main", false) testutil.Must(t, err) testutil.Must(t, deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "a/b"})) - _, err = deps.catalog.Commit(ctx, repo, "main", "first commit", "test", nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "main", "first commit", "test", nil, nil, nil, false) testutil.Must(t, err) const newBranchName = "main2" @@ -1530,7 +1530,7 @@ func TestController_CreateBranchHandler(t *testing.T) { uploadResp, err := uploadObjectHelper(t, ctx, clt, objPath, strings.NewReader(content), repo, newBranchName) verifyResponseOK(t, uploadResp, err) - if _, err := deps.catalog.Commit(ctx, repo, "main2", "commit 1", "some_user", nil, nil, nil); err != nil { + if _, err := deps.catalog.Commit(ctx, repo, "main2", "commit 1", "some_user", nil, nil, nil, false); err != nil { t.Fatalf("failed to commit 'repo1': %s", err) } resp2, err := clt.DiffRefsWithResponse(ctx, repo, "main", newBranchName, &apigen.DiffRefsParams{}) @@ -1637,7 +1637,7 @@ func TestController_CreateBranchHandler(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repo, onBlock(deps, "foo1"), "main", true) testutil.Must(t, err) testutil.Must(t, deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "a/b"}, graveler.WithForce(true))) - _, err = deps.catalog.Commit(ctx, repo, "main", "first commit", "test", nil, nil, nil, graveler.WithForce(true)) + _, err = deps.catalog.Commit(ctx, repo, "main", "first commit", "test", nil, nil, nil, false, graveler.WithForce(true)) testutil.Must(t, err) const newBranchName = "main2" @@ -1684,7 +1684,7 @@ func TestController_DiffRefsHandler(t *testing.T) { uploadResp, err := uploadObjectHelper(t, ctx, clt, fullPath, strings.NewReader(content), repoName, newBranchName) verifyResponseOK(t, uploadResp, err) - if _, err := deps.catalog.Commit(ctx, repoName, newBranchName, "commit 1", "some_user", nil, nil, nil); err != nil { + if _, err := deps.catalog.Commit(ctx, repoName, newBranchName, "commit 1", "some_user", nil, nil, nil, false); err != nil { t.Fatalf("failed to commit 'repo1': %s", err) } resp2, err := clt.DiffRefsWithResponse(ctx, repoName, "main", newBranchName, &apigen.DiffRefsParams{}) @@ -1830,7 +1830,7 @@ func TestController_UploadObjectHandler(t *testing.T) { } // commit - _, err = deps.catalog.Commit(ctx, "my-new-repo", "another-branch", "a commit!", "user1", nil, nil, nil) + _, err = deps.catalog.Commit(ctx, "my-new-repo", "another-branch", "a commit!", "user1", nil, nil, nil, false) testutil.Must(t, err) // overwrite after commit @@ -1912,7 +1912,7 @@ func TestController_DeleteBranchHandler(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, "my-new-repo", onBlock(deps, "foo1"), "main", false) testutil.Must(t, err) testutil.Must(t, deps.catalog.CreateEntry(ctx, "my-new-repo", "main", catalog.DBEntry{Path: "a/b"})) - _, err = deps.catalog.Commit(ctx, "my-new-repo", "main", "first commit", "test", nil, nil, nil) + _, err = deps.catalog.Commit(ctx, "my-new-repo", "main", "first commit", "test", nil, nil, nil, false) testutil.Must(t, err) _, err = deps.catalog.CreateBranch(ctx, "my-new-repo", "main2", "main") @@ -1956,7 +1956,7 @@ func TestController_DeleteBranchHandler(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repoName, onBlock(deps, "foo1"), "main", true) testutil.Must(t, err) testutil.Must(t, deps.catalog.CreateEntry(ctx, repoName, "main", catalog.DBEntry{Path: "a/b"}, graveler.WithForce(true))) - _, err = deps.catalog.Commit(ctx, repoName, "main", "first commit", "test", nil, nil, nil, graveler.WithForce(true)) + _, err = deps.catalog.Commit(ctx, repoName, "main", "first commit", "test", nil, nil, nil, false, graveler.WithForce(true)) testutil.Must(t, err) _, err = deps.catalog.CreateBranch(ctx, repoName, "main2", "main", graveler.WithForce(true)) @@ -3545,7 +3545,7 @@ func TestController_MergeIntoExplicitBranch(t *testing.T) { testutil.Must(t, err) err = deps.catalog.CreateEntry(ctx, repo, "branch1", catalog.DBEntry{Path: "foo/bar1", PhysicalAddress: "bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"}) testutil.Must(t, err) - _, err = deps.catalog.Commit(ctx, repo, "branch1", "some message", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "branch1", "some message", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) // test branch with mods @@ -3582,7 +3582,7 @@ func TestController_MergeDirtyBranch(t *testing.T) { testutil.Must(t, err) err = deps.catalog.CreateEntry(ctx, repo, "branch1", catalog.DBEntry{Path: "foo/bar2", PhysicalAddress: "bar2addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum2"}) testutil.Must(t, err) - _, err = deps.catalog.Commit(ctx, repo, "branch1", "some message", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "branch1", "some message", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) // merge branch1 to main (dirty) @@ -3601,7 +3601,7 @@ func TestController_CreateTag(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repo, onBlock(deps, repo), "main", false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar1", deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "foo/bar1", PhysicalAddress: "bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"})) - commit1, err := deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil) + commit1, err := deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) t.Run("ref", func(t *testing.T) { @@ -3711,7 +3711,7 @@ func TestController_CreateTag(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, readOnlyRepo, onBlock(deps, readOnlyRepo), "main", true) testutil.Must(t, err) testutil.MustDo(t, "create entry bar1", deps.catalog.CreateEntry(ctx, readOnlyRepo, "main", catalog.DBEntry{Path: "foo/bar2", PhysicalAddress: "bar2addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"}, graveler.WithForce(true))) - commit1, err := deps.catalog.Commit(ctx, readOnlyRepo, "main", "some message", DefaultUserID, nil, nil, nil, graveler.WithForce(true)) + commit1, err := deps.catalog.Commit(ctx, readOnlyRepo, "main", "some message", DefaultUserID, nil, nil, nil, false, graveler.WithForce(true)) testutil.Must(t, err) tagResp, err := clt.CreateTagWithResponse(ctx, readOnlyRepo, apigen.CreateTagJSONRequestBody{ Id: "tag1", @@ -3741,7 +3741,7 @@ func TestController_Revert(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repo, onBlock(deps, repo), "main", false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar1", deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "foo/bar1", PhysicalAddress: "bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"})) - _, err = deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) t.Run("ref", func(t *testing.T) { @@ -3791,14 +3791,14 @@ func TestController_Revert(t *testing.T) { testutil.Must(t, err) err = deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "merge/foo/bar1", PhysicalAddress: "merge1bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"}) testutil.Must(t, err) - _, err = deps.catalog.Commit(ctx, repo, "main", "first", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "main", "first", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) // create branch with one entry committed _, err = deps.catalog.CreateBranch(ctx, repo, "branch1", "main") testutil.Must(t, err) err = deps.catalog.CreateEntry(ctx, repo, "branch1", catalog.DBEntry{Path: "merge/foo/bar2", PhysicalAddress: "merge2bar2addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum2"}) testutil.Must(t, err) - _, err = deps.catalog.Commit(ctx, repo, "branch1", "second", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "branch1", "second", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) // merge branch1 to main mergeRef, err := deps.catalog.Merge(ctx, repo, "main", "branch1", DefaultUserID, "merge to main", catalog.Metadata{}, "") @@ -3817,7 +3817,7 @@ func TestController_Revert(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, readOnlyRepository, onBlock(deps, readOnlyRepository), "main", true) testutil.Must(t, err) testutil.MustDo(t, "create entry bar1", deps.catalog.CreateEntry(ctx, readOnlyRepository, "main", catalog.DBEntry{Path: "foo/bar2", PhysicalAddress: "bar2addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"}, graveler.WithForce(true))) - _, err = deps.catalog.Commit(ctx, readOnlyRepository, "main", "some message", DefaultUserID, nil, nil, nil, graveler.WithForce(true)) + _, err = deps.catalog.Commit(ctx, readOnlyRepository, "main", "some message", DefaultUserID, nil, nil, nil, false, graveler.WithForce(true)) testutil.Must(t, err) revertResp, err := clt.RevertBranchWithResponse(ctx, readOnlyRepository, "main", apigen.RevertBranchJSONRequestBody{Ref: "main"}) if revertResp.JSON403 == nil { @@ -3836,10 +3836,10 @@ func TestController_RevertConflict(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repo, onBlock(deps, repo), "main", false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar1", deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "foo/bar1", PhysicalAddress: "bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"})) - firstCommit, err := deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil) + firstCommit, err := deps.catalog.Commit(ctx, repo, "main", "some message", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) testutil.MustDo(t, "overriding entry bar1", deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "foo/bar1", PhysicalAddress: "bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum2"})) - _, err = deps.catalog.Commit(ctx, repo, "main", "some other message", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "main", "some other message", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) resp, err := clt.RevertBranchWithResponse(ctx, repo, "main", apigen.RevertBranchJSONRequestBody{Ref: firstCommit.Reference}) @@ -3857,7 +3857,7 @@ func TestController_CherryPick(t *testing.T) { _, err := deps.catalog.CreateRepository(ctx, repo, onBlock(deps, repo), "main", false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar1", deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: "foo/bar1", PhysicalAddress: "bar1addr", CreationDate: time.Now(), Size: 1, Checksum: "cksum1"})) - _, err = deps.catalog.Commit(ctx, repo, "main", "message1", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "main", "message1", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) for _, name := range []string{"branch1", "branch2", "branch3", "branch4", "dest-branch1", "dest-branch2", "dest-branch3", "dest-branch4"} { @@ -3866,25 +3866,25 @@ func TestController_CherryPick(t *testing.T) { } testutil.MustDo(t, "create entry bar2", deps.catalog.CreateEntry(ctx, repo, "branch1", catalog.DBEntry{Path: "foo/bar2", PhysicalAddress: "bar2addr", CreationDate: time.Now(), Size: 2, Checksum: "cksum2"})) - commit2, err := deps.catalog.Commit(ctx, repo, "branch1", "message2", DefaultUserID, nil, nil, nil) + commit2, err := deps.catalog.Commit(ctx, repo, "branch1", "message2", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar3", deps.catalog.CreateEntry(ctx, repo, "branch1", catalog.DBEntry{Path: "foo/bar3", PhysicalAddress: "bar3addr", CreationDate: time.Now(), Size: 3, Checksum: "cksum3"})) testutil.MustDo(t, "create entry bar4", deps.catalog.CreateEntry(ctx, repo, "branch1", catalog.DBEntry{Path: "foo/bar4", PhysicalAddress: "bar4addr", CreationDate: time.Now(), Size: 4, Checksum: "cksum4"})) - _, err = deps.catalog.Commit(ctx, repo, "branch1", "message34", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "branch1", "message34", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar6", deps.catalog.CreateEntry(ctx, repo, "branch2", catalog.DBEntry{Path: "foo/bar6", PhysicalAddress: "bar6addr", CreationDate: time.Now(), Size: 6, Checksum: "cksum6"})) testutil.MustDo(t, "create entry bar7", deps.catalog.CreateEntry(ctx, repo, "branch2", catalog.DBEntry{Path: "foo/bar7", PhysicalAddress: "bar7addr", CreationDate: time.Now(), Size: 7, Checksum: "cksum7"})) - _, err = deps.catalog.Commit(ctx, repo, "branch2", "message34", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "branch2", "message34", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar8", deps.catalog.CreateEntry(ctx, repo, "branch3", catalog.DBEntry{Path: "foo/bar8", PhysicalAddress: "bar8addr", CreationDate: time.Now(), Size: 8, Checksum: "cksum8"})) - _, err = deps.catalog.Commit(ctx, repo, "branch3", "message8", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "branch3", "message8", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) testutil.MustDo(t, "create entry bar2", deps.catalog.CreateEntry(ctx, repo, "branch4", catalog.DBEntry{Path: "foo/bar2", PhysicalAddress: "bar2addr4", CreationDate: time.Now(), Size: 24, Checksum: "cksum24"})) - _, err = deps.catalog.Commit(ctx, repo, "branch4", "message4", DefaultUserID, nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "branch4", "message4", DefaultUserID, nil, nil, nil, false) testutil.Must(t, err) _, err = deps.catalog.Merge(ctx, repo, "branch3", "branch1", DefaultUserID, @@ -4019,7 +4019,7 @@ func TestController_CherryPick(t *testing.T) { testutil.Must(t, err) } testutil.MustDo(t, "create entry bar2", deps.catalog.CreateEntry(ctx, readOnlyRepository, "branch1", catalog.DBEntry{Path: "foo/bar2", PhysicalAddress: "bar2addr", CreationDate: time.Now(), Size: 2, Checksum: "cksum2"}, graveler.WithForce(true))) - _, err = deps.catalog.Commit(ctx, readOnlyRepository, "branch1", "message2", DefaultUserID, nil, nil, nil, graveler.WithForce(true)) + _, err = deps.catalog.Commit(ctx, readOnlyRepository, "branch1", "message2", DefaultUserID, nil, nil, nil, false, graveler.WithForce(true)) testutil.Must(t, err) cherryResponse, err := clt.CherryPickWithResponse(ctx, readOnlyRepository, "dest-branch1", apigen.CherryPickJSONRequestBody{Ref: "branch1"}) @@ -4263,7 +4263,7 @@ func TestController_PrepareGarbageCollectionUncommitted(t *testing.T) { uploadResp, err := uploadObjectHelper(t, ctx, clt, objPath, strings.NewReader(objPath), repo, "main") verifyResponseOK(t, uploadResp, err) } - if _, err := deps.catalog.Commit(ctx, repo, "main", "committed objects", "some_user", nil, nil, nil); err != nil { + if _, err := deps.catalog.Commit(ctx, repo, "main", "committed objects", "some_user", nil, nil, nil, false); err != nil { t.Fatalf("failed to commit objects: %s", err) } verifyPrepareGarbageCollection(t, repo, 1, false) @@ -5062,7 +5062,7 @@ func TestController_DumpRestoreRepository(t *testing.T) { p := "foo/bar" + n err := deps.catalog.CreateEntry(ctx, repo, "main", catalog.DBEntry{Path: p, PhysicalAddress: onBlock(deps, "bar"+n+"addr"), CreationDate: time.Now(), Size: int64(i) + 1, Checksum: "cksum" + n}) testutil.MustDo(t, "create entry "+p, err) - _, err = deps.catalog.Commit(ctx, repo, "main", "commit"+n, "tester", nil, nil, nil) + _, err = deps.catalog.Commit(ctx, repo, "main", "commit"+n, "tester", nil, nil, nil, false) testutil.MustDo(t, "commit "+p, err) } diff --git a/pkg/catalog/catalog.go b/pkg/catalog/catalog.go index bd4b3f66a2e..d6798fa3ae1 100644 --- a/pkg/catalog/catalog.go +++ b/pkg/catalog/catalog.go @@ -1110,7 +1110,7 @@ func (c *Catalog) ResetEntries(ctx context.Context, repositoryID string, branch return c.Store.ResetPrefix(ctx, repository, branchID, keyPrefix, opts...) } -func (c *Catalog) Commit(ctx context.Context, repositoryID, branch, message, committer string, metadata Metadata, date *int64, sourceMetarange *string, opts ...graveler.SetOptionsFunc) (*CommitLog, error) { +func (c *Catalog) Commit(ctx context.Context, repositoryID, branch, message, committer string, metadata Metadata, date *int64, sourceMetarange *string, allowEmpty bool, opts ...graveler.SetOptionsFunc) (*CommitLog, error) { branchID := graveler.BranchID(branch) if err := validator.Validate([]validator.ValidateArg{ {Name: "repository", Value: repositoryID, Fn: graveler.ValidateRepositoryID}, @@ -1125,10 +1125,11 @@ func (c *Catalog) Commit(ctx context.Context, repositoryID, branch, message, com } p := graveler.CommitParams{ - Committer: committer, - Message: message, - Date: date, - Metadata: map[string]string(metadata), + Committer: committer, + Message: message, + Date: date, + Metadata: map[string]string(metadata), + AllowEmpty: allowEmpty, } if sourceMetarange != nil { x := graveler.MetaRangeID(*sourceMetarange) diff --git a/pkg/graveler/committed/manager.go b/pkg/graveler/committed/manager.go index 5ab0f6200ea..c4e6e7d0088 100644 --- a/pkg/graveler/committed/manager.go +++ b/pkg/graveler/committed/manager.go @@ -281,7 +281,7 @@ func (c *committedManager) merge(ctx context.Context, mctx mergeContext) (gravel return *newID, err } -func (c *committedManager) Commit(ctx context.Context, ns graveler.StorageNamespace, baseMetaRangeID graveler.MetaRangeID, changes graveler.ValueIterator, _ ...graveler.SetOptionsFunc) (graveler.MetaRangeID, graveler.DiffSummary, error) { +func (c *committedManager) Commit(ctx context.Context, ns graveler.StorageNamespace, baseMetaRangeID graveler.MetaRangeID, changes graveler.ValueIterator, allowEmpty bool, _ ...graveler.SetOptionsFunc) (graveler.MetaRangeID, graveler.DiffSummary, error) { mwWriter := c.metaRangeManager.NewWriter(ctx, ns, nil) defer func() { err := mwWriter.Abort() @@ -297,7 +297,7 @@ func (c *committedManager) Commit(ctx context.Context, ns graveler.StorageNamesp return "", summary, fmt.Errorf("get metarange ns=%s id=%s: %w", ns, baseMetaRangeID, err) } defer metaRangeIterator.Close() - summary, err = Commit(ctx, mwWriter, metaRangeIterator, changes, &CommitOptions{}) + summary, err = Commit(ctx, mwWriter, metaRangeIterator, changes, &CommitOptions{AllowEmpty: allowEmpty}) if err != nil { if !errors.Is(err, graveler.ErrUserVisible) { err = fmt.Errorf("commit ns=%s id=%s: %w", ns, baseMetaRangeID, err) diff --git a/pkg/graveler/graveler.go b/pkg/graveler/graveler.go index 6140ba89e5c..e571ffd5946 100644 --- a/pkg/graveler/graveler.go +++ b/pkg/graveler/graveler.go @@ -448,6 +448,7 @@ type CommitParams struct { Metadata Metadata // SourceMetaRange - If exists, use it directly. Fail if branch has uncommitted changes SourceMetaRange *MetaRangeID + AllowEmpty bool } type GarbageCollectionRunMetadata struct { @@ -919,7 +920,7 @@ type CommittedManager interface { // Commit is the act of taking an existing metaRange (snapshot) and applying a set of changes to it. // A change is either an entity to write/overwrite, or a tombstone to mark a deletion // it returns a new MetaRangeID that is expected to be immediately addressable - Commit(ctx context.Context, ns StorageNamespace, baseMetaRangeID MetaRangeID, changes ValueIterator, opts ...SetOptionsFunc) (MetaRangeID, DiffSummary, error) + Commit(ctx context.Context, ns StorageNamespace, baseMetaRangeID MetaRangeID, changes ValueIterator, allowEmpty bool, opts ...SetOptionsFunc) (MetaRangeID, DiffSummary, error) // GetMetaRange returns information where metarangeID is stored. GetMetaRange(ctx context.Context, ns StorageNamespace, metaRangeID MetaRangeID) (MetaRangeAddress, error) @@ -2020,7 +2021,7 @@ func (g *Graveler) Commit(ctx context.Context, repository *RepositoryRecord, bra } defer changes.Close() // returns err if the commit is empty (no changes) - commit.MetaRangeID, _, err = g.CommittedManager.Commit(ctx, storageNamespace, branchMetaRangeID, changes) + commit.MetaRangeID, _, err = g.CommittedManager.Commit(ctx, storageNamespace, branchMetaRangeID, changes, params.AllowEmpty) if err != nil { return nil, fmt.Errorf("commit: %w", err) } diff --git a/pkg/graveler/graveler_v2_test.go b/pkg/graveler/graveler_v2_test.go index 75c879f8a05..38168df5854 100644 --- a/pkg/graveler/graveler_v2_test.go +++ b/pkg/graveler/graveler_v2_test.go @@ -521,7 +521,7 @@ func TestGravelerCommit_v2(t *testing.T) { test.StagingManager.EXPECT().List(ctx, stagingToken1, gomock.Any()).Times(1).Return(testutils.NewFakeValueIterator([]*graveler.ValueRecord{})) test.StagingManager.EXPECT().List(ctx, stagingToken2, gomock.Any()).Times(1).Return(testutils.NewFakeValueIterator([]*graveler.ValueRecord{})) test.StagingManager.EXPECT().List(ctx, stagingToken3, gomock.Any()).Times(1).Return(testutils.NewFakeValueIterator([]*graveler.ValueRecord{})) - test.CommittedManager.EXPECT().Commit(ctx, repository.StorageNamespace, mr1ID, gomock.Any(), []graveler.SetOptionsFunc{}).Times(1).Return(graveler.MetaRangeID(""), graveler.DiffSummary{}, nil) + test.CommittedManager.EXPECT().Commit(ctx, repository.StorageNamespace, mr1ID, gomock.Any(), false, []graveler.SetOptionsFunc{}).Times(1).Return(graveler.MetaRangeID(""), graveler.DiffSummary{}, nil) test.RefManager.EXPECT().AddCommit(ctx, repository, gomock.Any()).Return(graveler.CommitID(""), nil) test.StagingManager.EXPECT().DropAsync(ctx, stagingToken1).Return(nil) test.StagingManager.EXPECT().DropAsync(ctx, stagingToken2).Return(nil) @@ -563,7 +563,7 @@ func TestGravelerCommit_v2(t *testing.T) { test.StagingManager.EXPECT().List(ctx, stagingToken1, gomock.Any()).Times(1).Return(testutils.NewFakeValueIterator([]*graveler.ValueRecord{})) test.StagingManager.EXPECT().List(ctx, stagingToken2, gomock.Any()).Times(1).Return(testutils.NewFakeValueIterator([]*graveler.ValueRecord{})) test.StagingManager.EXPECT().List(ctx, stagingToken3, gomock.Any()).Times(1).Return(testutils.NewFakeValueIterator([]*graveler.ValueRecord{})) - test.CommittedManager.EXPECT().Commit(ctx, repository.StorageNamespace, mr1ID, gomock.Any(), []graveler.SetOptionsFunc{}).Times(1).Return(graveler.MetaRangeID(""), graveler.DiffSummary{}, graveler.ErrNoChanges) + test.CommittedManager.EXPECT().Commit(ctx, repository.StorageNamespace, mr1ID, gomock.Any(), false, []graveler.SetOptionsFunc{}).Times(1).Return(graveler.MetaRangeID(""), graveler.DiffSummary{}, graveler.ErrNoChanges) val, err := test.Sut.Commit(ctx, repository, branch1ID, graveler.CommitParams{}) diff --git a/pkg/graveler/mock/graveler.go b/pkg/graveler/mock/graveler.go index 553eec24fec..d5834a9edf3 100644 --- a/pkg/graveler/mock/graveler.go +++ b/pkg/graveler/mock/graveler.go @@ -2429,9 +2429,9 @@ func (m *MockCommittedManager) EXPECT() *MockCommittedManagerMockRecorder { } // Commit mocks base method. -func (m *MockCommittedManager) Commit(ctx context.Context, ns graveler.StorageNamespace, baseMetaRangeID graveler.MetaRangeID, changes graveler.ValueIterator, opts ...graveler.SetOptionsFunc) (graveler.MetaRangeID, graveler.DiffSummary, error) { +func (m *MockCommittedManager) Commit(ctx context.Context, ns graveler.StorageNamespace, baseMetaRangeID graveler.MetaRangeID, changes graveler.ValueIterator, allowEmpty bool, opts ...graveler.SetOptionsFunc) (graveler.MetaRangeID, graveler.DiffSummary, error) { m.ctrl.T.Helper() - varargs := []interface{}{ctx, ns, baseMetaRangeID, changes} + varargs := []interface{}{ctx, ns, baseMetaRangeID, changes, allowEmpty} for _, a := range opts { varargs = append(varargs, a) } @@ -2443,9 +2443,9 @@ func (m *MockCommittedManager) Commit(ctx context.Context, ns graveler.StorageNa } // Commit indicates an expected call of Commit. -func (mr *MockCommittedManagerMockRecorder) Commit(ctx, ns, baseMetaRangeID, changes interface{}, opts ...interface{}) *gomock.Call { +func (mr *MockCommittedManagerMockRecorder) Commit(ctx, ns, baseMetaRangeID, changes, allowEmpty interface{}, opts ...interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - varargs := append([]interface{}{ctx, ns, baseMetaRangeID, changes}, opts...) + varargs := append([]interface{}{ctx, ns, baseMetaRangeID, changes, allowEmpty}, opts...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Commit", reflect.TypeOf((*MockCommittedManager)(nil).Commit), varargs...) } diff --git a/pkg/graveler/testutil/fakes.go b/pkg/graveler/testutil/fakes.go index 4d34cf8c63b..d24d8e61e38 100644 --- a/pkg/graveler/testutil/fakes.go +++ b/pkg/graveler/testutil/fakes.go @@ -94,7 +94,7 @@ func (c *CommittedFake) Import(_ context.Context, _ graveler.StorageNamespace, _ return c.MetaRangeID, nil } -func (c *CommittedFake) Commit(_ context.Context, _ graveler.StorageNamespace, baseMetaRangeID graveler.MetaRangeID, changes graveler.ValueIterator, _ ...graveler.SetOptionsFunc) (graveler.MetaRangeID, graveler.DiffSummary, error) { +func (c *CommittedFake) Commit(_ context.Context, _ graveler.StorageNamespace, baseMetaRangeID graveler.MetaRangeID, changes graveler.ValueIterator, allowEmpty bool, _ ...graveler.SetOptionsFunc) (graveler.MetaRangeID, graveler.DiffSummary, error) { if c.Err != nil { return "", graveler.DiffSummary{}, c.Err } diff --git a/pkg/samplerepo/samplecontent.go b/pkg/samplerepo/samplecontent.go index d27a376537f..ad646bf14a0 100644 --- a/pkg/samplerepo/samplecontent.go +++ b/pkg/samplerepo/samplecontent.go @@ -112,7 +112,7 @@ func PopulateSampleRepo(ctx context.Context, repo *catalog.Repository, cat *cata // if we succeeded, commit the changes // commit changes _, err = cat.Commit(ctx, repo.Name, repo.DefaultBranch, sampleRepoCommitMsg, - user.Username, map[string]string{}, swag.Int64(time.Now().Unix()), nil) + user.Username, map[string]string{}, swag.Int64(time.Now().Unix()), nil, false) return err }