Skip to content

Commit

Permalink
sqale-audit: finished searchObjectsIterative + tests
Browse files Browse the repository at this point in the history 
Sqale tests now support multiple performance monitors (test can override
default getting the one from SqaleRepositoryService).
Operation prefixes (short class names) are now part of the client call.
  • Loading branch information
@virgo47
virgo47 committed Sep 13, 2021
1 parent 6276370 commit 749ca45
Show file tree
Hide file tree
Showing 11 changed files with 489 additions and 138 deletions.
17 changes: 6 additions & 11 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 @@ -795,7 +795,7 @@ public <T extends ObjectType> int countObjects(Class<T> type, ObjectQuery query,
return 0;
}

return executeCountObject(type, query, options);
return executeCountObjects(type, query, options);
} catch (RepositoryException | RuntimeException e) {
throw handledGeneralException(e, operationResult);
} catch (Throwable t) {
Expand All @@ -806,7 +806,7 @@ public <T extends ObjectType> int countObjects(Class<T> type, ObjectQuery query,
}
}

private <T extends ObjectType> int executeCountObject(
private <T extends ObjectType> int executeCountObjects(
@NotNull Class<T> type,
ObjectQuery query,
Collection<SelectorOptions<GetOperationOptions>> options)
Expand Down Expand Up @@ -846,7 +846,7 @@ private <T extends ObjectType> int executeCountObject(
return new SearchResultList<>();
}

return executeSearchObject(type, query, options, OP_SEARCH_OBJECTS);
return executeSearchObjects(type, query, options, OP_SEARCH_OBJECTS);
} catch (RepositoryException | RuntimeException e) {
throw handledGeneralException(e, operationResult);
} catch (Throwable t) {
Expand All @@ -857,7 +857,7 @@ private <T extends ObjectType> int executeCountObject(
}
}

