diff --git a/api/swagger.yml b/api/swagger.yml index fd6492ebcf0..33d042302f8 100644 --- a/api/swagger.yml +++ b/api/swagger.yml @@ -90,6 +90,15 @@ components: required: false schema: type: string + + IfMatch: + in: header + name: If-Match + description: Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. + example: "2e9ec317e197e02e4264d128c2e7e681" + required: false + schema: + type: string NoTombstone: in: query @@ -5114,6 +5123,7 @@ paths: parameters: - $ref: "#/components/parameters/IfNoneMatch" + - $ref: "#/components/parameters/IfMatch" - in: query name: storageClass description: Deprecated, this capability will not be supported in future releases. diff --git a/clients/java/api/openapi.yaml b/clients/java/api/openapi.yaml index 583e79c6ca9..ec905e08c1e 100644 --- a/clients/java/api/openapi.yaml +++ b/clients/java/api/openapi.yaml @@ -5507,6 +5507,16 @@ paths: schema: type: string style: simple + - description: Set to the object's ETag to atomically allow operations only + if the object's current ETag matches the provided value. + example: 2e9ec317e197e02e4264d128c2e7e681 + explode: false + in: header + name: If-Match + required: false + schema: + type: string + style: simple - deprecated: true description: "Deprecated, this capability will not be supported in future\ \ releases." @@ -7719,6 +7729,17 @@ components: schema: type: string style: simple + IfMatch: + description: Set to the object's ETag to atomically allow operations only if + the object's current ETag matches the provided value. + example: 2e9ec317e197e02e4264d128c2e7e681 + explode: false + in: header + name: If-Match + required: false + schema: + type: string + style: simple NoTombstone: description: delete entry without tombstone when possible *EXPERIMENTAL* explode: true diff --git a/clients/java/docs/ObjectsApi.md b/clients/java/docs/ObjectsApi.md index 44c1ae28fdd..b49e61f066e 100644 --- a/clients/java/docs/ObjectsApi.md +++ b/clients/java/docs/ObjectsApi.md @@ -955,7 +955,7 @@ null (empty response body) # **uploadObject** -> ObjectStats uploadObject(repository, branch, path).ifNoneMatch(ifNoneMatch).storageClass(storageClass).force(force).content(content).execute(); +> ObjectStats uploadObject(repository, branch, path).ifNoneMatch(ifNoneMatch).ifMatch(ifMatch).storageClass(storageClass).force(force).content(content).execute(); @@ -1006,12 +1006,14 @@ public class Example { String branch = "branch_example"; // String | String path = "path_example"; // String | relative to the branch String ifNoneMatch = "*"; // String | Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported. + String ifMatch = "2e9ec317e197e02e4264d128c2e7e681"; // String | Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. String storageClass = "storageClass_example"; // String | Deprecated, this capability will not be supported in future releases. Boolean force = false; // Boolean | File content = new File("/path/to/file"); // File | Only a single file per upload which must be named \\\"content\\\". try { ObjectStats result = apiInstance.uploadObject(repository, branch, path) .ifNoneMatch(ifNoneMatch) + .ifMatch(ifMatch) .storageClass(storageClass) .force(force) .content(content) @@ -1036,6 +1038,7 @@ public class Example { | **branch** | **String**| | | | **path** | **String**| relative to the branch | | | **ifNoneMatch** | **String**| Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported. | [optional] | +| **ifMatch** | **String**| Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. | [optional] | | **storageClass** | **String**| Deprecated, this capability will not be supported in future releases. | [optional] | | **force** | **Boolean**| | [optional] [default to false] | | **content** | **File**| Only a single file per upload which must be named \\\"content\\\". | [optional] | diff --git a/clients/java/src/main/java/io/lakefs/clients/sdk/ObjectsApi.java b/clients/java/src/main/java/io/lakefs/clients/sdk/ObjectsApi.java index a26e2cc234e..6a09fbf559a 100644 --- a/clients/java/src/main/java/io/lakefs/clients/sdk/ObjectsApi.java +++ b/clients/java/src/main/java/io/lakefs/clients/sdk/ObjectsApi.java @@ -2133,7 +2133,7 @@ public okhttp3.Call executeAsync(final ApiCallback _callback) throws ApiEx public APIupdateObjectUserMetadataRequest updateObjectUserMetadata(String repository, String branch, String path, UpdateObjectUserMetadata updateObjectUserMetadata) { return new APIupdateObjectUserMetadataRequest(repository, branch, path, updateObjectUserMetadata); } - private okhttp3.Call uploadObjectCall(String repository, String branch, String path, String ifNoneMatch, String storageClass, Boolean force, File content, final ApiCallback _callback) throws ApiException { + private okhttp3.Call uploadObjectCall(String repository, String branch, String path, String ifNoneMatch, String ifMatch, String storageClass, Boolean force, File content, final ApiCallback _callback) throws ApiException { String basePath = null; // Operation Servers String[] localBasePaths = new String[] { }; @@ -2180,6 +2180,10 @@ private okhttp3.Call uploadObjectCall(String repository, String branch, String p localVarHeaderParams.put("If-None-Match", localVarApiClient.parameterToString(ifNoneMatch)); } + if (ifMatch != null) { + localVarHeaderParams.put("If-Match", localVarApiClient.parameterToString(ifMatch)); + } + final String[] localVarAccepts = { "application/json" }; @@ -2202,7 +2206,7 @@ private okhttp3.Call uploadObjectCall(String repository, String branch, String p } @SuppressWarnings("rawtypes") - private okhttp3.Call uploadObjectValidateBeforeCall(String repository, String branch, String path, String ifNoneMatch, String storageClass, Boolean force, File content, final ApiCallback _callback) throws ApiException { + private okhttp3.Call uploadObjectValidateBeforeCall(String repository, String branch, String path, String ifNoneMatch, String ifMatch, String storageClass, Boolean force, File content, final ApiCallback _callback) throws ApiException { // verify the required parameter 'repository' is set if (repository == null) { throw new ApiException("Missing the required parameter 'repository' when calling uploadObject(Async)"); @@ -2218,20 +2222,20 @@ private okhttp3.Call uploadObjectValidateBeforeCall(String repository, String br throw new ApiException("Missing the required parameter 'path' when calling uploadObject(Async)"); } - return uploadObjectCall(repository, branch, path, ifNoneMatch, storageClass, force, content, _callback); + return uploadObjectCall(repository, branch, path, ifNoneMatch, ifMatch, storageClass, force, content, _callback); } - private ApiResponse uploadObjectWithHttpInfo(String repository, String branch, String path, String ifNoneMatch, String storageClass, Boolean force, File content) throws ApiException { - okhttp3.Call localVarCall = uploadObjectValidateBeforeCall(repository, branch, path, ifNoneMatch, storageClass, force, content, null); + private ApiResponse uploadObjectWithHttpInfo(String repository, String branch, String path, String ifNoneMatch, String ifMatch, String storageClass, Boolean force, File content) throws ApiException { + okhttp3.Call localVarCall = uploadObjectValidateBeforeCall(repository, branch, path, ifNoneMatch, ifMatch, storageClass, force, content, null); Type localVarReturnType = new TypeToken(){}.getType(); return localVarApiClient.execute(localVarCall, localVarReturnType); } - private okhttp3.Call uploadObjectAsync(String repository, String branch, String path, String ifNoneMatch, String storageClass, Boolean force, File content, final ApiCallback _callback) throws ApiException { + private okhttp3.Call uploadObjectAsync(String repository, String branch, String path, String ifNoneMatch, String ifMatch, String storageClass, Boolean force, File content, final ApiCallback _callback) throws ApiException { - okhttp3.Call localVarCall = uploadObjectValidateBeforeCall(repository, branch, path, ifNoneMatch, storageClass, force, content, _callback); + okhttp3.Call localVarCall = uploadObjectValidateBeforeCall(repository, branch, path, ifNoneMatch, ifMatch, storageClass, force, content, _callback); Type localVarReturnType = new TypeToken(){}.getType(); localVarApiClient.executeAsync(localVarCall, localVarReturnType, _callback); return localVarCall; @@ -2242,6 +2246,7 @@ public class APIuploadObjectRequest { private final String branch; private final String path; private String ifNoneMatch; + private String ifMatch; private String storageClass; private Boolean force; private File content; @@ -2262,6 +2267,16 @@ public APIuploadObjectRequest ifNoneMatch(String ifNoneMatch) { return this; } + /** + * Set ifMatch + * @param ifMatch Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. (optional) + * @return APIuploadObjectRequest + */ + public APIuploadObjectRequest ifMatch(String ifMatch) { + this.ifMatch = ifMatch; + return this; + } + /** * Set storageClass * @param storageClass Deprecated, this capability will not be supported in future releases. (optional) @@ -2311,7 +2326,7 @@ public APIuploadObjectRequest content(File content) { */ public okhttp3.Call buildCall(final ApiCallback _callback) throws ApiException { - return uploadObjectCall(repository, branch, path, ifNoneMatch, storageClass, force, content, _callback); + return uploadObjectCall(repository, branch, path, ifNoneMatch, ifMatch, storageClass, force, content, _callback); } /** @@ -2332,7 +2347,7 @@ public okhttp3.Call buildCall(final ApiCallback _callback) throws ApiException { */ public ObjectStats execute() throws ApiException { - ApiResponse localVarResp = uploadObjectWithHttpInfo(repository, branch, path, ifNoneMatch, storageClass, force, content); + ApiResponse localVarResp = uploadObjectWithHttpInfo(repository, branch, path, ifNoneMatch, ifMatch, storageClass, force, content); return localVarResp.getData(); } @@ -2354,7 +2369,7 @@ public ObjectStats execute() throws ApiException { */ public ApiResponse executeWithHttpInfo() throws ApiException { - return uploadObjectWithHttpInfo(repository, branch, path, ifNoneMatch, storageClass, force, content); + return uploadObjectWithHttpInfo(repository, branch, path, ifNoneMatch, ifMatch, storageClass, force, content); } /** @@ -2376,7 +2391,7 @@ public ApiResponse executeWithHttpInfo() throws ApiException { */ public okhttp3.Call executeAsync(final ApiCallback _callback) throws ApiException { - return uploadObjectAsync(repository, branch, path, ifNoneMatch, storageClass, force, content, _callback); + return uploadObjectAsync(repository, branch, path, ifNoneMatch, ifMatch, storageClass, force, content, _callback); } } diff --git a/clients/java/src/test/java/io/lakefs/clients/sdk/ObjectsApiTest.java b/clients/java/src/test/java/io/lakefs/clients/sdk/ObjectsApiTest.java index d12b473ac78..70c1bf4ea4b 100644 --- a/clients/java/src/test/java/io/lakefs/clients/sdk/ObjectsApiTest.java +++ b/clients/java/src/test/java/io/lakefs/clients/sdk/ObjectsApiTest.java @@ -216,11 +216,13 @@ public void uploadObjectTest() throws ApiException { String branch = null; String path = null; String ifNoneMatch = null; + String ifMatch = null; String storageClass = null; Boolean force = null; File content = null; ObjectStats response = api.uploadObject(repository, branch, path) .ifNoneMatch(ifNoneMatch) + .ifMatch(ifMatch) .storageClass(storageClass) .force(force) .content(content) diff --git a/clients/python/docs/ObjectsApi.md b/clients/python/docs/ObjectsApi.md index 02034e34fd8..ae754e0f2c0 100644 --- a/clients/python/docs/ObjectsApi.md +++ b/clients/python/docs/ObjectsApi.md @@ -1087,7 +1087,7 @@ void (empty response body) [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) # **upload_object** -> ObjectStats upload_object(repository, branch, path, if_none_match=if_none_match, storage_class=storage_class, force=force, content=content) +> ObjectStats upload_object(repository, branch, path, if_none_match=if_none_match, if_match=if_match, storage_class=storage_class, force=force, content=content) @@ -1155,12 +1155,13 @@ with lakefs_sdk.ApiClient(configuration) as api_client: branch = 'branch_example' # str | path = 'path_example' # str | relative to the branch if_none_match = '*' # str | Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported. (optional) + if_match = '2e9ec317e197e02e4264d128c2e7e681' # str | Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. (optional) storage_class = 'storage_class_example' # str | Deprecated, this capability will not be supported in future releases. (optional) force = False # bool | (optional) (default to False) content = None # bytearray | Only a single file per upload which must be named \\\"content\\\". (optional) try: - api_response = api_instance.upload_object(repository, branch, path, if_none_match=if_none_match, storage_class=storage_class, force=force, content=content) + api_response = api_instance.upload_object(repository, branch, path, if_none_match=if_none_match, if_match=if_match, storage_class=storage_class, force=force, content=content) print("The response of ObjectsApi->upload_object:\n") pprint(api_response) except Exception as e: @@ -1178,6 +1179,7 @@ Name | Type | Description | Notes **branch** | **str**| | **path** | **str**| relative to the branch | **if_none_match** | **str**| Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported. | [optional] + **if_match** | **str**| Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. | [optional] **storage_class** | **str**| Deprecated, this capability will not be supported in future releases. | [optional] **force** | **bool**| | [optional] [default to False] **content** | **bytearray**| Only a single file per upload which must be named \\\"content\\\". | [optional] diff --git a/clients/python/lakefs_sdk/api/objects_api.py b/clients/python/lakefs_sdk/api/objects_api.py index 6f9c7f35953..36dc06f8a08 100644 --- a/clients/python/lakefs_sdk/api/objects_api.py +++ b/clients/python/lakefs_sdk/api/objects_api.py @@ -1621,13 +1621,13 @@ def update_object_user_metadata_with_http_info(self, repository : StrictStr, bra _request_auth=_params.get('_request_auth')) @validate_arguments - def upload_object(self, repository : StrictStr, branch : StrictStr, path : Annotated[StrictStr, Field(..., description="relative to the branch")], if_none_match : Annotated[Optional[StrictStr], Field(description="Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported.")] = None, storage_class : Annotated[Optional[StrictStr], Field(description="Deprecated, this capability will not be supported in future releases.")] = None, force : Optional[StrictBool] = None, content : Annotated[Optional[Union[StrictBytes, StrictStr]], Field(description="Only a single file per upload which must be named \\\"content\\\".")] = None, **kwargs) -> ObjectStats: # noqa: E501 + def upload_object(self, repository : StrictStr, branch : StrictStr, path : Annotated[StrictStr, Field(..., description="relative to the branch")], if_none_match : Annotated[Optional[StrictStr], Field(description="Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported.")] = None, if_match : Annotated[Optional[StrictStr], Field(description="Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value.")] = None, storage_class : Annotated[Optional[StrictStr], Field(description="Deprecated, this capability will not be supported in future releases.")] = None, force : Optional[StrictBool] = None, content : Annotated[Optional[Union[StrictBytes, StrictStr]], Field(description="Only a single file per upload which must be named \\\"content\\\".")] = None, **kwargs) -> ObjectStats: # noqa: E501 """upload_object # noqa: E501 This method makes a synchronous HTTP request by default. To make an asynchronous HTTP request, please pass async_req=True - >>> thread = api.upload_object(repository, branch, path, if_none_match, storage_class, force, content, async_req=True) + >>> thread = api.upload_object(repository, branch, path, if_none_match, if_match, storage_class, force, content, async_req=True) >>> result = thread.get() :param repository: (required) @@ -1638,6 +1638,8 @@ def upload_object(self, repository : StrictStr, branch : StrictStr, path : Annot :type path: str :param if_none_match: Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported. :type if_none_match: str + :param if_match: Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. + :type if_match: str :param storage_class: Deprecated, this capability will not be supported in future releases. :type storage_class: str :param force: @@ -1659,16 +1661,16 @@ def upload_object(self, repository : StrictStr, branch : StrictStr, path : Annot if '_preload_content' in kwargs: message = "Error! Please call the upload_object_with_http_info method with `_preload_content` instead and obtain raw data from ApiResponse.raw_data" # noqa: E501 raise ValueError(message) - return self.upload_object_with_http_info(repository, branch, path, if_none_match, storage_class, force, content, **kwargs) # noqa: E501 + return self.upload_object_with_http_info(repository, branch, path, if_none_match, if_match, storage_class, force, content, **kwargs) # noqa: E501 @validate_arguments - def upload_object_with_http_info(self, repository : StrictStr, branch : StrictStr, path : Annotated[StrictStr, Field(..., description="relative to the branch")], if_none_match : Annotated[Optional[StrictStr], Field(description="Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported.")] = None, storage_class : Annotated[Optional[StrictStr], Field(description="Deprecated, this capability will not be supported in future releases.")] = None, force : Optional[StrictBool] = None, content : Annotated[Optional[Union[StrictBytes, StrictStr]], Field(description="Only a single file per upload which must be named \\\"content\\\".")] = None, **kwargs) -> ApiResponse: # noqa: E501 + def upload_object_with_http_info(self, repository : StrictStr, branch : StrictStr, path : Annotated[StrictStr, Field(..., description="relative to the branch")], if_none_match : Annotated[Optional[StrictStr], Field(description="Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported.")] = None, if_match : Annotated[Optional[StrictStr], Field(description="Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value.")] = None, storage_class : Annotated[Optional[StrictStr], Field(description="Deprecated, this capability will not be supported in future releases.")] = None, force : Optional[StrictBool] = None, content : Annotated[Optional[Union[StrictBytes, StrictStr]], Field(description="Only a single file per upload which must be named \\\"content\\\".")] = None, **kwargs) -> ApiResponse: # noqa: E501 """upload_object # noqa: E501 This method makes a synchronous HTTP request by default. To make an asynchronous HTTP request, please pass async_req=True - >>> thread = api.upload_object_with_http_info(repository, branch, path, if_none_match, storage_class, force, content, async_req=True) + >>> thread = api.upload_object_with_http_info(repository, branch, path, if_none_match, if_match, storage_class, force, content, async_req=True) >>> result = thread.get() :param repository: (required) @@ -1679,6 +1681,8 @@ def upload_object_with_http_info(self, repository : StrictStr, branch : StrictSt :type path: str :param if_none_match: Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported. :type if_none_match: str + :param if_match: Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. + :type if_match: str :param storage_class: Deprecated, this capability will not be supported in future releases. :type storage_class: str :param force: @@ -1717,6 +1721,7 @@ def upload_object_with_http_info(self, repository : StrictStr, branch : StrictSt 'branch', 'path', 'if_none_match', + 'if_match', 'storage_class', 'force', 'content' @@ -1770,6 +1775,9 @@ def upload_object_with_http_info(self, repository : StrictStr, branch : StrictSt if _params['if_none_match']: _header_params['If-None-Match'] = _params['if_none_match'] + if _params['if_match']: + _header_params['If-Match'] = _params['if_match'] + # process the form parameters _form_params = [] _files = {} diff --git a/clients/rust/docs/ObjectsApi.md b/clients/rust/docs/ObjectsApi.md index 05336e7031b..3b67e7313b2 100644 --- a/clients/rust/docs/ObjectsApi.md +++ b/clients/rust/docs/ObjectsApi.md @@ -306,7 +306,7 @@ Name | Type | Description | Required | Notes ## upload_object -> models::ObjectStats upload_object(repository, branch, path, if_none_match, storage_class, force, content) +> models::ObjectStats upload_object(repository, branch, path, if_none_match, if_match, storage_class, force, content) ### Parameters @@ -318,6 +318,7 @@ Name | Type | Description | Required | Notes **branch** | **String** | | [required] | **path** | **String** | relative to the branch | [required] | **if_none_match** | Option<**String**> | Set to \"*\" to atomically allow the upload only if the key has no object yet. Other values are not supported. | | +**if_match** | Option<**String**> | Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. | | **storage_class** | Option<**String**> | Deprecated, this capability will not be supported in future releases. | | **force** | Option<**bool**> | | |[default to false] **content** | Option<**std::path::PathBuf**> | Only a single file per upload which must be named \\\"content\\\". | | diff --git a/clients/rust/src/apis/objects_api.rs b/clients/rust/src/apis/objects_api.rs index 9a0738cc06e..108cc8c5aa3 100644 --- a/clients/rust/src/apis/objects_api.rs +++ b/clients/rust/src/apis/objects_api.rs @@ -497,7 +497,7 @@ pub async fn update_object_user_metadata(configuration: &configuration::Configur } } -pub async fn upload_object(configuration: &configuration::Configuration, repository: &str, branch: &str, path: &str, if_none_match: Option<&str>, storage_class: Option<&str>, force: Option, content: Option) -> Result> { +pub async fn upload_object(configuration: &configuration::Configuration, repository: &str, branch: &str, path: &str, if_none_match: Option<&str>, if_match: Option<&str>, storage_class: Option<&str>, force: Option, content: Option) -> Result> { let local_var_configuration = configuration; let local_var_client = &local_var_configuration.client; @@ -518,6 +518,9 @@ pub async fn upload_object(configuration: &configuration::Configuration, reposit if let Some(local_var_param_value) = if_none_match { local_var_req_builder = local_var_req_builder.header("If-None-Match", local_var_param_value.to_string()); } + if let Some(local_var_param_value) = if_match { + local_var_req_builder = local_var_req_builder.header("If-Match", local_var_param_value.to_string()); + } if let Some(ref local_var_auth_conf) = local_var_configuration.basic_auth { local_var_req_builder = local_var_req_builder.basic_auth(local_var_auth_conf.0.to_owned(), local_var_auth_conf.1.to_owned()); }; diff --git a/docs/src/assets/js/swagger.yml b/docs/src/assets/js/swagger.yml index fd6492ebcf0..33d042302f8 100644 --- a/docs/src/assets/js/swagger.yml +++ b/docs/src/assets/js/swagger.yml @@ -90,6 +90,15 @@ components: required: false schema: type: string + + IfMatch: + in: header + name: If-Match + description: Set to the object's ETag to atomically allow operations only if the object's current ETag matches the provided value. + example: "2e9ec317e197e02e4264d128c2e7e681" + required: false + schema: + type: string NoTombstone: in: query @@ -5114,6 +5123,7 @@ paths: parameters: - $ref: "#/components/parameters/IfNoneMatch" + - $ref: "#/components/parameters/IfMatch" - in: query name: storageClass description: Deprecated, this capability will not be supported in future releases. diff --git a/modules/api/factory/build.go b/modules/api/factory/build.go index 305c0f3d9cd..14954a9ef74 100644 --- a/modules/api/factory/build.go +++ b/modules/api/factory/build.go @@ -5,10 +5,12 @@ import ( "net/http" "github.com/go-chi/chi/v5" + "github.com/treeverse/lakefs/pkg/api/apigen" "github.com/treeverse/lakefs/pkg/auth" "github.com/treeverse/lakefs/pkg/authentication" "github.com/treeverse/lakefs/pkg/block" "github.com/treeverse/lakefs/pkg/config" + "github.com/treeverse/lakefs/pkg/graveler" "github.com/treeverse/lakefs/pkg/license" "github.com/treeverse/lakefs/pkg/logging" "github.com/treeverse/lakefs/pkg/stats" @@ -38,3 +40,9 @@ func RegisterServices(ctx context.Context, sd ServiceDependencies, router *chi.M func NotImplementedIcebergCatalogHandler(w http.ResponseWriter, r *http.Request) { http.Error(w, "Iceberg REST Catalog Not Implemented", http.StatusNotImplemented) } + +// BuildConditionFromParams creates a graveler.ConditionFunc from upload params. +// Returns nil if no precondition is specified in the params. +func BuildConditionFromParams(params apigen.UploadObjectParams) *graveler.ConditionFunc { + return nil +} diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 3bb83e2b82d..72f3cbef032 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -25,6 +25,7 @@ import ( "github.com/go-openapi/swag" "github.com/gorilla/sessions" authacl "github.com/treeverse/lakefs/contrib/auth/acl" + "github.com/treeverse/lakefs/modules/api/factory" "github.com/treeverse/lakefs/pkg/actions" "github.com/treeverse/lakefs/pkg/api/apigen" "github.com/treeverse/lakefs/pkg/api/apiutil" @@ -3436,6 +3437,7 @@ func (c *Controller) UploadObject(w http.ResponseWriter, r *http.Request, reposi // once before uploading the body to save resources and time, // and then graveler will check again when passed a SetOptions. allowOverwrite := true + if params.IfNoneMatch != nil { if swag.StringValue((*string)(params.IfNoneMatch)) != "*" { writeError(w, r, http.StatusBadRequest, "Unsupported value for If-None-Match - Only \"*\" is supported") @@ -3454,6 +3456,12 @@ func (c *Controller) UploadObject(w http.ResponseWriter, r *http.Request, reposi allowOverwrite = false } + var setOpts []graveler.SetOptionsFunc + // Handle If-Match precondition + if condition := factory.BuildConditionFromParams(params); condition != nil { + setOpts = append(setOpts, graveler.WithCondition(*condition)) + } + // read request body parse multipart for "content" and upload the data contentType := catalog.ContentTypeOrDefault(r.Header.Get("Content-Type")) mediaType, p, err := mime.ParseMediaType(contentType) @@ -3542,7 +3550,9 @@ func (c *Controller) UploadObject(w http.ResponseWriter, r *http.Request, reposi } entry := entryBuilder.Build() - err = c.Catalog.CreateEntry(ctx, repo.Name, branch, entry, graveler.WithIfAbsent(!allowOverwrite), graveler.WithForce(swag.BoolValue(params.Force))) + // Combine all set options + setOpts = append(setOpts, graveler.WithIfAbsent(!allowOverwrite), graveler.WithForce(swag.BoolValue(params.Force))) + err = c.Catalog.CreateEntry(ctx, repo.Name, branch, entry, setOpts...) if errors.Is(err, graveler.ErrPreconditionFailed) { writeError(w, r, http.StatusPreconditionFailed, "path already exists") return diff --git a/pkg/catalog/catalog.go b/pkg/catalog/catalog.go index b31670041c5..89822747881 100644 --- a/pkg/catalog/catalog.go +++ b/pkg/catalog/catalog.go @@ -1149,6 +1149,24 @@ func addressTypeToCatalog(t Entry_AddressType) AddressType { } } +// EntryCondition adapts an Entry-level condition function to a graveler.ConditionFunc. +// It converts graveler Values to Entries before applying the condition, enabling Entry-based +// validation logic (e.g., Object metadata checks) to work with graveler's conditional operations. +func EntryCondition(conditionFunc func(*Entry) error) graveler.ConditionFunc { + return func(currentValue *graveler.Value) error { + if currentValue == nil { + return conditionFunc(nil) + } + + currentEntry, err := ValueToEntry(currentValue) + if err != nil { + return err + } + + return conditionFunc(currentEntry) + } +} + func (c *Catalog) CreateEntry(ctx context.Context, repositoryID string, branch string, entry DBEntry, opts ...graveler.SetOptionsFunc) error { branchID := graveler.BranchID(branch) ent := newEntryFromCatalogEntry(entry) diff --git a/pkg/catalog/catalog_test.go b/pkg/catalog/catalog_test.go index 40a69f59122..975ef324951 100644 --- a/pkg/catalog/catalog_test.go +++ b/pkg/catalog/catalog_test.go @@ -3,6 +3,7 @@ package catalog_test import ( "bytes" "context" + "errors" "fmt" "io" "net/url" @@ -942,3 +943,135 @@ func readPhysicalAddressesFromParquetObject(t *testing.T, repositoryID string, c } return records } + +func TestEntryCondition(t *testing.T) { + // Helper to create a graveler.Value from an Entry + createValueFromEntry := func(entry *catalog.Entry) *graveler.Value { + value, err := catalog.EntryToValue(entry) + require.NoError(t, err) + return value + } + + tests := []struct { + name string + entry *catalog.Entry + conditionFunc func(*catalog.Entry) error + expectedErr error + }{ + { + name: "condition passes with valid entry", + entry: &catalog.Entry{ + Address: "s3://bucket/key", + Size: 100, + ETag: "abc123", + }, + conditionFunc: func(e *catalog.Entry) error { + if e != nil && e.Size == 100 { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedErr: nil, + }, + { + name: "condition fails validation", + entry: &catalog.Entry{ + Address: "s3://bucket/key", + Size: 100, + ETag: "abc123", + }, + conditionFunc: func(e *catalog.Entry) error { + if e != nil && e.Size == 200 { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedErr: graveler.ErrPreconditionFailed, + }, + { + name: "condition with nil entry - expect nil", + entry: nil, + conditionFunc: func(e *catalog.Entry) error { + if e == nil { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedErr: nil, + }, + { + name: "condition with nil entry - expect non-nil", + entry: nil, + conditionFunc: func(e *catalog.Entry) error { + if e != nil { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedErr: graveler.ErrPreconditionFailed, + }, + { + name: "condition validates etag", + entry: &catalog.Entry{ + Address: "s3://bucket/key", + Size: 100, + ETag: "expected-etag", + }, + conditionFunc: func(e *catalog.Entry) error { + if e == nil { + return graveler.ErrPreconditionFailed + } + if e.ETag != "expected-etag" { + return graveler.ErrPreconditionFailed + } + return nil + }, + expectedErr: nil, + }, + { + name: "condition validates etag mismatch", + entry: &catalog.Entry{ + Address: "s3://bucket/key", + Size: 100, + ETag: "actual-etag", + }, + conditionFunc: func(e *catalog.Entry) error { + if e == nil { + return graveler.ErrPreconditionFailed + } + if e.ETag != "expected-etag" { + return graveler.ErrPreconditionFailed + } + return nil + }, + expectedErr: graveler.ErrPreconditionFailed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create the ConditionFunc using WithEntryCondition + conditionFunc := catalog.EntryCondition(tt.conditionFunc) + + // Apply it to SetOptions using graveler.WithCondition + opts := &graveler.SetOptions{} + graveler.WithCondition(conditionFunc)(opts) + + // Verify condition was set + require.NotNil(t, opts.Condition) + + // Convert entry to value (or nil) + var currentValue *graveler.Value + if tt.entry != nil { + currentValue = createValueFromEntry(tt.entry) + } + + // Execute the condition + err := opts.Condition(currentValue) + // Verify the result + if !errors.Is(err, tt.expectedErr) { + t.Errorf("WithEntryCondition() error = %v, expectedErr %v", err, tt.expectedErr) + } + }) + } +} diff --git a/pkg/graveler/graveler.go b/pkg/graveler/graveler.go index 93e7e81a52a..8ccc2496bc7 100644 --- a/pkg/graveler/graveler.go +++ b/pkg/graveler/graveler.go @@ -198,6 +198,7 @@ func WithStageOnly(v bool) GetOptionsFunc { } } +type ConditionFunc func(currentValue *Value) error type SetOptions struct { IfAbsent bool // MaxTries set number of times we try to perform the operation before we fail with BranchWriteMaxTries. @@ -214,6 +215,10 @@ type SetOptions struct { SquashMerge bool // NoTombstone will try to remove entry without setting a tombstone in KV NoTombstone bool + // Condition is a function that validates the current value before performing the Set. + // If the condition returns an error, the Set operation fails with that error. + // If the condition succeeds, the Set is performed using SetIf with the current value. + Condition ConditionFunc } type SetOptionsFunc func(opts *SetOptions) @@ -262,6 +267,12 @@ func WithNoTombstone(v bool) SetOptionsFunc { } } +func WithCondition(condition ConditionFunc) SetOptionsFunc { + return func(opts *SetOptions) { + opts.Condition = condition + } +} + // ListOptions controls list request defaults type ListOptions struct { // Shows entities marked as hidden @@ -1843,25 +1854,46 @@ func (g *Graveler) Set(ctx context.Context, repository *RepositoryRecord, branch log := g.log(ctx).WithFields(logging.Fields{"key": key, "operation": "set"}) err = g.safeBranchWrite(ctx, log, repository, branchID, safeBranchWriteOptions{MaxTries: options.MaxTries}, func(branch *Branch) error { - if !options.IfAbsent { + // Check if Condition is provided + hasCondition := options.Condition != nil + + if !options.IfAbsent && !hasCondition { return g.StagingManager.Set(ctx, branch.StagingToken, key, &value, false) } // verify the key not found - _, err := g.Get(ctx, repository, Ref(branchID), key) - if err == nil { // Entry found, return precondition failed + curValue, err := g.Get(ctx, repository, Ref(branchID), key) + if err == nil && options.IfAbsent { // Entry found, return precondition failed return ErrPreconditionFailed } - if !errors.Is(err, ErrNotFound) { + if err != nil && !errors.Is(err, ErrNotFound) { return err } - // update stage with new value only if key not found or tombstone - return g.StagingManager.Update(ctx, branch.StagingToken, key, func(currentValue *Value) (*Value, error) { - if currentValue == nil || currentValue.Identity == nil { - return &value, nil + // update stage with new value respecting the ifAbsent and condition + return g.StagingManager.Update(ctx, branch.StagingToken, key, func(stagingValue *Value) (*Value, error) { + var valueToCheck *Value + noValue := stagingValue == nil || stagingValue.Identity == nil + + switch { + case noValue: + // Nothing in staging, check against committed value + valueToCheck = curValue + case options.IfAbsent: + // Value exists in staging but IfAbsent is set + return nil, ErrSkipValueUpdate + default: + // Value exists in staging, check against it + valueToCheck = stagingValue + } + + // Run condition if provided + if hasCondition { + if err := options.Condition(valueToCheck); err != nil { + return nil, err + } } - return nil, ErrSkipValueUpdate + return &value, nil }) }, "set") return err diff --git a/pkg/graveler/graveler_test.go b/pkg/graveler/graveler_test.go index dfe16b44e88..39c15a418be 100644 --- a/pkg/graveler/graveler_test.go +++ b/pkg/graveler/graveler_test.go @@ -563,6 +563,137 @@ func TestGravelerSet_Advanced(t *testing.T) { }) } +func TestGraveler_SetWithCondition(t *testing.T) { + ctx := context.Background() + newSetVal := &graveler.ValueRecord{Key: []byte("key"), Value: &graveler.Value{Data: []byte("newValue"), Identity: []byte("newIdentity")}} + existingVal := &graveler.Value{Identity: []byte("existingIdentity"), Data: []byte("existingValue")} + + tests := []struct { + name string + existingValue *graveler.Value + existingInCommitted bool // If true, put value in committed instead of staging + condition func(*graveler.Value) error + expectedErr error + expectedValueResult *graveler.ValueRecord + }{ + { + name: "condition passes with existing value in staging", + existingValue: existingVal, + condition: func(v *graveler.Value) error { + if v != nil && string(v.Identity) == "existingIdentity" { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedValueResult: newSetVal, + }, + { + name: "condition fails with existing value in staging", + existingValue: existingVal, + condition: func(v *graveler.Value) error { + if v != nil && string(v.Identity) == "wrongIdentity" { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedErr: graveler.ErrPreconditionFailed, + }, + { + name: "condition passes with nil value", + existingValue: nil, + condition: func(v *graveler.Value) error { + if v == nil { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedValueResult: newSetVal, + }, + { + name: "condition fails with nil value", + existingValue: nil, + condition: func(v *graveler.Value) error { + if v != nil { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedErr: graveler.ErrPreconditionFailed, + }, + { + name: "condition passes with existing value in committed", + existingValue: existingVal, + existingInCommitted: true, + condition: func(v *graveler.Value) error { + if v != nil && string(v.Identity) == "existingIdentity" { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedValueResult: newSetVal, + }, + { + name: "condition fails with existing value in committed", + existingValue: existingVal, + existingInCommitted: true, + condition: func(v *graveler.Value) error { + if v != nil && string(v.Identity) == "wrongIdentity" { + return nil + } + return graveler.ErrPreconditionFailed + }, + expectedErr: graveler.ErrPreconditionFailed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + committedMgr := &testutil.CommittedFake{} + stagingMgr := &testutil.StagingFake{} + + if tt.existingValue != nil { + if tt.existingInCommitted { + // Populate committed storage + committedMgr.ValuesByKey = map[string]*graveler.Value{ + string(newSetVal.Key): tt.existingValue, + } + } else { + // Populate staging storage + stagingMgr.Values = map[string]map[string]*graveler.Value{ + "st": {string(newSetVal.Key): tt.existingValue}, + } + } + } + // RefManager mock setup - returns a random commit for the branch just to get to the committed manager and get the existing value + randomCommit := &graveler.Commit{} + refMgr := &testutil.RefsFake{ + Branch: &graveler.Branch{ + CommitID: "commit1", + StagingToken: "st", + }, + CommitID: "commit1", + Commits: map[graveler.CommitID]*graveler.Commit{"commit1": randomCommit}, + } + + store := newGraveler(t, committedMgr, stagingMgr, refMgr, nil, testutil.NewProtectedBranchesManagerFake()) + + // Create SetOptionsFunc with condition + withCondition := func(opts *graveler.SetOptions) { + opts.Condition = tt.condition + } + + err := store.Set(ctx, repository, "branch-1", newSetVal.Key, *newSetVal.Value, withCondition) + if !errors.Is(err, tt.expectedErr) { + t.Fatalf("Set() with condition - error: %v, expected: %v", err, tt.expectedErr) + } + + if err == nil { + require.Equal(t, tt.expectedValueResult, stagingMgr.LastSetValueRecord) + } + }) + } +} + func TestGravelerGet_Advanced(t *testing.T) { tests := []struct { name string