From 80887b8462385de7c45f7e15cc6e5965f07fa079 Mon Sep 17 00:00:00 2001 From: Kevin Roast Date: Wed, 6 Mar 2013 16:52:09 +0000 Subject: [PATCH] Merged BRANCHES/DEV/V4.1-BUG-FIX to HEAD 47670: ALF-18245 - BM-0013: Soak: Run 06: Search population of ScriptNode is expensive - Optimizations to improve the performance of the repository when retrieving the populating data needed for Share Search results - Additional service method to PersonService to allow retrieval of a person or null in a single operation - rather than exists() followed get() - Improvements to generate of full name strings for a Person - only retrieve minimum needed properties rather than allprops from cm:person - Improvements to generation of DisplayPath to avoid multiple permission checks git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@47674 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../public-services-security-context.xml | 1 + config/alfresco/script-services-context.xml | 1 + .../org/alfresco/repo/jscript/People.java | 32 +++++++-- .../alfresco/repo/jscript/ScriptUtils.java | 27 +++++++- .../security/person/PersonServiceImpl.java | 68 ++++++++++++++----- .../repo/security/person/PersonTest.java | 4 ++ .../service/cmr/security/PersonService.java | 14 ++++ 7 files changed, 122 insertions(+), 25 deletions(-) diff --git a/config/alfresco/public-services-security-context.xml b/config/alfresco/public-services-security-context.xml index 7b547be177..c3abd3c9d3 100644 --- a/config/alfresco/public-services-security-context.xml +++ b/config/alfresco/public-services-security-context.xml @@ -863,6 +863,7 @@ org.alfresco.service.cmr.security.PersonService.getPerson=ACL_ALLOW,AFTER_ACL_NODE.sys:base.ReadProperties + org.alfresco.service.cmr.security.PersonService.getPersonOrNull=ACL_ALLOW,AFTER_ACL_NODE.sys:base.ReadProperties org.alfresco.service.cmr.security.PersonService.personExists=ACL_ALLOW org.alfresco.service.cmr.security.PersonService.createMissingPeople=ACL_ALLOW org.alfresco.service.cmr.security.PersonService.setCreateMissingPeople=ACL_METHOD.ROLE_ADMINISTRATOR diff --git a/config/alfresco/script-services-context.xml b/config/alfresco/script-services-context.xml index 48d24af0c4..ddedc2b38d 100644 --- a/config/alfresco/script-services-context.xml +++ b/config/alfresco/script-services-context.xml @@ -67,6 +67,7 @@ + diff --git a/source/java/org/alfresco/repo/jscript/People.java b/source/java/org/alfresco/repo/jscript/People.java index 76b5c5e25b..5d67c4f441 100644 --- a/source/java/org/alfresco/repo/jscript/People.java +++ b/source/java/org/alfresco/repo/jscript/People.java @@ -837,23 +837,41 @@ public final class People extends BaseScopableProcessorExtension implements Init peopleRefs.toArray(people); return people; - } + } + /** * Gets the Person given the username * * @param username the username of the person to get * @return the person node (type cm:person) or null if no such person exists */ - public ScriptNode getPerson(String username) + public ScriptNode getPerson(final String username) { ParameterCheck.mandatoryString("Username", username); - ScriptNode person = null; - if (personService.personExists(username)) + final NodeRef personRef = personService.getPerson(username, false); + return new ScriptNode(personRef, services, getScope()); + } + + /** + * Faster helper when the script just wants to build the Full name for a person. + * Avoids complete getProperties() retrieval for a cm:person. + * + * @param username the username of the person to get Full name for + * @return full name for a person or null if the user does not exist in the system. + */ + public String getPersonFullName(final String username) + { + String name = null; + ParameterCheck.mandatoryString("Username", username); + final NodeRef personRef = personService.getPersonOrNull(username); + if (personRef != null) { - NodeRef personRef = personService.getPerson(username); - person = new ScriptNode(personRef, services, getScope()); + final NodeService nodeService = services.getNodeService(); + final String firstName = (String)nodeService.getProperty(personRef, ContentModel.PROP_FIRSTNAME); + final String lastName = (String)nodeService.getProperty(personRef, ContentModel.PROP_LASTNAME); + name = (firstName != null ? firstName + " " : "") + (lastName != null ? lastName : ""); } - return person; + return name; } /** diff --git a/source/java/org/alfresco/repo/jscript/ScriptUtils.java b/source/java/org/alfresco/repo/jscript/ScriptUtils.java index b6e453f219..1cfa6599e6 100644 --- a/source/java/org/alfresco/repo/jscript/ScriptUtils.java +++ b/source/java/org/alfresco/repo/jscript/ScriptUtils.java @@ -22,10 +22,12 @@ import java.util.Date; import java.util.Locale; import java.util.Map; +import org.alfresco.repo.security.permissions.noop.PermissionServiceNOOPImpl; import org.alfresco.service.ServiceRegistry; import org.alfresco.service.cmr.module.ModuleDetails; import org.alfresco.service.cmr.module.ModuleService; import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.util.ScriptPagingDetails; @@ -40,10 +42,13 @@ import org.springframework.extensions.surf.util.ISO8601DateFormat; public class ScriptUtils extends BaseScopableProcessorExtension { private final static String NAMESPACE_BEGIN = "" + QName.NAMESPACE_BEGIN; - + /** Services */ private ServiceRegistry services; + private NodeService unprotNodeService; + + /** * Sets the service registry * @@ -54,6 +59,26 @@ public class ScriptUtils extends BaseScopableProcessorExtension this.services = services; } + /** + * @param nodeService the NodeService to set + */ + public void setNodeService(NodeService nodeService) + { + this.unprotNodeService = nodeService; + } + + /** + * Function to return the cm:name display path for a node with minimum performance overhead. + * + * @param node ScriptNode + * @return cm:name based human readable display path for the give node. + */ + public String displayPath(ScriptNode node) + { + return this.unprotNodeService.getPath(node.nodeRef).toDisplayPath( + this.unprotNodeService, new PermissionServiceNOOPImpl()); + } + /** * Function to pad a string with zero '0' characters to the required length * diff --git a/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java b/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java index 236b8d9539..d8fb83303d 100644 --- a/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java +++ b/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java @@ -414,25 +414,56 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per return getPerson(userName, true); } + /** + * {@inheritDoc} + */ + public NodeRef getPersonOrNull(String userName) + { + return getPerson(userName, false, false); + } + /** * {@inheritDoc} */ public NodeRef getPerson(final String userName, final boolean autoCreateHomeFolderAndMissingPersonIfAllowed) { - return getPersonImpl(userName, autoCreateHomeFolderAndMissingPersonIfAllowed); + return getPerson(userName, autoCreateHomeFolderAndMissingPersonIfAllowed, true); } - - private NodeRef getPersonImpl(String userName, boolean autoCreateHomeFolderAndMissingPersonIfAllowed) + + private NodeRef getPerson( + final String userName, + final boolean autoCreateHomeFolderAndMissingPersonIfAllowed, + final boolean exceptionOrNull) { - if(userName == null) + // MT share - for activity service system callback + if (tenantService.isEnabled() && (AuthenticationUtil.SYSTEM_USER_NAME.equals(AuthenticationUtil.getRunAsUser())) && tenantService.isTenantUser(userName)) + { + final String tenantDomain = tenantService.getUserDomain(userName); + + return AuthenticationUtil.runAs(new AuthenticationUtil.RunAsWork() + { + public NodeRef doWork() throws Exception + { + return getPersonImpl(userName, autoCreateHomeFolderAndMissingPersonIfAllowed, exceptionOrNull); + } + }, tenantService.getDomainUser(AuthenticationUtil.getSystemUserName(), tenantDomain)); + } + else + { + return getPersonImpl(userName, autoCreateHomeFolderAndMissingPersonIfAllowed, exceptionOrNull); + } + } + + private NodeRef getPersonImpl( + final String userName, + final boolean autoCreateHomeFolderAndMissingPersonIfAllowed, + final boolean exceptionOrNull) + { + if (userName == null || userName.length() == 0) { return null; } - if(userName.length() == 0) - { - return null; - } - NodeRef personNode = getPersonOrNull(userName); + final NodeRef personNode = getPersonOrNullImpl(userName); if (personNode == null) { TxnReadState txnReadState = AlfrescoTransactionSupport.getTransactionReadState(); @@ -444,7 +475,10 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per } else { - throw new NoSuchPersonException(userName); + if (exceptionOrNull) + { + throw new NoSuchPersonException(userName); + } } } else if (autoCreateHomeFolderAndMissingPersonIfAllowed) @@ -459,10 +493,10 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per */ public boolean personExists(String caseSensitiveUserName) { - return getPersonOrNull(caseSensitiveUserName) != null; + return getPersonOrNullImpl(caseSensitiveUserName) != null; } - private NodeRef getPersonOrNull(String searchUserName) + private NodeRef getPersonOrNullImpl(String searchUserName) { Set allRefs = getFromCache(searchUserName); boolean addToCache = false; @@ -738,7 +772,7 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per */ public void setPersonProperties(String userName, Map properties, boolean autoCreateHomeFolder) { - NodeRef personNode = getPersonOrNull(userName); + NodeRef personNode = getPersonOrNullImpl(userName); if (personNode == null) { if (createMissingPeople()) @@ -1077,7 +1111,7 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per return; } - NodeRef personRef = getPersonOrNull(userName); + NodeRef personRef = getPersonOrNullImpl(userName); deletePersonImpl(userName, personRef); } @@ -1715,7 +1749,7 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per private void putToCache(String userName, Set refs) { String key = userName.toLowerCase(); - if(AlfrescoTransactionSupport.getTransactionId() != null && !TransactionalResourceHelper.getSet(DELETING_PERSON_SET_RESOURCE).contains(key)) + if (AlfrescoTransactionSupport.getTransactionId() != null && !TransactionalResourceHelper.getSet(DELETING_PERSON_SET_RESOURCE).contains(key)) { this.personCache.put(key, refs); } @@ -1733,7 +1767,7 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per */ public String getUserIdentifier(String caseSensitiveUserName) { - NodeRef nodeRef = getPersonOrNull(caseSensitiveUserName); + NodeRef nodeRef = getPersonOrNullImpl(caseSensitiveUserName); if ((nodeRef != null) && nodeService.exists(nodeRef)) { String realUserName = DefaultTypeConverter.INSTANCE.convert(String.class, nodeService.getProperty(nodeRef, ContentModel.PROP_USERNAME)); @@ -1857,7 +1891,7 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per return; } // Get the person - NodeRef personNodeRef = getPersonOrNull(userName); + NodeRef personNodeRef = getPersonOrNullImpl(userName); if (personNodeRef == null) { // Don't attempt to maintain enabled/disabled flag diff --git a/source/java/org/alfresco/repo/security/person/PersonTest.java b/source/java/org/alfresco/repo/security/person/PersonTest.java index 380d8a234f..c976da48b8 100644 --- a/source/java/org/alfresco/repo/security/person/PersonTest.java +++ b/source/java/org/alfresco/repo/security/person/PersonTest.java @@ -362,6 +362,8 @@ public class PersonTest extends TestCase } personService.createPerson(createDefaultProperties("andy", "Andy", "Hind", "andy@hind", "alfresco", rootNodeRef)); personService.getPerson("andy"); + // test getting with getPersonOrNull + assertNotNull(personService.getPersonOrNull("andy")); personService.deletePerson("andy"); try { @@ -372,6 +374,8 @@ public class PersonTest extends TestCase { } + // test getting with getPersonOrNull - no exception, just null returned + assertNull(personService.getPersonOrNull("andy")); } public void testCreateMissingPeople1() diff --git a/source/java/org/alfresco/service/cmr/security/PersonService.java b/source/java/org/alfresco/service/cmr/security/PersonService.java index 3f683d2c01..5ac684c364 100644 --- a/source/java/org/alfresco/service/cmr/security/PersonService.java +++ b/source/java/org/alfresco/service/cmr/security/PersonService.java @@ -65,6 +65,20 @@ public interface PersonService */ @Auditable(parameters = {"userName"}) public NodeRef getPerson(String userName); + + /** + * Get a person by userName. The person is store in the repository. No missing + * person objects will be created as a side effect of this call. If the person + * is missing from the repository null will be returned. + * + * @param userName - + * the userName key to find the person + * @return Returns the existing person node, or null if does not exist. + * + * @see #createMissingPeople() + */ + @Auditable(parameters = {"userName"}) + public NodeRef getPersonOrNull(String userName); /** * Retrieve the person NodeRef for a {@code username}, optionally creating