private <T extends ObjectType> SearchResultList<PrismObject<T>> executeSearchObject(
private <T extends ObjectType> SearchResultList<PrismObject<T>> executeSearchObjects(
@NotNull Class<T> type,
ObjectQuery query,
Collection<SelectorOptions<GetOperationOptions>> options,
Expand Down Expand Up @@ -1002,7 +1002,7 @@ private <T extends ObjectType> SearchResultMetadata executeSearchObjectsIterativ

// we don't call public searchObject to avoid subresults and query simplification
logSearchInputParameters(type, pagedQuery, "Search object iterative page");
List<PrismObject<T>> objects = executeSearchObject(
List<PrismObject<T>> objects = executeSearchObjects(
type, pagedQuery, options, OP_SEARCH_OBJECTS_ITERATIVE_PAGE);

// process page results
Expand Down Expand Up @@ -1303,7 +1303,7 @@ public <F extends FocusType> PrismObject<F> searchShadowOwner(String shadowOid,
.item(FocusType.F_LINK_REF).ref(shadowOid)
.build();
SearchResultList<PrismObject<FocusType>> result =
executeSearchObject(FocusType.class, query, options, OP_SEARCH_SHADOW_OWNER);
executeSearchObjects(FocusType.class, query, options, OP_SEARCH_SHADOW_OWNER);

if (result == null || result.isEmpty()) {
// account shadow owner was not found
Expand Down Expand Up @@ -1901,9 +1901,4 @@ private <T extends ObjectType> boolean pruneDiagnosticInformation(
return true;
}
}

@Override
public SqlPerformanceMonitorImpl getPerformanceMonitor() {
return performanceMonitor;
}
}
5 changes: 5 additions & 0 deletions repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleServiceBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ protected PrismContext prismContext() {
return sqlRepoContext.prismContext();
}

// implements also RepositoryService method (not declared in AuditService)
public SqlPerformanceMonitorImpl getPerformanceMonitor() {
return performanceMonitor;
}

// region exception handling

/**
Expand Down
153 changes: 74 additions & 79 deletions repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/SqaleAuditService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.time.Instant;
import java.util.*;
import javax.xml.datatype.Duration;
import javax.xml.datatype.XMLGregorianCalendar;

import com.querydsl.sql.ColumnMetadata;
import com.querydsl.sql.dml.DefaultMapper;
Expand All @@ -31,12 +32,17 @@
import com.evolveum.midpoint.audit.api.AuditReferenceValue;
import com.evolveum.midpoint.audit.api.AuditResultHandler;
import com.evolveum.midpoint.audit.api.AuditService;
import com.evolveum.midpoint.prism.Item;
import com.evolveum.midpoint.prism.ItemDefinition;
import com.evolveum.midpoint.prism.PrismValue;
import com.evolveum.midpoint.prism.SerializationOptions;
import com.evolveum.midpoint.prism.delta.ObjectDelta;
import com.evolveum.midpoint.prism.path.CanonicalItemPath;
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_ConditionEntry;
import com.evolveum.midpoint.prism.query.builder.S_MatchingRuleEntry;
import com.evolveum.midpoint.prism.util.CloneUtil;
import com.evolveum.midpoint.repo.api.SqlPerformanceMonitorsCollection;
import com.evolveum.midpoint.repo.sqale.*;
Expand Down Expand Up @@ -544,6 +550,16 @@ public SearchResultMetadata searchObjectsIterative(
}
}

/*
TODO: We should try to unify iterative search for repo and audit.
There are some obvious differences - like the provider of the page results - the differences need to be
captured before the common functionality.
Then there are little nuances in filter/ordering:
In repo there is no potential collision between provided filter/ordering and additional "technical" one for OID.
In audit these can collide, but perhaps not every situation needs to be optimized and we can let DB do the work
(e.g. superfluous timestamp conditions).
See also iterativeSearchCondition() comment and ideas there.
*/
private SearchResultMetadata executeSearchObjectsIterative(
ObjectQuery originalQuery,
AuditResultHandler handler,
Expand All @@ -555,14 +571,21 @@ private SearchResultMetadata executeSearchObjectsIterative(
// this is total requested size of the search
Integer maxSize = originalPaging != null ? originalPaging.getMaxSize() : null;

IterativeSearchOrdering ordering = new IterativeSearchOrdering(originalPaging);
List<? extends ObjectOrdering> providedOrdering = originalPaging != null
? originalPaging.getOrderingInstructions()
: null;
if (providedOrdering != null && providedOrdering.size() > 1) {
throw new RepositoryException("searchObjectsIterative() does not support ordering"
+ " by multiple paths (yet): " + providedOrdering);
}

ObjectQuery pagedQuery = prismContext().queryFactory().createQuery();
ObjectPaging paging = prismContext().queryFactory().createPaging();
if (originalPaging != null && originalPaging.getOrderingInstructions() != null) {
originalPaging.getOrderingInstructions().forEach(o ->
paging.addOrderingInstruction(o.getOrderBy(), o.getDirection()));
}
// TODO check of provided ordering
paging.addOrderingInstruction(AuditEventRecordType.F_REPO_ID, OrderDirection.ASCENDING);
pagedQuery.setPaging(paging);

Expand All @@ -581,9 +604,10 @@ private SearchResultMetadata executeSearchObjectsIterative(
}

// filterAnd() is quite null safe, even for both nulls
ObjectFilter originalFilter = originalQuery != null ? originalQuery.getFilter() : null;
pagedQuery.setFilter(ObjectQueryUtil.filterAnd(
originalQuery != null ? originalQuery.getFilter() : null,
lastOidCondition(lastProcessedObject, ordering),
originalFilter,
iterativeSearchCondition(lastProcessedObject, providedOrdering),
prismContext()));

// we don't call public searchObject to avoid subresults and query simplification
Expand Down Expand Up @@ -623,80 +647,75 @@ private SearchResultMetadata executeSearchObjectsIterative(
}
}

private void checkProvidedOrdering(List<? extends ObjectOrdering> providedOrdering) throws RepositoryException {
if (providedOrdering != null && providedOrdering.size() > 1) {
throw new RepositoryException("searchObjectsIterative() does not support ordering"
+ " by multiple paths (yet): " + providedOrdering);
}
}

private class IterativeSearchOrdering {

private final List<? extends ObjectOrdering> providedOrdering;

private IterativeSearchOrdering(ObjectPaging originalPaging) throws RepositoryException {
providedOrdering = originalPaging != null
? originalPaging.getOrderingInstructions()
: null;

// if (providedOrdering != null) {
// TODO
// if (providedOrdering.stream().anyMatch(o -> QNameUtil.equals()o.getOrderBy()))
// checkProvidedOrdering(providedOrdering);
// }
}
}

/**
* Without requested ordering, this is easy: `WHERE id > lastId AND timestamp > lastTimestamp`.
* Timestamp is used to help with partition pruning and is part of the PK anyway.
* Similar to {@link SqaleRepositoryService#lastOidCondition}.
*
* TODO, lots of possible improvements:
*
* But with outside ordering we need to respect it and for ordering by X, Y, Z use:
* * Just like in repo iterative search this is added to the original filter with `AND`.
* However, if `timestamp` is used in the original filter, it could be replaced by stricter
* `timestamp` condition based on the `lastProcessedObject` - this is not implemented yet.
* ** TODO: If provided, do we want to add another timestamp condition?
* Would it help with partitions?
* Probably, with ID condition + ordering, it's not a big deal to leave it out.
*
* ----
* -- the first part of WHERE with original conditions is taken care of outside of this method
* ... WHERE original conditions AND (
* X > last.X
* OR (X = last.X AND Y > last.Y)
* OR (X = last.X AND Y = last.Y AND Z > last.Z)
* OR (X = last.X AND Y = last.Y ...if all equal
* AND id > lastId AND timestamp > lastTimestamp) -- only here is ID + timestamp
* ----
* * Support for multiple order specifications from the client is not supported.
* Perhaps some short-term stateful filter/order object would be better to construct this,
* especially if it could be used in both repo and audit (with strict order attribute
* provided in constructor for example).
*
* This can be further complicated by the fact that both ID (F_REPO_ID) and timestamp
* (F_TIMESTAMP) can be part of custom ordering, in which case it must be omitted
* from the internally added conditions and ordering.
* [NOTE]
* ====
* Both `timestamp` and `repoId` is used for iterative search condition, but *only `repoId`*
* is used for additional ordering to assure strict reliable ordering.
* This can be further complicated by the fact that both `repoId` and `timestamp`
* can be part of custom ordering from client (this is different from repository, where `oid`
* is not valid for filter and ordering on the model level, even when it's usable for repository).
* If used on the top-level `AND` group, they can be be replaced by next iteration condition,
* which is likely more selective.
*
* This is suddenly much more fun, isn't it?
* Of course the condition `>` or `<` depends on `ASC` vs `DESC`.
* As for the ordering, if multiple ordering is used (*not supported yet*) if `repoId` is used,
* anything after it can be omitted as irrelevant.
* ====
*
* TODO: Currently, single path ordering is supported. Finish multi-path too.
* TODO: What about nullable columns?
*/
@Nullable
private <T extends ObjectType> ObjectFilter lastOidCondition(
AuditEventRecordType lastProcessedObject, IterativeSearchOrdering providedOrdering) {
/*
private ObjectFilter iterativeSearchCondition(
@Nullable AuditEventRecordType lastProcessedObject,
List<? extends ObjectOrdering> providedOrdering) {
if (lastProcessedObject == null) {
return null;
}

String lastProcessedOid = lastProcessedObject.getOid();
// TODO inspect originalFilter to detect timestamp/repoId conditions.
// Only top level AND filter should be checked, anything else is irrelevant
// for the decision whether to skip additional timestamp condition.
// BTW: We CANNOT skip repoId condition, that one is CRITICAL for proper iterating.

Long lastProcessedId = lastProcessedObject.getRepoId();
XMLGregorianCalendar lastProcessedTimestamp = lastProcessedObject.getTimestamp();
if (providedOrdering == null || providedOrdering.isEmpty()) {
return prismContext()
.queryFor(lastProcessedObject.getCompileTimeClass())
.item(OID_PATH).gt(lastProcessedOid).buildFilter();
.queryFor(AuditEventRecordType.class)
.item(AuditEventRecordType.F_REPO_ID).gt(lastProcessedId)
.and()
// timestamp of the next entry can be the same, we need greater-or-equal here
.item(AuditEventRecordType.F_TIMESTAMP).ge(lastProcessedTimestamp)
.buildFilter();
}

if (providedOrdering.size() == 1) {
ObjectOrdering objectOrdering = providedOrdering.get(0);
ItemPath orderByPath = objectOrdering.getOrderBy();
boolean asc = objectOrdering.getDirection() == OrderDirection.ASCENDING;
S_ConditionEntry filter = prismContext()
.queryFor(lastProcessedObject.getCompileTimeClass())
.queryFor(AuditEventRecordType.class)
.item(orderByPath);
//noinspection rawtypes
Item<PrismValue, ItemDefinition> item = lastProcessedObject.findItem(orderByPath);
@SuppressWarnings("unchecked")
Item<PrismValue, ItemDefinition<?>> item =
lastProcessedObject.asPrismContainerValue().findItem(orderByPath);
if (item.size() > 1) {
throw new IllegalArgumentException(
"Multi-value property for ordering is forbidden - item: " + item);
Expand All @@ -712,39 +731,15 @@ private <T extends ObjectType> ObjectFilter lastOidCondition(
.block()
.item(orderByPath).eq(item.getRealValue())
.and()
.item(OID_PATH);
return (asc ? filter.gt(lastProcessedOid) : filter.lt(lastProcessedOid))
.item(AuditEventRecordType.F_REPO_ID);
return (asc ? filter.gt(lastProcessedId) : filter.lt(lastProcessedId))
.endBlock()
.buildFilter();
}
}
*/

throw new IllegalArgumentException(
"Shouldn't get here with check in executeSearchObjectsIterative()");
/*
TODO: Unfinished - this is painful with fluent API. Should I call
prismContext().queryFor(lastProcessedObject.getCompileTimeClass()) for each component
and then use ObjectQueryUtil.filterAnd/Or?
// we need to handle the complicated case with externally provided ordering
S_FilterEntryOrEmpty orBlock = prismContext()
.queryFor(lastProcessedObject.getCompileTimeClass()).block();
orLoop:
for (ObjectOrdering orMasterOrdering : providedOrdering) {
Iterator<? extends ObjectOrdering> iterator = providedOrdering.iterator();
while (iterator.hasNext()) {
S_FilterEntryOrEmpty andBlock = orBlock.block();
ObjectOrdering ordering = iterator.next();
if (ordering.equals(orMasterOrdering)) {
// ...
continue orLoop;
}
orBlock = andBlock.endBlock();
}
}
return orBlock.endBlock().buildFilter();
*/
}

protected long registerOperationStart(String kind) {
Expand Down
Loading

0 comments on commit 749ca45

Please sign in to comment.