ZBUG-2619:Incorrect Member format in Dynamic DL

Author: swaatiTelus

store/src/java/com/zimbra/cs/account/soap/SoapDistributionList.java

@@ -52,7 +52,7 @@ class SoapDistributionList extends DistributionList implements SoapEntry {
         super(dlInfo.getName(), dlInfo.getId(),
                 new HashMap<String,Object>(), prov);
         // DistributionListMembershipInfo does not supply membership info
-        resetDlm(getRawAttrs());
+        addDlm(new ArrayList<String>(), getRawAttrs());
     }
 
     SoapDistributionList(DistributionListInfo dlInfo, Provisioning prov)
@@ -72,7 +72,7 @@ class SoapDistributionList extends DistributionList implements SoapEntry {
         attrs.put(Provisioning.A_zimbraId, dlInfo.getId());
 
         // DLInfo does not supply membership info
-        resetDlm(getRawAttrs());
+        addDlm(new ArrayList<String>(), getRawAttrs());
     }
 
     SoapDistributionList(Element e, Provisioning prov) throws ServiceException {
@@ -81,30 +81,10 @@ class SoapDistributionList extends DistributionList implements SoapEntry {
                 SoapProvisioning.getAttrs(e), prov);
         addDlm(e, getRawAttrs());
     }
-    
-    /**
-     * ZBUG-651: This method is modified as it is overriding the existing member/s value with zero 
-     * against zimbraMailForwardingAddress attribute.
-     * So now checking if members list is zero than it should not override.
-     * @param members This is list of members
-     * @param attrs This is list of attribute
-     * @return nothing
-     */
+
     private void addDlm(List <String> members, Map<String, Object> attrs) {
-    	if (members.size() > 0) {
-    		attrs.put(Provisioning.A_zimbraMailForwardingAddress,
+        attrs.put(Provisioning.A_zimbraMailForwardingAddress,
                 members.toArray(new String[members.size()]));
-    	}
-    }
-    
-    /**
-     * ZBUG-651: This method is added against the change in addDlm(), which provides blank list of members.
-     * @param attrs to put/override attribute with blank member list.
-     * @return nothing
-     */
-    private void resetDlm(Map<String, Object> attrs) {
-    	attrs.put(Provisioning.A_zimbraMailForwardingAddress,
-    			new String[0]);
     }
 
     private void addDlm(Element e, Map<String, Object> attrs) {
@@ -136,4 +116,4 @@ public void reload(SoapProvisioning prov) throws ServiceException {
         addDlm(dl, attrs);                
         setAttrs(attrs);
     }
-}
+}

store/src/java/com/zimbra/cs/account/soap/SoapDynamicGroup.java

@@ -83,7 +83,7 @@ public String[] getAllMembers() throws ServiceException {
         if (membersList == null) {
             return new String[0];
         } else {
-            return getMultiAttr(Provisioning.A_member);
+            return membersList.toArray(new String[membersList.size()]);
         }
     }
 

store/src/java/com/zimbra/cs/service/admin/GetAllDistributionLists.java

@@ -17,18 +17,20 @@
 
 package com.zimbra.cs.service.admin;
 
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
 import com.zimbra.common.account.Key;
-import com.zimbra.common.account.Key.DomainBy;
 import com.zimbra.common.service.ServiceException;
 import com.zimbra.common.soap.AdminConstants;
 import com.zimbra.common.soap.Element;
+import com.zimbra.cs.account.AccessManager.AttrRightChecker;
 import com.zimbra.cs.account.AccountServiceException;
 import com.zimbra.cs.account.DistributionList;
 import com.zimbra.cs.account.Domain;
+import com.zimbra.cs.account.DynamicGroup;
 import com.zimbra.cs.account.Group;
 import com.zimbra.cs.account.Provisioning;
 import com.zimbra.cs.account.accesscontrol.AdminRight;
