Merged FILE-FOLDER-API (5.2.0) to HEAD (5.2)

119311 jvonka: RA-388 / RA-638 : FileFolder API - optimise userInfo lookup for cm:creator & cm:modifier (especially when getting folder children)
   - also return userInfo for other in-built user props, eg. cm:owner, cm:lockOwner, cm:workingCopyOwner


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@126355 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/rest/api/impl/NodesImpl.java

@@ -30,6 +30,7 @@ import org.alfresco.rest.api.model.Folder;
 import org.alfresco.rest.api.model.Node;
 import org.alfresco.rest.api.model.PathInfo;
 import org.alfresco.rest.api.model.PathInfo.ElementInfo;
+import org.alfresco.rest.api.model.UserInfo;
 import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException;
 import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException;
 import org.alfresco.rest.framework.resource.content.BasicContentInfo;
@@ -144,7 +145,7 @@ public class NodesImpl implements Nodes
             ContentModel.ASPECT_LOCALIZED);
 
     private static final List<QName> EXCLUDED_PROPS = Arrays.asList(
-            // top-level basic info
+            // top-level minimal info
             ContentModel.PROP_NAME,
             ContentModel.PROP_MODIFIER,
             ContentModel.PROP_MODIFIED,
@@ -163,6 +164,13 @@ public class NodesImpl implements Nodes
             ContentModel.PROP_AUTO_VERSION_PROPS,
             ContentModel.PROP_AUTO_VERSION);
 
+    private static final List<QName> PROPS_USERLOOKUP = Arrays.asList(
+            ContentModel.PROP_CREATOR,
+            ContentModel.PROP_MODIFIER,
+            ContentModel.PROP_OWNER,
+            ContentModel.PROP_LOCK_OWNER,
+            ContentModel.PROP_WORKING_COPY_OWNER);
+
     private final static String PARAM_ISFOLDER = "isFolder";
     private final static String PARAM_NAME = "name";
     private final static String PARAM_CREATEDAT = "createdAt";
@@ -273,7 +281,7 @@ public class NodesImpl implements Nodes
     {
         NodeRef nodeRef = validateNode(nodeId);
 
-        return new Node(nodeRef, null, nodeService.getProperties(nodeRef), sr);
+        return new Node(nodeRef, null, nodeService.getProperties(nodeRef), null, sr);
     }
 
     /**
@@ -281,7 +289,7 @@ public class NodesImpl implements Nodes
      */
     public Node getNode(NodeRef nodeRef)
     {
-        return new Node(nodeRef, null, nodeService.getProperties(nodeRef), sr);
+        return new Node(nodeRef, null, nodeService.getProperties(nodeRef), null, sr);
     }
 
     private Type getType(NodeRef nodeRef)
@@ -306,7 +314,7 @@ public class NodesImpl implements Nodes
         {
             Map<QName, Serializable> properties = nodeService.getProperties(nodeRef);
 
-            Document doc = new Document(nodeRef, getParentNodeRef(nodeRef), properties, sr);
+            Document doc = new Document(nodeRef, getParentNodeRef(nodeRef), properties, null, sr);
 
             doc.setVersionLabel((String) properties.get(ContentModel.PROP_VERSION_LABEL));
             ContentData cd = (ContentData) properties.get(ContentModel.PROP_CONTENT);
@@ -344,7 +352,7 @@ public class NodesImpl implements Nodes
         {
             Map<QName, Serializable> properties = nodeService.getProperties(nodeRef);
 
-            Folder folder = new Folder(nodeRef, getParentNodeRef(nodeRef), properties, sr);
+            Folder folder = new Folder(nodeRef, getParentNodeRef(nodeRef), properties, null, sr);
             setCommonProps(folder, nodeRef, properties);
             return folder;
         }
@@ -461,14 +469,18 @@ public class NodesImpl implements Nodes
         NodeRef nodeRef = validateOrLookupNode(nodeId, path);
 
         QName typeQName = nodeService.getType(nodeRef);
-         List<QName> requestedProperties = createQNames(parameters.getSelectedProperties());
+        List<QName> requestedProperties = createQNames(parameters.getSelectedProperties());
-        return getFolderOrDocument(nodeRef, getParentNodeRef(nodeRef), typeQName, requestedProperties, false);
+        return getFolderOrDocument(nodeRef, getParentNodeRef(nodeRef), typeQName, requestedProperties, false, null);
     }
 
