Skip to content

Commit

Permalink
count/searchReferences cleaner query specification + related tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@virgo47
virgo47 committed Jan 23, 2023
1 parent 8f8d9da commit 5ae0299
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 45 deletions.
12 changes: 8 additions & 4 deletions repo/repo-api/src/main/java/com/evolveum/midpoint/repo/api/RepositoryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ public interface RepositoryService {
String OP_RETURN_UNUSED_VALUES_TO_SEQUENCE = "returnUnusedValuesToSequence";
String OP_EXECUTE_QUERY_DIAGNOSTICS = "executeQueryDiagnostics";
String OP_GET_OBJECT = "getObject";
String OP_SEARCH_SHADOW_OWNER = "searchShadowOwner";
String OP_SEARCH_OBJECTS = "searchObjects";
String OP_SEARCH_OBJECTS_ITERATIVE = "searchObjectsIterative";
String OP_SEARCH_OBJECTS_ITERATIVE_PAGE = "searchObjectsIterativePage";
Expand All @@ -152,7 +151,6 @@ public interface RepositoryService {
String MODIFY_OBJECT_DYNAMICALLY = CLASS_NAME_WITH_DOT + OP_MODIFY_OBJECT_DYNAMICALLY;
String GET_VERSION = CLASS_NAME_WITH_DOT + OP_GET_VERSION;
String SEARCH_OBJECTS_ITERATIVE = CLASS_NAME_WITH_DOT + OP_SEARCH_OBJECTS_ITERATIVE;
String SEARCH_SHADOW_OWNER = CLASS_NAME_WITH_DOT + OP_SEARCH_SHADOW_OWNER;
String ADVANCE_SEQUENCE = CLASS_NAME_WITH_DOT + OP_ADVANCE_SEQUENCE;
String RETURN_UNUSED_VALUES_TO_SEQUENCE = CLASS_NAME_WITH_DOT + OP_RETURN_UNUSED_VALUES_TO_SEQUENCE;
String EXECUTE_QUERY_DIAGNOSTICS = CLASS_NAME_WITH_DOT + OP_EXECUTE_QUERY_DIAGNOSTICS;
Expand Down Expand Up @@ -350,8 +348,9 @@ <T extends Containerable> SearchResultList<T> searchContainers(Class<T> type, Ob

/**
* Reference count - currently supporting roleMembershipRef and linkRef search.
* See {@link #searchReferences(ObjectQuery, Collection, OperationResult)} for more details.
*
* @param query mandatory query that has to have exactly one root OWNER-BY and additional REF filters
* @param query mandatory query
*/
int countReferences(
@Nullable ObjectQuery query,
Expand All @@ -363,7 +362,12 @@ int countReferences(
* This returns reference objects extracted from the actual object(s) that own them,
* but selection of which (and cardinality of the result list) is based on a repository search.
*
* @param query mandatory query that has to have exactly one root OWNER-BY and additional REF filters
* Query must not be null and its filter must be:
*
* * either a OWNER-BY filter,
* * or AND filter containing exactly one OWNER-BY filter and optionally one or more REF filters with empty path (self).
*
* @param query mandatory query with exactly one root OWNER-BY and additional REF filters
*/
@NotNull SearchResultList<ObjectReferenceType> searchReferences(
@NotNull ObjectQuery query,
Expand Down
70 changes: 34 additions & 36 deletions repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ public SqaleRepositoryService(
OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_GET_OBJECT)
.addQualifier(type.getSimpleName())
.setMinor()
.addParam("type", type.getName())
.addParam("oid", oid)
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam(OperationResult.PARAM_OID, oid)
.build();

PrismObject<T> object = null;
Expand Down Expand Up @@ -241,8 +241,8 @@ public <T extends ObjectType> String getVersion(

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_GET_VERSION)
.addQualifier(type.getSimpleName())
.addParam("type", type.getName())
.addParam("oid", oid)
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam(OperationResult.PARAM_OID, oid)
.build();

try {
Expand Down Expand Up @@ -302,8 +302,8 @@ public <T extends ObjectType> String addObject(

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_ADD_OBJECT)
.addQualifier(object.asObjectable().getClass().getSimpleName())
.addParam("object", object)
.addParam("options", options.toString())
.addParam(OperationResult.PARAM_OBJECT, object)
.addParam(OperationResult.PARAM_OPTIONS, options.toString())
.build();

try {
Expand Down Expand Up @@ -448,9 +448,9 @@ public <T extends ObjectType> ModifyObjectResult<T> modifyObject(

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_MODIFY_OBJECT)
.addQualifier(type.getSimpleName())
.addParam("type", type.getName())
.addParam("oid", oid)
.addParam("options", String.valueOf(options))
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam(OperationResult.PARAM_OID, oid)
.addParam(OperationResult.PARAM_OPTIONS, String.valueOf(options))
.addArbitraryObjectCollectionAsParam("modifications", modifications)
.build();

Expand Down Expand Up @@ -508,10 +508,10 @@ private <T extends ObjectType> ModifyObjectResult<T> executeModifyObject(

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_MODIFY_OBJECT_DYNAMICALLY)
.addQualifier(type.getSimpleName())
.addParam("type", type.getName())
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam("getOptions", String.valueOf(getOptions))
.addParam("modifyOptions", String.valueOf(modifyOptions))
.addParam("oid", oid)
.addParam(OperationResult.PARAM_OID, oid)
.build();

ModifyObjectResult<T> rv = null;
Expand Down Expand Up @@ -765,8 +765,8 @@ private void logTraceModifications(@NotNull Collection<? extends ItemDelta<?, ?>

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_DELETE_OBJECT)
.addQualifier(type.getSimpleName())
.addParam("type", type.getName())
.addParam("oid", oid)
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam(OperationResult.PARAM_OID, oid)
.build();
try {
return executeDeleteObject(type, oid, oidUuid);
Expand Down Expand Up @@ -831,8 +831,8 @@ public <T extends ObjectType> int countObjects(Class<T> type, ObjectQuery query,

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_COUNT_OBJECTS)
.addQualifier(type.getSimpleName())
.addParam("type", type.getName())
.addParam("query", query)
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam(OperationResult.PARAM_QUERY, query)
.build();

try {
Expand Down Expand Up @@ -881,9 +881,9 @@ private <T extends ObjectType> int executeCountObjects(

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_SEARCH_OBJECTS)
.addQualifier(type.getSimpleName())
.addParam("type", type.getName())
.addParam("query", query)
.addParam("options", String.valueOf(options))
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam(OperationResult.PARAM_QUERY, query)
.addParam(OperationResult.PARAM_OPTIONS, String.valueOf(options))
.build();

try {
Expand Down Expand Up @@ -937,8 +937,8 @@ public <T extends ObjectType> SearchResultMetadata searchObjectsIterative(

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_SEARCH_OBJECTS_ITERATIVE)
.addQualifier(type.getSimpleName())
.addParam("type", type.getName())
.addParam("query", query)
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam(OperationResult.PARAM_QUERY, query)
.build();

try {
Expand Down Expand Up @@ -1189,8 +1189,8 @@ public <T extends Containerable> int countContainers(Class<T> type, ObjectQuery
OperationResult operationResult =
parentResult.subresult(opNamePrefix + OP_COUNT_CONTAINERS)
.addQualifier(type.getSimpleName())
.addParam("type", type.getName())
.addParam("query", query)
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam(OperationResult.PARAM_QUERY, query)
.build();
try {
logSearchInputParameters(type, query, "Count containers");
Expand Down Expand Up @@ -1238,8 +1238,8 @@ public <T extends Containerable> SearchResultList<T> searchContainers(

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_SEARCH_CONTAINERS)
.addQualifier(type.getSimpleName())
.addParam("type", type.getName())
.addParam("query", query)
.addParam(OperationResult.PARAM_TYPE, type.getName())
.addParam(OperationResult.PARAM_QUERY, query)
.build();

try {
Expand Down Expand Up @@ -1284,7 +1284,7 @@ public int countReferences(
Objects.requireNonNull(parentResult, "Operation result must not be null.");

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_COUNT_REFERENCES)
.addParam("query", query)
.addParam(OperationResult.PARAM_QUERY, query)
.build();

try {
Expand Down Expand Up @@ -1330,7 +1330,7 @@ public int executeCountReferences(
Objects.requireNonNull(parentResult, "Operation result must not be null.");

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_SEARCH_REFERENCES)
.addParam("query", query)
.addParam(OperationResult.PARAM_QUERY, query)
.build();

try {
Expand Down Expand Up @@ -1387,11 +1387,9 @@ private OwnedByFilter extractOwnedByFilterForReferenceSearch(ObjectFilter filter
throws QueryException {
if (filter instanceof OwnedByFilter) {
return (OwnedByFilter) filter;
} else if (filter instanceof UnaryLogicalFilter) {
return extractOwnedByFilterForReferenceSearch(((UnaryLogicalFilter) filter).getFilter(), originalFilter);
} else if (filter instanceof NaryLogicalFilter) {
} else if (filter instanceof AndFilter) {
OwnedByFilter ownedByFilter = null;
for (ObjectFilter condition : ((NaryLogicalFilter) filter).getConditions()) {
for (ObjectFilter condition : ((AndFilter) filter).getConditions()) {
if (condition instanceof OwnedByFilter) {
if (ownedByFilter != null) {
throw new QueryException("Exactly one main OWNED-BY filter must be used"
Expand All @@ -1407,7 +1405,7 @@ private OwnedByFilter extractOwnedByFilterForReferenceSearch(ObjectFilter filter
return ownedByFilter;
} else {
throw new QueryException("Invalid filter for reference search: " + originalFilter
+ "\nReference search filter should be OWNED-BY filter or a logical filter with it.");
+ "\nReference search filter should be OWNED-BY filter or an AND filter containing it.");
}
}
// endregion
Expand Down Expand Up @@ -1481,7 +1479,7 @@ public long advanceSequence(String oid, OperationResult parentResult)
logger.debug("Advancing sequence {}", oid);

OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_ADVANCE_SEQUENCE)
.addParam("oid", oid)
.addParam(OperationResult.PARAM_OID, oid)
.build();

try {
Expand Down Expand Up @@ -1531,7 +1529,7 @@ public void returnUnusedValuesToSequence(

OperationResult operationResult =
parentResult.subresult(opNamePrefix + OP_RETURN_UNUSED_VALUES_TO_SEQUENCE)
.addParam("oid", oid)
.addParam(OperationResult.PARAM_OID, oid)
.build();

if (unusedValues == null || unusedValues.isEmpty()) {
Expand Down Expand Up @@ -1992,7 +1990,7 @@ public void unregisterConflictWatcher(ConflictWatcher watcher) {
public boolean hasConflict(ConflictWatcher watcher, OperationResult parentResult) {
OperationResult operationResult = parentResult.subresult(opNamePrefix + OP_HAS_CONFLICT)
.setMinor()
.addParam("oid", watcher.getOid())
.addParam(OperationResult.PARAM_OID, watcher.getOid())
.addParam("watcherClass", watcher.getClass().getName())
.build();

Expand Down Expand Up @@ -2026,8 +2024,8 @@ public <T extends ObjectType> void addDiagnosticInformation(Class<T> type, Strin
OperationResult operationResult =
parentResult.subresult(opNamePrefix + OP_ADD_DIAGNOSTIC_INFORMATION)
.addQualifier(type.getSimpleName())
.addParam("type", type)
.addParam("oid", oid)
.addParam(OperationResult.PARAM_TYPE, type)
.addParam(OperationResult.PARAM_OID, oid)
.build();
try {
PrismObject<T> object = getObject(type, oid, null, operationResult);
Expand Down
72 changes: 67 additions & 5 deletions repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1402,6 +1402,8 @@ public void test446SearchByRefWithComplexTargetFilter() {
@Test
public void test448SearchUserByAssignmentTargetRef() throws SchemaException {
searchObjectTest("User by Assignment targetRef with ref and target subfilter", UserType.class,
// TODO if AbstractRoleType.COMPLEX_TYPE is used, nothing is matched, because currently the repo
// does not "understand" the hierarchy and will not expand the type to IN (subtypes...) condition.
f -> f.ref(ItemPath.create(F_ASSIGNMENT, AssignmentType.F_TARGET_REF),
RoleType.COMPLEX_TYPE, relation2)
.item(RoleType.F_NAME).eq("role-ass-vs-ind"),
Expand Down Expand Up @@ -2518,6 +2520,27 @@ public void test802SearchReferenceWithAnyRelation() throws SchemaException {
.anyMatch(r -> refMatches(r, user4Oid, roleOneMoreOid, ORG_DEFAULT));
}

@Test
public void test803SearchReferenceWithAnyOfTwoRelationsUsingNestedOr() throws SchemaException {
when("searching role membership references to roles with any relation");
OperationResult operationResult = createOperationResult();
ObjectQuery refQuery = prismContext.queryForReferenceOwnedBy(UserType.class, UserType.F_ROLE_MEMBERSHIP_REF)
.and()
.block()
.item(ItemPath.SELF_PATH).ref(null, RoleType.COMPLEX_TYPE, relation1)
.or()
.item(ItemPath.SELF_PATH).ref(null, RoleType.COMPLEX_TYPE, relation2)
.endBlock()
.build();
SearchResultList<ObjectReferenceType> result = searchReferences(refQuery, operationResult, null);

then("expected refs are returned (none found for relation1, that's OK)");
assertThat(result)
.hasSize(2)
.anyMatch(r -> refMatches(r, user1Oid, roleOtherOid, relation2))
.anyMatch(r -> refMatches(r, user3Oid, roleAvIOid, relation2));
}

@Test
public void test810SearchLinkRefs() throws SchemaException {
when("searching link references (pure owned-by filter)");
Expand Down Expand Up @@ -2566,10 +2589,6 @@ public void test820SearchReferenceCanBeOrderedByTargetItem() throws SchemaExcept
.containsExactly(roleOtherOid, roleOneMoreOid);
}

// TODO test wrong ref type
// TODO test multiple owned-by (failing)
// TODO test non AND filter (OR and NOT on the top level have no meaning, this needs fixing in repo too)

@Test
public void test890ReferenceSearchWithNullQueryFails() {
OperationResult operationResult = createOperationResult();
Expand Down Expand Up @@ -2597,12 +2616,55 @@ public void test892ReferenceQueryFailsOnWrongTopLevelPath() {
assertThatThrownBy(
() -> prismContext
.queryForReferenceOwnedBy(UserType.class, UserType.F_ROLE_MEMBERSHIP_REF)
.or()
.and()
.item(UserType.F_ROLE_MEMBERSHIP_REF).ref("target-oid") // this fails
.build())
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Reference search only supports REF filter with SELF path (.) on the top level.");
}

@Test
public void test893ReferenceSearchFailsOnInvalidTopLevelLogicalFilter() {
OperationResult operationResult = createOperationResult();
// OR doesn't make much sense, we want to specify OWNED-BY AND REF filter(s).
// OR can be nested deeper though, that's not a problem.
assertThatThrownBy(
() -> searchReferences(prismContext
.queryForReferenceOwnedBy(UserType.class, UserType.F_ROLE_MEMBERSHIP_REF)
.or() // this fails on filter check in the repo
.item(ItemPath.SELF_PATH).ref(roleOtherOid)
.build(), operationResult, null))
.isInstanceOf(SystemException.class)
.hasMessageStartingWith("Invalid filter for reference search")
.hasMessageEndingWith("Reference search filter should be OWNED-BY filter or an AND filter containing it.");
}

@Test
public void test895ReferenceSearchForUnsupportedRefFails() {
OperationResult operationResult = createOperationResult();
assertThatThrownBy(
() -> searchReferences(prismContext
.queryForReferenceOwnedBy(UserType.class, UserType.F_ROLE_INFLUENCE_REF)
.build(), operationResult, null))
.isInstanceOf(SystemException.class)
.hasMessageMatching("Reference search is not supported for.*roleInfluenceRef");
}

@Test
public void test897ReferenceSearchWithMultipleOwnedByFails() {
OperationResult operationResult = createOperationResult();
assertThatThrownBy(
() -> searchReferences(prismContext
.queryForReferenceOwnedBy(UserType.class, UserType.F_ROLE_INFLUENCE_REF)
.and()
.ownedBy(UserType.class, UserType.F_ROLE_INFLUENCE_REF)
.build(), operationResult, null))
.isInstanceOf(SystemException.class)
.hasMessageStartingWith(
"Exactly one main OWNED-BY filter must be used for reference search, but multiple found.");
}

// TODO test multiple owned-by (failing)
// endregion

// region special cases
Expand Down

0 comments on commit 5ae0299

Please sign in to comment.