From a3321ab7166dba6879444074d8cac5a11c992f76 Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Thu, 15 Jan 2015 08:38:37 +0100
Subject: [PATCH] OO-1391: replace the search by userProperties map in User by
an "exists" using a new and independent mapping of user property.
---
.../basesecurity/BaseSecurityManager.java | 34 ++++--
.../olat/basesecurity/model/UserProperty.java | 111 ++++++++++++++++++
.../persistence/_spring/core_persistence.xml | 1 +
.../GetIdentitiesByPowerSearchTest.java | 15 ++-
4 files changed, 143 insertions(+), 18 deletions(-)
create mode 100644 src/main/java/org/olat/basesecurity/model/UserProperty.java
diff --git a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
index 6c4615e9a96..c72653c6f3f 100644
--- a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
+++ b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
@@ -853,11 +853,11 @@ public class BaseSecurityManager extends BasicManager implements BaseSecurity {
@Override
public SecurityGroup findSecurityGroupByName(String securityGroupName) {
StringBuilder sb = new StringBuilder();
- sb.append("select sgi from ").append(NamedGroupImpl.class.getName()).append(" as ngroup, ")
- .append(SecurityGroupImpl.class.getName()).append(" as sgi ")
- .append(" where ngroup.groupName=:groupName and ngroup.securityGroup=sgi");
+ sb.append("select sgi from ").append(NamedGroupImpl.class.getName()).append(" as ngroup ")
+ .append(" inner join ngroup.securityGroup sgi")
+ .append(" where ngroup.groupName=:groupName");
- List<SecurityGroup> group = this.dbInstance.getCurrentEntityManager()
+ List<SecurityGroup> group = dbInstance.getCurrentEntityManager()
.createQuery(sb.toString(), SecurityGroup.class)
.setParameter("groupName", securityGroupName)
.setHint("org.hibernate.cacheable", Boolean.TRUE)
@@ -1392,7 +1392,7 @@ public class BaseSecurityManager extends BasicManager implements BaseSecurity {
// In any case join with the user. Don't join-fetch user, this breaks the query
// because of the user fields (don't know exactly why this behaves like
// this)
- sb.append(" join ident.user as user ");
+ sb.append(" inner join fetch ident.user as user ");
if (hasGroups) {
// join over security group memberships
@@ -1461,23 +1461,27 @@ public class BaseSecurityManager extends BasicManager implements BaseSecurity {
// handle email fields special: search in all email fields
if (!emailProperties.isEmpty()) {
- needsUserPropertiesJoin = checkIntersectionInUserProperties(sb,needsUserPropertiesJoin, params.isUserPropertiesAsIntersectionSearch());
+ needsUserPropertiesJoin = checkIntersectionInUserProperties(sb, needsUserPropertiesJoin, params.isUserPropertiesAsIntersectionSearch());
boolean moreThanOne = emailProperties.size() > 1;
if (moreThanOne) sb.append("(");
boolean needsOr = false;
for (String key : emailProperties.keySet()) {
if (needsOr) sb.append(" or ");
- //fxdiff
+
+ sb.append(" exists (select prop").append(key).append(".value from userproperty prop").append(key).append(" where ")
+ .append(" prop").append(key).append(".propertyId.userId=user.key and prop").append(key).append(".propertyId.name ='").append(key).append("'")
+ .append(" and ");
if(dbVendor.equals("mysql")) {
- sb.append(" user.userProperties['").append(key).append("'] like :").append(key).append("_value ");
+ sb.append(" prop").append(key).append(".value like :").append(key).append("_value ");
} else {
- sb.append(" lower(user.userProperties['").append(key).append("']) like :").append(key).append("_value ");
+ sb.append(" lower(prop").append(key).append(".value) like :").append(key).append("_value ");
}
if(dbVendor.equals("oracle")) {
sb.append(" escape '\\'");
}
+ sb.append(")");
needsOr = true;
- }
+ }
if (moreThanOne) sb.append(")");
// cleanup
emailProperties.clear();
@@ -1485,15 +1489,19 @@ public class BaseSecurityManager extends BasicManager implements BaseSecurity {
// add other fields
for (String key : otherProperties.keySet()) {
- needsUserPropertiesJoin = checkIntersectionInUserProperties(sb,needsUserPropertiesJoin, params.isUserPropertiesAsIntersectionSearch());
+ needsUserPropertiesJoin = checkIntersectionInUserProperties(sb, needsUserPropertiesJoin, params.isUserPropertiesAsIntersectionSearch());
+ sb.append(" exists (select prop").append(key).append(".value from userproperty prop").append(key).append(" where ")
+ .append(" prop").append(key).append(".propertyId.userId=user.key and prop").append(key).append(".propertyId.name ='").append(key).append("'")
+ .append(" and ");
if(dbVendor.equals("mysql")) {
- sb.append(" user.userProperties['").append(key).append("'] like :").append(key).append("_value ");
+ sb.append(" prop").append(key).append(".value like :").append(key).append("_value ");
} else {
- sb.append(" lower(user.userProperties['").append(key).append("']) like :").append(key).append("_value ");
+ sb.append(" lower(prop").append(key).append(".value) like :").append(key).append("_value ");
}
if(dbVendor.equals("oracle")) {
sb.append(" escape '\\'");
}
+ sb.append(")");
needsAnd = true;
}
// cleanup
diff --git a/src/main/java/org/olat/basesecurity/model/UserProperty.java b/src/main/java/org/olat/basesecurity/model/UserProperty.java
new file mode 100644
index 00000000000..a7f622cf710
--- /dev/null
+++ b/src/main/java/org/olat/basesecurity/model/UserProperty.java
@@ -0,0 +1,111 @@
+/**
+ * <a href="http://www.openolat.org">
+ * OpenOLAT - Online Learning and Training</a><br>
+ * <p>
+ * Licensed under the Apache License, Version 2.0 (the "License"); <br>
+ * you may not use this file except in compliance with the License.<br>
+ * You may obtain a copy of the License at the
+ * <a href="http://www.apache.org/licenses/LICENSE-2.0">Apache homepage</a>
+ * <p>
+ * Unless required by applicable law or agreed to in writing,<br>
+ * software distributed under the License is distributed on an "AS IS" BASIS, <br>
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. <br>
+ * See the License for the specific language governing permissions and <br>
+ * limitations under the License.
+ * <p>
+ * Initial code contributed and copyrighted by<br>
+ * frentix GmbH, http://www.frentix.com
+ * <p>
+ */
+package org.olat.basesecurity.model;
+
+import java.io.Serializable;
+
+import javax.persistence.Column;
+import javax.persistence.Embeddable;
+import javax.persistence.EmbeddedId;
+import javax.persistence.Entity;
+import javax.persistence.Table;
+
+/**
+ *
+ * This is only an helper mapping for the power search. Don't use it to persist
+ * or update user property.
+ *
+ * Initial date: 14.01.2015<br>
+ * @author srosse, stephane.rosse@frentix.com, http://www.frentix.com
+ *
+ */
+@Table(name="o_userproperty")
+@Entity(name="userproperty")
+public class UserProperty {
+
+ @EmbeddedId
+ private UserPropertyId propertyId;
+
+ @Column(name="propvalue", nullable=false, insertable=false, updatable=false)
+ private String value;
+
+ public String getValue() {
+ return value;
+ }
+
+ public void setValue(String value) {
+ this.value = value;
+ }
+
+
+ @Embeddable
+ public static class UserPropertyId implements Serializable {
+
+ private static final long serialVersionUID = -1215870965613146741L;
+
+ @Column(name = "fk_user_id")
+ private Long userId;
+ @Column(name = "propname")
+ private String name;
+
+ public UserPropertyId() {
+ //
+ }
+
+ public Long getUserId() {
+ return userId;
+ }
+
+
+ public void setUserId(Long userId) {
+ this.userId = userId;
+ }
+
+
+ public String getName() {
+ return name;
+ }
+
+
+ public void setName(String name) {
+ this.name = name;
+ }
+
+
+ @Override
+ public int hashCode() {
+ return (userId == null ? 478 : userId.hashCode())
+ + (name == null ? 765912 : name.hashCode());
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if(this == obj) {
+ return true;
+ }
+ if(obj instanceof UserPropertyId) {
+ UserPropertyId id = (UserPropertyId)obj;
+ return userId != null && userId.equals(id.userId)
+ && name != null && name.equals(id.name);
+ }
+ return false;
+ }
+ }
+}
\ No newline at end of file
diff --git a/src/main/java/org/olat/core/commons/persistence/_spring/core_persistence.xml b/src/main/java/org/olat/core/commons/persistence/_spring/core_persistence.xml
index cf5e5776f78..65b2edbd6bd 100644
--- a/src/main/java/org/olat/core/commons/persistence/_spring/core_persistence.xml
+++ b/src/main/java/org/olat/core/commons/persistence/_spring/core_persistence.xml
@@ -92,6 +92,7 @@
<class>org.olat.basesecurity.model.GroupImpl</class>
<class>org.olat.basesecurity.model.GrantImpl</class>
<class>org.olat.basesecurity.model.GroupMembershipImpl</class>
+ <class>org.olat.basesecurity.model.UserProperty</class>
<class>org.olat.core.dispatcher.mapper.model.PersistedMapper</class>
<class>org.olat.core.commons.services.notifications.model.SubscriberImpl</class>
<class>org.olat.core.commons.services.notifications.model.PublisherImpl</class>
diff --git a/src/test/java/org/olat/basesecurity/GetIdentitiesByPowerSearchTest.java b/src/test/java/org/olat/basesecurity/GetIdentitiesByPowerSearchTest.java
index 2bc3ea68c97..3049c953a32 100644
--- a/src/test/java/org/olat/basesecurity/GetIdentitiesByPowerSearchTest.java
+++ b/src/test/java/org/olat/basesecurity/GetIdentitiesByPowerSearchTest.java
@@ -39,6 +39,7 @@ import java.util.UUID;
import junit.framework.Assert;
import org.junit.Test;
+import org.olat.core.commons.persistence.DB;
import org.olat.core.commons.persistence.DBFactory;
import org.olat.core.id.Identity;
import org.olat.core.id.Roles;
@@ -60,6 +61,8 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
private static final OLog log = Tracing.createLoggerFor(GetIdentitiesByPowerSearchTest.class);
+ @Autowired
+ private DB dbInstance;
@Autowired
private BaseSecurity baseSecurityManager;
@@ -69,12 +72,12 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
Identity ident = JunitTestHelper.createAndPersistIdentityAsUser("anIdentity-" + suffix);
Identity uniIdent = getOrCreateTestIdentity("extremegroovy-" + suffix);
Assert.assertNotNull(uniIdent);
- Identity deletedIdent = getOrCreateTestIdentity("delete");
+ Identity deletedIdent = getOrCreateTestIdentity("delete-" + suffix);
deletedIdent = baseSecurityManager.saveIdentityStatus(deletedIdent, Identity.STATUS_DELETED);
SecurityGroup admins = baseSecurityManager.findSecurityGroupByName(Constants.GROUP_ADMIN);
baseSecurityManager.addIdentityToSecurityGroup(deletedIdent, admins);
-
+ dbInstance.commitAndCloseSession();
// basic query to find all system users without restrictions
List<Identity> results = baseSecurityManager.getIdentitiesByPowerSearch(null, null, true, null, null, null, null, null, null, null, null);
@@ -116,12 +119,12 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
Identity ident = JunitTestHelper.createAndPersistIdentityAsUser("anIdentity-" + suffix);
Identity uniIdent = getOrCreateTestIdentity("extremegroovy-" + suffix);
Assert.assertNotNull(uniIdent);
- Identity deletedIdent = getOrCreateTestIdentity("delete");
+ Identity deletedIdent = getOrCreateTestIdentity("delete-" + suffix);
deletedIdent = baseSecurityManager.saveIdentityStatus(deletedIdent, Identity.STATUS_DELETED);
SecurityGroup admins = baseSecurityManager.findSecurityGroupByName(Constants.GROUP_ADMIN);
baseSecurityManager.addIdentityToSecurityGroup(deletedIdent, admins);
-
+ dbInstance.commitAndCloseSession();
//search institutional name with *zh2
Map<String, String> userProperties = new HashMap<String, String>();
@@ -198,6 +201,8 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
Assert.assertFalse(results.contains(ident));
Assert.assertTrue(results.contains(uniIdent));
Assert.assertFalse(results.contains(deletedIdent));
+
+ dbInstance.commitAndCloseSession();
}
@Test
@@ -220,7 +225,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
baseSecurityManager.addIdentityToSecurityGroup(ident, authors);
// security group search test
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
SecurityGroup[] groups1 = {admins};
SecurityGroup[] groups2 = {admins, authors};
SecurityGroup[] groups3 = {authors};
--
GitLab