From 7cb781c7639c29e663a9f853e57d98089263027f Mon Sep 17 00:00:00 2001 From: Gustavo Durand Date: Fri, 26 Feb 2016 17:10:15 -0500 Subject: [PATCH 1/3] performance fix for autocomplete plus related issues #2027, #1669, #2834 --- .../dataverse/ManageFilePermissionsPage.java | 22 +---------- .../iq/dataverse/ManageGroupsPage.java | 35 +++++------------- .../iq/dataverse/ManagePermissionsPage.java | 26 ++----------- .../iq/dataverse/RoleAssigneeServiceBean.java | 37 +++++++++++++++++++ .../users/AuthenticatedUser.java | 7 +++- .../webapp/explicitGroup-new-dialog.xhtml | 5 ++- src/main/webapp/manage-groups.xhtml | 5 ++- .../webapp/permissions-manage-files.xhtml | 8 +++- src/main/webapp/roles-assign.xhtml | 4 +- 9 files changed, 73 insertions(+), 76 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/ManageFilePermissionsPage.java b/src/main/java/edu/harvard/iq/dataverse/ManageFilePermissionsPage.java index 7d5c15a0867..7381df8a052 100644 --- a/src/main/java/edu/harvard/iq/dataverse/ManageFilePermissionsPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/ManageFilePermissionsPage.java @@ -268,7 +268,6 @@ private void revokeRole(Long roleAssignmentId) { */ private List selectedRoleAssignees; private List selectedFiles = new ArrayList(); - private final List roleAssigneeList = new ArrayList(); private AuthenticatedUser fileRequester; public List getSelectedRoleAssignees() { @@ -316,26 +315,7 @@ public void initAssignDialogForFileRequester(AuthenticatedUser au) { public List completeRoleAssignee(String query) { - if (roleAssigneeList.isEmpty()) { - for (AuthenticatedUser au : authenticationService.findAllAuthenticatedUsers()) { - roleAssigneeList.add(au); - } - for ( Group g : groupService.findGlobalGroups() ) { - roleAssigneeList.add( g ); - } - roleAssigneeList.addAll( explicitGroupService.findAvailableFor(dataset) ); - - } - List returnList = new ArrayList(); - for (RoleAssignee ra : roleAssigneeList) { - // @todo unsure if containsIgnore case will work for all locales - if ((StringUtils.containsIgnoreCase(ra.getDisplayInfo().getTitle(), query) - || StringUtils.containsIgnoreCase(ra.getIdentifier(), query)) - && (selectedRoleAssignees == null || !selectedRoleAssignees.contains(ra))) { - returnList.add(ra); - } - } - return returnList; + return roleAssigneeService.filterRoleAssignees(query, dataset, selectedRoleAssignees); } public void grantAccess(ActionEvent evt) { diff --git a/src/main/java/edu/harvard/iq/dataverse/ManageGroupsPage.java b/src/main/java/edu/harvard/iq/dataverse/ManageGroupsPage.java index b03b1997c31..9aaa060e4dc 100644 --- a/src/main/java/edu/harvard/iq/dataverse/ManageGroupsPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/ManageGroupsPage.java @@ -258,36 +258,21 @@ public void removeMemberFromSelectedGroup(RoleAssignee ra) { } public List completeRoleAssignee( String query ) { - // TODO eliminate redundancy - List roleAssigneeList = new ArrayList<>(); - // TODO push this to the authentication and group services. Below code retrieves all the users. - for (AuthenticatedUser au : authenticationService.findAllAuthenticatedUsers()) { - roleAssigneeList.add(au); - } - for ( Group g : groupService.findGlobalGroups() ) { - roleAssigneeList.add( g ); - } - roleAssigneeList.addAll( explicitGroupService.findAvailableFor(dataverse) ); - - List filteredList = new LinkedList(); - for (RoleAssignee ra : roleAssigneeList) { - // @todo unsure if containsIgnore case will work for all locales - // @todo maybe add some solr/lucene style searching, did-you-mean style? - if (StringUtils.containsIgnoreCase(ra.getDisplayInfo().getTitle(), query)) { - filteredList.add(ra); - } - } - // Remove assignees already assigned to this group + + List alreadyAssignedRoleAssignees = new ArrayList<>(); + if (this.getNewExplicitGroupRoleAssignees() != null) { - filteredList.removeAll(this.getNewExplicitGroupRoleAssignees()); + alreadyAssignedRoleAssignees.addAll(this.getNewExplicitGroupRoleAssignees()); } if (this.getSelectedGroupRoleAssignees() != null) { - filteredList.removeAll(this.getSelectedGroupRoleAssignees()); + alreadyAssignedRoleAssignees.addAll(this.getSelectedGroupRoleAssignees()); } if (this.getSelectedGroupAddRoleAssignees() != null) { - filteredList.removeAll(this.getSelectedGroupAddRoleAssignees()); - } - return filteredList; + alreadyAssignedRoleAssignees.addAll(this.getSelectedGroupAddRoleAssignees()); + } + + return roleAssigneeService.filterRoleAssignees(query, dataverse, alreadyAssignedRoleAssignees); + } /* diff --git a/src/main/java/edu/harvard/iq/dataverse/ManagePermissionsPage.java b/src/main/java/edu/harvard/iq/dataverse/ManagePermissionsPage.java index 83d48574bf9..8d1a006cec2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/ManagePermissionsPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/ManagePermissionsPage.java @@ -320,31 +320,11 @@ public void initAssigneeDialog(ActionEvent ae) { selectedRoleId = null; showNoMessages(); } - + public List completeRoleAssignee( String query ) { - List roleAssigneeList = new ArrayList<>(); - // TODO push this to the authentication and group services. Below code retrieves all the users. - for (AuthenticatedUser au : authenticationService.findAllAuthenticatedUsers()) { - roleAssigneeList.add(au); - } - for ( Group g : groupService.findGlobalGroups() ) { - roleAssigneeList.add( g ); - } - roleAssigneeList.addAll( explicitGroupService.findAvailableFor(dvObject) ); - - List filteredList = new LinkedList(); - for (RoleAssignee ra : roleAssigneeList) { - // @todo unsure if containsIgnore case will work for all locales - // @todo maybe add some solr/lucene style searching, did-you-mean style? - if ((StringUtils.containsIgnoreCase(ra.getDisplayInfo().getTitle(), query) - || StringUtils.containsIgnoreCase(ra.getIdentifier(), query)) - && (roleAssignSelectedRoleAssignees == null || !roleAssignSelectedRoleAssignees.contains(ra))) { - filteredList.add(ra); - } - } - return filteredList; + return roleAssigneeService.filterRoleAssignees(query, dvObject, roleAssignSelectedRoleAssignees); } - + public List getAvailableRoles() { List roles = new LinkedList<>(); if (dvObject != null && dvObject.getId() != null) { diff --git a/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java index 5ec6d7d6299..c269740957a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java @@ -3,13 +3,16 @@ import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.DataverseRole; import edu.harvard.iq.dataverse.authorization.RoleAssignee; +import edu.harvard.iq.dataverse.authorization.groups.Group; import edu.harvard.iq.dataverse.authorization.groups.GroupServiceBean; import edu.harvard.iq.dataverse.authorization.groups.impl.builtin.AllUsers; import edu.harvard.iq.dataverse.authorization.groups.impl.builtin.AuthenticatedUsers; import edu.harvard.iq.dataverse.authorization.groups.impl.explicit.ExplicitGroup; +import edu.harvard.iq.dataverse.authorization.groups.impl.explicit.ExplicitGroupServiceBean; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.GuestUser; import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -41,6 +44,9 @@ public class RoleAssigneeServiceBean { @EJB GroupServiceBean groupSvc; + @EJB + ExplicitGroupServiceBean explicitGroupSvc; + @EJB DataverseRoleServiceBean dataverseRoleService; @@ -265,6 +271,37 @@ private List getUserGroups(String roleAssigneeIdentifier){ .getResultList(); } + + public List filterRoleAssignees(String query, DvObject dvObject, List roleAssignSelectedRoleAssignees) { + List roleAssigneeList = new ArrayList<>(); + + em.createNamedQuery("AuthenticatedUser.filter", AuthenticatedUser.class) + .setParameter("query", "%" + query + "%") + .getResultList().stream() + .filter(ra -> roleAssignSelectedRoleAssignees == null || !roleAssignSelectedRoleAssignees.contains(ra)) + .forEach((ra) -> { + roleAssigneeList.add(ra); + }); + + groupSvc.findGlobalGroups().stream() + .filter(ra -> StringUtils.containsIgnoreCase(ra.getDisplayInfo().getTitle(), query)) + .filter(ra -> StringUtils.containsIgnoreCase(ra.getIdentifier(), query)) + .filter(ra -> roleAssignSelectedRoleAssignees == null || !roleAssignSelectedRoleAssignees.contains(ra)) + .forEach((ra) -> { + roleAssigneeList.add(ra); + }); + + explicitGroupSvc.findAvailableFor(dvObject).stream() + .filter(ra -> StringUtils.containsIgnoreCase(ra.getDisplayInfo().getTitle(), query)) + .filter(ra -> StringUtils.containsIgnoreCase(ra.getIdentifier(), query)) + .filter(ra -> roleAssignSelectedRoleAssignees == null || !roleAssignSelectedRoleAssignees.contains(ra)) + .forEach((ra) -> { + roleAssigneeList.add(ra); + }); + + return roleAssigneeList; + } + private void msg(String s){ //System.out.println(s); } diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java index e789a8dfb70..befcfbac5b8 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java @@ -33,7 +33,12 @@ @NamedQuery( name="AuthenticatedUser.findByEmail", query="select au from AuthenticatedUser au WHERE au.email=:email"), @NamedQuery( name="AuthenticatedUser.countOfIdentifier", - query="SELECT COUNT(a) FROM AuthenticatedUser a WHERE a.userIdentifier=:identifier") + query="SELECT COUNT(a) FROM AuthenticatedUser a WHERE a.userIdentifier=:identifier"), + @NamedQuery( name="AuthenticatedUser.filter", + query="select au from AuthenticatedUser au WHERE (" + + "au.userIdentifier like :query OR " + + "lower(concat(au.firstName,' ',au.lastName)) like lower(:query))") + }) @Entity public class AuthenticatedUser implements User, Serializable { diff --git a/src/main/webapp/explicitGroup-new-dialog.xhtml b/src/main/webapp/explicitGroup-new-dialog.xhtml index e9c336845b7..910ff17cbd1 100644 --- a/src/main/webapp/explicitGroup-new-dialog.xhtml +++ b/src/main/webapp/explicitGroup-new-dialog.xhtml @@ -57,7 +57,10 @@
diff --git a/src/main/webapp/roles-assign.xhtml b/src/main/webapp/roles-assign.xhtml index 6b4b9c36af0..93d47309e89 100644 --- a/src/main/webapp/roles-assign.xhtml +++ b/src/main/webapp/roles-assign.xhtml @@ -20,8 +20,8 @@
Date: Fri, 26 Feb 2016 20:40:24 -0500 Subject: [PATCH 2/3] some clenaup plus a fix in filtering logic #2027 --- .../iq/dataverse/RoleAssigneeServiceBean.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java index c269740957a..de286c1dec1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java @@ -12,9 +12,9 @@ import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.GuestUser; import java.util.ArrayList; -import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeMap; import java.util.logging.Logger; import javax.annotation.PostConstruct; @@ -275,6 +275,8 @@ private List getUserGroups(String roleAssigneeIdentifier){ public List filterRoleAssignees(String query, DvObject dvObject, List roleAssignSelectedRoleAssignees) { List roleAssigneeList = new ArrayList<>(); + // we get the users through a query that does the filtering through the db, + // so that we don't have to instantiate all of the RoleAssignee objects em.createNamedQuery("AuthenticatedUser.filter", AuthenticatedUser.class) .setParameter("query", "%" + query + "%") .getResultList().stream() @@ -283,22 +285,17 @@ public List filterRoleAssignees(String query, DvObject dvObject, L roleAssigneeList.add(ra); }); - groupSvc.findGlobalGroups().stream() - .filter(ra -> StringUtils.containsIgnoreCase(ra.getDisplayInfo().getTitle(), query)) - .filter(ra -> StringUtils.containsIgnoreCase(ra.getIdentifier(), query)) + // now we add groups to the list, both global and explicit + Set groups = groupSvc.findGlobalGroups(); + groups.addAll(explicitGroupSvc.findAvailableFor(dvObject)); + groups.stream() + .filter(ra -> StringUtils.containsIgnoreCase(ra.getDisplayInfo().getTitle(), query) + || StringUtils.containsIgnoreCase(ra.getIdentifier(), query)) .filter(ra -> roleAssignSelectedRoleAssignees == null || !roleAssignSelectedRoleAssignees.contains(ra)) .forEach((ra) -> { roleAssigneeList.add(ra); }); - explicitGroupSvc.findAvailableFor(dvObject).stream() - .filter(ra -> StringUtils.containsIgnoreCase(ra.getDisplayInfo().getTitle(), query)) - .filter(ra -> StringUtils.containsIgnoreCase(ra.getIdentifier(), query)) - .filter(ra -> roleAssignSelectedRoleAssignees == null || !roleAssignSelectedRoleAssignees.contains(ra)) - .forEach((ra) -> { - roleAssigneeList.add(ra); - }); - return roleAssigneeList; } From 46abb4bb8eedd39b324cb54b72564ea01dc3428c Mon Sep 17 00:00:00 2001 From: Gustavo Durand Date: Mon, 29 Feb 2016 17:09:16 -0500 Subject: [PATCH 3/3] Readded some things that got lost in merge --- .../iq/dataverse/ManageFilePermissionsPage.java | 14 +++++++------- .../iq/dataverse/ManagePermissionsPage.java | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/ManageFilePermissionsPage.java b/src/main/java/edu/harvard/iq/dataverse/ManageFilePermissionsPage.java index 7381df8a052..4c67c5b1342 100644 --- a/src/main/java/edu/harvard/iq/dataverse/ManageFilePermissionsPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/ManageFilePermissionsPage.java @@ -78,9 +78,9 @@ public class ManageFilePermissionsPage implements java.io.Serializable { DataverseSession session; Dataset dataset = new Dataset(); - private final Map> roleAssigneeMap = new HashMap(); - private final Map> fileMap = new HashMap(); - private final Map> fileAccessRequestMap = new HashMap(); + private final Map> roleAssigneeMap = new HashMap<>(); + private final Map> fileMap = new HashMap<>(); + private final Map> fileAccessRequestMap = new HashMap<>(); public Dataset getDataset() { return dataset; @@ -134,7 +134,7 @@ private void initMaps() { if (file.getFileMetadata() != null && (file.isRestricted() || file.getFileMetadata().isRestricted())) { // we get the direct role assignments assigned to the file List ras = roleService.directRoleAssignments(file); - List raList = new ArrayList<>(ras.size()); + List raList = new ArrayList<>(ras.size()); for (RoleAssignment ra : ras) { // for files, only show role assignments which can download if (ra.getRole().permissions().contains(Permission.DownloadFile)) { @@ -149,7 +149,7 @@ private void initMaps() { for (AuthenticatedUser au : file.getFileAccessRequesters()) { List requestedFiles = fileAccessRequestMap.get(au); if (requestedFiles == null) { - requestedFiles = new ArrayList(); + requestedFiles = new ArrayList<>(); fileAccessRequestMap.put(au, requestedFiles); } @@ -165,7 +165,7 @@ private void addFileToRoleAssignee(RoleAssignment assignment) { RoleAssignee ra = roleAssigneeService.getRoleAssignee(assignment.getAssigneeIdentifier()); List assignments = roleAssigneeMap.get(ra); if (assignments == null) { - assignments = new ArrayList(); + assignments = new ArrayList<>(); roleAssigneeMap.put(ra, assignments); } @@ -267,7 +267,7 @@ private void revokeRole(Long roleAssignmentId) { grant access dialog */ private List selectedRoleAssignees; - private List selectedFiles = new ArrayList(); + private List selectedFiles = new ArrayList<>(); private AuthenticatedUser fileRequester; public List getSelectedRoleAssignees() { diff --git a/src/main/java/edu/harvard/iq/dataverse/ManagePermissionsPage.java b/src/main/java/edu/harvard/iq/dataverse/ManagePermissionsPage.java index 8d1a006cec2..36902669816 100644 --- a/src/main/java/edu/harvard/iq/dataverse/ManagePermissionsPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/ManagePermissionsPage.java @@ -183,7 +183,7 @@ public List getRoles() { if (dvObject != null && dvObject.getId() != null) { return roleService.findByOwnerId(dvObject.getId()); } - return new ArrayList(); + return new ArrayList<>(); } public void createNewRole(ActionEvent e) {