diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java index 800f982034d..c09d87af1d5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java @@ -729,6 +729,12 @@ protected OmKeyInfo prepareFileInfo( dbKeyInfo.setModificationTime(keyArgs.getModificationTime()); dbKeyInfo.setUpdateID(transactionLogIndex, isRatisEnabled); dbKeyInfo.setReplicationConfig(replicationConfig); + + // Construct a new metadata map from KeyArgs. + // Clear the old one when the key is overwritten. + dbKeyInfo.getMetadata().clear(); + dbKeyInfo.getMetadata().putAll(KeyValueUtil.getFromProtobuf( + keyArgs.getMetadataList())); return dbKeyInfo; } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java index afc60e2947b..953e457d199 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java @@ -25,10 +25,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; -import java.util.UUID; -import java.util.stream.Collectors; import java.util.Map; +import java.util.Collections; import java.util.HashMap; +import java.util.UUID; +import java.util.stream.Collectors; import org.apache.hadoop.hdds.client.ECReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; @@ -40,15 +41,17 @@ import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; + +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo; import org.apache.hadoop.ozone.om.lock.OzoneLockProvider; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.KeyValue; -import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; -import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos @@ -66,8 +69,10 @@ import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_INDICATOR; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS; import static org.apache.hadoop.ozone.om.request.OMRequestTestUtils.addVolumeAndBucketToDB; +import static org.apache.hadoop.ozone.om.request.OMRequestTestUtils.createOmKeyInfo; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.NOT_A_FILE; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.when; @@ -460,6 +465,107 @@ public void testValidateAndUpdateCacheWithInvalidPath( Assertions.assertNull(omKeyInfo); } + + @ParameterizedTest + @MethodSource("data") + public void testOverwritingExistingMetadata( + boolean setKeyPathLock, boolean setFileSystemPaths) throws Exception { + when(ozoneManager.getOzoneLockProvider()).thenReturn( + new OzoneLockProvider(setKeyPathLock, setFileSystemPaths)); + + addVolumeAndBucketToDB(volumeName, bucketName, omMetadataManager, + getBucketLayout()); + + Map initialMetadata = + Collections.singletonMap("initialKey", "initialValue"); + OMRequest initialRequest = + createKeyRequest(false, 0, keyName, initialMetadata); + OMKeyCreateRequest initialOmKeyCreateRequest = + new OMKeyCreateRequest(initialRequest, getBucketLayout()); + OMClientResponse initialResponse = + initialOmKeyCreateRequest.validateAndUpdateCache(ozoneManager, 100L); + verifyMetadataInResponse(initialResponse, initialMetadata); + + // We have to add the key to the key table, as validateAndUpdateCache only + // updates the cache and not the DB. + OmKeyInfo keyInfo = createOmKeyInfo(volumeName, bucketName, keyName, + replicationType, replicationFactor); + keyInfo.setMetadata(initialMetadata); + omMetadataManager.getKeyTable(initialOmKeyCreateRequest.getBucketLayout()) + .put(getOzoneKey(), keyInfo); + + Map updatedMetadata = + Collections.singletonMap("initialKey", "updatedValue"); + OMRequest updatedRequest = + createKeyRequest(false, 0, keyName, updatedMetadata); + OMKeyCreateRequest updatedOmKeyCreateRequest = + new OMKeyCreateRequest(updatedRequest, getBucketLayout()); + + OMClientResponse updatedResponse = + updatedOmKeyCreateRequest.validateAndUpdateCache(ozoneManager, 101L); + verifyMetadataInResponse(updatedResponse, updatedMetadata); + } + + @ParameterizedTest + @MethodSource("data") + public void testCreationWithoutMetadataFollowedByOverwriteWithMetadata( + boolean setKeyPathLock, boolean setFileSystemPaths) throws Exception { + when(ozoneManager.getOzoneLockProvider()).thenReturn( + new OzoneLockProvider(setKeyPathLock, setFileSystemPaths)); + addVolumeAndBucketToDB(volumeName, bucketName, omMetadataManager, + getBucketLayout()); + + // Create the key request without any initial metadata + OMRequest createRequestWithoutMetadata = createKeyRequest(false, 0, keyName, + null); // Passing 'null' for metadata + OMKeyCreateRequest createOmKeyCreateRequest = + new OMKeyCreateRequest(createRequestWithoutMetadata, getBucketLayout()); + + // Perform the create operation without any metadata + OMClientResponse createResponse = + createOmKeyCreateRequest.validateAndUpdateCache(ozoneManager, 100L); + // Verify that no metadata exists in the response + assertThat( + createResponse.getOMResponse().getCreateKeyResponse().getKeyInfo() + .getMetadataList()).isEmpty(); + + OmKeyInfo keyInfo = createOmKeyInfo(volumeName, bucketName, keyName, + replicationType, replicationFactor); + omMetadataManager.getKeyTable(createOmKeyCreateRequest.getBucketLayout()) + .put(getOzoneKey(), keyInfo); + + // Define new metadata for the overwrite operation + Map overwriteMetadata = new HashMap<>(); + overwriteMetadata.put("newKey", "newValue"); + + // Overwrite the previously created key with new metadata + OMRequest overwriteRequestWithMetadata = + createKeyRequest(false, 0, keyName, overwriteMetadata); + OMKeyCreateRequest overwriteOmKeyCreateRequest = + new OMKeyCreateRequest(overwriteRequestWithMetadata, getBucketLayout()); + + // Perform the overwrite operation and capture the response + OMClientResponse overwriteResponse = + overwriteOmKeyCreateRequest.validateAndUpdateCache(ozoneManager, 101L); + // Verify the new metadata is correctly applied in the response + verifyMetadataInResponse(overwriteResponse, overwriteMetadata); + } + + + private void verifyMetadataInResponse(OMClientResponse response, + Map expectedMetadata) { + // Extract metadata from the response + List metadataList = + response.getOMResponse().getCreateKeyResponse().getKeyInfo() + .getMetadataList(); + Assertions.assertEquals(expectedMetadata.size(), metadataList.size()); + metadataList.forEach(kv -> { + String expectedValue = expectedMetadata.get(kv.getKey()); + Assertions.assertEquals(expectedValue, kv.getValue(), + "Metadata value mismatch for key: " + kv.getKey()); + }); + } + /** * This method calls preExecute and verify the modified request. * @param originalOMRequest @@ -541,24 +647,51 @@ protected OMRequest createKeyRequest(boolean isMultipartKey, int partNumber) { private OMRequest createKeyRequest(boolean isMultipartKey, int partNumber, String keyName) { + return createKeyRequest(isMultipartKey, partNumber, keyName, null); + } + /** + * Create OMRequest which encapsulates a CreateKeyRequest, optionally + * with metadata. + * + * @param isMultipartKey Indicates if the key is part of a multipart upload. + * @param partNumber The part number for multipart uploads, ignored if + * isMultipartKey is false. + * @param keyName The name of the key to create or update. + * @param metadata Optional metadata for the key. Pass null or an empty + * map if no metadata is to be set. + * @return OMRequest configured with the provided parameters. + */ + protected OMRequest createKeyRequest(boolean isMultipartKey, int partNumber, + String keyName, + Map metadata) { KeyArgs.Builder keyArgs = KeyArgs.newBuilder() .setVolumeName(volumeName).setBucketName(bucketName) .setKeyName(keyName).setIsMultipartKey(isMultipartKey) .setFactor(replicationFactor).setType(replicationType) .setLatestVersionLocation(true); + // Configure for multipart upload, if applicable if (isMultipartKey) { keyArgs.setDataSize(dataSize).setMultipartNumber(partNumber); } + // Include metadata, if provided + if (metadata != null && !metadata.isEmpty()) { + metadata.forEach((key, value) -> keyArgs.addMetadata(KeyValue.newBuilder() + .setKey(key) + .setValue(value) + .build())); + } + OzoneManagerProtocolProtos.CreateKeyRequest createKeyRequest = CreateKeyRequest.newBuilder().setKeyArgs(keyArgs).build(); return OMRequest.newBuilder() .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey) .setClientId(UUID.randomUUID().toString()) - .setCreateKeyRequest(createKeyRequest).build(); + .setCreateKeyRequest(createKeyRequest) + .build(); } private OMRequest createKeyRequest(