ayushtkn commented on code in PR #5696:
URL: https://github.com/apache/hadoop/pull/5696#discussion_r1219823209
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/security/TestRouterSecurityManager.java:
##########
@@ -259,6 +262,15 @@ private static String[] getUserGroupForTesting() {
return groupsForTesting;
}
+ @Test
+ public void testGetTopTokenRealOwners() throws Exception {
Review Comment:
this test passes only when run with the entire class, but fails if run alone
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -712,13 +715,21 @@ public long getCurrentTokensCount() {
@Override
public String getTopTokenRealOwners() {
- RouterSecurityManager mgr =
- this.router.getRpcServer().getRouterSecurityManager();
- if (mgr != null && mgr.getSecretManager() != null) {
- return JSON.toString(mgr.getSecretManager()
- .getTopTokenRealOwners(this.topTokenRealOwners));
+ String topTokenRealOwnersString = "";
+ RouterSecurityManager routerSecurityManager =
+ this.router.getRpcServer().getRouterSecurityManager();
+ if (routerSecurityManager == null ||
routerSecurityManager.getSecretManager() == null)
+ return topTokenRealOwnersString;
+ try {
+ List<Metrics2Util.NameValuePair> topOwners =
routerSecurityManager.getSecretManager()
+ .getTopTokenRealOwners(this.topTokenRealOwners);
+ if (!topOwners.isEmpty()) {
+ topTokenRealOwnersString = new
ObjectMapper().writeValueAsString(topOwners);
+ }
+ } catch (JsonProcessingException jpe) {
+ LOG.error("Unable to parse the top token real owners to string {}",
jpe.getMessage());
}
- return "";
+ return topTokenRealOwnersString;
Review Comment:
Can we use a method from ``JsonUtil.java``
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/security/TestRouterSecurityManager.java:
##########
@@ -259,6 +262,15 @@ private static String[] getUserGroupForTesting() {
return groupsForTesting;
}
+ @Test
+ public void testGetTopTokenRealOwners() throws Exception {
+ RouterMBean bean = FederationTestUtils.getBean(
+ ROUTER_BEAN, RouterMBean.class);
+ String topTokenRealOwners = bean.getTopTokenRealOwners();
+ assertNotNull("The value of output should not be null " +
topTokenRealOwners, topTokenRealOwners);
+ assertTrue("The value contains incorrect message " + topTokenRealOwners,
!topTokenRealOwners.contains("NameValuePair"));
Review Comment:
can we assert some output which should be there post this fix rather than
what shouldn't be there?
btw. rather than ``!``, you could have used ``assertFalse``
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
