Skip to content

Commit

Permalink
MID-7860 fix: iterative search with order by polystring is correct now
Browse files Browse the repository at this point in the history
(cherry picked from commit 7886521)
  • Loading branch information
virgo47 committed Apr 13, 2022
1 parent bc8e1c5 commit 574a8f7
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@
import com.evolveum.midpoint.prism.path.ItemPath;
import com.evolveum.midpoint.prism.polystring.PolyString;
import com.evolveum.midpoint.prism.query.*;
import com.evolveum.midpoint.prism.query.builder.S_AtomicFilterExit;
import com.evolveum.midpoint.prism.query.builder.S_ConditionEntry;
import com.evolveum.midpoint.prism.query.builder.S_MatchingRuleEntry;
import com.evolveum.midpoint.prism.xml.XmlTypeConverter;
import com.evolveum.midpoint.repo.api.*;
import com.evolveum.midpoint.repo.api.query.ObjectFilterExpressionEvaluator;
Expand Down Expand Up @@ -69,10 +69,12 @@
import com.evolveum.midpoint.schema.util.ObjectQueryUtil;
import com.evolveum.midpoint.schema.util.ObjectTypeUtil;
import com.evolveum.midpoint.util.PrettyPrinter;
import com.evolveum.midpoint.util.QNameUtil;
import com.evolveum.midpoint.util.exception.*;
import com.evolveum.midpoint.util.logging.Trace;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
import com.evolveum.prism.xml.ns._public.query_3.SearchFilterType;
import com.evolveum.prism.xml.ns._public.types_3.PolyStringType;

/**
* Repository implementation based on SQL, JDBC and Querydsl without any ORM.
Expand Down Expand Up @@ -1093,11 +1095,18 @@ private <T extends ObjectType> ObjectFilter lastOidCondition(
// "By default, null values sort as if larger than any non-null value; that is,
// NULLS FIRST is the default for DESC order, and NULLS LAST otherwise."
} else {
S_MatchingRuleEntry matchingRuleEntry =
asc ? filter.gt(item.getRealValue()) : filter.lt(item.getRealValue());
filter = matchingRuleEntry.or()
// see MID-7860
boolean isPolyString = QNameUtil.match(
PolyStringType.COMPLEX_TYPE, item.getDefinition().getTypeName());

Object realValue = item.getRealValue();
S_AtomicFilterExit subfilter = asc
? (isPolyString ? filter.gt(realValue).matchingOrig() : filter.gt(realValue))
: (isPolyString ? filter.lt(realValue).matchingOrig() : filter.lt(realValue));
S_ConditionEntry subfilter2 = subfilter.or()
.block()
.item(orderByPath).eq(item.getRealValue())
.item(orderByPath);
filter = (isPolyString ? subfilter2.eq(realValue).matchingOrig() : subfilter2.eq(realValue))
.and()
.item(OID_PATH);
return (asc ? filter.gt(lastProcessedOid) : filter.lt(lastProcessedOid))
Expand Down Expand Up @@ -1329,7 +1338,7 @@ public <F extends FocusType> PrismObject<F> searchShadowOwner(String shadowOid,

@Override
public long advanceSequence(String oid, OperationResult parentResult)
throws ObjectNotFoundException, SchemaException {
throws ObjectNotFoundException {
UUID oidUuid = checkOid(oid);
Validate.notNull(parentResult, "Operation result must not be null.");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ public void initObjects() throws Exception {
.costCenter(String.valueOf(i / 10)); // 10 per cost center
repositoryService.addObject(user.asPrismObject(), null, result);
}
// MID-7860: Special name that breaks iteration conditions which should be only by orig.
// If additional conditions for further "pages" use strict poly match (which is default)
// then both orig and norm is used for GT/LT operations and it doesn't work for some values.
repositoryService.addObject(new UserType(prismContext)
.name("α-user-0001")
.asPrismObject(), null, result);
}

@BeforeMethod
Expand Down Expand Up @@ -265,7 +271,7 @@ public void test120SearchIterativeWithMaxSize() throws Exception {
when("calling search iterative");
SearchResultMetadata metadata = searchObjectsIterative(query, operationResult);

then("result metadata is not null and reports partial result (because of the break)");
then("result metadata is not null and not partial result");
assertThat(metadata).isNotNull();
assertThat(metadata.getApproxNumberOfAllResults()).isEqualTo(testHandler.getCounter());
assertThat(metadata.isPartialResults()).isFalse();
Expand All @@ -279,7 +285,7 @@ public void test120SearchIterativeWithMaxSize() throws Exception {
assertThat(testHandler.getCounter()).isEqualTo(101);
}

@Test
@Test(description = "MID-7860")
public void test125SearchIterativeWithCustomOrdering() throws Exception {
OperationResult operationResult = createOperationResult();
SqlPerformanceMonitorImpl pm = getPerformanceMonitor();
Expand All @@ -294,7 +300,7 @@ public void test125SearchIterativeWithCustomOrdering() throws Exception {
when("calling search iterative");
SearchResultMetadata metadata = searchObjectsIterative(query, operationResult);

then("result metadata is not null and reports partial result (because of the break)");
then("result metadata is not null and not partial result");
assertThatOperationResult(operationResult).isSuccess();
assertThat(metadata).isNotNull();
assertThat(metadata.getApproxNumberOfAllResults()).isEqualTo(testHandler.getCounter());
Expand All @@ -305,7 +311,7 @@ public void test125SearchIterativeWithCustomOrdering() throws Exception {
REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1);
assertTypicalPageOperationCount(metadata);

and("all objects were processed in proper order");
and("all objects were processed");
QUser u = aliasFor(QUser.class);
try (JdbcSession jdbcSession = startReadOnlyTransaction()) {
List<String> result = jdbcSession.newQuery()
Expand All @@ -321,6 +327,36 @@ public void test125SearchIterativeWithCustomOrdering() throws Exception {
}
}

@Test
public void test130SearchIterativeWithCustomOrderingByName() throws Exception {
OperationResult operationResult = createOperationResult();

given("query with custom ordering by name (poly-string)");
ObjectQuery query = prismContext.queryFor(UserType.class)
.asc(UserType.F_NAME)
.build();

when("calling search iterative");
SearchResultMetadata metadata = searchObjectsIterative(query, operationResult);

then("result metadata is not null and not partial result");
assertThatOperationResult(operationResult).isSuccess();
assertThat(metadata).isNotNull();
assertThat(metadata.getApproxNumberOfAllResults()).isEqualTo(testHandler.getCounter());
assertThat(metadata.isPartialResults()).isFalse();

and("all objects were processed");
QUser u = aliasFor(QUser.class);
try (JdbcSession jdbcSession = startReadOnlyTransaction()) {
long processed = jdbcSession.newQuery()
.from(u)
.where(u.employeeNumber.startsWith(getTestNumber()))
.fetchCount();

assertThat(processed).isEqualTo(count(QUser.class)); // all users should be processed
}
}

@SafeVarargs
private SearchResultMetadata searchObjectsIterative(
ObjectQuery query,
Expand Down Expand Up @@ -354,6 +390,9 @@ private int getConfiguredPageSize() {
.getIterativeSearchByPagingBatchSize();
}

/**
* Counts processed objects and changes user's employee number (test+count).
*/
private class TestResultHandler implements ResultHandler<UserType> {

private final AtomicInteger counter = new AtomicInteger();
Expand Down

0 comments on commit 574a8f7

Please sign in to comment.