automatically skip empty roles/group from review list (#2607)

servers/zms/src/main/java/com/yahoo/athenz/zms/store/impl/jdbc/JDBCConnection.java

@@ -655,15 +655,16 @@ public class JDBCConnection implements ObjectStoreConnection {
     private static final String SQL_GET_ROLE_REVIEW_LIST  = "SELECT domain.name AS domain_name, role.name AS role_name,"
             + " role.member_expiry_days, role.service_expiry_days, role.group_expiry_days, role.member_review_days,"
             + " role.service_review_days, role.group_review_days, role.last_reviewed_time, role.created FROM role"
-            + " JOIN domain ON role.domain_id=domain.domain_id WHERE role.trust='' AND"
+            + " JOIN domain ON role.domain_id=domain.domain_id JOIN role_member ON role.role_id=role_member.role_id WHERE role.trust='' AND"
             + " (role.member_expiry_days!=0 OR role.service_expiry_days!=0 OR role.group_expiry_days!=0 OR"
             + " role.member_review_days!=0 OR role.service_review_days!=0 OR role.group_review_days!=0) AND"
             + " role.domain_id IN (SELECT domain.domain_id FROM domain JOIN role ON role.domain_id=domain.domain_id"
             + " JOIN role_member ON role.role_id=role_member.role_id WHERE role_member.principal_id=? AND"
             + " role_member.active=true AND role.name='admin') ORDER BY domain.name, role.name;";
     private static final String SQL_GET_GROUP_REVIEW_LIST = "SELECT domain.name AS domain_name, principal_group.name AS group_name,"
             + " principal_group.member_expiry_days, principal_group.service_expiry_days, principal_group.last_reviewed_time,"
-            + " principal_group.created FROM principal_group JOIN domain ON principal_group.domain_id=domain.domain_id WHERE"
+            + " principal_group.created FROM principal_group JOIN domain ON principal_group.domain_id=domain.domain_id"
+            + " JOIN principal_group_member ON principal_group.group_id=principal_group_member.group_id WHERE"
             + " (principal_group.member_expiry_days!=0 OR principal_group.service_expiry_days!=0) AND"
             + " principal_group.domain_id IN (SELECT domain.domain_id FROM domain JOIN role ON"
             + " role.domain_id=domain.domain_id JOIN role_member ON role.role_id=role_member.role_id"

servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSObjectReviewTest.java

@@ -182,6 +182,7 @@ public void testGetRolesForReview() {
 
         meta = new RoleMeta().setMemberReviewDays(30).setLastReviewedDate(past15Days);
         zmsImpl.putRoleMeta(rsrcCtx1, "domain3", "role1", auditRef, null, meta);
+        zmsImpl.putRoleMeta(rsrcCtx1, "domain3", "role4", auditRef, null, meta);
 
         // we should get back no roles in domain1 and domain3
 
@@ -191,10 +192,13 @@ public void testGetRolesForReview() {
         assertTrue(reviewObjects.getList().isEmpty());
 
         // now let's set the expiry to a value 15 days for domain3
-        // and we should get back that entry in our list
+        // and we should get back that entry in our list.
+        // we're also setting the value 15 days for role4 in domain3,
+        // but we should not get it back since it has no members
 
         meta = new RoleMeta().setServiceExpiryDays(15);
         zmsImpl.putRoleMeta(rsrcCtx1, "domain3", "role1", auditRef, null, meta);
+        zmsImpl.putRoleMeta(rsrcCtx1, "domain3", "role4", auditRef, null, meta);
 
         reviewObjects = zmsImpl.getRolesForReview(rsrcCtx1, principal.getFullName());
         assertNotNull(reviewObjects);
@@ -290,6 +294,7 @@ public void testGetGroupsForReview() {
 
         meta = new GroupMeta().setServiceExpiryDays(30).setLastReviewedDate(past15Days);
         zmsImpl.putGroupMeta(rsrcCtx1, "domain3", "group1", auditRef, null, meta);
+        zmsImpl.putGroupMeta(rsrcCtx1, "domain3", "group4", auditRef, null, meta);
 
         // we should get back no groups in domain1 and domain3
 
@@ -299,10 +304,13 @@ public void testGetGroupsForReview() {
         assertTrue(reviewObjects.getList().isEmpty());
 
         // now let's set the expiry to a value 15 days for domain3
-        // and we should get back that entry in our list
+        // and we should get back that entry in our list.
+        // we'll also set the value 15 days for group4 in domain3,
+        // but we won't get it back since it has no members
 
         meta = new GroupMeta().setServiceExpiryDays(15);
         zmsImpl.putGroupMeta(rsrcCtx1, "domain3", "group1", auditRef, null, meta);
+        zmsImpl.putGroupMeta(rsrcCtx1, "domain3", "group4", auditRef, null, meta);
 
         reviewObjects = zmsImpl.getGroupsForReview(rsrcCtx1, principal.getFullName());
         assertNotNull(reviewObjects);
@@ -364,6 +372,10 @@ private void insertRecordsForRoleReviewTest(final String principal) {
         // Create role1 in domain3 only principal
         role = zmsTestInitializer.createRoleObject("domain3", "role1", null, roleMembers);
         zmsImpl.putRole(ctx, "domain3", "role1", auditRef, false, null, role);
+
+        // create role4 in domain1 with no members
+        role = zmsTestInitializer.createRoleObject("domain3", "role4", null, null);
+        zmsImpl.putRole(ctx, "domain3", "role4", auditRef, false, null, role);
     }
 
     private void insertRecordsForGroupReviewTest(final String principal) {
@@ -399,6 +411,10 @@ private void insertRecordsForGroupReviewTest(final String principal) {
         // Create group1 in domain3 only principal
         group = zmsTestInitializer.createGroupObject("domain3", "group1", groupMembers);
         zmsImpl.putGroup(ctx, "domain3", "group1", auditRef, false, null, group);
+
+        // create group4 in domain3 with no members
+        group = zmsTestInitializer.createGroupObject("domain3", "group4", null);
+        zmsImpl.putGroup(ctx, "domain3", "group4", auditRef, false, null, group);
     }
 
     private void createDomain(final String domainName, final String principal) {