Skip to content

Commit

Permalink
[SYNCOPE-1855] Rewriting JPAAnySearchDAO to reduce subqueries (#962)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ilgrosso
ilgrosso authored Jan 23, 2025
1 parent e4ce8ef commit 061c94e
Show file tree
Hide file tree
Showing 74 changed files with 2,335 additions and 3,436 deletions.
4 changes: 2 additions & 2 deletions core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/RealmLogic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
import org.apache.syncope.core.persistence.api.dao.search.AttrCond;
import org.apache.syncope.core.persistence.api.dao.search.SearchCond;
import org.apache.syncope.core.persistence.api.entity.Realm;
import org.apache.syncope.core.persistence.api.search.SyncopePage;
import org.apache.syncope.core.persistence.api.entity.user.User;
import org.apache.syncope.core.persistence.api.search.SyncopePage;
import org.apache.syncope.core.provisioning.api.PropagationByResource;
import org.apache.syncope.core.provisioning.api.data.RealmDataBinder;
import org.apache.syncope.core.provisioning.api.propagation.PropagationManager;
Expand Down Expand Up @@ -219,7 +219,7 @@ public ProvisioningResult<RealmTO> delete(final String fullPath) {
Set<String> adminRealms = Set.of(realm.getFullPath());
AnyCond keyCond = new AnyCond(AttrCond.Type.ISNOTNULL);
keyCond.setSchema("key");
SearchCond allMatchingCond = SearchCond.getLeaf(keyCond);
SearchCond allMatchingCond = SearchCond.of(keyCond);
long users = searchDAO.count(realm, true, adminRealms, allMatchingCond, AnyTypeKind.USER);
long groups = searchDAO.count(realm, true, adminRealms, allMatchingCond, AnyTypeKind.GROUP);
long anyObjects = searchDAO.count(realm, true, adminRealms, allMatchingCond, AnyTypeKind.ANY_OBJECT);
Expand Down
2 changes: 1 addition & 1 deletion core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/SyncopeLogic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public Page<GroupTO> searchAssignableGroups(
termCond = new AnyCond(AttrCond.Type.ISNOTNULL);
termCond.setSchema("key");
}
SearchCond searchCond = SearchCond.getLeaf(termCond);
SearchCond searchCond = SearchCond.of(termCond);

long count = anySearchDAO.count(base, true, SyncopeConstants.FULL_ADMIN_REALMS, searchCond, AnyTypeKind.GROUP);

Expand Down
10 changes: 1 addition & 9 deletions core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/AnyDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@
import org.apache.syncope.core.persistence.api.entity.Any;
import org.apache.syncope.core.persistence.api.entity.DerSchema;
import org.apache.syncope.core.persistence.api.entity.ExternalResource;
import org.apache.syncope.core.persistence.api.entity.PlainAttrUniqueValue;
import org.apache.syncope.core.persistence.api.entity.PlainAttrValue;
import org.apache.syncope.core.persistence.api.entity.PlainSchema;
import org.apache.syncope.core.persistence.api.entity.Schema;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
Expand All @@ -45,11 +42,6 @@ public interface AnyDAO<A extends Any<?>> extends DAO<A> {

A authFind(String key);

List<A> findByPlainAttrValue(PlainSchema schema, PlainAttrValue attrValue, boolean ignoreCaseMatch);

Optional<A> findByPlainAttrUniqueValue(
PlainSchema schema, PlainAttrUniqueValue attrUniqueValue, boolean ignoreCaseMatch);

/**
* Find any objects by derived attribute value. This method could fail if one or more string literals contained
* into the derived attribute value provided derive from identifier (schema key) replacement. When you are going to
Expand All @@ -73,7 +65,7 @@ Optional<A> findByPlainAttrUniqueValue(
default SearchCond getAllMatchingCond() {
AnyCond idCond = new AnyCond(AttrCond.Type.ISNOTNULL);
idCond.setSchema("id");
return SearchCond.getLeaf(idCond);
return SearchCond.of(idCond);
}

<S extends Schema> AllowedSchemas<S> findAllowedSchemas(A any, Class<S> reference);
Expand Down
41 changes: 25 additions & 16 deletions ...ence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/search/SearchCond.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.apache.syncope.core.persistence.api.dao.search;

import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import org.apache.commons.lang3.builder.EqualsBuilder;
Expand Down Expand Up @@ -45,10 +46,10 @@ public enum Type {

private SearchCond right;

public static SearchCond getLeaf(final AbstractSearchCond leaf) {
public static SearchCond of(final AbstractSearchCond leaf) {
SearchCond cond;
if (leaf instanceof SearchCond) {
cond = (SearchCond) leaf;
if (leaf instanceof SearchCond searchCond) {
cond = searchCond;
} else {
cond = new SearchCond();
cond.leaf = leaf;
Expand All @@ -59,15 +60,15 @@ public static SearchCond getLeaf(final AbstractSearchCond leaf) {
return cond;
}

public static SearchCond getNotLeaf(final AbstractSearchCond leaf) {
SearchCond cond = getLeaf(leaf);
public static SearchCond negate(final AbstractSearchCond leaf) {
SearchCond cond = of(leaf);

cond.type = Type.NOT_LEAF;

return cond;
}

public static SearchCond getAnd(final SearchCond left, final SearchCond right) {
private static SearchCond and(final SearchCond left, final SearchCond right) {
SearchCond cond = new SearchCond();

cond.type = Type.AND;
Expand All @@ -77,17 +78,21 @@ public static SearchCond getAnd(final SearchCond left, final SearchCond right) {
return cond;
}

public static SearchCond getAnd(final List<SearchCond> conditions) {
public static SearchCond and(final List<SearchCond> conditions) {
if (conditions.size() == 1) {
return conditions.get(0);
} else if (conditions.size() > 2) {
return getAnd(conditions.get(0), getAnd(conditions.subList(1, conditions.size())));
return and(conditions.get(0), and(conditions.subList(1, conditions.size())));
} else {
return getAnd(conditions.get(0), conditions.get(1));
return and(conditions.get(0), conditions.get(1));
}
}

public static SearchCond getOr(final SearchCond left, final SearchCond right) {
public static SearchCond and(final SearchCond... conditions) {
return and(Arrays.asList(conditions));
}

private static SearchCond or(final SearchCond left, final SearchCond right) {
SearchCond cond = new SearchCond();

cond.type = Type.OR;
Expand All @@ -97,16 +102,20 @@ public static SearchCond getOr(final SearchCond left, final SearchCond right) {
return cond;
}

public static SearchCond getOr(final List<SearchCond> conditions) {
public static SearchCond or(final List<SearchCond> conditions) {
if (conditions.size() == 1) {
return conditions.get(0);
} else if (conditions.size() > 2) {
return getOr(conditions.get(0), getOr(conditions.subList(1, conditions.size())));
return or(conditions.get(0), or(conditions.subList(1, conditions.size())));
} else {
return getOr(conditions.get(0), conditions.get(1));
return or(conditions.get(0), conditions.get(1));
}
}

public static SearchCond or(final SearchCond... conditions) {
return or(Arrays.asList(conditions));
}

public Optional<AnyTypeCond> getAnyTypeCond() {
return Optional.ofNullable(leaf instanceof AnyTypeCond ? (AnyTypeCond) leaf : null);
}
Expand All @@ -126,8 +135,8 @@ public String hasAnyTypeCond() {
switch (type) {
case LEAF:
case NOT_LEAF:
if (leaf instanceof AnyTypeCond) {
anyTypeName = ((AnyTypeCond) leaf).getAnyTypeKey();
if (leaf instanceof AnyTypeCond anyTypeCond) {
anyTypeName = anyTypeCond.getAnyTypeKey();
}
break;

Expand All @@ -148,7 +157,7 @@ public String hasAnyTypeCond() {
}

@SuppressWarnings("unchecked")
public <T extends AbstractSearchCond> Optional<T> getLeaf(final Class<T> clazz) {
public <T extends AbstractSearchCond> Optional<T> asLeaf(final Class<T> clazz) {
return Optional.ofNullable((T) (clazz.isInstance(leaf) ? leaf : null));
}

Expand Down
42 changes: 21 additions & 21 deletions ...e-api/src/main/java/org/apache/syncope/core/persistence/api/search/SearchCondVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,67 +134,67 @@ protected SearchCond visitPrimitive(final SearchCondition<SearchBean> sc) {
: AttrCond.Type.LIKE);
}

leaf = SearchCond.getLeaf(attrCond);
leaf = SearchCond.of(attrCond);
} else {
switch (specialAttrName.get()) {
case TYPE:
AnyTypeCond typeCond = new AnyTypeCond();
typeCond.setAnyTypeKey(value);
leaf = SearchCond.getLeaf(typeCond);
leaf = SearchCond.of(typeCond);
break;

case AUX_CLASSES:
AuxClassCond auxClassCond = new AuxClassCond();
auxClassCond.setAuxClass(value);
leaf = SearchCond.getLeaf(auxClassCond);
leaf = SearchCond.of(auxClassCond);
break;

case RESOURCES:
ResourceCond resourceCond = new ResourceCond();
resourceCond.setResource(value);
leaf = SearchCond.getLeaf(resourceCond);
leaf = SearchCond.of(resourceCond);
break;

case GROUPS:
MembershipCond groupCond = new MembershipCond();
groupCond.setGroup(value);
leaf = SearchCond.getLeaf(groupCond);
leaf = SearchCond.of(groupCond);
break;

case RELATIONSHIPS:
RelationshipCond relationshipCond = new RelationshipCond();
relationshipCond.setAnyObject(value);
leaf = SearchCond.getLeaf(relationshipCond);
leaf = SearchCond.of(relationshipCond);
break;

case RELATIONSHIP_TYPES:
RelationshipTypeCond relationshipTypeCond = new RelationshipTypeCond();
relationshipTypeCond.setRelationshipTypeKey(value);
leaf = SearchCond.getLeaf(relationshipTypeCond);
leaf = SearchCond.of(relationshipTypeCond);
break;

case ROLES:
RoleCond roleCond = new RoleCond();
roleCond.setRole(value);
leaf = SearchCond.getLeaf(roleCond);
leaf = SearchCond.of(roleCond);
break;

case PRIVILEGES:
PrivilegeCond privilegeCond = new PrivilegeCond();
privilegeCond.setPrivilege(value);
leaf = SearchCond.getLeaf(privilegeCond);
leaf = SearchCond.of(privilegeCond);
break;

case DYNREALMS:
DynRealmCond dynRealmCond = new DynRealmCond();
dynRealmCond.setDynRealm(value);
leaf = SearchCond.getLeaf(dynRealmCond);
leaf = SearchCond.of(dynRealmCond);
break;

case MEMBER:
MemberCond memberCond = new MemberCond();
memberCond.setMember(value);
leaf = SearchCond.getLeaf(memberCond);
leaf = SearchCond.of(memberCond);
break;

default:
Expand All @@ -203,41 +203,41 @@ protected SearchCond visitPrimitive(final SearchCondition<SearchBean> sc) {
}
}
if (ct == ConditionType.NOT_EQUALS) {
Optional<AttrCond> notEquals = leaf.getLeaf(AttrCond.class);
Optional<AttrCond> notEquals = leaf.asLeaf(AttrCond.class);
if (notEquals.isPresent() && notEquals.get().getType() == AttrCond.Type.ISNULL) {
notEquals.get().setType(AttrCond.Type.ISNOTNULL);
} else {
leaf = SearchCond.getNotLeaf(leaf);
leaf = SearchCond.negate(leaf);
}
}
break;

case GREATER_OR_EQUALS:
attrCond.setType(AttrCond.Type.GE);
leaf = SearchCond.getLeaf(attrCond);
leaf = SearchCond.of(attrCond);
break;

case GREATER_THAN:
attrCond.setType(AttrCond.Type.GT);
leaf = SearchCond.getLeaf(attrCond);
leaf = SearchCond.of(attrCond);
break;

case LESS_OR_EQUALS:
attrCond.setType(AttrCond.Type.LE);
leaf = SearchCond.getLeaf(attrCond);
leaf = SearchCond.of(attrCond);
break;

case LESS_THAN:
attrCond.setType(AttrCond.Type.LT);
leaf = SearchCond.getLeaf(attrCond);
leaf = SearchCond.of(attrCond);
break;

default:
throw new IllegalArgumentException(String.format("Condition type %s is not supported", ct.name()));
}

// SYNCOPE-1293: explicitly re-process to allow 'token==$null' or 'token!=$null'
Optional<AttrCond> reprocess = leaf.getLeaf(AttrCond.class).
Optional<AttrCond> reprocess = leaf.asLeaf(AttrCond.class).
filter(cond -> "token".equals(cond.getSchema())
&& (cond.getType() == AttrCond.Type.ISNULL || cond.getType() == AttrCond.Type.ISNOTNULL)
&& cond.getExpression() == null);
Expand All @@ -246,7 +246,7 @@ protected SearchCond visitPrimitive(final SearchCondition<SearchBean> sc) {
tokenCond.setSchema(reprocess.get().getSchema());
tokenCond.setType(reprocess.get().getType());
tokenCond.setExpression(null);
leaf = SearchCond.getLeaf(tokenCond);
leaf = SearchCond.of(tokenCond);
}

return leaf;
Expand All @@ -263,11 +263,11 @@ protected SearchCond visitCompound(final SearchCondition<SearchBean> sc) {
SearchCond compound;
switch (sc.getConditionType()) {
case AND:
compound = SearchCond.getAnd(searchConds);
compound = SearchCond.and(searchConds);
break;

case OR:
compound = SearchCond.getOr(searchConds);
compound = SearchCond.or(searchConds);
break;

default:
Expand Down
Loading

0 comments on commit 061c94e

Please sign in to comment.