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/12/05 07:44:25 UTC

[GitHub] [ozone] devmadhuu opened a new pull request, #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

devmadhuu opened a new pull request, #4042:
URL: https://github.com/apache/ozone/pull/4042

   Currently, the /namsepace/summary endpoint carries only count data. Since it is a summary endpoint at the path level, we could make it more useful with other information as well.
   
   Sample volume and bucket summary API responses
   
   For volumes, we can add fields in OzoneVolume:
   {
   	"path": "/s3v",
   	"type": "VOLUME",
   	"counts": {
   		"volumes": 0,
   		"buckets": 1,
   		"directories": 3,
   		"object-store-prefixes": 0,
   		"keys": 2
   	},
   	"dbinfo": {
   		"metadata": {},
   		"name": "s3v",
   		"admin": "hadoop",
   		"owner": "hadoop",
   		"quotaInBytes": -1,
   		"quotaInNamespace": -1,
   		"usedNamespace": 1,
   		"creationTime": "2021-07-27T18:21:40.251Z",
   		"modificationTime": "2021-07-27T18:21:40.251Z",
   		"acls": []
   	},
   	"status": "OK"
   }
   For buckets, we can add fields in OzoneBucket:
   {
   	"path": "/s3v/s3bucket",
   	"type": "BUCKET",
   	"counts": {
   		"volumes": 0,
   		"buckets": 0,
   		"directories": 3,
   		"object-store-prefixes": 0,
   		"keys": 20
   	},
   	"dbinfo": {
   		"metadata": {
   			"ozone.om.metadata.layout": "PREFIX",
   			"ozone.om.enable.filesystem.paths": "true"
   		},
   		"volumeName": "s3v",
   		"name": "s3bucket",
   		"storageType": "DISK",
   		"versioning": false,
   		"usedBytes": 536870912,
   		"usedNamespace": 2,
   		"creationTime": "2021-07-27T19:33:54.574Z",
   		"modificationTime": "2021-07-27T19:33:54.574Z",
   		"encryptionKeyName": null,
   		"sourceVolume": null,
   		"sourceBucket": null,
   		"quotaInBytes": -1,
   		"quotaInNamespace": -1
   	},
   	"status": "OK"
   }
   
   https://issues.apache.org/jira/browse/HDDS-5541
   
   Patch is tested with writing and executing Junit test cases inside "TestNSSummaryEndpointWithLegacy" and "TestNSSummaryEndpointWithFSO"
   


-- 
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


[GitHub] [ozone] kerneltime commented on pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #4042:
URL: https://github.com/apache/ozone/pull/4042#issuecomment-1337789611

   @swamirishi @aswinshakil can you please take a look?


