aswinshakil commented on code in PR #5814:
URL: https://github.com/apache/ozone/pull/5814#discussion_r1430715915
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java:
##########
@@ -77,6 +78,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws
IOException {
OzoneManagerProtocolProtos.KeyArgs keyArgs = deleteKeyRequest.getKeyArgs();
+
OmUtils.verifyKeyNameWithSnapshotReservedWordForDeletion(deleteKeyRequest.getKeyArgs().getKeyName());
Review Comment:
Nit: We just use the keyPath in line 83 here.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequest.java:
##########
@@ -28,21 +29,37 @@
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
- .DeleteKeyRequest;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
- .OMRequest;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
- .KeyArgs;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* Tests OmKeyDelete request.
*/
public class TestOMKeyDeleteRequest extends TestOMKeyRequest {
- @Test
- public void testPreExecute() throws Exception {
- doPreExecute(createDeleteKeyRequest());
+ @ParameterizedTest
+ @ValueSource(strings = {"keyName", "a/b/keyName", "a/.snapshot/keyName",
"a.snapshot/b/keyName"})
+ public void testPreExecute(String testKeyName) throws Exception {
+ doPreExecute(createDeleteKeyRequest(testKeyName));
+ }
+
+ @ParameterizedTest
+ @CsvSource(value = {".snapshot,Cannot delete key with reserved name:
.snapshot",
+ ".snapshot/snapName,Cannot delete key under path reserved for snapshot:
.snapshot/",
+ ".snapshot/snapName/keyName,Cannot delete key under path reserved for
snapshot: .snapshot/"})
+ public void testPreExecuteFailure(String testKeyName,
+ String expectedExceptionMessage) {
+ OMKeyDeleteRequest deleteKeyRequest =
+ getOmKeyDeleteRequest(createDeleteKeyRequest(testKeyName));
+ OMException omException = Assertions.assertThrows(OMException.class,
+ () -> deleteKeyRequest.preExecute(ozoneManager));
+ System.out.println(omException.getMessage());
Review Comment:
This can be removed. I believe it was added for testing the exception
message.
--
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]
