diff --git a/api/swagger.yml b/api/swagger.yml index a8a2c44cb13..b54ad7d58f0 100644 --- a/api/swagger.yml +++ b/api/swagger.yml @@ -645,6 +645,11 @@ components: force: type: boolean default: false + description: Allow merge into a read-only branch or into a branch with the same content + allow_empty: + type: boolean + default: false + description: Allow merge when the branches have the same content BranchCreation: type: object diff --git a/clients/java-legacy/api/openapi.yaml b/clients/java-legacy/api/openapi.yaml index eccab3f7789..0454ce89c79 100644 --- a/clients/java-legacy/api/openapi.yaml +++ b/clients/java-legacy/api/openapi.yaml @@ -7762,6 +7762,7 @@ components: force: false message: message strategy: strategy + allow_empty: false properties: message: type: string @@ -7777,6 +7778,12 @@ components: type: string force: default: false + description: Allow merge into a read-only branch or into a branch with the + same content + type: boolean + allow_empty: + default: false + description: Allow merge when the branches have the same content type: boolean type: object BranchCreation: diff --git a/clients/java-legacy/docs/Merge.md b/clients/java-legacy/docs/Merge.md index c03c5939760..fefa0445dc7 100644 --- a/clients/java-legacy/docs/Merge.md +++ b/clients/java-legacy/docs/Merge.md @@ -10,7 +10,8 @@ Name | Type | Description | Notes **message** | **String** | | [optional] **metadata** | **Map<String, String>** | | [optional] **strategy** | **String** | In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ('dest-wins') or from the source branch('source-wins'). In case no selection is made, the merge process will fail in case of a conflict | [optional] -**force** | **Boolean** | | [optional] +**force** | **Boolean** | Allow merge into a read-only branch or into a branch with the same content | [optional] +**allowEmpty** | **Boolean** | Allow merge when the branches have the same content | [optional] diff --git a/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/Merge.java b/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/Merge.java index 1fd987f35d4..2509d15ddd6 100644 --- a/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/Merge.java +++ b/clients/java-legacy/src/main/java/io/lakefs/clients/api/model/Merge.java @@ -48,6 +48,10 @@ public class Merge { @SerializedName(SERIALIZED_NAME_FORCE) private Boolean force = false; + public static final String SERIALIZED_NAME_ALLOW_EMPTY = "allow_empty"; + @SerializedName(SERIALIZED_NAME_ALLOW_EMPTY) + private Boolean allowEmpty = false; + public Merge message(String message) { @@ -133,11 +137,11 @@ public Merge force(Boolean force) { } /** - * Get force + * Allow merge into a read-only branch or into a branch with the same content * @return force **/ @javax.annotation.Nullable - @ApiModelProperty(value = "") + @ApiModelProperty(value = "Allow merge into a read-only branch or into a branch with the same content") public Boolean getForce() { return force; @@ -149,6 +153,29 @@ public void setForce(Boolean force) { } + public Merge allowEmpty(Boolean allowEmpty) { + + this.allowEmpty = allowEmpty; + return this; + } + + /** + * Allow merge when the branches have the same content + * @return allowEmpty + **/ + @javax.annotation.Nullable + @ApiModelProperty(value = "Allow merge when the branches have the same content") + + public Boolean getAllowEmpty() { + return allowEmpty; + } + + + public void setAllowEmpty(Boolean allowEmpty) { + this.allowEmpty = allowEmpty; + } + + @Override public boolean equals(Object o) { if (this == o) { @@ -161,12 +188,13 @@ public boolean equals(Object o) { return Objects.equals(this.message, merge.message) && Objects.equals(this.metadata, merge.metadata) && Objects.equals(this.strategy, merge.strategy) && - Objects.equals(this.force, merge.force); + Objects.equals(this.force, merge.force) && + Objects.equals(this.allowEmpty, merge.allowEmpty); } @Override public int hashCode() { - return Objects.hash(message, metadata, strategy, force); + return Objects.hash(message, metadata, strategy, force, allowEmpty); } @Override @@ -177,6 +205,7 @@ public String toString() { sb.append(" metadata: ").append(toIndentedString(metadata)).append("\n"); sb.append(" strategy: ").append(toIndentedString(strategy)).append("\n"); sb.append(" force: ").append(toIndentedString(force)).append("\n"); + sb.append(" allowEmpty: ").append(toIndentedString(allowEmpty)).append("\n"); sb.append("}"); return sb.toString(); } diff --git a/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/MergeTest.java b/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/MergeTest.java index aee2a5db5e3..1076fb3732c 100644 --- a/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/MergeTest.java +++ b/clients/java-legacy/src/test/java/io/lakefs/clients/api/model/MergeTest.java @@ -75,4 +75,12 @@ public void forceTest() { // TODO: test force } + /** + * Test the property 'allowEmpty' + */ + @Test + public void allowEmptyTest() { + // TODO: test allowEmpty + } + } diff --git a/clients/java/api/openapi.yaml b/clients/java/api/openapi.yaml index 94a94dc8d4b..f7602a2495e 100644 --- a/clients/java/api/openapi.yaml +++ b/clients/java/api/openapi.yaml @@ -7736,6 +7736,7 @@ components: force: false message: message strategy: strategy + allow_empty: false properties: message: type: string @@ -7751,6 +7752,12 @@ components: type: string force: default: false + description: Allow merge into a read-only branch or into a branch with the + same content + type: boolean + allow_empty: + default: false + description: Allow merge when the branches have the same content type: boolean type: object BranchCreation: diff --git a/clients/java/docs/Merge.md b/clients/java/docs/Merge.md index 1a2f865b2b0..7c7a6d108d8 100644 --- a/clients/java/docs/Merge.md +++ b/clients/java/docs/Merge.md @@ -10,7 +10,8 @@ |**message** | **String** | | [optional] | |**metadata** | **Map<String, String>** | | [optional] | |**strategy** | **String** | In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ('dest-wins') or from the source branch('source-wins'). In case no selection is made, the merge process will fail in case of a conflict | [optional] | -|**force** | **Boolean** | | [optional] | +|**force** | **Boolean** | Allow merge into a read-only branch or into a branch with the same content | [optional] | +|**allowEmpty** | **Boolean** | Allow merge when the branches have the same content | [optional] | diff --git a/clients/java/src/main/java/io/lakefs/clients/sdk/model/Merge.java b/clients/java/src/main/java/io/lakefs/clients/sdk/model/Merge.java index b154b697b21..2c380740f60 100644 --- a/clients/java/src/main/java/io/lakefs/clients/sdk/model/Merge.java +++ b/clients/java/src/main/java/io/lakefs/clients/sdk/model/Merge.java @@ -70,6 +70,10 @@ public class Merge { @SerializedName(SERIALIZED_NAME_FORCE) private Boolean force = false; + public static final String SERIALIZED_NAME_ALLOW_EMPTY = "allow_empty"; + @SerializedName(SERIALIZED_NAME_ALLOW_EMPTY) + private Boolean allowEmpty = false; + public Merge() { } @@ -151,7 +155,7 @@ public Merge force(Boolean force) { } /** - * Get force + * Allow merge into a read-only branch or into a branch with the same content * @return force **/ @javax.annotation.Nullable @@ -164,6 +168,27 @@ public void setForce(Boolean force) { this.force = force; } + + public Merge allowEmpty(Boolean allowEmpty) { + + this.allowEmpty = allowEmpty; + return this; + } + + /** + * Allow merge when the branches have the same content + * @return allowEmpty + **/ + @javax.annotation.Nullable + public Boolean getAllowEmpty() { + return allowEmpty; + } + + + public void setAllowEmpty(Boolean allowEmpty) { + this.allowEmpty = allowEmpty; + } + /** * A container for additional, undeclared properties. * This is a holder for any undeclared properties as specified with @@ -222,13 +247,14 @@ public boolean equals(Object o) { return Objects.equals(this.message, merge.message) && Objects.equals(this.metadata, merge.metadata) && Objects.equals(this.strategy, merge.strategy) && - Objects.equals(this.force, merge.force)&& + Objects.equals(this.force, merge.force) && + Objects.equals(this.allowEmpty, merge.allowEmpty)&& Objects.equals(this.additionalProperties, merge.additionalProperties); } @Override public int hashCode() { - return Objects.hash(message, metadata, strategy, force, additionalProperties); + return Objects.hash(message, metadata, strategy, force, allowEmpty, additionalProperties); } @Override @@ -239,6 +265,7 @@ public String toString() { sb.append(" metadata: ").append(toIndentedString(metadata)).append("\n"); sb.append(" strategy: ").append(toIndentedString(strategy)).append("\n"); sb.append(" force: ").append(toIndentedString(force)).append("\n"); + sb.append(" allowEmpty: ").append(toIndentedString(allowEmpty)).append("\n"); sb.append(" additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n"); sb.append("}"); return sb.toString(); @@ -266,6 +293,7 @@ private String toIndentedString(Object o) { openapiFields.add("metadata"); openapiFields.add("strategy"); openapiFields.add("force"); + openapiFields.add("allow_empty"); // a set of required properties/fields (JSON key names) openapiRequiredFields = new HashSet(); diff --git a/clients/java/src/test/java/io/lakefs/clients/sdk/model/MergeTest.java b/clients/java/src/test/java/io/lakefs/clients/sdk/model/MergeTest.java index e5c1a56f8cf..72b19d25a0e 100644 --- a/clients/java/src/test/java/io/lakefs/clients/sdk/model/MergeTest.java +++ b/clients/java/src/test/java/io/lakefs/clients/sdk/model/MergeTest.java @@ -71,4 +71,12 @@ public void forceTest() { // TODO: test force } + /** + * Test the property 'allowEmpty' + */ + @Test + public void allowEmptyTest() { + // TODO: test allowEmpty + } + } diff --git a/clients/python-legacy/docs/Merge.md b/clients/python-legacy/docs/Merge.md index 359cb60cd6e..157875ad2ef 100644 --- a/clients/python-legacy/docs/Merge.md +++ b/clients/python-legacy/docs/Merge.md @@ -7,7 +7,8 @@ Name | Type | Description | Notes **message** | **str** | | [optional] **metadata** | **{str: (str,)}** | | [optional] **strategy** | **str** | In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ('dest-wins') or from the source branch('source-wins'). In case no selection is made, the merge process will fail in case of a conflict | [optional] -**force** | **bool** | | [optional] if omitted the server will use the default value of False +**force** | **bool** | Allow merge into a read-only branch or into a branch with the same content | [optional] if omitted the server will use the default value of False +**allow_empty** | **bool** | Allow merge when the branches have the same content | [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] [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) diff --git a/clients/python-legacy/docs/RefsApi.md b/clients/python-legacy/docs/RefsApi.md index b66a3d0afa8..01a832dddf7 100644 --- a/clients/python-legacy/docs/RefsApi.md +++ b/clients/python-legacy/docs/RefsApi.md @@ -468,6 +468,7 @@ with lakefs_client.ApiClient(configuration) as api_client: }, strategy="strategy_example", force=False, + allow_empty=False, ) # Merge | (optional) # example passing only required values which don't have defaults set diff --git a/clients/python-legacy/lakefs_client/model/merge.py b/clients/python-legacy/lakefs_client/model/merge.py index 0d90baf0a4c..5504c34cd20 100644 --- a/clients/python-legacy/lakefs_client/model/merge.py +++ b/clients/python-legacy/lakefs_client/model/merge.py @@ -86,6 +86,7 @@ def openapi_types(): 'metadata': ({str: (str,)},), # noqa: E501 'strategy': (str,), # noqa: E501 'force': (bool,), # noqa: E501 + 'allow_empty': (bool,), # noqa: E501 } @cached_property @@ -98,6 +99,7 @@ def discriminator(): 'metadata': 'metadata', # noqa: E501 'strategy': 'strategy', # noqa: E501 'force': 'force', # noqa: E501 + 'allow_empty': 'allow_empty', # noqa: E501 } read_only_vars = { @@ -144,7 +146,8 @@ def _from_openapi_data(cls, *args, **kwargs): # noqa: E501 message (str): [optional] # noqa: E501 metadata ({str: (str,)}): [optional] # noqa: E501 strategy (str): In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ('dest-wins') or from the source branch('source-wins'). In case no selection is made, the merge process will fail in case of a conflict. [optional] # noqa: E501 - force (bool): [optional] if omitted the server will use the default value of False # noqa: E501 + force (bool): Allow merge into a read-only branch or into a branch with the same content. [optional] if omitted the server will use the default value of False # noqa: E501 + allow_empty (bool): Allow merge when the branches have the same content. [optional] if omitted the server will use the default value of False # noqa: E501 """ _check_type = kwargs.pop('_check_type', True) @@ -229,7 +232,8 @@ def __init__(self, *args, **kwargs): # noqa: E501 message (str): [optional] # noqa: E501 metadata ({str: (str,)}): [optional] # noqa: E501 strategy (str): In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ('dest-wins') or from the source branch('source-wins'). In case no selection is made, the merge process will fail in case of a conflict. [optional] # noqa: E501 - force (bool): [optional] if omitted the server will use the default value of False # noqa: E501 + force (bool): Allow merge into a read-only branch or into a branch with the same content. [optional] if omitted the server will use the default value of False # noqa: E501 + allow_empty (bool): Allow merge when the branches have the same content. [optional] if omitted the server will use the default value of False # noqa: E501 """ _check_type = kwargs.pop('_check_type', True) diff --git a/clients/python/docs/Merge.md b/clients/python/docs/Merge.md index fcf15eaa5b6..6b0e40f1083 100644 --- a/clients/python/docs/Merge.md +++ b/clients/python/docs/Merge.md @@ -8,7 +8,8 @@ Name | Type | Description | Notes **message** | **str** | | [optional] **metadata** | **Dict[str, str]** | | [optional] **strategy** | **str** | In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ('dest-wins') or from the source branch('source-wins'). In case no selection is made, the merge process will fail in case of a conflict | [optional] -**force** | **bool** | | [optional] [default to False] +**force** | **bool** | Allow merge into a read-only branch or into a branch with the same content | [optional] [default to False] +**allow_empty** | **bool** | Allow merge when the branches have the same content | [optional] [default to False] ## Example diff --git a/clients/python/lakefs_sdk/models/merge.py b/clients/python/lakefs_sdk/models/merge.py index 95d1984fc30..274f42988b3 100644 --- a/clients/python/lakefs_sdk/models/merge.py +++ b/clients/python/lakefs_sdk/models/merge.py @@ -32,8 +32,9 @@ class Merge(BaseModel): message: Optional[StrictStr] = None metadata: Optional[Dict[str, StrictStr]] = None strategy: Optional[StrictStr] = Field(None, description="In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ('dest-wins') or from the source branch('source-wins'). In case no selection is made, the merge process will fail in case of a conflict") - force: Optional[StrictBool] = False - __properties = ["message", "metadata", "strategy", "force"] + force: Optional[StrictBool] = Field(False, description="Allow merge into a read-only branch or into a branch with the same content") + allow_empty: Optional[StrictBool] = Field(False, description="Allow merge when the branches have the same content") + __properties = ["message", "metadata", "strategy", "force", "allow_empty"] class Config: """Pydantic configuration""" @@ -74,7 +75,8 @@ def from_dict(cls, obj: dict) -> Merge: "message": obj.get("message"), "metadata": obj.get("metadata"), "strategy": obj.get("strategy"), - "force": obj.get("force") if obj.get("force") is not None else False + "force": obj.get("force") if obj.get("force") is not None else False, + "allow_empty": obj.get("allow_empty") if obj.get("allow_empty") is not None else False }) return _obj diff --git a/clients/python/test/test_merge.py b/clients/python/test/test_merge.py index fc2c8dff7b3..d22970adb56 100644 --- a/clients/python/test/test_merge.py +++ b/clients/python/test/test_merge.py @@ -44,7 +44,8 @@ def make_instance(self, include_optional): 'key' : '' }, strategy = '', - force = True + force = True, + allow_empty = True ) else : return Merge( diff --git a/clients/rust/docs/Merge.md b/clients/rust/docs/Merge.md index 4d0fa3538af..a51ace6df16 100644 --- a/clients/rust/docs/Merge.md +++ b/clients/rust/docs/Merge.md @@ -7,7 +7,8 @@ Name | Type | Description | Notes **message** | Option<**String**> | | [optional] **metadata** | Option<**std::collections::HashMap**> | | [optional] **strategy** | Option<**String**> | In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ('dest-wins') or from the source branch('source-wins'). In case no selection is made, the merge process will fail in case of a conflict | [optional] -**force** | Option<**bool**> | | [optional][default to false] +**force** | Option<**bool**> | Allow merge into a read-only branch or into a branch with the same content | [optional][default to false] +**allow_empty** | Option<**bool**> | Allow merge when the branches have the same content | [optional][default to false] [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) diff --git a/clients/rust/src/models/merge.rs b/clients/rust/src/models/merge.rs index 95322fb035c..8dedae1f09b 100644 --- a/clients/rust/src/models/merge.rs +++ b/clients/rust/src/models/merge.rs @@ -19,8 +19,12 @@ pub struct Merge { /// In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ('dest-wins') or from the source branch('source-wins'). In case no selection is made, the merge process will fail in case of a conflict #[serde(rename = "strategy", skip_serializing_if = "Option::is_none")] pub strategy: Option, + /// Allow merge into a read-only branch or into a branch with the same content #[serde(rename = "force", skip_serializing_if = "Option::is_none")] pub force: Option, + /// Allow merge when the branches have the same content + #[serde(rename = "allow_empty", skip_serializing_if = "Option::is_none")] + pub allow_empty: Option, } impl Merge { @@ -30,6 +34,7 @@ impl Merge { metadata: None, strategy: None, force: None, + allow_empty: None, } } } diff --git a/cmd/lakectl/cmd/merge.go b/cmd/lakectl/cmd/merge.go index 5402bdd3d28..473a842c787 100644 --- a/cmd/lakectl/cmd/merge.go +++ b/cmd/lakectl/cmd/merge.go @@ -10,7 +10,7 @@ import ( const ( mergeCmdMinArgs = 2 - mergeCmdMaxArgs = 5 + mergeCmdMaxArgs = 6 mergeCreateTemplate = `Merged "{{.Merge.FromRef|yellow}}" into "{{.Merge.ToRef|yellow}}" to get "{{.Result.Reference|green}}". ` @@ -36,24 +36,31 @@ var mergeCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { message, kvPairs := getCommitFlags(cmd) client := getClient() + sourceRef := MustParseBranchURI("source ref", args[0]) destinationRef := MustParseBranchURI("destination ref", args[1]) strategy := Must(cmd.Flags().GetString("strategy")) + force := Must(cmd.Flags().GetBool("force")) + allowEmpty := Must(cmd.Flags().GetBool("allow-empty")) + fmt.Println("Source:", sourceRef) fmt.Println("Destination:", destinationRef) + if destinationRef.Repository != sourceRef.Repository { Die("both references must belong to the same repository", 1) } - if strategy != "dest-wins" && strategy != "source-wins" && strategy != "" { Die("Invalid strategy value. Expected \"dest-wins\" or \"source-wins\"", 1) } body := apigen.MergeIntoBranchJSONRequestBody{ - Message: &message, - Metadata: &apigen.Merge_Metadata{AdditionalProperties: kvPairs}, - Strategy: &strategy, + Message: &message, + Metadata: &apigen.Merge_Metadata{AdditionalProperties: kvPairs}, + Strategy: &strategy, + Force: &force, + AllowEmpty: &allowEmpty, } + resp, err := client.MergeIntoBranchWithResponse(cmd.Context(), destinationRef.Repository, sourceRef.Ref, destinationRef.Ref, body) if resp != nil && resp.JSON409 != nil { Die("Conflict found.", 1) @@ -78,7 +85,10 @@ var mergeCmd = &cobra.Command{ //nolint:gochecknoinits func init() { - mergeCmd.Flags().String("strategy", "", "In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch (\"dest-wins\") or from the source branch(\"source-wins\"). In case no selection is made, the merge process will fail in case of a conflict") + flags := mergeCmd.Flags() + flags.String("strategy", "", "In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch (\"dest-wins\") or from the source branch(\"source-wins\"). In case no selection is made, the merge process will fail in case of a conflict") + flags.Bool("force", false, "Allow merge into a read-only branch or into a branch with the same content") + flags.Bool("allow-empty", false, "Allow merge when the branches have the same content") withCommitFlags(mergeCmd, true) rootCmd.AddCommand(mergeCmd) } diff --git a/docs/assets/js/swagger.yml b/docs/assets/js/swagger.yml index a8a2c44cb13..b54ad7d58f0 100644 --- a/docs/assets/js/swagger.yml +++ b/docs/assets/js/swagger.yml @@ -645,6 +645,11 @@ components: force: type: boolean default: false + description: Allow merge into a read-only branch or into a branch with the same content + allow_empty: + type: boolean + default: false + description: Allow merge when the branches have the same content BranchCreation: type: object diff --git a/docs/reference/cli.md b/docs/reference/cli.md index dedfc80d257..d9ad8c11d1d 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -2738,7 +2738,9 @@ lakectl merge [flags] {:.no_toc} ``` + --allow-empty Allow merge when the branches have the same content --allow-empty-message allow an empty commit message (default true) + --force Allow merge into a read-only branch or into a branch with the same content -h, --help help for merge -m, --message string commit message --meta strings key value pair in the form of key=value diff --git a/pkg/api/controller.go b/pkg/api/controller.go index a0e81cf8cd1..120fea0781a 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -4723,7 +4723,9 @@ func (c *Controller) MergeIntoBranch(w http.ResponseWriter, r *http.Request, bod swag.StringValue(body.Message), metadata, swag.StringValue(body.Strategy), - graveler.WithForce(swag.BoolValue(body.Force))) + graveler.WithForce(swag.BoolValue(body.Force)), + graveler.WithAllowEmpty(swag.BoolValue(body.AllowEmpty)), + ) if errors.Is(err, graveler.ErrConflictFound) { writeResponse(w, r, http.StatusConflict, apigen.MergeResult{ diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index c9fa8129583..e612163ff41 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -3805,6 +3805,45 @@ func TestController_MergeDirtyBranch(t *testing.T) { } } +func TestController_MergeBranchWithNoChanges(t *testing.T) { + clt, _ := setupClientWithAdmin(t) + ctx := context.Background() + + repoName := testUniqueRepoName() + repoResp, err := clt.CreateRepositoryWithResponse(ctx, &apigen.CreateRepositoryParams{}, apigen.CreateRepositoryJSONRequestBody{ + DefaultBranch: apiutil.Ptr("main"), + Name: repoName, + StorageNamespace: "mem://", + }) + verifyResponseOK(t, repoResp, err) + + branch1Resp, err := clt.CreateBranchWithResponse(ctx, repoName, apigen.CreateBranchJSONRequestBody{Name: "branch1", Source: "main"}) + verifyResponseOK(t, branch1Resp, err) + + branch2Resp, err := clt.CreateBranchWithResponse(ctx, repoName, apigen.CreateBranchJSONRequestBody{Name: "branch2", Source: "main"}) + verifyResponseOK(t, branch2Resp, err) + + mergeResp, err := clt.MergeIntoBranchWithResponse(ctx, repoName, "branch2", "branch1", apigen.MergeIntoBranchJSONRequestBody{ + Message: apiutil.Ptr("Merge branch2 to branch1"), + }) + testutil.MustDo(t, "perform merge with no changes", err) + if mergeResp.JSON400 == nil || !strings.HasSuffix(mergeResp.JSON400.Message, graveler.ErrNoChanges.Error()) { + t.Errorf("Merge branches with no changes should fail with ErrNoChanges, got %+v", mergeResp) + } + + mergeWithAllowEmptyFlagResp, err := clt.MergeIntoBranchWithResponse(ctx, repoName, "branch2", "branch1", apigen.MergeIntoBranchJSONRequestBody{ + Message: apiutil.Ptr("Merge branch2 to branch1"), + AllowEmpty: swag.Bool(true), + }) + verifyResponseOK(t, mergeWithAllowEmptyFlagResp, err) + + mergeWithForceFlagResp, err := clt.MergeIntoBranchWithResponse(ctx, repoName, "branch2", "branch1", apigen.MergeIntoBranchJSONRequestBody{ + Message: apiutil.Ptr("Merge branch2 to branch1"), + Force: swag.Bool(true), + }) + verifyResponseOK(t, mergeWithForceFlagResp, err) +} + func TestController_CreateTag(t *testing.T) { clt, deps := setupClientWithAdmin(t) ctx := context.Background() diff --git a/pkg/graveler/committed/manager.go b/pkg/graveler/committed/manager.go index c4e6e7d0088..e5dc7858ddc 100644 --- a/pkg/graveler/committed/manager.go +++ b/pkg/graveler/committed/manager.go @@ -200,15 +200,19 @@ func (c *committedManager) Import(ctx context.Context, ns graveler.StorageNamesp return c.merge(ctx, mctx) } -func (c *committedManager) Merge(ctx context.Context, ns graveler.StorageNamespace, destination, source, base graveler.MetaRangeID, strategy graveler.MergeStrategy, _ ...graveler.SetOptionsFunc) (graveler.MetaRangeID, error) { - if source == base { +func (c *committedManager) Merge(ctx context.Context, ns graveler.StorageNamespace, destination, source, base graveler.MetaRangeID, strategy graveler.MergeStrategy, opts ...graveler.SetOptionsFunc) (graveler.MetaRangeID, error) { + options := graveler.NewSetOptions(opts) + + if source == base && !options.AllowEmpty && !options.Force { // no changes on source return "", graveler.ErrNoChanges } + if destination == base { // changes introduced only on source return source, nil } + mctx := mergeContext{ strategy: strategy, ns: ns, diff --git a/pkg/graveler/graveler.go b/pkg/graveler/graveler.go index 9de55390e69..c07cf819566 100644 --- a/pkg/graveler/graveler.go +++ b/pkg/graveler/graveler.go @@ -191,10 +191,20 @@ type SetOptions struct { MaxTries int // Force set to true will bypass repository read-only protection. Force bool + // AllowEmpty set to true will allow committing an empty commit. + AllowEmpty bool } type SetOptionsFunc func(opts *SetOptions) +func NewSetOptions(opts []SetOptionsFunc) *SetOptions { + options := &SetOptions{} + for _, opt := range opts { + opt(options) + } + return options +} + func WithIfAbsent(v bool) SetOptionsFunc { return func(opts *SetOptions) { opts.IfAbsent = v @@ -207,6 +217,12 @@ func WithForce(v bool) SetOptionsFunc { } } +func WithAllowEmpty(v bool) SetOptionsFunc { + return func(opts *SetOptions) { + opts.AllowEmpty = v + } +} + // function/methods receiving the following basic types could assume they passed validation // StorageNamespace is the URI to the storage location @@ -1092,10 +1108,7 @@ func (g *Graveler) SetRepositoryMetadata(ctx context.Context, repository *Reposi } func (g *Graveler) WriteRange(ctx context.Context, repository *RepositoryRecord, it ValueIterator, opts ...SetOptionsFunc) (*RangeInfo, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return nil, ErrReadOnlyRepository } @@ -1103,10 +1116,7 @@ func (g *Graveler) WriteRange(ctx context.Context, repository *RepositoryRecord, } func (g *Graveler) WriteMetaRange(ctx context.Context, repository *RepositoryRecord, ranges []*RangeInfo, opts ...SetOptionsFunc) (*MetaRangeInfo, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return nil, ErrReadOnlyRepository } @@ -1118,10 +1128,7 @@ func (g *Graveler) StageObject(ctx context.Context, stagingToken string, object } func (g *Graveler) WriteMetaRangeByIterator(ctx context.Context, repository *RepositoryRecord, it ValueIterator, opts ...SetOptionsFunc) (*MetaRangeID, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return nil, ErrReadOnlyRepository } @@ -1138,10 +1145,7 @@ func GenerateStagingToken(repositoryID RepositoryID, branchID BranchID) StagingT } func (g *Graveler) CreateBranch(ctx context.Context, repository *RepositoryRecord, branchID BranchID, ref Ref, opts ...SetOptionsFunc) (*Branch, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return nil, ErrReadOnlyRepository } @@ -1210,10 +1214,7 @@ func (g *Graveler) CreateBranch(ctx context.Context, repository *RepositoryRecor } func (g *Graveler) UpdateBranch(ctx context.Context, repository *RepositoryRecord, branchID BranchID, ref Ref, opts ...SetOptionsFunc) (*Branch, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return nil, ErrReadOnlyRepository } @@ -1306,10 +1307,7 @@ func (g *Graveler) GetTag(ctx context.Context, repository *RepositoryRecord, tag } func (g *Graveler) CreateTag(ctx context.Context, repository *RepositoryRecord, tagID TagID, commitID CommitID, opts ...SetOptionsFunc) error { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -1369,10 +1367,7 @@ func (g *Graveler) CreateTag(ctx context.Context, repository *RepositoryRecord, } func (g *Graveler) DeleteTag(ctx context.Context, repository *RepositoryRecord, tagID TagID, opts ...SetOptionsFunc) error { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -1455,10 +1450,7 @@ func (g *Graveler) ListBranches(ctx context.Context, repository *RepositoryRecor } func (g *Graveler) DeleteBranch(ctx context.Context, repository *RepositoryRecord, branchID BranchID, opts ...SetOptionsFunc) error { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -1667,10 +1659,7 @@ func (g *Graveler) Set(ctx context.Context, repository *RepositoryRecord, branch return ErrWriteToProtectedBranch } - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -1759,10 +1748,7 @@ func (g *Graveler) Delete(ctx context.Context, repository *RepositoryRecord, bra return ErrWriteToProtectedBranch } - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -1786,10 +1772,7 @@ func (g *Graveler) DeleteBatch(ctx context.Context, repository *RepositoryRecord return ErrWriteToProtectedBranch } - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -1943,10 +1926,7 @@ func (g *Graveler) Commit(ctx context.Context, repository *RepositoryRecord, bra return "", ErrCommitToProtectedBranch } - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return "", ErrReadOnlyRepository } @@ -2078,10 +2058,7 @@ func (g *Graveler) Commit(ctx context.Context, repository *RepositoryRecord, bra } func (g *Graveler) CreateCommitRecord(ctx context.Context, repository *RepositoryRecord, commitID CommitID, commit Commit, opts ...SetOptionsFunc) error { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -2149,10 +2126,7 @@ func CommitExists(ctx context.Context, repository *RepositoryRecord, commitID Co } func (g *Graveler) AddCommit(ctx context.Context, repository *RepositoryRecord, commit Commit, opts ...SetOptionsFunc) (CommitID, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return "", ErrReadOnlyRepository } @@ -2268,10 +2242,7 @@ func (g *Graveler) ResetHard(ctx context.Context, repository *RepositoryRecord, return ErrWriteToProtectedBranch } - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository @@ -2325,10 +2296,7 @@ func (g *Graveler) Reset(ctx context.Context, repository *RepositoryRecord, bran return ErrWriteToProtectedBranch } - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -2400,10 +2368,7 @@ func (g *Graveler) ResetKey(ctx context.Context, repository *RepositoryRecord, b return ErrWriteToProtectedBranch } - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -2444,10 +2409,7 @@ func (g *Graveler) ResetPrefix(ctx context.Context, repository *RepositoryRecord return ErrWriteToProtectedBranch } - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -2509,10 +2471,7 @@ type CommitIDAndSummary struct { // That is, try to apply the diff from C2 to C1 on the tip of the branch. // If the commit is a merge commit, 'parentNumber' is the parent number (1-based) relative to which the revert is done. func (g *Graveler) Revert(ctx context.Context, repository *RepositoryRecord, branchID BranchID, ref Ref, parentNumber int, commitParams CommitParams, opts ...SetOptionsFunc) (CommitID, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return "", ErrReadOnlyRepository } @@ -2596,10 +2555,7 @@ func (g *Graveler) Revert(ctx context.Context, repository *RepositoryRecord, bra // CherryPick creates a new commit on the given branch, with the changes from the given commit. // If the commit is a merge commit, 'parentNumber' is the parent number (1-based) relative to which the cherry-pick is done. func (g *Graveler) CherryPick(ctx context.Context, repository *RepositoryRecord, branchID BranchID, ref Ref, parentNumber *int, committer string, opts ...SetOptionsFunc) (CommitID, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return "", ErrReadOnlyRepository } @@ -2691,10 +2647,7 @@ func (g *Graveler) CherryPick(ctx context.Context, repository *RepositoryRecord, } func (g *Graveler) Merge(ctx context.Context, repository *RepositoryRecord, destination BranchID, source Ref, commitParams CommitParams, strategy string, opts ...SetOptionsFunc) (CommitID, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return "", ErrReadOnlyRepository } @@ -2758,7 +2711,7 @@ func (g *Graveler) Merge(ctx context.Context, repository *RepositoryRecord, dest return nil, ErrInvalidMergeStrategy } - metaRangeID, err := g.CommittedManager.Merge(ctx, storageNamespace, toCommit.MetaRangeID, fromCommit.MetaRangeID, baseCommit.MetaRangeID, mergeStrategy) + metaRangeID, err := g.CommittedManager.Merge(ctx, storageNamespace, toCommit.MetaRangeID, fromCommit.MetaRangeID, baseCommit.MetaRangeID, mergeStrategy, opts...) if err != nil { if !errors.Is(err, ErrUserVisible) { err = fmt.Errorf("merge in CommitManager: %w", err) @@ -2864,10 +2817,7 @@ func (g *Graveler) retryRepoMetadataUpdate(ctx context.Context, repository *Repo } func (g *Graveler) Import(ctx context.Context, repository *RepositoryRecord, destination BranchID, source MetaRangeID, commitParams CommitParams, prefixes []Prefix, opts ...SetOptionsFunc) (CommitID, error) { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return "", ErrReadOnlyRepository } @@ -3117,10 +3067,7 @@ func (g *Graveler) SetHooksHandler(handler HooksHandler) { } func (g *Graveler) LoadCommits(ctx context.Context, repository *RepositoryRecord, metaRangeID MetaRangeID, opts ...SetOptionsFunc) error { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -3169,10 +3116,7 @@ func (g *Graveler) LoadCommits(ctx context.Context, repository *RepositoryRecord } func (g *Graveler) LoadBranches(ctx context.Context, repository *RepositoryRecord, metaRangeID MetaRangeID, opts ...SetOptionsFunc) error { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository } @@ -3205,10 +3149,7 @@ func (g *Graveler) LoadBranches(ctx context.Context, repository *RepositoryRecor } func (g *Graveler) LoadTags(ctx context.Context, repository *RepositoryRecord, metaRangeID MetaRangeID, opts ...SetOptionsFunc) error { - options := &SetOptions{} - for _, opt := range opts { - opt(options) - } + options := NewSetOptions(opts) if repository.ReadOnly && !options.Force { return ErrReadOnlyRepository }