@@ -48,7 +50,7 @@ public boolean domainAuthSufficient(Map context) {
     }
 
     public Element handle(Element request, Map<String, Object> context) throws ServiceException {
-	    
+
         ZimbraSoapContext zsc = getZimbraSoapContext(context);
         Provisioning prov = Provisioning.getInstance();
 
@@ -69,7 +71,7 @@ public Element handle(Element request, Map<String, Object> context) throws Servi
                 throw ServiceException.INVALID_REQUEST("unknown value for by: "+key, null);
             }
             if (domain == null)
-                throw AccountServiceException.NO_SUCH_DOMAIN(value);            
+                throw AccountServiceException.NO_SUCH_DOMAIN(value);
         }
 
         if (isDomainAdminOnly(zsc)) {
@@ -79,40 +81,68 @@ public Element handle(Element request, Map<String, Object> context) throws Servi
             domain = getAuthTokenAccountDomain(zsc);
         }
 
-        AdminAccessControl aac = AdminAccessControl.getAdminAccessControl(zsc);
-        
         if (domain != null) {
             response = zsc.createElement(AdminConstants.GET_ALL_DISTRIBUTION_LISTS_RESPONSE);
-            doDomain(zsc, response, domain, aac);
+            doDomain(zsc, response, domain);
         } else {
             response = zsc.createElement(AdminConstants.GET_ALL_DISTRIBUTION_LISTS_RESPONSE);
             List domains = prov.getAllDomains();
             for (Iterator dit=domains.iterator(); dit.hasNext(); ) {
                 Domain dm = (Domain) dit.next();
-                doDomain(zsc, response, dm, aac);                
+                doDomain(zsc, response, dm);
             }
         }
-        return response;        
+        return response;
     }
-    
-    private void doDomain(ZimbraSoapContext zsc, Element e, Domain d, AdminAccessControl aac) throws ServiceException {
+
+    private void doDomain(ZimbraSoapContext zsc, Element e, Domain d) throws ServiceException {
         List dls = Provisioning.getInstance().getAllGroups(d);
+        Element eDL = null;
+        AdminAccessControl aac = null;
+        AttrRightChecker arc = null;
         for (Iterator it = dls.iterator(); it.hasNext(); ) {
             Group dl = (Group) it.next();
             boolean hasRightToList = true;
+            boolean allowMembers = true;
+
             if (dl.isDynamic()) {
-                // TODO: fix me
+                aac = checkDynamicGroupRight(zsc, (DynamicGroup) dl, AdminRight.PR_ALWAYS_ALLOW);
+                arc = aac.getAttrRightChecker(dl);
                 hasRightToList = true;
+                allowMembers = arc == null ? true : arc.allowAttr(Provisioning.A_member);
             } else {
+                aac = checkDistributionListRight(zsc,
+                        (DistributionList) dl, AdminRight.PR_ALWAYS_ALLOW);
+                arc = aac.getAttrRightChecker(dl);
+                allowMembers = arc == null ? true : arc.allowAttr(Provisioning.A_zimbraMailForwardingAddress);
                 hasRightToList = aac.hasRightsToList(dl, Admin.R_listDistributionList, null);
             }
-            
             if (hasRightToList) {
-                GetDistributionList.encodeDistributionList(e, dl, false, false, null, aac.getAttrRightChecker(dl));
+                eDL = GetDistributionList.encodeDistributionList(e, dl, true, false, null, arc);
+            }
+            if (allowMembers) {
+                encodeMembers(e, eDL, dl);
             }
-        }        
+        }
     }
-    
+
+    private void encodeMembers(Element response, Element dlElement, Group group)
+            throws ServiceException {
+       String[] members;
+       if (group instanceof DynamicGroup) {
+           members = ((DynamicGroup)group).getAllMembers(true);
+       } else {
+           members = group.getAllMembers();
+       }
+
+       Arrays.sort(members);
+       for (int i = 0; i < members.length; i++) {
+           dlElement.addElement(AdminConstants.E_DLM).setText(members[i]);
+       }
+
+       response.addAttribute(AdminConstants.A_TOTAL, members.length);
+   }
+
     @Override
     public void docRights(List<AdminRight> relatedRights, List<String> notes) {
         relatedRights.add(Admin.R_listDistributionList);