You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/10/11 08:03:55 UTC

[GitHub] [ozone] sadanand48 commented on a diff in pull request #3774: HDDS-7253. Fix exception when '/' in key name

sadanand48 commented on code in PR #3774:
URL: https://github.com/apache/ozone/pull/3774#discussion_r991928728


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -1221,28 +1223,46 @@ private OzoneFileStatus getOzoneFileStatus(OmKeyArgs args,
       // Check if the key is a file.
       String fileKeyBytes = metadataManager.getOzoneKey(
               volumeName, bucketName, keyName);
-      fileKeyInfo = metadataManager
-          .getKeyTable(getBucketLayout(metadataManager, volumeName, bucketName))
-          .get(fileKeyBytes);
+      BucketLayout layout =
+          getBucketLayout(metadataManager, volumeName, bucketName);
+      fileKeyInfo = metadataManager.getKeyTable(layout).get(fileKeyBytes);
+      String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
 
       // Check if the key is a directory.
       if (fileKeyInfo == null) {
-        String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
         String dirKeyBytes = metadataManager.getOzoneKey(
                 volumeName, bucketName, dirKey);
-        OmKeyInfo dirKeyInfo = metadataManager.getKeyTable(
-                getBucketLayout(metadataManager, volumeName, bucketName))
-            .get(dirKeyBytes);
-        if (dirKeyInfo != null) {
-          return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true);
+        dirKeyInfo = metadataManager.getKeyTable(layout).get(dirKeyBytes);
+
+        // Check if the key is a prefix.
+        // Some keys may contain '/' Ozone will treat '/' as directory separator
+        // such as : key name is 'a/b/c', 'a' and 'b' may not really exist,
+        // but Ozone treats 'a' and 'b' as a directory.
+        // we need create a fake directory 'a' or 'a/b'
+        if (dirKeyInfo == null) {
+          Table.KeyValue<String, OmKeyInfo> keyValue =
+              metadataManager.getKeyTable(layout).iterator().seek(fileKeyBytes);
+          if (keyValue != null) {
+            Path fullPath = Paths.get(keyValue.getValue().getKeyName());
+            Path subPath = Paths.get(dirKey);
+            OmKeyInfo omKeyInfo = keyValue.getValue();
+            if (fullPath.startsWith(subPath)) {
+              // create fake directory
+              fakeDirKeyInfo = createDirectoryKey(
+                  omKeyInfo,
+                  dirKey);
+            }
+          }
         }
       }
+    } catch (Exception e) {

Review Comment:
   Let's remove this catch as the method already throws IOException and the caller will have to catch the exception.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java:
##########
@@ -1273,6 +1273,67 @@ public void testListStatus() throws IOException {
     }
   }
 
+  @Test
+  public void testGetFileStatus() throws IOException {
+    // create a key
+    String keyName = RandomStringUtils.randomAlphabetic(5);
+    OmKeyArgs keyArgs = createBuilder()
+        .setKeyName(keyName)
+        .setLatestVersionLocation(true)
+        .build();
+    writeClient.createFile(keyArgs, false, false);
+    OpenKeySession keySession = writeClient.createFile(keyArgs, true, true);
+    keyArgs.setLocationInfoList(
+        keySession.getKeyInfo().getLatestVersionLocations().getLocationList());
+    writeClient.commitKey(keyArgs, keySession.getId());
+    OzoneFileStatus ozoneFileStatus = keyManager.getFileStatus(keyArgs);
+    Assert.assertEquals(keyName, ozoneFileStatus.getKeyInfo().getFileName());
+  }
+
+  @Test
+  public void testGetFileStatusWithFakeDir() throws IOException {
+    String parentDir = "dir1";
+    String key = "key1";
+    String fullKeyName = parentDir + OZONE_URI_DELIMITER + key;
+    OzoneFileStatus ozoneFileStatus;
+
+    // create a key "dir1/key1"
+    OmKeyArgs keyArgs = createBuilder().setKeyName(fullKeyName).build();
+    OpenKeySession keySession = writeClient.createFile(keyArgs, true, true);

Review Comment:
   ```suggestion
       OpenKeySession keySession = writeClient.openKey(keyArgs);
   ```
   createFile will create an intermediate dir instead openKey should be used here



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org