Skip to content

Commit

Permalink
Improve OID-less ref filter implementation
Browse files Browse the repository at this point in the history
1. The default for oidNullAsAny is now TRUE (was false).
2. Repo query interpreter rejects oid=null if oidNullAsAny=false.
3. Ref filter construction in query builder was cleaned up a little bit.

This is to resolve MID-5987.
  • Loading branch information
mederly committed Jan 15, 2020
1 parent 1512ed0 commit 169241c
Show file tree
Hide file tree
Showing 11 changed files with 165 additions and 25 deletions.
Expand Up @@ -149,7 +149,7 @@ protected ObjectQuery createObjectQuery() {
.getAllRelationsFor(RelationKindType.DELEGATION);
ObjectFilter deputyFilter = getParentPage().getPrismContext().queryFor(AssignmentType.class)
.item(AssignmentType.F_TARGET_REF)
.ref(delegationRelations.toArray(new QName[0]))
.refRelation(delegationRelations.toArray(new QName[0]))
.buildFilter();

QName targetType = getAssignmentType();
Expand Down
Expand Up @@ -44,7 +44,6 @@
import com.evolveum.midpoint.util.logging.TraceManager;
import com.evolveum.midpoint.web.component.MultiCompositedButtonPanel;
import com.evolveum.midpoint.web.component.MultiFunctinalButtonDto;
import com.evolveum.midpoint.web.component.MultifunctionalButton;
import com.evolveum.midpoint.web.component.data.column.*;
import com.evolveum.midpoint.web.component.menu.cog.ButtonInlineMenuItem;
import com.evolveum.midpoint.web.component.menu.cog.InlineMenuItem;
Expand Down Expand Up @@ -379,7 +378,7 @@ protected ObjectQuery createObjectQuery(){
ObjectQuery query = getParentPage().getPrismContext().queryFor(AssignmentType.class)
.not()
.item(AssignmentType.F_TARGET_REF)
.ref(delegationRelations.toArray(new QName[0]))
.refRelation(delegationRelations.toArray(new QName[0]))
.build();
query.addFilter(getPrismContext().queryFactory().createNot(archetypeFilter));
return query;
Expand Down
Expand Up @@ -83,7 +83,7 @@ protected ObjectQuery createObjectQuery() {
return getParentPage().getPrismContext().queryFor(AssignmentType.class)
.block()
.item(AssignmentType.F_TARGET_REF)
.ref(SchemaConstants.ORG_CONSENT)
.refRelation(SchemaConstants.ORG_CONSENT)
.endBlock()
.build();
}
Expand Down
Expand Up @@ -24,4 +24,9 @@ public interface RefFilter extends ValueFilter<PrismReferenceValue, PrismReferen

void setRelationNullAsAny(boolean relationNullAsAny);

boolean isOidNullAsAny();

boolean isTargetTypeNullAsAny();

boolean isRelationNullAsAny();
}
Expand Up @@ -9,6 +9,7 @@

import com.evolveum.midpoint.prism.PrismProperty;
import com.evolveum.midpoint.prism.PrismReferenceValue;
import com.evolveum.midpoint.prism.query.RefFilter;

import javax.xml.namespace.QName;
import java.util.Collection;
Expand Down Expand Up @@ -39,12 +40,15 @@ public interface S_ConditionEntry {
S_MatchingRuleEntry contains(Object value);
S_MatchingRuleEntry containsPoly(String orig, String norm);
S_MatchingRuleEntry containsPoly(String orig);
S_AtomicFilterExit ref(QName... relations); // TODO is this supported by repo query interpreter?
S_AtomicFilterExit refRelation(QName... relations);
S_AtomicFilterExit refType(QName... targetTypeName);
S_AtomicFilterExit ref(PrismReferenceValue... value);
S_AtomicFilterExit ref(Collection<PrismReferenceValue> values);
S_AtomicFilterExit ref(Collection<PrismReferenceValue> values, boolean nullTypeAsAny); // beware, 'nullTypeAsAny' is supported only by built-in match(..) method
S_AtomicFilterExit ref(String... oid);
S_AtomicFilterExit ref(String oid, QName targetTypeName);
S_AtomicFilterExit ref(Collection<PrismReferenceValue> values, boolean nullTypeAsAny); // beware, nullTypeAsAny=false is supported only by built-in match(..) method
S_AtomicFilterExit ref(Collection<PrismReferenceValue> values, boolean nullOidAsAny, boolean nullTypeAsAny); // beware, nullTypeAsAny=false and nullOidAsAny=false are supported only by built-in match(..) method
S_AtomicFilterExit ref(RefFilter filter);
S_AtomicFilterExit ref(String... oid); // TODO define semantics for oid == null
S_AtomicFilterExit ref(String oid, QName targetTypeName); // TODO define semantics for oid == null
S_AtomicFilterExit isNull();

}
Expand Up @@ -24,10 +24,9 @@
public class RefFilterImpl extends ValueFilterImpl<PrismReferenceValue, PrismReferenceDefinition> implements RefFilter {
private static final long serialVersionUID = 1L;

// these are currently supported only by built-in match(..) method; e.g. the repo query interpreter simply ignores them
private boolean oidNullAsAny = false;
private boolean targetTypeNullAsAny = true; // "true" to be consistent with the repo implementation
private boolean relationNullAsAny = false; // currently not supported at all
private boolean oidNullAsAny = true; // "false" is not supported by repo
private boolean targetTypeNullAsAny = true; // "true" to be consistent with the repo implementation; "false" is ignored by repo
private boolean relationNullAsAny = false; // currently ignored

public RefFilterImpl(@NotNull ItemPath fullPath, @Nullable PrismReferenceDefinition definition,
@Nullable List<PrismReferenceValue> values, @Nullable ExpressionWrapper expression) {
Expand Down Expand Up @@ -141,4 +140,18 @@ public void setRelationNullAsAny(boolean relationNullAsAny) {
this.relationNullAsAny = relationNullAsAny;
}

@Override
public boolean isOidNullAsAny() {
return oidNullAsAny;
}

@Override
public boolean isTargetTypeNullAsAny() {
return targetTypeNullAsAny;
}

@Override
public boolean isRelationNullAsAny() {
return relationNullAsAny;
}
}
Expand Up @@ -215,19 +215,26 @@ public S_MatchingRuleEntry containsPoly(String orig) {
}

@Override
public S_AtomicFilterExit ref(QName... relations) {
public S_AtomicFilterExit refRelation(QName... relations) {
List<PrismReferenceValue> values = new ArrayList<>();
for (QName relation : relations) {
PrismReferenceValue ref = new PrismReferenceValueImpl();
ref.setRelation(relation);
values.add(ref);
}
RefFilter filter = RefFilterImpl.createReferenceEqual(itemPath, referenceDefinition, values);
filter.setOidNullAsAny(true);
filter.setTargetTypeNullAsAny(true);
return new R_AtomicFilter(this, filter);
return ref(values);
}

@Override
public S_AtomicFilterExit refType(QName... targetTypeNames) {
List<PrismReferenceValue> values = new ArrayList<>();
for (QName targetTypeName : targetTypeNames) {
PrismReferenceValue ref = new PrismReferenceValueImpl();
ref.setTargetType(targetTypeName);
values.add(ref);
}
return ref(values);
}

@Override
public S_AtomicFilterExit ref(PrismReferenceValue... values) {
Expand All @@ -240,28 +247,40 @@ public S_AtomicFilterExit ref(PrismReferenceValue... values) {

@Override
public S_AtomicFilterExit ref(Collection<PrismReferenceValue> values) {
return ref(values, true);
return ref(values, true, true);
}

@Override
public S_AtomicFilterExit ref(Collection<PrismReferenceValue> values, boolean nullTypeAsAny) {
return ref(values, true, nullTypeAsAny);
}

@Override
public S_AtomicFilterExit ref(Collection<PrismReferenceValue> values, boolean nullOidAsAny, boolean nullTypeAsAny) {
RefFilter filter = RefFilterImpl.createReferenceEqual(itemPath, referenceDefinition, values);
filter.setOidNullAsAny(nullOidAsAny);
filter.setTargetTypeNullAsAny(nullTypeAsAny);
return ref(filter);
}

@Override
public S_AtomicFilterExit ref(RefFilter filter) {
return new R_AtomicFilter(this, filter);
}

@Override
public S_AtomicFilterExit ref(String... oids) {
// TODO reconsider oid == null case
if (oids.length == 1 && oids[0] == null) {
return ref(Collections.emptyList());
} else {
// when OIDs are specified, we allow any type
return ref(Arrays.stream(oids).map(oid -> new PrismReferenceValueImpl(oid)).collect(Collectors.toList()), true);
return ref(Arrays.stream(oids).map(oid -> new PrismReferenceValueImpl(oid)).collect(Collectors.toList()));
}
}

@Override
public S_AtomicFilterExit ref(String oid, QName targetTypeName) {
// TODO reconsider oid == null case
if (oid != null) {
return ref(new PrismReferenceValueImpl(oid, targetTypeName));
} else {
Expand Down
Expand Up @@ -249,7 +249,7 @@ public void testRefNegative() throws Exception {
public void testRefRelationNegative() throws Exception {
PrismObject<UserType> user = PrismTestUtil.parseObject(PrismInternalTestUtil.USER_JACK_FILE_XML);
ObjectFilter filter = getPrismContext().queryFor(UserType.class)
.item(UserType.F_ACCOUNT_REF).ref(new QName("a-relation"))
.item(UserType.F_ACCOUNT_REF).refRelation(new QName("a-relation"))
.buildFilter();
boolean match = ObjectQuery.match(user, filter, matchingRuleRegistry);
AssertJUnit.assertFalse("filter matches object, but it should not", match);
Expand Down
Expand Up @@ -40,6 +40,9 @@
import java.util.zip.ZipInputStream;
import java.util.zip.ZipOutputStream;

import static java.util.Collections.emptySet;
import static java.util.Collections.singleton;

/**
* @author semancik
*
Expand Down Expand Up @@ -819,4 +822,8 @@ public static String readZipFile(File file, Charset charset) throws IOException
}
}
}

public static Set<String> singletonOrEmptySet(String value) {
return value != null ? singleton(value) : emptySet();
}
}
Expand Up @@ -1132,6 +1132,87 @@ public void test0144QueryUserAccountRefNotNull() throws Exception {
}
}

@Test
public void test0146QueryUserAccountRefByType() throws Exception {
Session session = open();
try {
ObjectQuery query = prismContext.queryFor(UserType.class)
.item(UserType.F_LINK_REF).refType(ShadowType.COMPLEX_TYPE)
.build();
String real = getInterpretedQuery2(session, UserType.class, query);
String expected = "select\n"
+ " u.oid,\n"
+ " u.fullObject\n"
+ "from\n"
+ " RUser u\n"
+ " left join u.linkRef l\n"
+ "where\n"
+ " (\n"
+ " l.relation in (:relation) and\n"
+ " l.type = :type\n"
+ " )\n";
assertEqualsIgnoreWhitespace(expected, real);
} finally {
close(session);
}
}

@Test
public void test0147QueryUserAccountRefByRelation() throws Exception {
Session session = open();
try {
ObjectQuery query = prismContext.queryFor(UserType.class)
.item(UserType.F_LINK_REF).refRelation(prismContext.getDefaultRelation())
.build();
String real = getInterpretedQuery2(session, UserType.class, query);
String expected = "select\n"
+ " u.oid,\n"
+ " u.fullObject\n"
+ "from\n"
+ " RUser u\n"
+ " left join u.linkRef l\n"
+ "where\n"
+ " l.relation in (:relation)\n";
assertEqualsIgnoreWhitespace(expected, real);
} finally {
close(session);
}
}

@Test
public void test0148QueryUserAccountRefComplex() throws Exception {
Session session = open();
try {
PrismReferenceValue value1 = prismContext.itemFactory().createReferenceValue(null, ShadowType.COMPLEX_TYPE);
PrismReferenceValue value2 = prismContext.itemFactory().createReferenceValue("abcdef", ShadowType.COMPLEX_TYPE);
ObjectQuery query = prismContext.queryFor(UserType.class)
.item(UserType.F_LINK_REF).ref(value1, value2)
.build();
String real = getInterpretedQuery2(session, UserType.class, query);
String expected = "select\n"
+ " u.oid,\n"
+ " u.fullObject\n"
+ "from\n"
+ " RUser u\n"
+ " left join u.linkRef l\n"
+ "where\n"
+ " (\n"
+ " (\n"
+ " l.relation in (:relation) and\n"
+ " l.type = :type\n"
+ " ) or\n"
+ " (\n"
+ " l.targetOid = :targetOid and\n"
+ " l.relation in (:relation2) and\n"
+ " l.type = :type2\n"
+ " )\n"
+ " )\n";
assertEqualsIgnoreWhitespace(expected, real);
} finally {
close(session);
}
}

@Test
public void test0150QueryUserAssignmentTargetRef() throws Exception {
Session session = open();
Expand Down
Expand Up @@ -67,9 +67,19 @@ public Condition interpretInternal() throws QueryException {
Set<String> oids = new HashSet<>();
Set<QName> relations = new HashSet<>();
Set<QName> targetTypes = new HashSet<>();
boolean valuesWithWildcardOid = false;
boolean valuesWithSpecifiedOid = false;
for (PrismReferenceValue value : values) {
if (value.getOid() != null) {
oids.add(value.getOid());
valuesWithSpecifiedOid = true;
} else {
if (filter.isOidNullAsAny()) {
valuesWithWildcardOid = true;
} else {
throw new QueryException("Null OID is not allowed in the reference query. "
+ "If you'd like to search for missing reference, use empty list of values.");
}
}
if (value.getRelation() == null) {
relations.add(context.getPrismContext().getDefaultRelation());
Expand All @@ -81,14 +91,16 @@ public Condition interpretInternal() throws QueryException {
targetTypes.add(qualifyTypeName(value.getTargetType()));
}

if (relations.size() > 1 || targetTypes.size() > 1) {
// we must use 'OR' clause
if (valuesWithWildcardOid && valuesWithSpecifiedOid || relations.size() > 1 || targetTypes.size() > 1) {
// We must use 'OR' clause
OrCondition rootOr = hibernateQuery.createOr();
values.forEach(prv -> rootOr
.add(createRefCondition(hibernateQuery, Collections.singleton(prv.getOid()), prv.getRelation(), prv.getTargetType())));
.add(createRefCondition(hibernateQuery,
MiscUtil.singletonOrEmptySet(prv.getOid()), prv.getRelation(), prv.getTargetType())));
return rootOr;
} else {
return createRefCondition(hibernateQuery, oids, MiscUtil.extractSingleton(relations), MiscUtil.extractSingleton(targetTypes));
return createRefCondition(hibernateQuery, oids,
MiscUtil.extractSingleton(relations), MiscUtil.extractSingleton(targetTypes));
}
}

Expand Down

0 comments on commit 169241c

Please sign in to comment.