-    private Node getFolderOrDocument(final NodeRef nodeRef, NodeRef parentNodeRef, QName typeQName, List<QName> selectedProperties, boolean minimalnfo)
+    private Node getFolderOrDocument(final NodeRef nodeRef, NodeRef parentNodeRef, QName typeQName, List<QName> selectedProperties, boolean minimalInfo, Map<String,UserInfo> mapUserInfo)
     {
+        if (mapUserInfo == null) {
+            mapUserInfo = new HashMap<>(2);
+        }
+
         PathInfo pathInfo = null;
-        if (!minimalnfo)
+        if (!minimalInfo)
         {
             pathInfo = lookupPathInfo(nodeRef);
         }
@@ -480,21 +492,21 @@ public class NodesImpl implements Nodes
 
         if (type.equals(Type.DOCUMENT))
         {
-            node = new Document(nodeRef, parentNodeRef, properties, sr);
+            node = new Document(nodeRef, parentNodeRef, properties, mapUserInfo, sr);
         }
         else if (type.equals(Type.FOLDER))
         {
             // container/folder
-            node = new Folder(nodeRef, parentNodeRef, properties, sr);
+            node = new Folder(nodeRef, parentNodeRef, properties, mapUserInfo, sr);
         }
         else
         {
             throw new InvalidArgumentException("Node is not a folder or file");
         }
 
-        if (!minimalnfo)
+        if (! minimalInfo)
         {
-            node.setProperties(mapProperties(properties, selectedProperties));
+            node.setProperties(mapProperties(properties, selectedProperties, mapUserInfo));
             node.setAspectNames(mapAspects(nodeService.getAspects(nodeRef)));
         }
 
@@ -606,7 +618,7 @@ public class NodesImpl implements Nodes
         return new PathInfo(pathStr, isComplete, pathElements);
     }
 
-    protected Map<String, Object> mapProperties(Map<QName, Serializable> nodeProps, List<QName> selectedProperties)
+    protected Map<String, Object> mapProperties(Map<QName, Serializable> nodeProps, List<QName> selectedProperties, Map<String,UserInfo> mapUserInfo)
     {
         Map<String, Object> props = null;
         if (!selectedProperties.isEmpty())
@@ -617,6 +629,9 @@ public class NodesImpl implements Nodes
                 Serializable value = nodeProps.get(qName);
                 if (value != null)
                 {
+                    if (PROPS_USERLOOKUP.contains(qName)) {
+                        value = Node.lookupUserInfo((String)value, mapUserInfo, sr.getPersonService());
+                    }
                     props.put(qName.toPrefixString(namespaceService), value);
                 }
             }
