diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java index 7237157b472..60810949201 100644 --- a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java +++ b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java @@ -593,9 +593,30 @@ private ModifyObjectResult modifyObjectInternal( invokeConflictWatchers(w -> w.beforeModifyObject(prismObject)); PrismObject originalObject = prismObject.clone(); // for result later - modifications = updateContext.execute(modifications); - updateContext.jdbcSession().commit(); - + boolean reindex = options.isForceReindex(); + + if (reindex) { + // UpdateTables is false, we want only to process modifications on fullObject + // do not modify nested items. + modifications = updateContext.execute(modifications, false); + // We delete original object and cascade of referenced tables, this will also + // remove additional rows, which may not be present in full object + // after desync + updateContext.jdbcSession().newDelete(updateContext.entityPath()) + .where(updateContext.entityPath().oid.eq(updateContext.objectOid())) + .execute(); + try { + // We add object again, this will ensure recreation of all indices and correct + // table rows again + new AddObjectContext<>(sqlRepoContext, updateContext.getPrismObject()) + .executeReindexed(updateContext.jdbcSession()); + } catch (SchemaException | ObjectAlreadyExistsException e) { + throw new RepositoryException("Update with reindex failed", e); + } + } else { + modifications = updateContext.execute(modifications); + updateContext.jdbcSession().commit(); + } logger.trace("OBJECT after:\n{}", prismObject.debugDumpLazily()); if (!modifications.isEmpty()) { @@ -774,7 +795,7 @@ public int countObjects(Class 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) { @@ -785,7 +806,7 @@ public int countObjects(Class type, ObjectQuery query, } } - private int executeCountObject( + private int executeCountObjects( @NotNull Class type, ObjectQuery query, Collection> options) @@ -825,7 +846,7 @@ private 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) { @@ -836,7 +857,7 @@ private int executeCountObject( } } - private SearchResultList> executeSearchObject( + private SearchResultList> executeSearchObjects( @NotNull Class type, ObjectQuery query, Collection> options, @@ -981,7 +1002,7 @@ private SearchResultMetadata executeSearchObjectsIterativ // we don't call public searchObject to avoid subresults and query simplification logSearchInputParameters(type, pagedQuery, "Search object iterative page"); - List> objects = executeSearchObject( + List> objects = executeSearchObjects( type, pagedQuery, options, OP_SEARCH_OBJECTS_ITERATIVE_PAGE); // process page results @@ -1282,7 +1303,7 @@ public PrismObject searchShadowOwner(String shadowOid, .item(FocusType.F_LINK_REF).ref(shadowOid) .build(); SearchResultList> 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 @@ -1880,9 +1901,4 @@ private boolean pruneDiagnosticInformation( return true; } } - - @Override - public SqlPerformanceMonitorImpl getPerformanceMonitor() { - return performanceMonitor; - } } diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleServiceBase.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleServiceBase.java index 0206770c123..ffa6ba387e6 100644 --- a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleServiceBase.java +++ b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleServiceBase.java @@ -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 /** diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/SqaleAuditService.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/SqaleAuditService.java index bf564414bfa..e2609bad4a0 100644 --- a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/SqaleAuditService.java +++ b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/SqaleAuditService.java @@ -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; @@ -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.*; @@ -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, @@ -555,7 +571,13 @@ private SearchResultMetadata executeSearchObjectsIterative( // this is total requested size of the search Integer maxSize = originalPaging != null ? originalPaging.getMaxSize() : null; - IterativeSearchOrdering ordering = new IterativeSearchOrdering(originalPaging); + List 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(); @@ -563,6 +585,7 @@ private SearchResultMetadata executeSearchObjectsIterative( 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); @@ -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 @@ -623,69 +647,63 @@ private SearchResultMetadata executeSearchObjectsIterative( } } - private void checkProvidedOrdering(List 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 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 ObjectFilter lastOidCondition( - AuditEventRecordType lastProcessedObject, IterativeSearchOrdering providedOrdering) { - /* + private ObjectFilter iterativeSearchCondition( + @Nullable AuditEventRecordType lastProcessedObject, + List 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) { @@ -693,10 +711,11 @@ private ObjectFilter lastOidCondition( 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 item = lastProcessedObject.findItem(orderByPath); + @SuppressWarnings("unchecked") + Item> item = + lastProcessedObject.asPrismContainerValue().findItem(orderByPath); if (item.size() > 1) { throw new IllegalArgumentException( "Multi-value property for ordering is forbidden - item: " + item); @@ -712,39 +731,15 @@ private 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 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) { diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/update/AddObjectContext.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/update/AddObjectContext.java index d48faf38207..54e81c0cfde 100644 --- a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/update/AddObjectContext.java +++ b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/update/AddObjectContext.java @@ -50,18 +50,14 @@ public AddObjectContext( */ public String execute() throws SchemaException, ObjectAlreadyExistsException { - try { + try (JdbcSession jdbcSession = repositoryContext.newJdbcSession().startTransaction()) { object.setVersion("1"); // initial add always uses 1 as version number - Class schemaObjectClass = object.getCompileTimeClass(); - objectType = MObjectType.fromSchemaType(schemaObjectClass); - rootMapping = repositoryContext.getMappingBySchemaType(schemaObjectClass); - root = rootMapping.defaultAlias(); - + initContexts(); if (object.getOid() == null) { - return addObjectWithoutOid(); + return addObjectWithoutOid(jdbcSession); } else { // this also handles overwrite after ObjectNotFoundException - return addObjectWithOid(); + return addObjectWithOid(jdbcSession); } } catch (QueryException e) { // Querydsl exception, not ours Throwable cause = e.getCause(); @@ -72,59 +68,76 @@ public String execute() } } - private String addObjectWithOid() throws SchemaException { - long lastCid = new ContainerValueIdGenerator(object).generateForNewObject(); - try (JdbcSession jdbcSession = repositoryContext.newJdbcSession().startTransaction()) { - S schemaObject = object.asObjectable(); - R row = rootMapping.toRowObjectWithoutFullObject(schemaObject, jdbcSession); - row.containerIdSeq = lastCid + 1; - rootMapping.setFullObject(row, schemaObject); - - UUID oid = jdbcSession.newInsert(root) - // default populate mapper ignores null, that's good, especially for objectType - .populate(row) - .executeWithKey(root.oid); - - row.objectType = objectType; // sub-entities can use it, now it's safe to set it - rootMapping.storeRelatedEntities(row, schemaObject, jdbcSession); - - jdbcSession.commit(); - return Objects.requireNonNull(oid, "OID of inserted object can't be null") - .toString(); + public void executeReindexed(JdbcSession jdbcSession) + throws SchemaException, ObjectAlreadyExistsException { + try { + initContexts(); + addObjectWithOid(jdbcSession); + } catch (QueryException e) { // Querydsl exception, not ours + Throwable cause = e.getCause(); + if (cause instanceof PSQLException) { + SqaleUtils.handlePostgresException((PSQLException) cause); + } + throw e; } } - private String addObjectWithoutOid() throws SchemaException { - try (JdbcSession jdbcSession = repositoryContext.newJdbcSession().startTransaction()) { - S schemaObject = object.asObjectable(); - R row = rootMapping.toRowObjectWithoutFullObject(schemaObject, jdbcSession); - - // first insert without full object, because we don't know the OID yet - UUID oid = jdbcSession.newInsert(root) - // default populate mapper ignores null, that's good, especially for objectType - .populate(row) - .executeWithKey(root.oid); - String oidString = - Objects.requireNonNull(oid, "OID of inserted object can't be null") - .toString(); - object.setOid(oidString); - - long lastCid = new ContainerValueIdGenerator(object).generateForNewObject(); - - // now to update full object with known OID - rootMapping.setFullObject(row, schemaObject); - jdbcSession.newUpdate(root) - .set(root.fullObject, row.fullObject) - .set(root.containerIdSeq, lastCid + 1) - .where(root.oid.eq(oid)) - .execute(); - - row.oid = oid; - row.objectType = objectType; // sub-entities can use it, now it's safe to set it - rootMapping.storeRelatedEntities(row, schemaObject, jdbcSession); - - jdbcSession.commit(); - return oidString; - } + private void initContexts() { + Class schemaObjectClass = object.getCompileTimeClass(); + objectType = MObjectType.fromSchemaType(schemaObjectClass); + rootMapping = repositoryContext.getMappingBySchemaType(schemaObjectClass); + root = rootMapping.defaultAlias(); + } + + private String addObjectWithOid(JdbcSession jdbcSession) throws SchemaException { + long lastCid = new ContainerValueIdGenerator(object).generateForNewObject(); + S schemaObject = object.asObjectable(); + R row = rootMapping.toRowObjectWithoutFullObject(schemaObject, jdbcSession); + row.containerIdSeq = lastCid + 1; + rootMapping.setFullObject(row, schemaObject); + + UUID oid = jdbcSession.newInsert(root) + // default populate mapper ignores null, that's good, especially for objectType + .populate(row) + .executeWithKey(root.oid); + + row.objectType = objectType; // sub-entities can use it, now it's safe to set it + rootMapping.storeRelatedEntities(row, schemaObject, jdbcSession); + + jdbcSession.commit(); + return Objects.requireNonNull(oid, "OID of inserted object can't be null") + .toString(); + } + + private String addObjectWithoutOid(JdbcSession jdbcSession) throws SchemaException { + S schemaObject = object.asObjectable(); + R row = rootMapping.toRowObjectWithoutFullObject(schemaObject, jdbcSession); + + // first insert without full object, because we don't know the OID yet + UUID oid = jdbcSession.newInsert(root) + // default populate mapper ignores null, that's good, especially for objectType + .populate(row) + .executeWithKey(root.oid); + String oidString = + Objects.requireNonNull(oid, "OID of inserted object can't be null") + .toString(); + object.setOid(oidString); + + long lastCid = new ContainerValueIdGenerator(object).generateForNewObject(); + + // now to update full object with known OID + rootMapping.setFullObject(row, schemaObject); + jdbcSession.newUpdate(root) + .set(root.fullObject, row.fullObject) + .set(root.containerIdSeq, lastCid + 1) + .where(root.oid.eq(oid)) + .execute(); + + row.oid = oid; + row.objectType = objectType; // sub-entities can use it, now it's safe to set it + rootMapping.storeRelatedEntities(row, schemaObject, jdbcSession); + + jdbcSession.commit(); + return oidString; } } diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/update/RootUpdateContext.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/update/RootUpdateContext.java index b1f94c607e1..a12c739d2ec 100644 --- a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/update/RootUpdateContext.java +++ b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/update/RootUpdateContext.java @@ -83,7 +83,12 @@ public QObjectMapping mapping() { public Collection> execute( Collection> modifications) throws SchemaException, RepositoryException { + return execute(modifications, true); + } + public Collection> execute( + Collection> modifications, boolean updateTables) + throws SchemaException, RepositoryException { PrismObject prismObject = getPrismObject(); // I reassign here, we DON'T want original modifications to be used further by accident @@ -101,7 +106,7 @@ public QObjectMapping mapping() { for (ItemDelta modification : modifications) { try { - processModification(modification); + processModification(modification, updateTables); } catch (IllegalArgumentException e) { logger.warn("Modification failed with '{}': {}", e, modification); throw new SystemException("Modification failed: " + modification, e); @@ -114,13 +119,14 @@ public QObjectMapping mapping() { return modifications; } - private void processModification(ItemDelta modification) + private void processModification(ItemDelta modification, boolean updateTables) throws RepositoryException, SchemaException { cidGenerator.processModification(modification); resolveContainerIdsForValuesToDelete(modification); modification.applyTo(getPrismObject()); - - new DelegatingItemDeltaProcessor(this).process(modification); + if (updateTables) { + new DelegatingItemDeltaProcessor(this).process(modification); + } } private void resolveContainerIdsForValuesToDelete(ItemDelta modification) { diff --git a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/SqaleRepoBaseTest.java b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/SqaleRepoBaseTest.java index 93334a703fb..f06c220486a 100644 --- a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/SqaleRepoBaseTest.java +++ b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/SqaleRepoBaseTest.java @@ -25,7 +25,9 @@ import com.evolveum.midpoint.prism.*; import com.evolveum.midpoint.prism.path.ItemName; +import com.evolveum.midpoint.prism.query.ObjectQuery; import com.evolveum.midpoint.repo.api.perf.OperationPerformanceInformation; +import com.evolveum.midpoint.repo.sqale.audit.SqaleAuditService; import com.evolveum.midpoint.repo.sqale.qmodel.common.QUri; import com.evolveum.midpoint.repo.sqale.qmodel.ext.MExtItem; import com.evolveum.midpoint.repo.sqale.qmodel.ext.MExtItemHolderType; @@ -37,11 +39,16 @@ import com.evolveum.midpoint.repo.sqlbase.JdbcSession; import com.evolveum.midpoint.repo.sqlbase.perfmon.SqlPerformanceMonitorImpl; import com.evolveum.midpoint.repo.sqlbase.querydsl.FlexibleRelationalPathBase; +import com.evolveum.midpoint.schema.GetOperationOptions; import com.evolveum.midpoint.schema.RelationRegistry; +import com.evolveum.midpoint.schema.SearchResultList; +import com.evolveum.midpoint.schema.SelectorOptions; +import com.evolveum.midpoint.schema.result.OperationResult; import com.evolveum.midpoint.test.util.AbstractSpringTest; import com.evolveum.midpoint.test.util.InfraTestMixin; import com.evolveum.midpoint.util.QNameUtil; import com.evolveum.midpoint.util.exception.SchemaException; +import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType; import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowAttributesType; import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowType; @@ -49,6 +56,9 @@ public class SqaleRepoBaseTest extends AbstractSpringTest implements InfraTestMixin { + public static final String REPO_OP_PREFIX = SqaleRepositoryService.class.getSimpleName() + '.'; + public static final String AUDIT_OP_PREFIX = SqaleAuditService.class.getSimpleName() + '.'; + @Autowired protected SqaleRepositoryService repositoryService; @Autowired protected SqaleRepoContext sqlRepoContext; @Autowired protected SqaleRepositoryConfiguration repositoryConfiguration; @@ -268,10 +278,8 @@ protected void assertSingleOperationRecorded(String opKind) { } protected void assertOperationRecordedCount(String opKind, int count) { - // TODO see comment in SqaleServiceBase.registerOperationStart - opKind = "SqaleRepositoryService." + opKind; Map pmAllData = - repositoryService.getPerformanceMonitor() + getPerformanceMonitor() .getGlobalPerformanceInformation().getAllData(); OperationPerformanceInformation operationInfo = pmAllData.get(opKind); if (count != 0) { @@ -349,8 +357,13 @@ private String extKey(Containerable extContainer, String itemName, MExtItemHolde } } + /** Returns performance monitor from repository service, override to get the one from audit. */ + protected SqlPerformanceMonitorImpl getPerformanceMonitor() { + return repositoryService.getPerformanceMonitor(); + } + protected void clearPerformanceMonitor() { - SqlPerformanceMonitorImpl pm = repositoryService.getPerformanceMonitor(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); pm.clearGlobalPerformanceInformation(); assertThat(pm.getGlobalPerformanceInformation().getAllData()).isEmpty(); } @@ -362,6 +375,24 @@ protected void refreshOrgClosureForce() { } } + /** Low-level shortcut for {@link SqaleRepositoryService#searchObjects}, no checks. */ + @SafeVarargs + @NotNull + protected final SearchResultList repositorySearchObjects( + @NotNull Class type, + ObjectQuery query, + OperationResult operationResult, + SelectorOptions... selectorOptions) + throws SchemaException { + return repositoryService.searchObjects( + type, + query, + selectorOptions != null && selectorOptions.length != 0 + ? List.of(selectorOptions) : null, + operationResult) + .map(p -> p.asObjectable()); + } + /** * Helper to make setting shadow attributes easier. * diff --git a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleAuditSearchIterativeTest.java b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleAuditSearchIterativeTest.java new file mode 100644 index 00000000000..f71568746c2 --- /dev/null +++ b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleAuditSearchIterativeTest.java @@ -0,0 +1,325 @@ +/* + * Copyright (C) 2010-2021 Evolveum and contributors + * + * This work is dual-licensed under the Apache License 2.0 + * and European Union Public License. See LICENSE file for details. + */ +package com.evolveum.midpoint.repo.sqale.func; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.Random; +import java.util.function.Predicate; + +import org.springframework.beans.factory.annotation.Autowired; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.evolveum.midpoint.audit.api.AuditEventRecord; +import com.evolveum.midpoint.audit.api.AuditResultHandler; +import com.evolveum.midpoint.audit.api.AuditService; +import com.evolveum.midpoint.init.AuditServiceProxy; +import com.evolveum.midpoint.prism.query.ObjectQuery; +import com.evolveum.midpoint.repo.sqale.SqaleRepoBaseTest; +import com.evolveum.midpoint.repo.sqale.audit.SqaleAuditService; +import com.evolveum.midpoint.repo.sqale.audit.qmodel.QAuditEventRecord; +import com.evolveum.midpoint.repo.sqale.audit.qmodel.QAuditEventRecordMapping; +import com.evolveum.midpoint.repo.sqlbase.perfmon.SqlPerformanceMonitorImpl; +import com.evolveum.midpoint.schema.GetOperationOptions; +import com.evolveum.midpoint.schema.SearchResultMetadata; +import com.evolveum.midpoint.schema.SelectorOptions; +import com.evolveum.midpoint.schema.result.OperationResult; +import com.evolveum.midpoint.task.api.test.NullTaskImpl; +import com.evolveum.midpoint.util.exception.SchemaException; +import com.evolveum.midpoint.xml.ns._public.common.audit_3.AuditEventRecordType; +import com.evolveum.midpoint.xml.ns._public.common.common_3.UserType; + +/** + * This tests {@link SqaleAuditService#searchObjectsIterative} in a gray-box style, + * knowing that it uses {@link SqaleAuditService#executeSearchObjects} internally. + * Inspired by {@link SqaleRepoSearchIterativeTest}. + */ +public class SqaleAuditSearchIterativeTest extends SqaleRepoBaseTest { + + private final TestResultHandler testHandler = new TestResultHandler(); + + // default page size for iterative search, reset before each test + private static final int ITERATION_PAGE_SIZE = 100; + + @Autowired private AuditService auditService; + + private final long startTimestamp = System.currentTimeMillis(); + + // alias for queries, can't be static/final, the mapping it's not yet initialized + private QAuditEventRecord aer; + private SqlPerformanceMonitorImpl performanceMonitor; + + @BeforeClass + public void initObjects() throws Exception { + performanceMonitor = + ((AuditServiceProxy) auditService).getImplementation(SqaleAuditService.class) + .getPerformanceMonitor(); + + aer = QAuditEventRecordMapping.get().defaultAlias(); + clearAudit(); + + OperationResult result = createOperationResult(); + + long timestamp = startTimestamp; + Random random = new Random(); + // we will create two full "pages" of data + for (int i = 1; i <= ITERATION_PAGE_SIZE * 2; i++) { + AuditEventRecord record = new AuditEventRecord(); + record.setParameter(paramString(i)); + record.setTimestamp(timestamp); + auditService.audit(record, NullTaskImpl.INSTANCE, result); + + // 50% chance to change the timestamp by up to a second + timestamp += random.nextInt(2) * random.nextInt(1000); + } + } + + // We need to pad the param string so it's possible to compare it. + private String paramString(int i) { + return String.format("%05d", i); + } + + @BeforeMethod + public void resetTestHandler() { + testHandler.reset(); + repositoryConfiguration.setIterativeSearchByPagingBatchSize(ITERATION_PAGE_SIZE); + } + + @Test + public void test100SearchIterativeWithNoneFilter() throws Exception { + OperationResult operationResult = createOperationResult(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); + pm.clearGlobalPerformanceInformation(); + + given("query with top level NONE filter"); + ObjectQuery query = prismContext.queryFor(UserType.class) + .none() + .build(); + + when("calling search iterative"); + SearchResultMetadata searchResultMetadata = + searchObjectsIterative(query, operationResult); + + then("no operation is performed"); + assertThatOperationResult(operationResult).isSuccess(); + assertThat(searchResultMetadata).isNotNull(); + assertThat(searchResultMetadata.getApproxNumberOfAllResults()).isZero(); + // this is not the main part, just documenting that currently we short circuit the operation + assertOperationRecordedCount( + AUDIT_OP_PREFIX + AuditService.OP_SEARCH_OBJECTS_ITERATIVE, 0); + // this is important - no actual search was called + assertOperationRecordedCount( + AUDIT_OP_PREFIX + AuditService.OP_SEARCH_OBJECTS_ITERATIVE_PAGE, 0); + } + + @Test + public void test110SearchIterativeWithEmptyFilter() throws Exception { + OperationResult operationResult = createOperationResult(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); + pm.clearGlobalPerformanceInformation(); + + when("calling search iterative with null query"); + SearchResultMetadata metadata = + searchObjectsIterative(null, operationResult); + + then("result metadata is not null and reports the handled objects"); + assertThatOperationResult(operationResult).isSuccess(); + assertThat(metadata).isNotNull(); + assertThat(metadata.getApproxNumberOfAllResults()).isEqualTo(testHandler.processedCount()); + assertThat(metadata.isPartialResults()).isFalse(); + assertThat(metadata.getPagingCookie()).isNotNull(); + + and("search operations were called"); + assertOperationRecordedCount(AUDIT_OP_PREFIX + AuditService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertTypicalPageOperationCount(metadata); + + and("all objects of the specified type (here User) were processed"); + assertThat(testHandler.processedCount()).isEqualTo(count(aer)); + } + + @Test + public void test111SearchIterativeWithLastPageNotFull() throws Exception { + OperationResult operationResult = createOperationResult(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); + pm.clearGlobalPerformanceInformation(); + given("total result count not multiple of the page size"); + repositoryConfiguration.setIterativeSearchByPagingBatchSize(47); + + when("calling search iterative with null query"); + SearchResultMetadata metadata = + searchObjectsIterative(null, operationResult); + + then("result metadata is not null and reports the handled objects"); + assertThatOperationResult(operationResult).isSuccess(); + assertThat(metadata).isNotNull(); + assertThat(metadata.getApproxNumberOfAllResults()).isEqualTo(testHandler.processedCount()); + assertThat(metadata.isPartialResults()).isFalse(); + assertThat(metadata.getPagingCookie()).isNotNull(); + + and("search operations were called"); + assertOperationRecordedCount(AUDIT_OP_PREFIX + AuditService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertTypicalPageOperationCount(metadata); + + and("all objects of the specified type were processed"); + assertThat(testHandler.processedCount()).isEqualTo(count(aer)); + } + + @Test + public void test115SearchIterativeWithBreakingConditionCheckingOidOrdering() throws Exception { + OperationResult operationResult = createOperationResult(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); + pm.clearGlobalPerformanceInformation(); + + given("condition that breaks iterative search based on parameter (count)"); + testHandler.setStoppingPredicate(aer -> aer.getParameter().equals(paramString(ITERATION_PAGE_SIZE))); + + when("calling search iterative with null query"); + SearchResultMetadata metadata = searchObjectsIterative(null, operationResult); + + then("result metadata is not null and reports partial result (because of the break)"); + assertThat(metadata).isNotNull(); + assertThat(metadata.getApproxNumberOfAllResults()).isEqualTo(testHandler.processedCount()); + assertThat(metadata.isPartialResults()).isTrue(); // extremely likely with enough items + + and("search operations were called"); + assertOperationRecordedCount(AUDIT_OP_PREFIX + AuditService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertTypicalPageOperationCount(metadata); + + and("all objects up to specified parameter value were processed"); + assertThat(testHandler.processedCount()) + .isEqualTo(count(aer, aer.parameter.loe(paramString(ITERATION_PAGE_SIZE)))); + } + + @Test + public void test120SearchIterativeWithMaxSize() throws Exception { + OperationResult operationResult = createOperationResult(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); + pm.clearGlobalPerformanceInformation(); + + given("query with maxSize specified"); + ObjectQuery query = prismContext.queryFor(UserType.class) + .maxSize(101) + .build(); + + when("calling search iterative"); + SearchResultMetadata metadata = searchObjectsIterative(query, operationResult); + + then("result metadata is not null and reports partial result (because of the break)"); + assertThat(metadata).isNotNull(); + assertThat(metadata.getApproxNumberOfAllResults()).isEqualTo(testHandler.processedCount()); + assertThat(metadata.isPartialResults()).isFalse(); + + and("search operations were called"); + assertOperationRecordedCount( + AUDIT_OP_PREFIX + AuditService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertTypicalPageOperationCount(metadata); + + and("specified amount of objects was processed"); + assertThat(testHandler.processedCount()).isEqualTo(101); + } + + @Test + public void test125SearchIterativeWithCustomOrdering() throws Exception { + OperationResult operationResult = createOperationResult(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); + pm.clearGlobalPerformanceInformation(); + + given("query with custom ordering"); + int limit = 47; + ObjectQuery query = prismContext.queryFor(AuditEventRecordType.class) + .asc(AuditEventRecordType.F_PARAMETER) + .maxSize(limit) + .build(); + + when("calling search iterative"); + SearchResultMetadata metadata = searchObjectsIterative(query, operationResult); + + then("result metadata is not null and reports partial result (because of the break)"); + assertThatOperationResult(operationResult).isSuccess(); + assertThat(metadata).isNotNull(); + assertThat(metadata.getApproxNumberOfAllResults()).isEqualTo(testHandler.processedCount()); + assertThat(metadata.getApproxNumberOfAllResults()).isEqualTo(limit); + assertThat(metadata.isPartialResults()).isFalse(); // everything was processed + + and("search operations were called"); + assertOperationRecordedCount( + AUDIT_OP_PREFIX + AuditService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertTypicalPageOperationCount(metadata); + + and("all objects were processed in proper order"); + Iterator processedItems = testHandler.processedEvents.iterator(); + for (int i = 1; i < limit; i++) { + assertThat(processedItems.next().getParameter()).isEqualTo(paramString(i)); + } + } + + @SafeVarargs + private SearchResultMetadata searchObjectsIterative( + ObjectQuery query, + OperationResult operationResult, + SelectorOptions... selectorOptions) + throws SchemaException { + + display("QUERY: " + query); + return auditService.searchObjectsIterative( + query, + testHandler, + selectorOptions != null && selectorOptions.length != 0 + ? List.of(selectorOptions) : null, + operationResult); + } + + private void assertTypicalPageOperationCount(SearchResultMetadata metadata) { + boolean lastRowCausingPartialResult = metadata.isPartialResults() + && metadata.getApproxNumberOfAllResults() % getConfiguredPageSize() == 0; + + assertOperationRecordedCount( + AUDIT_OP_PREFIX + AuditService.OP_SEARCH_OBJECTS_ITERATIVE_PAGE, + metadata.getApproxNumberOfAllResults() / getConfiguredPageSize() + + (lastRowCausingPartialResult ? 0 : 1)); + } + + private int getConfiguredPageSize() { + return sqlRepoContext.getJdbcRepositoryConfiguration() + .getIterativeSearchByPagingBatchSize(); + } + + private static class TestResultHandler implements AuditResultHandler { + + private final List processedEvents = new ArrayList<>(); + private Predicate stoppingPredicate; + + public void reset() { + processedEvents.clear(); + stoppingPredicate = o -> false; + } + + public int processedCount() { + return processedEvents.size(); + } + + public void setStoppingPredicate(Predicate stoppingPredicate) { + this.stoppingPredicate = stoppingPredicate; + } + + @Override + public boolean handle(AuditEventRecordType eventRecord, OperationResult parentResult) { + processedEvents.add(eventRecord); + return !stoppingPredicate.test(eventRecord); // true means continue, so we need NOT + } + } + + @Override + protected SqlPerformanceMonitorImpl getPerformanceMonitor() { + return performanceMonitor; + } +} diff --git a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoAddDeleteObjectTest.java b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoAddDeleteObjectTest.java index 888dd6be0bb..9496fe873f6 100644 --- a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoAddDeleteObjectTest.java +++ b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoAddDeleteObjectTest.java @@ -365,7 +365,7 @@ public void test150AddOperationUpdatesPerformanceMonitor() then("performance monitor is updated"); assertThatOperationResult(result).isSuccess(); - assertSingleOperationRecorded(RepositoryService.OP_ADD_OBJECT); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_ADD_OBJECT); } @Test @@ -384,7 +384,7 @@ public void test151OverwriteOperationUpdatesPerformanceMonitor() then("performance monitor is updated"); assertThatOperationResult(result).isSuccess(); - assertSingleOperationRecorded(RepositoryService.OP_ADD_OBJECT_OVERWRITE); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_ADD_OBJECT_OVERWRITE); } @Test @@ -2428,7 +2428,7 @@ public void test920DeleteOperationUpdatesPerformanceMonitor() then("performance monitor is updated"); assertThatOperationResult(result).isSuccess(); - assertSingleOperationRecorded(RepositoryService.OP_DELETE_OBJECT); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_DELETE_OBJECT); } @Test diff --git a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoModifyObjectTest.java b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoModifyObjectTest.java index 9087617f677..f3cb3069cad 100644 --- a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoModifyObjectTest.java +++ b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoModifyObjectTest.java @@ -9,15 +9,13 @@ import static java.util.Comparator.comparing; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; import java.time.Instant; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.UUID; +import java.util.*; import java.util.stream.Collectors; import javax.xml.namespace.QName; @@ -26,23 +24,25 @@ import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; -import com.evolveum.midpoint.prism.Item; -import com.evolveum.midpoint.prism.PrismContainerValue; -import com.evolveum.midpoint.prism.PrismContext; -import com.evolveum.midpoint.prism.SerializationOptions; +import com.evolveum.midpoint.prism.*; import com.evolveum.midpoint.prism.delta.ItemDelta; import com.evolveum.midpoint.prism.delta.ObjectDelta; import com.evolveum.midpoint.prism.path.ItemName; import com.evolveum.midpoint.prism.path.ItemPath; import com.evolveum.midpoint.prism.polystring.PolyString; +import com.evolveum.midpoint.prism.query.ObjectQuery; +import com.evolveum.midpoint.repo.api.RepoModifyOptions; import com.evolveum.midpoint.repo.api.RepositoryService; import com.evolveum.midpoint.repo.sqale.SqaleRepoBaseTest; +import com.evolveum.midpoint.repo.sqale.SqaleUtils; import com.evolveum.midpoint.repo.sqale.jsonb.Jsonb; import com.evolveum.midpoint.repo.sqale.qmodel.accesscert.*; import com.evolveum.midpoint.repo.sqale.qmodel.assignment.*; import com.evolveum.midpoint.repo.sqale.qmodel.common.MContainerType; import com.evolveum.midpoint.repo.sqale.qmodel.focus.MUser; import com.evolveum.midpoint.repo.sqale.qmodel.focus.QUser; +import com.evolveum.midpoint.repo.sqale.qmodel.focus.QUserMapping; +import com.evolveum.midpoint.repo.sqale.qmodel.object.MObject; import com.evolveum.midpoint.repo.sqale.qmodel.object.MObjectType; import com.evolveum.midpoint.repo.sqale.qmodel.ref.MReference; import com.evolveum.midpoint.repo.sqale.qmodel.ref.QObjectReference; @@ -53,8 +53,10 @@ import com.evolveum.midpoint.repo.sqale.qmodel.shadow.QShadow; import com.evolveum.midpoint.repo.sqale.qmodel.task.MTask; import com.evolveum.midpoint.repo.sqale.qmodel.task.QTask; +import com.evolveum.midpoint.repo.sqlbase.JdbcSession; import com.evolveum.midpoint.schema.GetOperationOptions; import com.evolveum.midpoint.schema.SchemaService; +import com.evolveum.midpoint.schema.SearchResultList; import com.evolveum.midpoint.schema.SelectorOptions; import com.evolveum.midpoint.schema.result.OperationResult; import com.evolveum.midpoint.schema.util.ObjectTypeUtil; @@ -3398,7 +3400,171 @@ public void test920ModifyOperationUpdatesPerformanceMonitor() then("performance monitor is updated"); assertThatOperationResult(result).isSuccess(); - assertSingleOperationRecorded(RepositoryService.OP_MODIFY_OBJECT); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_MODIFY_OBJECT); + } + + @Test + public void test950ModifyOperationWithReindexUpdatesPerformanceMonitor() + throws SchemaException, ObjectNotFoundException, ObjectAlreadyExistsException { + OperationResult result = createOperationResult(); + given("Full object was modified in database"); + clearPerformanceMonitor(); + when("object is modified in the repository"); + + RepoModifyOptions options = RepoModifyOptions.createForceReindex(); + repositoryService.modifyObject(UserType.class, user1Oid, Collections.emptyList(), options, result); + + then("performance monitor is updated"); + assertThatOperationResult(result).isSuccess(); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_MODIFY_OBJECT); + } + + @Test + public void test951ReindexAfterManualChangeOfFullObject() + throws SchemaException, ObjectNotFoundException, ObjectAlreadyExistsException { + OperationResult result = createOperationResult(); + + UserType user = new UserType(prismContext).name("corrupted") + .beginAssignment() + .policySituation("kept") + .end() + .beginAssignment() + .policySituation("removed") + .end(); + + String oid = repositoryService.addObject(user.asPrismObject(), + null, result); + + when("full object is modified in the database (indices are desynced from full object)"); + + user = repositoryService.getObject(UserType.class, oid, null, result).asObjectable(); + user.getAssignment().remove(1); // remove removed + user.beginAssignment().policySituation("added"); + QUserMapping mapping = QUserMapping.getUserMapping(); + byte[] fullObject = mapping.createFullObject(user); + try (JdbcSession session = mapping.repositoryContext().newJdbcSession().startTransaction()) { + session.newUpdate(mapping.defaultAlias()) + .set(mapping.defaultAlias().fullObject, fullObject) + .where(mapping.defaultAlias().oid.eq(SqaleUtils.oidToUUid(oid))) + .execute(); + session.commit(); + } + + assertPolicySituationFound("kept", 1, result); + assertPolicySituationFound("removed", 1, result); + assertPolicySituationFound("added", 0, result); + + given("object is reindexed"); + + RepoModifyOptions options = RepoModifyOptions.createForceReindex(); + repositoryService.modifyObject(UserType.class, oid, Collections.emptyList(), options, result); + + then("indices are updated according new full object"); + + assertPolicySituationFound("kept", 1, result); + assertPolicySituationFound("removed", 0, result); + assertPolicySituationFound("added", 1, result); + } + + private void assertPolicySituationFound(String situation, int count, OperationResult result) throws SchemaException { + ObjectQuery query = prismContext.queryFor(UserType.class) + .item(UserType.F_ASSIGNMENT, AssignmentType.F_POLICY_SITUATION).eq(situation) + .build(); + SearchResultList> found = repositoryService.searchObjects(UserType.class, query, null, result); + assertEquals(found.size(), count, "Found situation count does not match."); + } + + @Test + public void test952ReindexFixingColumnsOutOfSync() + throws SchemaException, ObjectNotFoundException, ObjectAlreadyExistsException { + OperationResult result = createOperationResult(); + + given("user existing in the repository"); + String name = "user" + getTestNumber(); + UserType user = new UserType(prismContext).name(name) + .emailAddress(name + "@goodmail.com") + .subtype("subtype-1") + .subtype("subtype-2") + .assignment(new AssignmentType(prismContext) + .order(1)); + String oid = repositoryService.addObject(user.asPrismObject(), null, result); + + and("object aggregate is modified in the DB (simulating out-of-sync data)"); + QAssignment a = QAssignmentMapping.getAssignmentMapping().defaultAlias(); + UUID oidUuid = UUID.fromString(oid); + try (JdbcSession jdbcSession = startTransaction()) { + jdbcSession.newInsert(a) + .set(a.ownerOid, oidUuid) + .set(a.cid, -1L) + .set(a.containerType, MContainerType.ASSIGNMENT) + .set(a.ownerType, MObjectType.USER) + .set(a.orderValue, 952) // test number, hopefully unique + .execute(); + + QUser u = QUserMapping.getUserMapping().defaultAlias(); + jdbcSession.newUpdate(u) + .set(u.emailAddress, "bad@badmail.com") + .set(u.subtypes, new String[] { "subtype-952" }) + .set(u.costCenter, "invasive value") + .where(u.oid.eq(oidUuid)) + .execute(); + + jdbcSession.commit(); + } + + and("search provides obviously bad results"); + assertThat(repositorySearchObjects(UserType.class, + prismContext.queryFor(UserType.class) + .item(UserType.F_EMAIL_ADDRESS).eq(name + "@goodmail.com").build(), result)) + .isEmpty(); // can't find by the original mail, that's wrong + assertThat(repositorySearchObjects(UserType.class, + prismContext.queryFor(UserType.class) + .item(UserType.F_COST_CENTER).eq("invasive value").build(), result)) + .hasSize(1); // can find by bad value, that's wrong + assertThat(repositorySearchObjects(UserType.class, + prismContext.queryFor(UserType.class) + .item(UserType.F_ASSIGNMENT, AssignmentType.F_ORDER).eq(952).build(), result)) + .hasSize(1); // can find by wrong assignment + assertThat(repositorySearchObjects(UserType.class, + prismContext.queryFor(UserType.class) + .item(UserType.F_SUBTYPE).eq("subtype-952").build(), result)) + .hasSize(1); // can find by wrong subtype + + when("reindex is called to fix it"); + repositoryService.modifyObject(UserType.class, oid, Collections.emptyList(), + RepoModifyOptions.createForceReindex(), result); + + then("the searches work as expected"); + assertThat(repositorySearchObjects(UserType.class, + prismContext.queryFor(UserType.class) + .item(UserType.F_EMAIL_ADDRESS).eq(name + "@goodmail.com").build(), result)) + .hasSize(1) + .extracting(o -> o.getOid()) + .containsExactlyInAnyOrder(oid); // can find by the original mail, that's good! + assertThat(repositorySearchObjects(UserType.class, + prismContext.queryFor(UserType.class) + .item(UserType.F_COST_CENTER).eq("invasive value").build(), result)) + .isEmpty(); // good + assertThat(repositorySearchObjects(UserType.class, + prismContext.queryFor(UserType.class) + .item(UserType.F_ASSIGNMENT, AssignmentType.F_ORDER).eq(952).build(), result)) + .isEmpty(); // good + assertThat(repositorySearchObjects(UserType.class, + prismContext.queryFor(UserType.class) + .item(UserType.F_ASSIGNMENT, AssignmentType.F_ORDER).eq(1).build(), result)) + .extracting(o -> o.getOid()) + .contains(oid); // good, can have more results, but our user is there too + + and("database values are as good as new"); + MUser userRow = selectObjectByOid(QUser.class, oid); + assertThat(userRow.emailAddress).isEqualTo(name + "@goodmail.com"); // tested by search above as well + assertThat(userRow.costCenter).isNull(); + assertThat(userRow.subtypes).containsExactlyInAnyOrder("subtype-1", "subtype-2"); + + List assRows = select(a, a.ownerOid.eq(oidUuid)); + // single row with proper order is returned, wrong row is gone + assertThat(assRows).hasSize(1); + assertThat(assRows.get(0).orderValue).isEqualTo(1); } @Test diff --git a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchIterativeTest.java b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchIterativeTest.java index e9388928aec..6c77768d926 100644 --- a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchIterativeTest.java +++ b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchIterativeTest.java @@ -35,7 +35,7 @@ /** * This tests {@link SqaleRepositoryService#searchObjectsIterative} in a gray-box style, - * knowing that it uses {@link SqaleRepositoryService#searchObjects} internally. + * knowing that it uses {@link SqaleRepositoryService#executeSearchObjects} internally. * We're not only interested in the fact that it iterates over all the objects matching criteria, * but we also want to assure that the internal paging is strictly sequential. * Each test can take a bit longer (~500ms) because the handler updates the objects @@ -69,7 +69,7 @@ public void resetTestHandler() { @Test public void test100SearchIterativeWithNoneFilter() throws Exception { OperationResult operationResult = createOperationResult(); - SqlPerformanceMonitorImpl pm = repositoryService.getPerformanceMonitor(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); pm.clearGlobalPerformanceInformation(); given("query with top level NONE filter"); @@ -86,15 +86,17 @@ public void test100SearchIterativeWithNoneFilter() throws Exception { assertThat(searchResultMetadata).isNotNull(); assertThat(searchResultMetadata.getApproxNumberOfAllResults()).isZero(); // this is not the main part, just documenting that currently we short circuit the operation - assertOperationRecordedCount(RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 0); + assertOperationRecordedCount( + REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 0); // this is important - no actual search was called - assertOperationRecordedCount(RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE_PAGE, 0); + assertOperationRecordedCount( + REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE_PAGE, 0); } @Test public void test110SearchIterativeWithEmptyFilter() throws Exception { OperationResult operationResult = createOperationResult(); - SqlPerformanceMonitorImpl pm = repositoryService.getPerformanceMonitor(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); pm.clearGlobalPerformanceInformation(); when("calling search iterative with null query"); @@ -110,7 +112,8 @@ public void test110SearchIterativeWithEmptyFilter() throws Exception { assertThat(UUID.fromString(metadata.getPagingCookie())).isNotNull(); and("search operations were called"); - assertOperationRecordedCount(RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertOperationRecordedCount( + REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); assertTypicalPageOperationCount(metadata); and("all objects of the specified type (here User) were processed"); @@ -120,7 +123,7 @@ public void test110SearchIterativeWithEmptyFilter() throws Exception { @Test public void test111SearchIterativeWithLastPageNotFull() throws Exception { OperationResult operationResult = createOperationResult(); - SqlPerformanceMonitorImpl pm = repositoryService.getPerformanceMonitor(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); pm.clearGlobalPerformanceInformation(); given("total result count not multiple of the page size"); repositoryConfiguration.setIterativeSearchByPagingBatchSize(47); @@ -138,7 +141,8 @@ public void test111SearchIterativeWithLastPageNotFull() throws Exception { assertThat(UUID.fromString(metadata.getPagingCookie())).isNotNull(); and("search operations were called"); - assertOperationRecordedCount(RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertOperationRecordedCount( + REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); assertTypicalPageOperationCount(metadata); and("all objects of the specified type were processed"); @@ -148,11 +152,11 @@ public void test111SearchIterativeWithLastPageNotFull() throws Exception { @Test public void test115SearchIterativeWithBreakingConditionCheckingOidOrdering() throws Exception { OperationResult operationResult = createOperationResult(); - SqlPerformanceMonitorImpl pm = repositoryService.getPerformanceMonitor(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); pm.clearGlobalPerformanceInformation(); String midOid = "80000000-0000-0000-0000-000000000000"; - given("condition that breaks iterative based on UUID"); + given("condition that breaks iterative search based on UUID"); testHandler.setStoppingPredicate(u -> u.getOid().compareTo(midOid) >= 0); when("calling search iterative with null query"); @@ -164,7 +168,8 @@ public void test115SearchIterativeWithBreakingConditionCheckingOidOrdering() thr assertThat(metadata.isPartialResults()).isTrue(); // extremely likely with enough items and("search operations were called"); - assertOperationRecordedCount(RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertOperationRecordedCount( + REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); assertTypicalPageOperationCount(metadata); and("all objects up to specified UUID were processed"); @@ -177,7 +182,7 @@ public void test115SearchIterativeWithBreakingConditionCheckingOidOrdering() thr @Test public void test120SearchIterativeWithMaxSize() throws Exception { OperationResult operationResult = createOperationResult(); - SqlPerformanceMonitorImpl pm = repositoryService.getPerformanceMonitor(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); pm.clearGlobalPerformanceInformation(); given("query with maxSize specified"); @@ -194,7 +199,8 @@ public void test120SearchIterativeWithMaxSize() throws Exception { assertThat(metadata.isPartialResults()).isFalse(); and("search operations were called"); - assertOperationRecordedCount(RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertOperationRecordedCount( + REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); assertTypicalPageOperationCount(metadata); and("specified amount of objects was processed"); @@ -204,7 +210,7 @@ public void test120SearchIterativeWithMaxSize() throws Exception { @Test public void test125SearchIterativeWithCustomOrdering() throws Exception { OperationResult operationResult = createOperationResult(); - SqlPerformanceMonitorImpl pm = repositoryService.getPerformanceMonitor(); + SqlPerformanceMonitorImpl pm = getPerformanceMonitor(); pm.clearGlobalPerformanceInformation(); given("query with custom ordering"); @@ -223,7 +229,8 @@ public void test125SearchIterativeWithCustomOrdering() throws Exception { assertThat(metadata.isPartialResults()).isFalse(); // everything was processed and("search operations were called"); - assertOperationRecordedCount(RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); + assertOperationRecordedCount( + REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE, 1); assertTypicalPageOperationCount(metadata); and("all objects were processed in proper order"); @@ -264,7 +271,8 @@ private void assertTypicalPageOperationCount(SearchResultMetadata metadata) { boolean lastRowCausingPartialResult = metadata.isPartialResults() && metadata.getApproxNumberOfAllResults() % getConfiguredPageSize() == 0; - assertOperationRecordedCount(RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE_PAGE, + assertOperationRecordedCount( + REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS_ITERATIVE_PAGE, metadata.getApproxNumberOfAllResults() / getConfiguredPageSize() + (lastRowCausingPartialResult ? 0 : 1)); } diff --git a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchTest.java b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchTest.java index 86a4cdab723..13af72158cb 100644 --- a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchTest.java +++ b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchTest.java @@ -2006,7 +2006,7 @@ public void test950SearchOperationUpdatesPerformanceMonitor() throws SchemaExcep then("performance monitor is updated"); assertThatOperationResult(operationResult).isSuccess(); - assertSingleOperationRecorded(RepositoryService.OP_SEARCH_OBJECTS); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_SEARCH_OBJECTS); } @Test @@ -2153,24 +2153,6 @@ private SearchResultList searchObjects( return repositorySearchObjects(type, query, operationResult, selectorOptions); } - /** Low-level shortcut for {@link SqaleRepositoryService#searchObjects}, no checks. */ - @SafeVarargs - @NotNull - private SearchResultList repositorySearchObjects( - @NotNull Class type, - ObjectQuery query, - OperationResult operationResult, - SelectorOptions... selectorOptions) - throws SchemaException { - return repositoryService.searchObjects( - type, - query, - selectorOptions != null && selectorOptions.length != 0 - ? List.of(selectorOptions) : null, - operationResult) - .map(p -> p.asObjectable()); - } - private SearchResultList searchContainerTest( String description, Class type, Function filter) throws SchemaException { diff --git a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSmokeTest.java b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSmokeTest.java index 7dcd9f548f4..28d36778f2f 100644 --- a/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSmokeTest.java +++ b/repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSmokeTest.java @@ -169,7 +169,7 @@ public void test100AddObject() throws ObjectAlreadyExistsException, SchemaExcept assertThat(user.getOid()).isEqualTo(userOid); assertThat(selectObjectByOid(QUser.class, userOid)).isNotNull(); assertThatOperationResult(result).isSuccess(); - assertSingleOperationRecorded(RepositoryService.OP_ADD_OBJECT); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_ADD_OBJECT); } @Test @@ -193,7 +193,7 @@ public void test110DeleteObject() throws Exception { assertThat(deleteResult).isNotNull(); assertThatOperationResult(result).isSuccess(); assertThat(selectNullableObjectByOid(QUser.class, userOid)).isNull(); - assertSingleOperationRecorded(RepositoryService.OP_DELETE_OBJECT); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_DELETE_OBJECT); } @Test @@ -214,7 +214,7 @@ public void test200GetObject() throws Exception { then("object is obtained and performance monitor is updated"); assertThatOperationResult(result).isSuccess(); assertThat(object).isNotNull(); - assertSingleOperationRecorded(RepositoryService.OP_GET_OBJECT); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_GET_OBJECT); } @Test @@ -234,7 +234,7 @@ public void test210GetVersion() throws Exception { then("non-null version string is obtained and performance monitor is updated"); assertThatOperationResult(result).isSuccess(); assertThat(version).isNotNull(); - assertSingleOperationRecorded(RepositoryService.OP_GET_VERSION); + assertSingleOperationRecorded(REPO_OP_PREFIX + RepositoryService.OP_GET_VERSION); } @Test diff --git a/repo/system-init/src/main/java/com/evolveum/midpoint/init/AuditServiceProxy.java b/repo/system-init/src/main/java/com/evolveum/midpoint/init/AuditServiceProxy.java index 5a0e0d58cee..5bf70165199 100644 --- a/repo/system-init/src/main/java/com/evolveum/midpoint/init/AuditServiceProxy.java +++ b/repo/system-init/src/main/java/com/evolveum/midpoint/init/AuditServiceProxy.java @@ -260,4 +260,14 @@ public SearchResultMetadata searchObjectsIterative( return new SearchResultMetadata(); } + + public T getImplementation(Class implementationType) { + for (AuditService service : services) { + if (implementationType.isInstance(service)) { + return implementationType.cast(service); + } + } + + return null; + } }