From 59b0cfee08c6e05af1e18fe6dc86cc202efda4fa Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Mon, 28 Oct 2019 15:23:42 +0100 Subject: [PATCH] Improve provisioning code a bit Traditional things like "resource object" vs. "shadow" clarification, elimination of variable value rewriting, redundant checking of LOGGER.isTraceEnabled, etc. --- .../impl/ProvisioningServiceImpl.java | 4 +- .../provisioning/impl/ShadowCache.java | 84 ++++++++----------- .../shadowmanager/ShadowDeltaComputer.java | 22 ++--- .../impl/shadowmanager/ShadowManager.java | 8 +- .../impl/sync/ChangeProcessor.java | 6 +- 5 files changed, 54 insertions(+), 70 deletions(-) diff --git a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ProvisioningServiceImpl.java b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ProvisioningServiceImpl.java index c100adcde71..c6cdea11a48 100644 --- a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ProvisioningServiceImpl.java +++ b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ProvisioningServiceImpl.java @@ -950,9 +950,7 @@ public SearchResultMetadata searchObjectsIterative(Class< Validate.notNull(parentResult, "Operation result must not be null."); Validate.notNull(handler, "Handler must not be null."); - if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Start of (iterative) search objects. Query:\n{}", query != null ? query.debugDump(1) : " (null)"); - } + LOGGER.trace("Start of (iterative) search objects. Query:\n{}", DebugUtil.debugDumpLazily(query, 1)); OperationResult result = parentResult.createSubresult(ProvisioningService.class.getName() + ".searchObjectsIterative"); result.setSummarizeSuccesses(true); diff --git a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ShadowCache.java b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ShadowCache.java index d3818a618da..6923bf5dc6a 100644 --- a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ShadowCache.java +++ b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ShadowCache.java @@ -220,7 +220,7 @@ public PrismObject getShadow(String oid, PrismObject rep return resultShadow; } - PrismObject resourceShadow = null; + PrismObject resourceObject; Collection> primaryIdentifiers = ShadowUtil.getPrimaryIdentifiers(repositoryShadow); if (primaryIdentifiers == null || primaryIdentifiers.isEmpty()) { @@ -257,7 +257,7 @@ public PrismObject getShadow(String oid, PrismObject rep try { - resourceShadow = resourceObjectConverter.getResourceObject(ctx, identifiers, true, parentResult); + resourceObject = resourceObjectConverter.getResourceObject(ctx, identifiers, true, parentResult); } catch (ObjectNotFoundException e) { // This may be OK, e.g. for connectors that have running async add operation. @@ -280,30 +280,30 @@ public PrismObject getShadow(String oid, PrismObject rep } } - LOGGER.trace("Shadow returned by ResourceObjectConverter:\n{}", resourceShadow.debugDumpLazily(1)); + LOGGER.trace("Shadow returned by ResourceObjectConverter:\n{}", resourceObject.debugDumpLazily(1)); // Resource shadow may have different auxiliary object classes than // the original repo shadow. Make sure we have the definition that // applies to resource shadow. We will fix repo shadow later. // BUT we need also information about kind/intent and these information is only // in repo shadow, therefore the following 2 lines.. - resourceShadow.asObjectable().setKind(repositoryShadow.asObjectable().getKind()); - resourceShadow.asObjectable().setIntent(repositoryShadow.asObjectable().getIntent()); - ProvisioningContext shadowCtx = ctx.spawn(resourceShadow); + resourceObject.asObjectable().setKind(repositoryShadow.asObjectable().getKind()); + resourceObject.asObjectable().setIntent(repositoryShadow.asObjectable().getIntent()); + ProvisioningContext shadowCtx = ctx.spawn(resourceObject); resourceManager.modifyResourceAvailabilityStatus(resource.asPrismObject(), AvailabilityStatusType.UP, parentResult); if (LOGGER.isTraceEnabled()) { LOGGER.trace("Shadow from repository:\n{}", repositoryShadow.debugDump(1)); - LOGGER.trace("Resource object fetched from resource:\n{}", resourceShadow.debugDump(1)); + LOGGER.trace("Resource object fetched from resource:\n{}", resourceObject.debugDump(1)); } - repositoryShadow = shadowManager.updateShadow(shadowCtx, resourceShadow, null, repositoryShadow, shadowState, parentResult); + repositoryShadow = shadowManager.updateShadow(shadowCtx, resourceObject, null, repositoryShadow, shadowState, parentResult); LOGGER.trace("Repository shadow after update:\n{}", repositoryShadow.debugDumpLazily(1)); // Complete the shadow by adding attributes from the resource object // This also completes the associations by adding shadowRefs - PrismObject assembledShadow = completeShadow(shadowCtx, resourceShadow, repositoryShadow, false, parentResult); + PrismObject assembledShadow = completeShadow(shadowCtx, resourceObject, repositoryShadow, false, parentResult); LOGGER.trace("Shadow when assembled:\n{}", assembledShadow.debugDumpLazily(1)); PrismObject resultShadow = futurizeShadow(ctx, repositoryShadow, assembledShadow, options, now); @@ -334,13 +334,8 @@ public PrismObject getShadow(String oid, PrismObject rep } } finally { // We need to record the fetch down here. Now it is certain that we - // are going to fetch from resource - // (we do not have raw/noFetch option) - // TODO: is this correct behaviour? don't we really want to record - // fetch for protected objects? - if (!ShadowUtil.isProtected(resourceShadow)) { - InternalMonitor.recordCount(InternalCounters.SHADOW_FETCH_OPERATION_COUNT); - } + // are going to fetch from resource (we do not have raw/noFetch option) + InternalMonitor.recordCount(InternalCounters.SHADOW_FETCH_OPERATION_COUNT); } } @@ -2017,26 +2012,25 @@ public SearchResultMetadata searchObjectsIterative(final ProvisioningContext ctx ObjectQuery attributeQuery = createAttributeQuery(query); - ResultHandler resultHandler = (PrismObject resourceShadow, OperationResult objResult) -> { - LOGGER.trace("Found resource object\n{}", resourceShadow.debugDumpLazily(1)); + ResultHandler resultHandler = (PrismObject resourceObject, OperationResult objResult) -> { + LOGGER.trace("Found resource object\n{}", resourceObject.debugDumpLazily(1)); PrismObject resultShadow; try { // The shadow does not have any kind or intent at this point. // But at least locate the definition using object classes. - ProvisioningContext estimatedShadowCtx = shadowCaretaker.reapplyDefinitions(ctx, resourceShadow); + ProvisioningContext estimatedShadowCtx = shadowCaretaker.reapplyDefinitions(ctx, resourceObject); // Try to find shadow that corresponds to the resource object. if (readFromRepository) { PrismObject repoShadow = acquireRepositoryShadow( - estimatedShadowCtx, resourceShadow, true, isDoDiscovery, parentResult); + estimatedShadowCtx, resourceObject, true, isDoDiscovery, objResult); // This determines the definitions exactly. How the repo // shadow should have proper kind/intent ProvisioningContext shadowCtx = shadowCaretaker.applyAttributesDefinition(ctx, repoShadow); // TODO: shadowState - repoShadow = shadowManager.updateShadow(shadowCtx, resourceShadow, null, repoShadow, - null, parentResult); + repoShadow = shadowManager.updateShadow(shadowCtx, resourceObject, null, repoShadow, null, objResult); - resultShadow = completeShadow(shadowCtx, resourceShadow, repoShadow, isDoDiscovery, objResult); + resultShadow = completeShadow(shadowCtx, resourceObject, repoShadow, isDoDiscovery, objResult); // TODO do we want also to futurize the shadow like in getObject? @@ -2050,7 +2044,7 @@ public SearchResultMetadata searchObjectsIterative(final ProvisioningContext ctx } } else { - resultShadow = resourceShadow; + resultShadow = resourceObject; } validateShadow(resultShadow, readFromRepository); @@ -2137,19 +2131,17 @@ private PrismObject acquireRepositoryShadow(ProvisioningContext ctx, throws SchemaException, ConfigurationException, ObjectNotFoundException, CommunicationException, SecurityViolationException, GenericConnectorException, ExpressionEvaluationException, EncryptionException { - PrismObject repoShadow = shadowManager.lookupLiveShadowInRepository(ctx, resourceShadow, parentResult); + PrismObject existingRepoShadow = shadowManager.lookupLiveShadowInRepository(ctx, resourceShadow, parentResult); - if (repoShadow != null) { + if (existingRepoShadow != null) { if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Found shadow object in the repository {}", ShadowUtil.shortDumpShadow(repoShadow)); + LOGGER.trace("Found shadow object in the repository {}", ShadowUtil.shortDumpShadow(existingRepoShadow)); } - return repoShadow; + return existingRepoShadow; } - if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Shadow object (in repo) corresponding to the resource object (on the resource) was not found. " - + "The repo shadow will be created. The resource object:\n{}", resourceShadow); - } + LOGGER.trace("Shadow object (in repo) corresponding to the resource object (on the resource) was not found. " + + "The repo shadow will be created. The resource object:\n{}", resourceShadow); // TODO: do something about shadows with mathcing secondary identifiers? We do not need to care about these any longer, do we? //MID-5844 @@ -2189,9 +2181,10 @@ private PrismObject acquireRepositoryShadow(ProvisioningContext ctx, // not exist in the repository we need to create the shadow to align repo state to the // reality (resource) + PrismObject createdRepoShadow; try { - repoShadow = shadowManager.addDiscoveredRepositoryShadow(ctx, resourceShadow, parentResult); + createdRepoShadow = shadowManager.addDiscoveredRepositoryShadow(ctx, resourceShadow, parentResult); } catch (ObjectAlreadyExistsException e) { // Conflict! But we haven't supplied an OID and we have checked for existing shadow before, @@ -2225,15 +2218,12 @@ private PrismObject acquireRepositoryShadow(ProvisioningContext ctx, return conflictingShadow; } - resourceShadow.setOid(repoShadow.getOid()); - ObjectReferenceType resourceRef = resourceShadow.asObjectable().getResourceRef(); - if (resourceRef == null) { - resourceRef = new ObjectReferenceType(); - resourceRef.asReferenceValue().setObject(ctx.getResource().asPrismObject()); - resourceShadow.asObjectable().setResourceRef(resourceRef); - } else { - resourceRef.asReferenceValue().setObject(ctx.getResource().asPrismObject()); + resourceShadow.setOid(createdRepoShadow.getOid()); + ShadowType resourceShadowBean = resourceShadow.asObjectable(); + if (resourceShadowBean.getResourceRef() == null) { + resourceShadowBean.setResourceRef(new ObjectReferenceType()); } + resourceShadowBean.getResourceRef().asReferenceValue().setObject(ctx.getResource().asPrismObject()); if (isDoDiscovery) { // We have object for which there was no shadow. Which means that midPoint haven't known about this shadow before. @@ -2242,17 +2232,17 @@ private PrismObject acquireRepositoryShadow(ProvisioningContext ctx, notifyResourceObjectChangeListeners(resourceShadow, ctx.getResource().asPrismObject(), true); } + PrismObject finalRepoShadow; if (unknownIntent) { // Intent may have been changed during the notifyChange processing. // Re-read the shadow if necessary. - repoShadow = shadowManager.fixShadow(ctx, repoShadow, parentResult); - } - - if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Final repo shadow (created):\n{}", repoShadow.debugDump(1)); + finalRepoShadow = shadowManager.fixShadow(ctx, createdRepoShadow, parentResult); + } else { + finalRepoShadow = createdRepoShadow; } - return repoShadow; + LOGGER.trace("Final repo shadow (created and possibly re-read):\n{}", finalRepoShadow.debugDumpLazily(1)); + return finalRepoShadow; } private ObjectQuery createAttributeQuery(ObjectQuery query) throws SchemaException { diff --git a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowDeltaComputer.java b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowDeltaComputer.java index 058f10c25cf..34547735257 100644 --- a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowDeltaComputer.java +++ b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowDeltaComputer.java @@ -57,15 +57,15 @@ public class ShadowDeltaComputer { @NotNull ObjectDelta computeShadowDelta(@NotNull ProvisioningContext ctx, - @NotNull PrismObject repoShadowOld, PrismObject resourceShadowNew, + @NotNull PrismObject repoShadowOld, PrismObject currentResourceObject, ObjectDelta resourceObjectDelta, ShadowState shadowState) throws ObjectNotFoundException, SchemaException, CommunicationException, ConfigurationException, ExpressionEvaluationException { ObjectDelta computedShadowDelta = repoShadowOld.createModifyDelta(); - RefinedObjectClassDefinition ocDef = ctx.computeCompositeObjectClassDefinition(resourceShadowNew); - PrismContainer currentResourceAttributes = resourceShadowNew.findContainer(ShadowType.F_ATTRIBUTES); + RefinedObjectClassDefinition ocDef = ctx.computeCompositeObjectClassDefinition(currentResourceObject); + PrismContainer currentResourceAttributes = currentResourceObject.findContainer(ShadowType.F_ATTRIBUTES); PrismContainer oldRepoAttributes = repoShadowOld.findContainer(ShadowType.F_ATTRIBUTES); ShadowType oldRepoShadowType = repoShadowOld.asObjectable(); @@ -75,17 +75,17 @@ ObjectDelta computeShadowDelta(@NotNull ProvisioningContext ctx, processAttributes(computedShadowDelta, incompleteCacheableItems, oldRepoAttributes, currentResourceAttributes, resourceObjectDelta, ocDef, cachingStrategy); - PolyString currentShadowName = ShadowUtil.determineShadowName(resourceShadowNew); + PolyString currentShadowName = ShadowUtil.determineShadowName(currentResourceObject); PolyString oldRepoShadowName = repoShadowOld.getName(); - if (!currentShadowName.equalsOriginalValue(oldRepoShadowName)) { + if (currentShadowName != null && !currentShadowName.equalsOriginalValue(oldRepoShadowName)) { PropertyDelta shadowNameDelta = prismContext.deltaFactory().property().createModificationReplaceProperty(ShadowType.F_NAME, - repoShadowOld.getDefinition(),currentShadowName); + repoShadowOld.getDefinition(), currentShadowName); computedShadowDelta.addModification(shadowNameDelta); } PropertyDelta auxOcDelta = ItemUtil.diff( repoShadowOld.findProperty(ShadowType.F_AUXILIARY_OBJECT_CLASS), - resourceShadowNew.findProperty(ShadowType.F_AUXILIARY_OBJECT_CLASS)); + currentResourceObject.findProperty(ShadowType.F_AUXILIARY_OBJECT_CLASS)); if (auxOcDelta != null) { computedShadowDelta.addModification(auxOcDelta); } @@ -105,10 +105,10 @@ ObjectDelta computeShadowDelta(@NotNull ProvisioningContext ctx, } else if (cachingStrategy == CachingStategyType.PASSIVE) { - compareUpdateProperty(computedShadowDelta, SchemaConstants.PATH_ACTIVATION_ADMINISTRATIVE_STATUS, resourceShadowNew, repoShadowOld); - compareUpdateProperty(computedShadowDelta, SchemaConstants.PATH_ACTIVATION_VALID_FROM, resourceShadowNew, repoShadowOld); - compareUpdateProperty(computedShadowDelta, SchemaConstants.PATH_ACTIVATION_VALID_TO, resourceShadowNew, repoShadowOld); - compareUpdateProperty(computedShadowDelta, SchemaConstants.PATH_ACTIVATION_LOCKOUT_STATUS, resourceShadowNew, repoShadowOld); + compareUpdateProperty(computedShadowDelta, SchemaConstants.PATH_ACTIVATION_ADMINISTRATIVE_STATUS, currentResourceObject, repoShadowOld); + compareUpdateProperty(computedShadowDelta, SchemaConstants.PATH_ACTIVATION_VALID_FROM, currentResourceObject, repoShadowOld); + compareUpdateProperty(computedShadowDelta, SchemaConstants.PATH_ACTIVATION_VALID_TO, currentResourceObject, repoShadowOld); + compareUpdateProperty(computedShadowDelta, SchemaConstants.PATH_ACTIVATION_LOCKOUT_STATUS, currentResourceObject, repoShadowOld); if (incompleteCacheableItems.isEmpty()) { CachingMetadataType cachingMetadata = new CachingMetadataType(); diff --git a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowManager.java b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowManager.java index a923d05b525..027d75003b5 100644 --- a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowManager.java +++ b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowManager.java @@ -691,17 +691,13 @@ private void processQueryMatchingRuleFilter(ObjectFilter filter, RefinedObje // Used when new resource object is discovered public PrismObject addDiscoveredRepositoryShadow(ProvisioningContext ctx, PrismObject resourceShadow, OperationResult parentResult) throws SchemaException, ConfigurationException, ObjectNotFoundException, CommunicationException, ObjectAlreadyExistsException, ExpressionEvaluationException, EncryptionException { - if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Adding new shadow from resource object:\n{}", resourceShadow.debugDump(1)); - } + LOGGER.trace("Adding new shadow from resource object:\n{}", resourceShadow.debugDumpLazily(1)); PrismObject repoShadow = createRepositoryShadow(ctx, resourceShadow); ConstraintsChecker.onShadowAddOperation(repoShadow.asObjectable()); String oid = repositoryService.addObject(repoShadow, null, parentResult); repoShadow.setOid(oid); LOGGER.debug("Added new shadow (from resource object): {}", repoShadow); - if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Added new shadow (from resource object):\n{}", repoShadow.debugDump(1)); - } + LOGGER.trace("Added new shadow (from resource object):\n{}", repoShadow.debugDumpLazily(1)); return repoShadow; } diff --git a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/sync/ChangeProcessor.java b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/sync/ChangeProcessor.java index 0c28110f2c4..d71327c60c2 100644 --- a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/sync/ChangeProcessor.java +++ b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/sync/ChangeProcessor.java @@ -279,10 +279,10 @@ private void preProcessChange(ProvisioningContext ctx, Change change, OperationR assert change.getCurrentResourceObject() != null || change.isDelete(); if (change.getCurrentResourceObject() != null) { // TODO do we need to complete the shadow now? Why? MID-5834 - PrismObject currentShadow = shadowCache.completeShadow(ctx, change.getCurrentResourceObject(), oldShadow, false, parentResult); - change.setCurrentResourceObject(currentShadow); + PrismObject currentResourceObjectShadowized = shadowCache.completeShadow(ctx, change.getCurrentResourceObject(), oldShadow, false, parentResult); + change.setCurrentResourceObject(currentResourceObjectShadowized); // TODO: shadowState MID-5834 - shadowManager.updateShadow(ctx, currentShadow, change.getObjectDelta(), oldShadow, null, parentResult); + shadowManager.updateShadow(ctx, currentResourceObjectShadowized, change.getObjectDelta(), oldShadow, null, parentResult); } if (change.getObjectDelta() != null && change.getObjectDelta().getOid() == null) {