@@ -651,18 +666,9 @@ public class NodesImpl implements Nodes
 
     public CollectionWithPagingInfo<Node> getChildren(String parentFolderNodeId, Parameters parameters)
     {
-        // TODO do we want to support path with list folder children ?
+        final NodeRef parentNodeRef = validateOrLookupNode(parentFolderNodeId, null);
-        String path = null;
-        // String path = parameters.getParameter("path");
 
-        final NodeRef parentNodeRef = validateOrLookupNode(parentFolderNodeId, path);
+        final boolean minimalInfo = (parameters.getSelectedProperties().size() == 0);
-
-        // TODO
-        // map - where (filter) properties - including isFolder
-        // map - orderBy (sort) properties - including isFolder
-
-        // TODO refactor & fix !
-        final boolean minimalnfo = (parameters.getSelectedProperties().size() == 0);
         final List<QName> requestedProperties = createQNames(parameters.getSelectedProperties());
 
         boolean includeFolders = true;
@@ -722,6 +728,8 @@ public class NodesImpl implements Nodes
 
         final PagingResults<FileInfo> pagingResults = fileFolderService.list(parentNodeRef, includeFiles, includeFolders, ignoreTypeQNames, sortProps, pagingRequest);
 
+        final Map<String, UserInfo> mapUserInfo = new HashMap<>(10);
+
         final List<FileInfo> page = pagingResults.getPage();
         List<Node> nodes = new AbstractList<Node>()
         {
@@ -730,8 +738,8 @@ public class NodesImpl implements Nodes
             {
                 FileInfo fInfo = page.get(index);
 
-                // basic info by default (unless "select"ed otherwise)
+                // minimal info by default (unless "select"ed otherwise)
-                return getFolderOrDocument(fInfo.getNodeRef(), parentNodeRef, fInfo.getType(), requestedProperties, minimalnfo);
+                return getFolderOrDocument(fInfo.getNodeRef(), parentNodeRef, fInfo.getType(), requestedProperties, minimalInfo, mapUserInfo);
             }
 
             @Override
@@ -789,7 +797,7 @@ public class NodesImpl implements Nodes
         {
             for (Entry<String, Object> entry : nodeInfo.getProperties().entrySet())
             {
-                QName propQName = QName.createQName((String)entry.getKey(), namespaceService);
+                QName propQName = QName.createQName(entry.getKey(), namespaceService);
                 props.put(propQName, (Serializable)entry.getValue());
             }
         }
@@ -827,7 +835,7 @@ public class NodesImpl implements Nodes
         {
             for (Entry<String, Object> entry : nodeInfo.getProperties().entrySet())
             {
-                QName propQName = QName.createQName((String)entry.getKey(), namespaceService);
+                QName propQName = QName.createQName(entry.getKey(), namespaceService);
                 props.put(propQName, (Serializable)entry.getValue());
             }
         }

source/java/org/alfresco/rest/api/model/Document.java

@@ -48,9 +48,9 @@ public class Document extends Node
         super();
     }
 
-    public Document(NodeRef nodeRef, NodeRef parentNodeRef, Map<QName, Serializable> nodeProps, ServiceRegistry sr)
+    public Document(NodeRef nodeRef, NodeRef parentNodeRef, Map<QName, Serializable> nodeProps, Map<String, UserInfo> mapUserInfo, ServiceRegistry sr)
     {
-        super(nodeRef, parentNodeRef, nodeProps, sr);
+        super(nodeRef, parentNodeRef, nodeProps, mapUserInfo, sr);
 
         Serializable val = nodeProps.get(ContentModel.PROP_CONTENT);
 

source/java/org/alfresco/rest/api/model/Folder.java

@@ -44,9 +44,9 @@ public class Folder extends Node
         super();
     }
 
-    public Folder(NodeRef nodeRef, NodeRef parentNodeRef, Map<QName, Serializable> nodeProps, ServiceRegistry sr)
+    public Folder(NodeRef nodeRef, NodeRef parentNodeRef, Map<QName, Serializable> nodeProps, Map<String, UserInfo> mapUserInfo, ServiceRegistry sr)
     {
-        super(nodeRef, parentNodeRef, nodeProps, sr);
+        super(nodeRef, parentNodeRef, nodeProps, mapUserInfo, sr);
     }
 
     @Override

source/java/org/alfresco/rest/api/model/Node.java

@@ -20,6 +20,7 @@ package org.alfresco.rest.api.model;
 
 import java.io.Serializable;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -28,6 +29,7 @@ import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.rest.framework.resource.UniqueId;
 import org.alfresco.service.ServiceRegistry;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.security.NoSuchPersonException;
 import org.alfresco.service.cmr.security.PersonService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.util.EqualsHelper;
@@ -60,9 +62,7 @@ public class Node implements Comparable<Node>
 
     protected Map<String, Object> properties;
 
-    // TODO fixme !
+    public Node(NodeRef nodeRef, NodeRef parentNodeRef, Map<QName, Serializable> nodeProps, Map<String, UserInfo> mapUserInfo, ServiceRegistry sr)
-    // also need to optionally pass in user map - eg. when listing children (to avoid multiple lookups for same user)
-    public Node(NodeRef nodeRef, NodeRef parentNodeRef, Map<QName, Serializable> nodeProps, ServiceRegistry sr)
     {
         if(nodeRef == null)
         {
@@ -72,7 +72,7 @@ public class Node implements Comparable<Node>
         this.nodeRef = nodeRef;
         this.parentNodeRef = parentNodeRef;
 
-        mapMinimalInfo(nodeProps, sr);
+        mapMinimalInfo(nodeProps, mapUserInfo, sr);
     }
 
     protected Object getValue(Map<String, PropertyData<?>> props, String name)
@@ -86,39 +86,52 @@ public class Node implements Comparable<Node>
     {
     }
 
-    protected void mapMinimalInfo(Map<QName, Serializable> nodeProps, ServiceRegistry sr)
+    protected void mapMinimalInfo(Map<QName, Serializable> nodeProps,  Map<String, UserInfo> mapUserInfo, ServiceRegistry sr)
     {
         PersonService personService = sr.getPersonService();
 
-        // TODO review backwards compat' for favorites & others (eg. set guid explicitly where still needed)
-        //this.guid = nodeRef;
-        //this.title = (String)nodeProps.get(ContentModel.PROP_TITLE);
-        //this.description = (String)nodeProps.get(ContentModel.PROP_DESCRIPTION);
-        //this.createdBy = (String)nodeProps.get(ContentModel.PROP_CREATOR);
-        //this.modifiedBy = (String)nodeProps.get(ContentModel.PROP_MODIFIER);
-
         this.name = (String)nodeProps.get(ContentModel.PROP_NAME);
 
+        if (mapUserInfo == null) {
+            // minor: save one lookup if creator & modifier are the same
+            mapUserInfo = new HashMap<>(2);
+        }
+
         this.createdAt = (Date)nodeProps.get(ContentModel.PROP_CREATED);
-        this.createdByUser = lookupUserInfo((String)nodeProps.get(ContentModel.PROP_CREATOR), personService);
+        this.createdByUser = lookupUserInfo((String)nodeProps.get(ContentModel.PROP_CREATOR), mapUserInfo, personService);
 
         this.modifiedAt = (Date)nodeProps.get(ContentModel.PROP_MODIFIED);
-        this.modifiedByUser = lookupUserInfo((String)nodeProps.get(ContentModel.PROP_MODIFIER), personService);
+        this.modifiedByUser = lookupUserInfo((String)nodeProps.get(ContentModel.PROP_MODIFIER), mapUserInfo, personService);
     }
 
-    // TODO refactor & optimise to avoid multiple person lookups
+    public static UserInfo lookupUserInfo(String userName, Map<String, UserInfo> mapUserInfo, PersonService personService) {
-    private UserInfo lookupUserInfo(final String userName, final PersonService personService) {
 
-        String sysUserName = AuthenticationUtil.getSystemUserName();
+        UserInfo userInfo = mapUserInfo.get(userName);
-        if (userName.equals(sysUserName) || (AuthenticationUtil.isMtEnabled() && userName.startsWith(sysUserName+"@")))
+        if (userInfo == null)
         {
-            return new UserInfo(userName, userName, "");
+            String sysUserName = AuthenticationUtil.getSystemUserName();
-        }
+            if (userName.equals(sysUserName) || (AuthenticationUtil.isMtEnabled() && userName.startsWith(sysUserName + "@")))
-        else
+            {
-        {
+                userInfo = new UserInfo(userName, userName, "");
-            PersonService.PersonInfo pInfo = personService.getPerson(personService.getPerson(userName));
+            }
-            return new UserInfo(userName, pInfo.getFirstName(), pInfo.getLastName());
+            else
+            {
+                try
+                {
+                    PersonService.PersonInfo pInfo = personService.getPerson(personService.getPerson(userName));
+                    userInfo = new UserInfo(userName, pInfo.getFirstName(), pInfo.getLastName());
+                }
+                catch (NoSuchPersonException nspe)
+                {
+                    // belts-and-braces (seen in dev/test env, eg. userName = Bobd58ba329-b702-41ee-a9ae-2b3c7029b5bc
+                    userInfo = new UserInfo(userName, userName, "");
+                }
+
+            }
+
+            mapUserInfo.put(userName, userInfo);
         }
+        return userInfo;
     }
 
     @UniqueId

source/java/org/alfresco/rest/api/model/UserInfo.java

@@ -19,12 +19,14 @@
 
 package org.alfresco.rest.api.model;
 
+import java.io.Serializable;
+
 /**
  * Representation of a user info
  *
  * @author janv
  */
-public class UserInfo
+public class UserInfo implements Serializable
 {
     private String userName;
     private String displayName;

source/java/org/alfresco/rest/api/nodes/NodesEntityResource.java

@@ -63,7 +63,7 @@ public class NodesEntityResource implements
     /**
      * Returns information regarding the node 'nodeId' - folder or document
      * 
-     * @param nodeId String id of node (folder or document) - will also accept well-known aliases, eg. "-root-" or "-my-"
+     * @param nodeId String id of node (folder or document) - will also accept well-known aliases, eg. "-root-", "-my-", "-shared-"
      * 
      * Optional parameters:
      * - path