-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042901399


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {

Review Comment:
   @hemantk-12 - I couldn't understand this.  Pls elaborate why unnecessary. 



-- 
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


[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
hemantk-12 commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042905660


##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestNSSummaryEndpointWithLegacy.java:
##########
@@ -389,15 +399,47 @@ public void testUtility() {
 
   @Test
   public void testGetBasicInfoRoot() throws Exception {
+    String username = "myuser";
+    OmPrefixInfo omPrefixInfo = getOmPrefixInfoForTest(ROOT_PATH,
+        IAccessAuthorizer.ACLIdentityType.USER,
+        username,
+        IAccessAuthorizer.ACLType.WRITE,
+        ACCESS);
+    omPrefixInfo.getMetadata().put("key", "value");
+    reconOMMetadataManager.getPrefixTable()
+        .put(OzoneConsts.OM_KEY_PREFIX, omPrefixInfo);
     // Test root basics
     Response rootResponse = nsSummaryEndpoint.getBasicInfo(ROOT_PATH);
     NamespaceSummaryResponse rootResponseObj =
         (NamespaceSummaryResponse) rootResponse.getEntity();
     Assert.assertEquals(EntityType.ROOT, rootResponseObj.getEntityType());
-    Assert.assertEquals(2, rootResponseObj.getNumVolume());
-    Assert.assertEquals(4, rootResponseObj.getNumBucket());
-    Assert.assertEquals(5, rootResponseObj.getNumTotalDir());
-    Assert.assertEquals(10, rootResponseObj.getNumTotalKey());
+    Assert.assertEquals(2, rootResponseObj.getCountStats().getNumVolume());
+    Assert.assertEquals(4, rootResponseObj.getCountStats().getNumBucket());
+    Assert.assertEquals(5, rootResponseObj.getCountStats().getNumTotalDir());
+    Assert.assertEquals(10, rootResponseObj.getCountStats().getNumTotalKey());

Review Comment:
   My bad, it has been fixed.



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042087314


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/BucketEntityHandler.java:
##########
@@ -49,18 +52,60 @@ public BucketEntityHandler(
   @Override
   public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
-    NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.BUCKET);
+
     String[] names = getNames();
     assert (names.length == 2);
     long bucketObjectId = getBucketHandler().getBucketObjectId(names);
-    namespaceSummaryResponse
-      .setNumTotalDir(getTotalDirCount(bucketObjectId));
-    namespaceSummaryResponse.setNumTotalKey(getTotalKeyCount(bucketObjectId));
+
+    CountStats countStats = new CountStats(
+        -1, -1,
+        getTotalDirCount(bucketObjectId), getTotalKeyCount(bucketObjectId));
+    NamespaceSummaryResponse namespaceSummaryResponse =
+        NamespaceSummaryResponse.newBuilder()
+            .setEntityType(EntityType.BUCKET)
+            .setCountStats(countStats)
+            .setObjectDBInfo(getBucketObjDbInfo(names))
+            .setStatus(ResponseStatus.OK)
+            .build();
 
     return namespaceSummaryResponse;
   }
 
+  private BucketObjectDBInfo getBucketObjDbInfo(String[] names)
+      throws IOException {
+    String volName = names[0];
+    String bucketName = names[1];
+    String bucketKey = getOmMetadataManager().
+        getBucketKey(volName, bucketName);
+    OmBucketInfo omBucketInfo =
+        getOmMetadataManager().getBucketTable().getSkipCache(bucketKey);
+    BucketObjectDBInfo bucketObjectDBInfo = new BucketObjectDBInfo();

Review Comment:
   Thanks @hemantk-12 for review. Changes done. Pls re-review. 



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/DirectoryEntityHandler.java:
##########
@@ -55,15 +58,34 @@ public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
     // path should exist so we don't need any extra verification/null check
     long dirObjectId = getBucketHandler().getDirObjectId(getNames());
+    CountStats countStats = new CountStats(
+        -1, -1,
+        getTotalDirCount(dirObjectId), getTotalKeyCount(dirObjectId));
     NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.DIRECTORY);
-    namespaceSummaryResponse
-        .setNumTotalDir(getTotalDirCount(dirObjectId));
-    namespaceSummaryResponse.setNumTotalKey(getTotalKeyCount(dirObjectId));
+        NamespaceSummaryResponse.newBuilder()
+            .setEntityType(EntityType.DIRECTORY)
+            .setCountStats(countStats)
+            .setObjectDBInfo(getDirectoryObjDbInfo(getNames()))
+            .setStatus(ResponseStatus.OK)
+            .build();
 
     return namespaceSummaryResponse;
   }
 
+  private ObjectDBInfo getDirectoryObjDbInfo(String[] names)
+      throws IOException {
+    OmDirectoryInfo dirInfo = getBucketHandler().getDirInfo(names);
+    ObjectDBInfo objectDBInfo = new ObjectDBInfo();

Review Comment:
   Thanks @hemantk-12 for review. Changes done. Pls re-review. 



-- 
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


[GitHub] [ozone] siddhantsangwan merged pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
siddhantsangwan merged PR #4042:
URL: https://github.com/apache/ozone/pull/4042


-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042994385


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {

Review Comment:
   @hemantk-12  - As discussed, I have implemented without builder and with plain constructor. Pls re-review.



-- 
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


[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
hemantk-12 commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1041619243


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/recon/TestReconWithOzoneManagerFSO.java:
##########
@@ -128,8 +128,8 @@ public void testNamespaceSummaryAPI() throws Exception {
     NamespaceSummaryResponse entity =
             (NamespaceSummaryResponse) basicInfo.getEntity();
     Assert.assertSame(entity.getEntityType(), EntityType.DIRECTORY);
-    Assert.assertEquals(1, entity.getNumTotalKey());
-    Assert.assertEquals(0, entity.getNumTotalDir());
+    Assert.assertEquals(1, entity.getCountStats().getNumTotalKey());

Review Comment:
   Why aren't we asserting `getNumVolume()` and `getNumBucket()` here?



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/DirectoryEntityHandler.java:
##########
@@ -55,15 +58,34 @@ public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
     // path should exist so we don't need any extra verification/null check
     long dirObjectId = getBucketHandler().getDirObjectId(getNames());
+    CountStats countStats = new CountStats(
+        -1, -1,
+        getTotalDirCount(dirObjectId), getTotalKeyCount(dirObjectId));
     NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.DIRECTORY);
-    namespaceSummaryResponse
-        .setNumTotalDir(getTotalDirCount(dirObjectId));
-    namespaceSummaryResponse.setNumTotalKey(getTotalKeyCount(dirObjectId));
+        NamespaceSummaryResponse.newBuilder()
+            .setEntityType(EntityType.DIRECTORY)
+            .setCountStats(countStats)
+            .setObjectDBInfo(getDirectoryObjDbInfo(getNames()))
+            .setStatus(ResponseStatus.OK)
+            .build();
 
     return namespaceSummaryResponse;
   }
 
+  private ObjectDBInfo getDirectoryObjDbInfo(String[] names)
+      throws IOException {
+    OmDirectoryInfo dirInfo = getBucketHandler().getDirInfo(names);
+    ObjectDBInfo objectDBInfo = new ObjectDBInfo();

Review Comment:
   Same as `BucketObjectDBInfo`, create constructor or builder which takes `OmDirectoryInfo` and create `ObjectDBInfo` object.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/VolumeEntityHandler.java:
##########
@@ -63,12 +64,41 @@ public NamespaceSummaryResponse getSummaryResponse()
       totalKey += getTotalKeyCount(bucketObjectId);
     }
 
-    namespaceSummaryResponse.setNumTotalDir(totalDir);
-    namespaceSummaryResponse.setNumTotalKey(totalKey);
+    CountStats countStats = new CountStats(
+        -1, buckets.size(), totalDir, totalKey);
+
+    NamespaceSummaryResponse namespaceSummaryResponse =
+        NamespaceSummaryResponse.newBuilder()
+            .setEntityType(EntityType.VOLUME)
+            .setCountStats(countStats)
+            .setObjectDBInfo(getVolumeObjDbInfo(names))
+            .setStatus(ResponseStatus.OK)
+            .build();
 
     return namespaceSummaryResponse;
   }
 
+  private VolumeObjectDBInfo getVolumeObjDbInfo(String[] names)
+      throws IOException {
+    String dbVolumeKey = getOmMetadataManager().getVolumeKey(names[0]);
+    OmVolumeArgs volumeArgs =
+        getOmMetadataManager().getVolumeTable().getSkipCache(dbVolumeKey);
+    VolumeObjectDBInfo objectDBInfo = new VolumeObjectDBInfo();

Review Comment:
   
   Same as `BucketObjectDBInfo`, create constructor or builder which takes `OmVolumeArgs` and create `VolumeObjectDBInfo` object.
   



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/BucketEntityHandler.java:
##########
@@ -49,18 +52,60 @@ public BucketEntityHandler(
   @Override
   public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
-    NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.BUCKET);
+
     String[] names = getNames();
     assert (names.length == 2);
     long bucketObjectId = getBucketHandler().getBucketObjectId(names);
-    namespaceSummaryResponse
-      .setNumTotalDir(getTotalDirCount(bucketObjectId));
-    namespaceSummaryResponse.setNumTotalKey(getTotalKeyCount(bucketObjectId));
+
+    CountStats countStats = new CountStats(
+        -1, -1,
+        getTotalDirCount(bucketObjectId), getTotalKeyCount(bucketObjectId));
+    NamespaceSummaryResponse namespaceSummaryResponse =
+        NamespaceSummaryResponse.newBuilder()
+            .setEntityType(EntityType.BUCKET)
+            .setCountStats(countStats)
+            .setObjectDBInfo(getBucketObjDbInfo(names))
+            .setStatus(ResponseStatus.OK)
+            .build();
 
     return namespaceSummaryResponse;
   }
 
+  private BucketObjectDBInfo getBucketObjDbInfo(String[] names)
+      throws IOException {
+    String volName = names[0];
+    String bucketName = names[1];
+    String bucketKey = getOmMetadataManager().
+        getBucketKey(volName, bucketName);
+    OmBucketInfo omBucketInfo =
+        getOmMetadataManager().getBucketTable().getSkipCache(bucketKey);
+    BucketObjectDBInfo bucketObjectDBInfo = new BucketObjectDBInfo();

Review Comment:
   Create a constructor or a builder which takes `OmBucketInfo` in `BucketObjectDBInfo`. Something like this. This way encapsulation will be preserved.
   
   ```
     public BucketObjectDBInfo(OmBucketInfo omBucketInfo) {
       this.metadata = omBucketInfo.getMetadata();
       this.volumeName = omBucketInfo.getVolumeName();
       this.name = omBucketInfo.getBucketName();
       this.quotaInBytes = omBucketInfo.getQuotaInBytes();
       this.quotaInNamespace = omBucketInfo.getQuotaInNamespace();
       this.usedNamespace = omBucketInfo.getUsedNamespace();
       this.creationTime = omBucketInfo.getCreationTime();
       this.modificationTime = omBucketInfo.getModificationTime();
       this.acls = omBucketInfo.getAcls();
       this.sourceBucket = omBucketInfo.getSourceBucket();
       this.sourceVolume = omBucketInfo.getSourceVolume();
       this.encryptionKeyInfo = omBucketInfo.getEncryptionKeyInfo();
       this.versioningEnabled = omBucketInfo.getIsVersionEnabled();
       this.storageType = omBucketInfo.getStorageType();
       this.defaultReplicationConfig = omBucketInfo.getDefaultReplicationConfig();
       this.bucketLayout = omBucketInfo.getBucketLayout();
       this.owner = omBucketInfo.getOwner();
     }
   ```
   
   Builder would be preferred tho.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  /** volume name from om db. */
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  /** storage type for the object. */
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  /** is versioning enabled for the object. */
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  /** used bytes for the object. */
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  /**
+   * Bucket encryption key info if encryption is enabled.
+   */
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  /**
+   * Optional default replication for bucket.
+   */
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  /** source volume. */

Review Comment:
   Java doc comments, like this, don't provide any information as such. I would recommend to remove obvious comments like this everywhere.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/KeyEntityHandler.java:
##########
@@ -46,10 +49,39 @@ public KeyEntityHandler(
   @Override
   public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
-    NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.KEY);
+    CountStats countStats = new CountStats(
+        -1, -1,
+        -1, 0);
+    return NamespaceSummaryResponse.newBuilder()
+        .setEntityType(EntityType.KEY)
+        .setCountStats(countStats)
+        .setObjectDBInfo(getKeyDbObjectInfo(getNames()))
+        .setStatus(ResponseStatus.OK)
+        .build();
 
-    return namespaceSummaryResponse;
+  }
+
+  private ObjectDBInfo getKeyDbObjectInfo(String[] names)
+      throws IOException {
+    OmKeyInfo keyInfo = getBucketHandler().getKeyInfo(names);
+    KeyObjectDBInfo keyObjectDBInfo = new KeyObjectDBInfo();

Review Comment:
   Same as `BucketObjectDBInfo`, create constructor or builder which takes `OmKeyInfo` and create `KeyObjectDBInfo` object.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/RootEntityHandler.java:
##########
@@ -48,26 +53,44 @@ public RootEntityHandler(
   @Override
   public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
-    NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.ROOT);
+
     List<OmVolumeArgs> volumes = listVolumes();
-    namespaceSummaryResponse.setNumVolume(volumes.size());
     List<OmBucketInfo> allBuckets = listBucketsUnderVolume(null);
-    namespaceSummaryResponse.setNumBucket(allBuckets.size());
     int totalNumDir = 0;
     long totalNumKey = 0L;
     for (OmBucketInfo bucket : allBuckets) {
       long bucketObjectId = bucket.getObjectID();
       totalNumDir += getTotalDirCount(bucketObjectId);
       totalNumKey += getTotalKeyCount(bucketObjectId);
     }
+    CountStats countStats = new CountStats(
+        volumes.size(), allBuckets.size(), totalNumDir, totalNumKey);
 
-    namespaceSummaryResponse.setNumTotalDir(totalNumDir);
-    namespaceSummaryResponse.setNumTotalKey(totalNumKey);
+    NamespaceSummaryResponse namespaceSummaryResponse =
+        NamespaceSummaryResponse.newBuilder()
+            .setEntityType(EntityType.ROOT)
+            .setCountStats(countStats)
+            .setObjectDBInfo(getPrefixObjDbInfo())
+            .setStatus(ResponseStatus.OK)
+            .build();
 
     return namespaceSummaryResponse;
   }
 
+  private ObjectDBInfo getPrefixObjDbInfo()
+      throws IOException {
+    OmPrefixInfo omPrefixInfo =
+        getOmMetadataManager().getPrefixTable().
+            getSkipCache(OzoneConsts.OM_KEY_PREFIX);
+    ObjectDBInfo objectDBInfo = new ObjectDBInfo();

Review Comment:
   Same as `BucketObjectDBInfo`, create constructor or builder which takes `OmPrefixInfo` and create `ObjectDBInfo` object.
   



##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestNSSummaryEndpointWithLegacy.java:
##########
@@ -389,15 +399,47 @@ public void testUtility() {
 
   @Test
   public void testGetBasicInfoRoot() throws Exception {
+    String username = "myuser";
+    OmPrefixInfo omPrefixInfo = getOmPrefixInfoForTest(ROOT_PATH,
+        IAccessAuthorizer.ACLIdentityType.USER,
+        username,
+        IAccessAuthorizer.ACLType.WRITE,
+        ACCESS);
+    omPrefixInfo.getMetadata().put("key", "value");
+    reconOMMetadataManager.getPrefixTable()
+        .put(OzoneConsts.OM_KEY_PREFIX, omPrefixInfo);
     // Test root basics
     Response rootResponse = nsSummaryEndpoint.getBasicInfo(ROOT_PATH);
     NamespaceSummaryResponse rootResponseObj =
         (NamespaceSummaryResponse) rootResponse.getEntity();
     Assert.assertEquals(EntityType.ROOT, rootResponseObj.getEntityType());
-    Assert.assertEquals(2, rootResponseObj.getNumVolume());
-    Assert.assertEquals(4, rootResponseObj.getNumBucket());
-    Assert.assertEquals(5, rootResponseObj.getNumTotalDir());
-    Assert.assertEquals(10, rootResponseObj.getNumTotalKey());
+    Assert.assertEquals(2, rootResponseObj.getCountStats().getNumVolume());
+    Assert.assertEquals(4, rootResponseObj.getCountStats().getNumBucket());
+    Assert.assertEquals(5, rootResponseObj.getCountStats().getNumTotalDir());
+    Assert.assertEquals(10, rootResponseObj.getCountStats().getNumTotalKey());

Review Comment:
   This is a repetitive code. I think moving this to a function would be nicer.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/BucketEntityHandler.java:
##########
@@ -49,18 +52,60 @@ public BucketEntityHandler(
   @Override
   public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
-    NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.BUCKET);
+
     String[] names = getNames();
     assert (names.length == 2);
     long bucketObjectId = getBucketHandler().getBucketObjectId(names);
-    namespaceSummaryResponse
-      .setNumTotalDir(getTotalDirCount(bucketObjectId));
-    namespaceSummaryResponse.setNumTotalKey(getTotalKeyCount(bucketObjectId));
+
+    CountStats countStats = new CountStats(
+        -1, -1,
+        getTotalDirCount(bucketObjectId), getTotalKeyCount(bucketObjectId));
+    NamespaceSummaryResponse namespaceSummaryResponse =

Review Comment:
   ```suggestion
       return NamespaceSummaryResponse.newBuilder()
             .setEntityType(EntityType.BUCKET)
             .setCountStats(countStats)
             .setObjectDBInfo(getBucketObjDbInfo(names))
             .setStatus(ResponseStatus.OK)
             .build();
   ```



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042994918


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {

Review Comment:
   > Is it really a subclass of `ObjectDBInfo`? Or you just did it because they share some common properties?
   > 
   > It also doesn't follow the correct hierarchy model. Java doc comment says `VolumeObjectDBInfo` encapsulates the low level volume info, `BucketObjectDBInfo` encapsulates the low level bucket info and `ObjectDBInfo`Encapsulates the low level volume/bucket/dir info.
   > 
   > So hierarchy should be Volume -> Bucket -> Dir -> Key but according to your implementation Dir -> Volume, Bucket and Key. Which is wrong to me.
   > 
   > Please correct it. If you want to extract out common properties, create another class something `CommonPropertyClass` (Please come up with better name).
   > 
   > ```
   > VolumeObjectDBInfo extands CommonPropertyClass
   > BucketObjectDBInfo extands CommonPropertyClass
   > ObjectDBInfo extands CommonPropertyClass
   > KeyObjectDBInfo extands CommonPropertyClass
   > ```
   
   @hemantk-12 - As discussed, we have kept the name same as ObjectDBInfo. Pls re-review.



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042995940


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/BucketEntityHandler.java:
##########
@@ -49,16 +52,34 @@ public BucketEntityHandler(
   @Override
   public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
-    NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.BUCKET);
+
     String[] names = getNames();
     assert (names.length == 2);
     long bucketObjectId = getBucketHandler().getBucketObjectId(names);
-    namespaceSummaryResponse
-      .setNumTotalDir(getTotalDirCount(bucketObjectId));
-    namespaceSummaryResponse.setNumTotalKey(getTotalKeyCount(bucketObjectId));
 
-    return namespaceSummaryResponse;
+    CountStats countStats = new CountStats(
+        -1, -1,
+        getTotalDirCount(bucketObjectId), getTotalKeyCount(bucketObjectId));
+    return
+        NamespaceSummaryResponse.newBuilder()
+            .setEntityType(EntityType.BUCKET)
+            .setCountStats(countStats)
+            .setObjectDBInfo(getBucketObjDbInfo(names))
+            .setStatus(ResponseStatus.OK)
+            .build();

Review Comment:
   @hemantk-12  - this is fixed. pls re-review.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java:
##########
@@ -80,7 +80,8 @@ public Response getBasicInfo(
     NamespaceSummaryResponse namespaceSummaryResponse;
     if (!isInitializationComplete()) {
       namespaceSummaryResponse =
-          new NamespaceSummaryResponse(EntityType.UNKNOWN);
+          NamespaceSummaryResponse.newBuilder().

Review Comment:
   @hemantk-12  - this is fixed. pls re-review.



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042911848


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {
+    return new BucketObjectDBInfo.Builder();
+  }
+
+  public BucketObjectDBInfo() {
+
+  }
+
+  public BucketObjectDBInfo(Builder b) {

Review Comment:
   @hemantk-12 - there can be multiple ways to implement builder pattern. We should just make sure that OOPS principles being followed. You may want to look below link as well.
   
   https://www.geeksforgeeks.org/builder-pattern-in-java/



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042995405


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {
+    return new BucketObjectDBInfo.Builder();
+  }
+
+  public BucketObjectDBInfo() {
+
+  }
+
+  public BucketObjectDBInfo(Builder b) {
+    this.setMetadata(b.getOmBucketInfo().getMetadata());
+    this.setVolumeName(b.getOmBucketInfo().getVolumeName());
+    this.setName(b.getOmBucketInfo().getBucketName());
+    this.setQuotaInBytes(b.getOmBucketInfo().getQuotaInBytes());
+    this.setQuotaInNamespace(
+        b.getOmBucketInfo().getQuotaInNamespace());
+    this.setUsedNamespace(b.getOmBucketInfo().getUsedNamespace());
+    this.setCreationTime(b.getOmBucketInfo().getCreationTime());
+    this.setModificationTime(
+        b.getOmBucketInfo().getModificationTime());
+    this.setAcls(b.getOmBucketInfo().getAcls());
+    this.setSourceBucket(b.getOmBucketInfo().getSourceBucket());
+    this.setSourceVolume(b.getOmBucketInfo().getSourceVolume());
+    this.setBekInfo(b.getOmBucketInfo().getEncryptionKeyInfo());
+    this.setVersioningEnabled(
+        b.getOmBucketInfo().getIsVersionEnabled());
+    this.setStorageType(b.getOmBucketInfo().getStorageType());
+    this.setDefaultReplicationConfig(
+        b.getOmBucketInfo().getDefaultReplicationConfig());
+    this.setBucketLayout(b.getOmBucketInfo().getBucketLayout());
+    this.setOwner(b.getOmBucketInfo().getOwner());
+  }
+
+  public String getVolumeName() {
+    return volumeName;
+  }
+
+  public void setVolumeName(String volumeName) {
+    this.volumeName = volumeName;
+  }
+
+  public StorageType getStorageType() {
+    return storageType;
+  }
+
+  public void setStorageType(StorageType storageType) {
+    this.storageType = storageType;
+  }
+
+  public String getUsedBytes() {
+    return usedBytes;
+  }
+
+  public void setUsedBytes(String usedBytes) {
+    this.usedBytes = usedBytes;
+  }
+
+  public BucketEncryptionKeyInfo getBekInfo() {
+    return bekInfo;
+  }
+
+  public void setBekInfo(BucketEncryptionKeyInfo bekInfo) {
+    this.bekInfo = bekInfo;
+  }
+
+  public DefaultReplicationConfig getDefaultReplicationConfig() {
+    return defaultReplicationConfig;
+  }
+
+  public void setDefaultReplicationConfig(
+      DefaultReplicationConfig defaultReplicationConfig) {
+    this.defaultReplicationConfig = defaultReplicationConfig;
+  }
+
+  public String getSourceVolume() {
+    return sourceVolume;
+  }
+
+  public void setSourceVolume(String sourceVolume) {
+    this.sourceVolume = sourceVolume;
+  }
+
+  public String getSourceBucket() {
+    return sourceBucket;
+  }
+
+  public void setSourceBucket(String sourceBucket) {
+    this.sourceBucket = sourceBucket;
+  }
+
+  public boolean isVersioningEnabled() {
+    return isVersioningEnabled;
+  }
+
+  public void setVersioningEnabled(boolean versioningEnabled) {
+    isVersioningEnabled = versioningEnabled;
+  }
+
+  public BucketLayout getBucketLayout() {
+    return bucketLayout;
+  }
+
+  public void setBucketLayout(BucketLayout bucketLayout) {
+    this.bucketLayout = bucketLayout;
+  }
+
+  public String getOwner() {
+    return owner;
+  }
+
+  public void setOwner(String owner) {
+    this.owner = owner;
+  }
+
+  /**
+   * Builder for BucketObjectDBInfo.
+   */
+  @SuppressWarnings("checkstyle:hiddenfield")
+  public static final class Builder {

Review Comment:
   @hemantk-12 - As discussed, implemented with plain constructor Pls re-review.



-- 
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


[GitHub] [ozone] siddhantsangwan commented on pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on PR #4042:
URL: https://github.com/apache/ozone/pull/4042#issuecomment-1378523191

   @devmadhuu Thanks for the contribution. Merging this now that we have approvals.


-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1055127988


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/CountStats.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+/** Count stats which tells the number of
+ * volumes/buckets/dir/files etc.
+ * */
+public class CountStats {
+    /** Total number of volumes under root, -1 for other types. */

Review Comment:
   @smengcl  - This alignment issue is fixed. Pls re-review.



-- 
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


[GitHub] [ozone] devmadhuu commented on pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on PR #4042:
URL: https://github.com/apache/ozone/pull/4042#issuecomment-1336885283

   @smengcl @dombizita , pls review


-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042902143


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {
+    return new BucketObjectDBInfo.Builder();
+  }
+
+  public BucketObjectDBInfo() {
+
+  }
+
+  public BucketObjectDBInfo(Builder b) {
+    this.setMetadata(b.getOmBucketInfo().getMetadata());
+    this.setVolumeName(b.getOmBucketInfo().getVolumeName());
+    this.setName(b.getOmBucketInfo().getBucketName());
+    this.setQuotaInBytes(b.getOmBucketInfo().getQuotaInBytes());
+    this.setQuotaInNamespace(
+        b.getOmBucketInfo().getQuotaInNamespace());
+    this.setUsedNamespace(b.getOmBucketInfo().getUsedNamespace());
+    this.setCreationTime(b.getOmBucketInfo().getCreationTime());
+    this.setModificationTime(
+        b.getOmBucketInfo().getModificationTime());
+    this.setAcls(b.getOmBucketInfo().getAcls());
+    this.setSourceBucket(b.getOmBucketInfo().getSourceBucket());
+    this.setSourceVolume(b.getOmBucketInfo().getSourceVolume());
+    this.setBekInfo(b.getOmBucketInfo().getEncryptionKeyInfo());
+    this.setVersioningEnabled(
+        b.getOmBucketInfo().getIsVersionEnabled());
+    this.setStorageType(b.getOmBucketInfo().getStorageType());
+    this.setDefaultReplicationConfig(
+        b.getOmBucketInfo().getDefaultReplicationConfig());
+    this.setBucketLayout(b.getOmBucketInfo().getBucketLayout());
+    this.setOwner(b.getOmBucketInfo().getOwner());
+  }
+
+  public String getVolumeName() {
+    return volumeName;
+  }
+
+  public void setVolumeName(String volumeName) {
+    this.volumeName = volumeName;
+  }
+
+  public StorageType getStorageType() {
+    return storageType;
+  }
+
+  public void setStorageType(StorageType storageType) {
+    this.storageType = storageType;
+  }
+
+  public String getUsedBytes() {
+    return usedBytes;
+  }
+
+  public void setUsedBytes(String usedBytes) {
+    this.usedBytes = usedBytes;
+  }
+
+  public BucketEncryptionKeyInfo getBekInfo() {
+    return bekInfo;
+  }
+
+  public void setBekInfo(BucketEncryptionKeyInfo bekInfo) {
+    this.bekInfo = bekInfo;
+  }
+
+  public DefaultReplicationConfig getDefaultReplicationConfig() {
+    return defaultReplicationConfig;
+  }
+
+  public void setDefaultReplicationConfig(
+      DefaultReplicationConfig defaultReplicationConfig) {
+    this.defaultReplicationConfig = defaultReplicationConfig;
+  }
+
+  public String getSourceVolume() {
+    return sourceVolume;
+  }
+
+  public void setSourceVolume(String sourceVolume) {
+    this.sourceVolume = sourceVolume;
+  }
+
+  public String getSourceBucket() {
+    return sourceBucket;
+  }
+
+  public void setSourceBucket(String sourceBucket) {
+    this.sourceBucket = sourceBucket;
+  }
+
+  public boolean isVersioningEnabled() {
+    return isVersioningEnabled;
+  }
+
+  public void setVersioningEnabled(boolean versioningEnabled) {
+    isVersioningEnabled = versioningEnabled;
+  }
+
+  public BucketLayout getBucketLayout() {
+    return bucketLayout;
+  }
+
+  public void setBucketLayout(BucketLayout bucketLayout) {
+    this.bucketLayout = bucketLayout;
+  }
+
+  public String getOwner() {
+    return owner;
+  }
+
+  public void setOwner(String owner) {
+    this.owner = owner;
+  }
+
+  /**
+   * Builder for BucketObjectDBInfo.
+   */
+  @SuppressWarnings("checkstyle:hiddenfield")
+  public static final class Builder {

Review Comment:
   @hemantk-12 - This is just a specific pattern you mentioned. Builder pattern can be written in many ways. We can discuss over this in Slack channel. I have pinged you. You can let me know when you are free to discuss.



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1055127384


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/LegacyBucketHandler.java:
##########
@@ -322,4 +323,11 @@ public Table<String, OmKeyInfo> getKeyTable() {
         getOmMetadataManager().getKeyTable(getBucketLayout());
     return keyTable;
   }
+
+  @Override
+  public OmDirectoryInfo getDirInfo(String[] names) throws IOException {
+    return OmDirectoryInfo.newBuilder()
+        .setName(names[2])

Review Comment:
   @smengcl  - this check is not needed here as above code flow hits only when the path will be directory else not.



-- 
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


[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
hemantk-12 commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042653693


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {
+    return new BucketObjectDBInfo.Builder();
+  }
+
+  public BucketObjectDBInfo() {
+
+  }
+
+  public BucketObjectDBInfo(Builder b) {

Review Comment:
   This is not how builder pattern works. Please change all the builder to this way.
   
   ```suggestion
   
     public BucketObjectDBInfo(Map<String, String> metadata,
                               String name,
                               String volumeName,
                               StorageType storageType,
                               boolean isVersioningEnabled,
                               BucketEncryptionKeyInfo bekInfo,
                               DefaultReplicationConfig defaultReplicationConfig,
                               String sourceVolume,
                               String sourceBucket,
                               BucketLayout bucketLayout,
                               String owner,
                               long quotaInBytes,
                               long quotaInNamespace,
                               long usedNamespace,
                               long creationTime,
                               long modificationTime,
                               List<OzoneAcl> acls) {
   
       super.setMetadata(metadata);
       super.setName(name);
       super.setQuotaInBytes(quotaInBytes);
       super.setQuotaInNamespace(quotaInNamespace);
       super.setUsedNamespace(usedNamespace);
       super.setCreationTime(creationTime);
       super.setModificationTime(modificationTime);
       super.setAcls(acls);
   
       this.volumeName = volumeName;
       this.sourceBucket = sourceVolume;
       this.sourceVolume = sourceBucket;
       this.isVersioningEnabled = isVersioningEnabled;
       this.storageType = storageType;
       this.defaultReplicationConfig = defaultReplicationConfig;
       this.bucketLayout = bucketLayout;
       this.owner = owner;
       this.bekInfo = bekInfo;
     }
   ```
   
   More: https://www.baeldung.com/java-builder-pattern-freebuilder



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {
+    return new BucketObjectDBInfo.Builder();
+  }
+
+  public BucketObjectDBInfo() {
+
+  }
+
+  public BucketObjectDBInfo(Builder b) {
+    this.setMetadata(b.getOmBucketInfo().getMetadata());
+    this.setVolumeName(b.getOmBucketInfo().getVolumeName());
+    this.setName(b.getOmBucketInfo().getBucketName());
+    this.setQuotaInBytes(b.getOmBucketInfo().getQuotaInBytes());
+    this.setQuotaInNamespace(
+        b.getOmBucketInfo().getQuotaInNamespace());
+    this.setUsedNamespace(b.getOmBucketInfo().getUsedNamespace());
+    this.setCreationTime(b.getOmBucketInfo().getCreationTime());
+    this.setModificationTime(
+        b.getOmBucketInfo().getModificationTime());
+    this.setAcls(b.getOmBucketInfo().getAcls());
+    this.setSourceBucket(b.getOmBucketInfo().getSourceBucket());
+    this.setSourceVolume(b.getOmBucketInfo().getSourceVolume());
+    this.setBekInfo(b.getOmBucketInfo().getEncryptionKeyInfo());
+    this.setVersioningEnabled(
+        b.getOmBucketInfo().getIsVersionEnabled());
+    this.setStorageType(b.getOmBucketInfo().getStorageType());
+    this.setDefaultReplicationConfig(
+        b.getOmBucketInfo().getDefaultReplicationConfig());
+    this.setBucketLayout(b.getOmBucketInfo().getBucketLayout());
+    this.setOwner(b.getOmBucketInfo().getOwner());
+  }
+
+  public String getVolumeName() {
+    return volumeName;
+  }
+
+  public void setVolumeName(String volumeName) {
+    this.volumeName = volumeName;
+  }
+
+  public StorageType getStorageType() {
+    return storageType;
+  }
+
+  public void setStorageType(StorageType storageType) {
+    this.storageType = storageType;
+  }
+
+  public String getUsedBytes() {
+    return usedBytes;
+  }
+
+  public void setUsedBytes(String usedBytes) {
+    this.usedBytes = usedBytes;
+  }
+
+  public BucketEncryptionKeyInfo getBekInfo() {
+    return bekInfo;
+  }
+
+  public void setBekInfo(BucketEncryptionKeyInfo bekInfo) {
+    this.bekInfo = bekInfo;
+  }
+
+  public DefaultReplicationConfig getDefaultReplicationConfig() {
+    return defaultReplicationConfig;
+  }
+
+  public void setDefaultReplicationConfig(
+      DefaultReplicationConfig defaultReplicationConfig) {
+    this.defaultReplicationConfig = defaultReplicationConfig;
+  }
+
+  public String getSourceVolume() {
+    return sourceVolume;
+  }
+
+  public void setSourceVolume(String sourceVolume) {
+    this.sourceVolume = sourceVolume;
+  }
+
+  public String getSourceBucket() {
+    return sourceBucket;
+  }
+
+  public void setSourceBucket(String sourceBucket) {
+    this.sourceBucket = sourceBucket;
+  }
+
+  public boolean isVersioningEnabled() {
+    return isVersioningEnabled;
+  }
+
+  public void setVersioningEnabled(boolean versioningEnabled) {
+    isVersioningEnabled = versioningEnabled;
+  }
+
+  public BucketLayout getBucketLayout() {
+    return bucketLayout;
+  }
+
+  public void setBucketLayout(BucketLayout bucketLayout) {
+    this.bucketLayout = bucketLayout;
+  }
+
+  public String getOwner() {
+    return owner;
+  }
+
+  public void setOwner(String owner) {
+    this.owner = owner;
+  }
+
+  /**
+   * Builder for BucketObjectDBInfo.
+   */
+  @SuppressWarnings("checkstyle:hiddenfield")
+  public static final class Builder {

Review Comment:
   ```suggestion
     public static class Builder {
       private final OmBucketInfo omBucketInfo;
   
       public Builder(OmBucketInfo omBucketInfo) {
         this.omBucketInfo = omBucketInfo;
       }
   
       public BucketObjectDBInfo build() {
         if (this.omBucketInfo == null) {
           return new BucketObjectDBInfo();
         }
         return new BucketObjectDBInfo(omBucketInfo.getMetadata(),
             omBucketInfo.getBucketName(),
             omBucketInfo.getVolumeName(),
             omBucketInfo.getStorageType(),
             omBucketInfo.getIsVersionEnabled(),
             omBucketInfo.getEncryptionKeyInfo(),
             omBucketInfo.getDefaultReplicationConfig(),
             omBucketInfo.getSourceVolume(),
             omBucketInfo.getSourceBucket(),
             omBucketInfo.getBucketLayout(),
             omBucketInfo.getOwner(),
             omBucketInfo.getQuotaInBytes(),
             omBucketInfo.getQuotaInNamespace(),
             omBucketInfo.getUsedNamespace(),
             omBucketInfo.getCreationTime(),
             omBucketInfo.getModificationTime(),
             omBucketInfo.getAcls());
       }
     }
   ```



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {

Review Comment:
   Is it really a subclass of `ObjectDBInfo`? Or you just did it because they share some common properties?
   
   It also doesn't follow the correct hierarchy model.
   Java doc comment says `VolumeObjectDBInfo` encapsulates the low level volume info, `BucketObjectDBInfo` encapsulates the low level bucket info and `ObjectDBInfo`Encapsulates the low level volume/bucket/dir info.
   
   So hierarchy should be Volume -> Bucket -> Dir -> Key but according to your implementation Dir -> Volume, Bucket and Key. Which is wrong to me.
   
   Please correct it. If you want to extract out common properties, create another class something `CommonPropertyClass` (Please come up with better name).
   
   ```
   VolumeObjectDBInfo extands CommonPropertyClass
   BucketObjectDBInfo extands CommonPropertyClass
   ObjectDBInfo extands CommonPropertyClass
   KeyObjectDBInfo extands CommonPropertyClass
   ```



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {
+    return new BucketObjectDBInfo.Builder();
+  }
+
+  public BucketObjectDBInfo() {
+
+  }
+
+  public BucketObjectDBInfo(Builder b) {
+    this.setMetadata(b.getOmBucketInfo().getMetadata());
+    this.setVolumeName(b.getOmBucketInfo().getVolumeName());
+    this.setName(b.getOmBucketInfo().getBucketName());
+    this.setQuotaInBytes(b.getOmBucketInfo().getQuotaInBytes());
+    this.setQuotaInNamespace(
+        b.getOmBucketInfo().getQuotaInNamespace());
+    this.setUsedNamespace(b.getOmBucketInfo().getUsedNamespace());
+    this.setCreationTime(b.getOmBucketInfo().getCreationTime());
+    this.setModificationTime(
+        b.getOmBucketInfo().getModificationTime());
+    this.setAcls(b.getOmBucketInfo().getAcls());
+    this.setSourceBucket(b.getOmBucketInfo().getSourceBucket());
+    this.setSourceVolume(b.getOmBucketInfo().getSourceVolume());
+    this.setBekInfo(b.getOmBucketInfo().getEncryptionKeyInfo());
+    this.setVersioningEnabled(
+        b.getOmBucketInfo().getIsVersionEnabled());
+    this.setStorageType(b.getOmBucketInfo().getStorageType());
+    this.setDefaultReplicationConfig(
+        b.getOmBucketInfo().getDefaultReplicationConfig());
+    this.setBucketLayout(b.getOmBucketInfo().getBucketLayout());
+    this.setOwner(b.getOmBucketInfo().getOwner());
+  }
+
+  public String getVolumeName() {
+    return volumeName;
+  }
+
+  public void setVolumeName(String volumeName) {

Review Comment:
   Why do we need setter? Is it for jackson serialization?



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NSSummaryEndpoint.java:
##########
@@ -80,7 +80,8 @@ public Response getBasicInfo(
     NamespaceSummaryResponse namespaceSummaryResponse;
     if (!isInitializationComplete()) {
       namespaceSummaryResponse =
-          new NamespaceSummaryResponse(EntityType.UNKNOWN);
+          NamespaceSummaryResponse.newBuilder().

Review Comment:
   ```suggestion
             NamespaceSummaryResponse.newBuilder()
                 .setEntityType(EntityType.UNKNOWN)
                 .setStatus(ResponseStatus.INITIALIZING)
                 .build();
   ```



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ObjectDBInfo.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
+import org.apache.hadoop.ozone.om.helpers.OmPrefixInfo;
+
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Encapsulates the low level volume/bucket/dir info.
+ */
+public class ObjectDBInfo {

Review Comment:
   Should we just call it `ObjectInfo`? Why DB in the name?



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/BucketEntityHandler.java:
##########
@@ -49,16 +52,34 @@ public BucketEntityHandler(
   @Override
   public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
-    NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.BUCKET);
+
     String[] names = getNames();
     assert (names.length == 2);
     long bucketObjectId = getBucketHandler().getBucketObjectId(names);
-    namespaceSummaryResponse
-      .setNumTotalDir(getTotalDirCount(bucketObjectId));
-    namespaceSummaryResponse.setNumTotalKey(getTotalKeyCount(bucketObjectId));
 
-    return namespaceSummaryResponse;
+    CountStats countStats = new CountStats(
+        -1, -1,
+        getTotalDirCount(bucketObjectId), getTotalKeyCount(bucketObjectId));
+    return
+        NamespaceSummaryResponse.newBuilder()
+            .setEntityType(EntityType.BUCKET)
+            .setCountStats(countStats)
+            .setObjectDBInfo(getBucketObjDbInfo(names))
+            .setStatus(ResponseStatus.OK)
+            .build();

Review Comment:
   It could be in one line. Please change it everywhere.
   
   ```suggestion
       return NamespaceSummaryResponse.newBuilder()
           .setEntityType(EntityType.VOLUME)
           .setCountStats(countStats)
           .setObjectDBInfo(getVolumeObjDbInfo(names))
           .setStatus(ResponseStatus.OK)
           .build();
   ```



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {

Review Comment:
   This is unnecessary function.



##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestNSSummaryEndpointWithLegacy.java:
##########
@@ -389,15 +399,47 @@ public void testUtility() {
 
   @Test
   public void testGetBasicInfoRoot() throws Exception {
+    String username = "myuser";
+    OmPrefixInfo omPrefixInfo = getOmPrefixInfoForTest(ROOT_PATH,
+        IAccessAuthorizer.ACLIdentityType.USER,
+        username,
+        IAccessAuthorizer.ACLType.WRITE,
+        ACCESS);
+    omPrefixInfo.getMetadata().put("key", "value");
+    reconOMMetadataManager.getPrefixTable()
+        .put(OzoneConsts.OM_KEY_PREFIX, omPrefixInfo);
     // Test root basics
     Response rootResponse = nsSummaryEndpoint.getBasicInfo(ROOT_PATH);
     NamespaceSummaryResponse rootResponseObj =
         (NamespaceSummaryResponse) rootResponse.getEntity();
     Assert.assertEquals(EntityType.ROOT, rootResponseObj.getEntityType());
-    Assert.assertEquals(2, rootResponseObj.getNumVolume());
-    Assert.assertEquals(4, rootResponseObj.getNumBucket());
-    Assert.assertEquals(5, rootResponseObj.getNumTotalDir());
-    Assert.assertEquals(10, rootResponseObj.getNumTotalKey());
+    Assert.assertEquals(2, rootResponseObj.getCountStats().getNumVolume());
+    Assert.assertEquals(4, rootResponseObj.getCountStats().getNumBucket());
+    Assert.assertEquals(5, rootResponseObj.getCountStats().getNumTotalDir());
+    Assert.assertEquals(10, rootResponseObj.getCountStats().getNumTotalKey());

Review Comment:
   I don't think this is addressed.



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042995686


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {
+    return new BucketObjectDBInfo.Builder();
+  }
+
+  public BucketObjectDBInfo() {
+
+  }
+
+  public BucketObjectDBInfo(Builder b) {

Review Comment:
   @hemantk-12 - As discussed, implemented with plain constructor Pls re-review.



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042087581


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/KeyEntityHandler.java:
##########
@@ -46,10 +49,39 @@ public KeyEntityHandler(
   @Override
   public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
-    NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.KEY);
+    CountStats countStats = new CountStats(
+        -1, -1,
+        -1, 0);
+    return NamespaceSummaryResponse.newBuilder()
+        .setEntityType(EntityType.KEY)
+        .setCountStats(countStats)
+        .setObjectDBInfo(getKeyDbObjectInfo(getNames()))
+        .setStatus(ResponseStatus.OK)
+        .build();
 
-    return namespaceSummaryResponse;
+  }
+
+  private ObjectDBInfo getKeyDbObjectInfo(String[] names)
+      throws IOException {
+    OmKeyInfo keyInfo = getBucketHandler().getKeyInfo(names);
+    KeyObjectDBInfo keyObjectDBInfo = new KeyObjectDBInfo();

Review Comment:
   Thanks @hemantk-12 for review. Changes done. Pls re-review. 



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/VolumeEntityHandler.java:
##########
@@ -63,12 +64,41 @@ public NamespaceSummaryResponse getSummaryResponse()
       totalKey += getTotalKeyCount(bucketObjectId);
     }
 
-    namespaceSummaryResponse.setNumTotalDir(totalDir);
-    namespaceSummaryResponse.setNumTotalKey(totalKey);
+    CountStats countStats = new CountStats(
+        -1, buckets.size(), totalDir, totalKey);
+
+    NamespaceSummaryResponse namespaceSummaryResponse =
+        NamespaceSummaryResponse.newBuilder()
+            .setEntityType(EntityType.VOLUME)
+            .setCountStats(countStats)
+            .setObjectDBInfo(getVolumeObjDbInfo(names))
+            .setStatus(ResponseStatus.OK)
+            .build();
 
     return namespaceSummaryResponse;
   }
 
+  private VolumeObjectDBInfo getVolumeObjDbInfo(String[] names)
+      throws IOException {
+    String dbVolumeKey = getOmMetadataManager().getVolumeKey(names[0]);
+    OmVolumeArgs volumeArgs =
+        getOmMetadataManager().getVolumeTable().getSkipCache(dbVolumeKey);
+    VolumeObjectDBInfo objectDBInfo = new VolumeObjectDBInfo();

Review Comment:
   Thanks @hemantk-12 for review. Changes done. Pls re-review. 



-- 
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


[GitHub] [ozone] smengcl commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
smengcl commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1054813575


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/CountStats.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+/** Count stats which tells the number of
+ * volumes/buckets/dir/files etc.
+ * */
+public class CountStats {
+    /** Total number of volumes under root, -1 for other types. */

Review Comment:
   Alignment
   ```suggestion
     /** Total number of volumes under root, -1 for other types. */
   ```



-- 
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


[GitHub] [ozone] dombizita commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
dombizita commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1064686773


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/LegacyBucketHandler.java:
##########
@@ -322,4 +323,11 @@ public Table<String, OmKeyInfo> getKeyTable() {
         getOmMetadataManager().getKeyTable(getBucketLayout());
     return keyTable;
   }
+
+  @Override
+  public OmDirectoryInfo getDirInfo(String[] names) throws IOException {
+    return OmDirectoryInfo.newBuilder()
+        .setName(names[2])

Review Comment:
   could you explain more, what guarantees that we can't call this `getDirInfo` method only when the path is a directory? this is inside `BucketHandler` and a legacy bucket doesn't necessarily have a directory inside. 



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")

Review Comment:
   what is the difference between `volumeName` and `sourceVolume` (in line 52)?



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042087095


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/recon/TestReconWithOzoneManagerFSO.java:
##########
@@ -128,8 +128,8 @@ public void testNamespaceSummaryAPI() throws Exception {
     NamespaceSummaryResponse entity =
             (NamespaceSummaryResponse) basicInfo.getEntity();
     Assert.assertSame(entity.getEntityType(), EntityType.DIRECTORY);
-    Assert.assertEquals(1, entity.getNumTotalKey());
-    Assert.assertEquals(0, entity.getNumTotalDir());
+    Assert.assertEquals(1, entity.getCountStats().getNumTotalKey());

Review Comment:
   Thanks @hemantk-12 for review. Changes done. Pls re-review. This was existing case, I added those missing as you mentioned.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/BucketEntityHandler.java:
##########
@@ -49,18 +52,60 @@ public BucketEntityHandler(
   @Override
   public NamespaceSummaryResponse getSummaryResponse()
           throws IOException {
-    NamespaceSummaryResponse namespaceSummaryResponse =
-            new NamespaceSummaryResponse(EntityType.BUCKET);
+
     String[] names = getNames();
     assert (names.length == 2);
     long bucketObjectId = getBucketHandler().getBucketObjectId(names);
-    namespaceSummaryResponse
-      .setNumTotalDir(getTotalDirCount(bucketObjectId));
-    namespaceSummaryResponse.setNumTotalKey(getTotalKeyCount(bucketObjectId));
+
+    CountStats countStats = new CountStats(
+        -1, -1,
+        getTotalDirCount(bucketObjectId), getTotalKeyCount(bucketObjectId));
+    NamespaceSummaryResponse namespaceSummaryResponse =

Review Comment:
   Thanks @hemantk-12 for review. Changes done. Pls re-review. 



-- 
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


[GitHub] [ozone] smengcl commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
smengcl commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1054810045


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/LegacyBucketHandler.java:
##########
@@ -322,4 +323,11 @@ public Table<String, OmKeyInfo> getKeyTable() {
         getOmMetadataManager().getKeyTable(getBucketLayout());
     return keyTable;
   }
+
+  @Override
+  public OmDirectoryInfo getDirInfo(String[] names) throws IOException {
+    return OmDirectoryInfo.newBuilder()
+        .setName(names[2])

Review Comment:
   If `names` has less than 3 elements, iirc it throws `ArrayIndexOutOfBoundsException` which is a `RuntimeException` and won't be caught by `IOException`.
   
   Add a `Preconditions.checkArgument` check 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


[GitHub] [ozone] smengcl commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
smengcl commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1065016580


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/LegacyBucketHandler.java:
##########
@@ -322,4 +323,11 @@ public Table<String, OmKeyInfo> getKeyTable() {
         getOmMetadataManager().getKeyTable(getBucketLayout());
     return keyTable;
   }
+
+  @Override
+  public OmDirectoryInfo getDirInfo(String[] names) throws IOException {
+    return OmDirectoryInfo.newBuilder()
+        .setName(names[2])

Review Comment:
   @devmadhuu I get that the only caller (at the moment) calling this method won't cause issues. I'm fine with either adding or not adding the check but I would prefer the former.
   
   Same goes for the method in `FSOBucketHandler`.



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1065346710


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")

Review Comment:
   > what is the difference between `volumeName` and `sourceVolume` (in line 52)?
   
   @dombizita  - SourceVolume is used if bucket is a symbolic link.



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042901653


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {
+    return new BucketObjectDBInfo.Builder();
+  }
+
+  public BucketObjectDBInfo() {
+
+  }
+
+  public BucketObjectDBInfo(Builder b) {
+    this.setMetadata(b.getOmBucketInfo().getMetadata());
+    this.setVolumeName(b.getOmBucketInfo().getVolumeName());
+    this.setName(b.getOmBucketInfo().getBucketName());
+    this.setQuotaInBytes(b.getOmBucketInfo().getQuotaInBytes());
+    this.setQuotaInNamespace(
+        b.getOmBucketInfo().getQuotaInNamespace());
+    this.setUsedNamespace(b.getOmBucketInfo().getUsedNamespace());
+    this.setCreationTime(b.getOmBucketInfo().getCreationTime());
+    this.setModificationTime(
+        b.getOmBucketInfo().getModificationTime());
+    this.setAcls(b.getOmBucketInfo().getAcls());
+    this.setSourceBucket(b.getOmBucketInfo().getSourceBucket());
+    this.setSourceVolume(b.getOmBucketInfo().getSourceVolume());
+    this.setBekInfo(b.getOmBucketInfo().getEncryptionKeyInfo());
+    this.setVersioningEnabled(
+        b.getOmBucketInfo().getIsVersionEnabled());
+    this.setStorageType(b.getOmBucketInfo().getStorageType());
+    this.setDefaultReplicationConfig(
+        b.getOmBucketInfo().getDefaultReplicationConfig());
+    this.setBucketLayout(b.getOmBucketInfo().getBucketLayout());
+    this.setOwner(b.getOmBucketInfo().getOwner());
+  }
+
+  public String getVolumeName() {
+    return volumeName;
+  }
+
+  public void setVolumeName(String volumeName) {

Review Comment:
   @hemantk-12 - yes it is needed for JSON serialization



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042900850


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {

Review Comment:
   @hemantk-12 , thanks for your review. Here we don't have to follow that Volume -> Bucket -> Dir -> Key
   Because this ObjectDBInfo is a parent class with common properties.
   Because this is RocksDB object level info properties and I feel giving name as ObjectDBInfo is justifying name... as this is no specific objectDB info and attributes can be related to any object which is common... So either we can give name as CommonPropertyClass or ObjectDBInfo, how does it matter. It is just an implementation specific I believe.



-- 
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


[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
hemantk-12 commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042914150


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  @JsonProperty("sourceVolume")
+  private String sourceVolume;
+
+  @JsonProperty("sourceBucket")
+  private String sourceBucket;
+
+  @JsonProperty("bucketLayout")
+  private BucketLayout bucketLayout;
+
+  @JsonProperty("owner")
+  private String owner;
+
+  public static BucketObjectDBInfo.Builder newBuilder() {

Review Comment:
   Because `new BucketObjectDBInfo.Builder();` can be used instead of `BucketObjectDBInfo.newBuilder() `. It is not providing any benefit. 
   
   If it is general pattern followed in the code base, please go ahead with it.



-- 
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


[GitHub] [ozone] smengcl commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
smengcl commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1065017764


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  @JsonProperty("volumeName")

Review Comment:
   `sourceVolume` is used for bucket link iirc.
   
   https://github.com/apache/ozone/blob/85e7cd1867ec9000df798c74ab0f9cf153936a5d/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java#L4306



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1065349620


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/LegacyBucketHandler.java:
##########
@@ -322,4 +323,11 @@ public Table<String, OmKeyInfo> getKeyTable() {
         getOmMetadataManager().getKeyTable(getBucketLayout());
     return keyTable;
   }
+
+  @Override
+  public OmDirectoryInfo getDirInfo(String[] names) throws IOException {
+    return OmDirectoryInfo.newBuilder()
+        .setName(names[2])

Review Comment:
   > 
   
   @dombizita  - getDirInfo method has caller only through DirectoryEntityHandler and call stack for DirectoryEntityHandler will be determined after validating the path , if path is directory, then only DirectoryEntityHandler will be returned by EntityHandler.getEntityHandler , so there is no way the current code flow will hit getDirInfo method if path is not directory. First two indexes will be taken by Volume and Bucket in path and 3rd index will be surely a directory. Please check the caller hierarchy of this method. We can have zoom call to explain by sharing my screen.



-- 
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


[GitHub] [ozone] dombizita commented on pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
dombizita commented on PR #4042:
URL: https://github.com/apache/ozone/pull/4042#issuecomment-1378450423

   thanks for updating your patch @devmadhuu, it looks good to me! 


-- 
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


[GitHub] [ozone] dombizita commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
dombizita commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1065640509


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/LegacyBucketHandler.java:
##########
@@ -322,4 +323,11 @@ public Table<String, OmKeyInfo> getKeyTable() {
         getOmMetadataManager().getKeyTable(getBucketLayout());
     return keyTable;
   }
+
+  @Override
+  public OmDirectoryInfo getDirInfo(String[] names) throws IOException {
+    return OmDirectoryInfo.newBuilder()
+        .setName(names[2])

Review Comment:
   I get that currently this is only called when it is guaranteed that the path is a directory. but if someone later adds something and doesn't know how this method works the scenario can happen which @smengcl wrote above. all the classes that are extending the `EntityHandler` abstract class can do a `OmDirectoryInfo omDirectoryInfo = getBucketHandler().getDirInfo(names)`, even if the `names` length is less than 3. I am also fine with either way, just wanted to clarify what I meant. 



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042087834


##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestNSSummaryEndpointWithLegacy.java:
##########
@@ -389,15 +399,47 @@ public void testUtility() {
 
   @Test
   public void testGetBasicInfoRoot() throws Exception {
+    String username = "myuser";
+    OmPrefixInfo omPrefixInfo = getOmPrefixInfoForTest(ROOT_PATH,
+        IAccessAuthorizer.ACLIdentityType.USER,
+        username,
+        IAccessAuthorizer.ACLType.WRITE,
+        ACCESS);
+    omPrefixInfo.getMetadata().put("key", "value");
+    reconOMMetadataManager.getPrefixTable()
+        .put(OzoneConsts.OM_KEY_PREFIX, omPrefixInfo);
     // Test root basics
     Response rootResponse = nsSummaryEndpoint.getBasicInfo(ROOT_PATH);
     NamespaceSummaryResponse rootResponseObj =
         (NamespaceSummaryResponse) rootResponse.getEntity();
     Assert.assertEquals(EntityType.ROOT, rootResponseObj.getEntityType());
-    Assert.assertEquals(2, rootResponseObj.getNumVolume());
-    Assert.assertEquals(4, rootResponseObj.getNumBucket());
-    Assert.assertEquals(5, rootResponseObj.getNumTotalDir());
-    Assert.assertEquals(10, rootResponseObj.getNumTotalKey());
+    Assert.assertEquals(2, rootResponseObj.getCountStats().getNumVolume());
+    Assert.assertEquals(4, rootResponseObj.getCountStats().getNumBucket());
+    Assert.assertEquals(5, rootResponseObj.getCountStats().getNumTotalDir());
+    Assert.assertEquals(10, rootResponseObj.getCountStats().getNumTotalKey());

Review Comment:
   Thanks @hemantk-12 for review. Changes done. Pls re-review. 



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/BucketObjectDBInfo.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.protocol.StorageType;
+import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+
+/**
+ * Encapsulates the low level bucket info.
+ */
+public class BucketObjectDBInfo extends ObjectDBInfo {
+  /** volume name from om db. */
+  @JsonProperty("volumeName")
+  private String volumeName;
+
+  /** storage type for the object. */
+  @JsonProperty("storageType")
+  private StorageType storageType;
+
+  /** is versioning enabled for the object. */
+  @JsonProperty("versioning")
+  private boolean isVersioningEnabled;
+
+  /** used bytes for the object. */
+  @JsonProperty("usedBytes")
+  private String usedBytes;
+
+  /**
+   * Bucket encryption key info if encryption is enabled.
+   */
+  @JsonProperty("encryptionInfo")
+  private BucketEncryptionKeyInfo bekInfo;
+
+  /**
+   * Optional default replication for bucket.
+   */
+  @JsonProperty("replicationConfigInfo")
+  private DefaultReplicationConfig defaultReplicationConfig;
+
+  /** source volume. */

Review Comment:
   Thanks @hemantk-12 for review. Changes done. Pls re-review. 



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042901081


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ObjectDBInfo.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
+import org.apache.hadoop.ozone.om.helpers.OmPrefixInfo;
+
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Encapsulates the low level volume/bucket/dir info.
+ */
+public class ObjectDBInfo {

Review Comment:
   @hemantk-12 - It is because these attributes are very specific to what we save in RocksDB. Pls also you can go through JIRA details.



-- 
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


[GitHub] [ozone] devmadhuu commented on a diff in pull request #4042: HDDS-5541. Namespace summary endpoint can carry basic information on the entity as well.

Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #4042:
URL: https://github.com/apache/ozone/pull/4042#discussion_r1042995211


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ObjectDBInfo.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.recon.api.types;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
+import org.apache.hadoop.ozone.om.helpers.OmPrefixInfo;
+
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Encapsulates the low level volume/bucket/dir info.
+ */
+public class ObjectDBInfo {

Review Comment:
   @hemantk-12 - As discussed, we have kept the name same as ObjectDBInfo. Pls re-review.



-- 
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