adoroszlai commented on code in PR #6273:
URL: https://github.com/apache/ozone/pull/6273#discussion_r1503829061
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -953,6 +959,27 @@ protected OmKeyInfo wrapUncommittedBlocksAsPseudoKey(
return pseudoKeyInfo;
}
+ /**
+ * Updates the metadata of an OmKeyInfo object with new metadata.
+ *
+ * @param dbKeyInfo The existing OmKeyInfo object whose metadata is to be
updated.
+ * @param newMetadata The new metadata map to update the existing metadata
with.
+ */
+ protected void updateMetadata(OmKeyInfo dbKeyInfo,
+ Map<String, String> newMetadata) {
+ if (dbKeyInfo == null || newMetadata == null || newMetadata.isEmpty()) {
+ return;
+ }
+ Map<String, String> existingMetadata = dbKeyInfo.getMetadata();
+ // Update existing metadata with new entries or values from newMetadata
+ for (Map.Entry<String, String> entry : newMetadata.entrySet()) {
+ String key = entry.getKey();
+ String value = entry.getValue();
+ // Update or add new metadata entry
+ existingMetadata.put(key, value);
+ }
+ }
+
Review Comment:
Now this new method is unused, please remove it.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java:
##########
@@ -464,6 +468,108 @@ 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
+ assertTrue(
+ createResponse.getOMResponse().getCreateKeyResponse().getKeyInfo()
+ .getMetadataList().isEmpty());
Review Comment:
Please use `assertThat`, which provides better message in case of failure:
```suggestion
assertThat(
createResponse.getOMResponse().getCreateKeyResponse().getKeyInfo()
.getMetadataList()).isEmpty();
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java:
##########
@@ -464,6 +468,108 @@ 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
+ assertTrue(
+ 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(),
+ "Metadata size mismatch.");
Review Comment:
```suggestion
assertEquals(expectedMetadata.size(), metadataList.size());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
