HDDS-10324. Metadata are not updated when keys are overwritten. (apac…

…he#6273)
jojochuang · Feb 28, 2024 · aa68aec · aa68aec
1 parent 1830fe2
commit aa68aec
Show file tree

Hide file tree

Showing 2 changed files with 149 additions and 8 deletions.
diff --git a/...zone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/...zone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
@@ -775,6 +775,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/...-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java b/...-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.RatisReplicationConfig;
@@ -41,14 +42,16 @@
 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.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,6 +69,7 @@
 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;
@@ -464,6 +468,107 @@ public void testValidateAndUpdateCacheWithInvalidPath(
     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<String, String> 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,
+        replicationConfig).build();
+    keyInfo.setMetadata(initialMetadata);
+    omMetadataManager.getKeyTable(initialOmKeyCreateRequest.getBucketLayout())
+        .put(getOzoneKey(), keyInfo);
+
+    Map<String, String> 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,
+        replicationConfig).build();
+    omMetadataManager.getKeyTable(createOmKeyCreateRequest.getBucketLayout())
+        .put(getOzoneKey(), keyInfo);
+
+    // Define new metadata for the overwrite operation
+    Map<String, String> 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<String, String> expectedMetadata) {
+    // Extract metadata from the response
+    List<KeyValue> metadataList =
+        response.getOMResponse().getCreateKeyResponse().getKeyInfo()
+            .getMetadataList();
+    assertEquals(expectedMetadata.size(), metadataList.size());
+    metadataList.forEach(kv -> {
+      String expectedValue = expectedMetadata.get(kv.getKey());
+      assertEquals(expectedValue, kv.getValue(),
+          "Metadata value mismatch for key: " + kv.getKey());
+    });
+  }
+
   /**
    * This method calls preExecute and verify the modified request.
    * @param originalOMRequest
@@ -543,25 +648,55 @@ 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<String, String> metadata) {
     KeyArgs.Builder keyArgs = KeyArgs.newBuilder()
-        .setVolumeName(volumeName).setBucketName(bucketName)
-        .setKeyName(keyName).setIsMultipartKey(isMultipartKey)
-        .setFactor(((RatisReplicationConfig) replicationConfig).getReplicationFactor())
+        .setVolumeName(volumeName)
+        .setBucketName(bucketName)
+        .setKeyName(keyName)
+        .setIsMultipartKey(isMultipartKey)
+        .setFactor(
+            ((RatisReplicationConfig) replicationConfig).getReplicationFactor())
         .setType(replicationConfig.getReplicationType())
         .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(