From df0e89066d89eb356e8665b9f54f2c861cb65253 Mon Sep 17 00:00:00 2001 From: Radovan Semancik Date: Mon, 3 Sep 2018 09:15:56 +0200 Subject: [PATCH] More fixes related to error handling. --- .../model/impl/lens/Construction.java | 27 +++++--- .../model/impl/util/ModelImplUtils.java | 5 -- .../intest/negative/TestAssignmentErrors.java | 27 ++++---- .../intest/negative/TestBrokenResources.java | 64 +++++++++++++------ 4 files changed, 75 insertions(+), 48 deletions(-) diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/Construction.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/Construction.java index a2b9fc57cb5..7f90a564927 100644 --- a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/Construction.java +++ b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/Construction.java @@ -94,6 +94,8 @@ public class Construction extends AbstractConstruction extends AbstractConstruction auxiliaryObjectClassDefinitions; private AssignmentPathVariables assignmentPathVariables = null; private PrismContainerDefinition associationContainerDefinition; - private PrismObject systemConfiguration; // only to - // provide - // $configuration - // variable - // (MID-2372) + private PrismObject systemConfiguration; // only to provide $configuration variable (MID-2372) private LensProjectionContext projectionContext; public Construction(ConstructionType constructionType, ObjectType source) { @@ -322,12 +320,21 @@ public ResourceType getResource(Task task, OperationResult result) return resource; } - public void evaluate(Task task, OperationResult result) + public void evaluate(Task task, OperationResult parentResult) throws SchemaException, ExpressionEvaluationException, ObjectNotFoundException, SecurityViolationException, ConfigurationException, CommunicationException { - assignmentPathVariables = LensUtil.computeAssignmentPathVariables(getAssignmentPath()); - evaluateKindIntentObjectClass(task, result); - evaluateAttributes(task, result); - evaluateAssociations(task, result); + // Subresult is needed here. If something fails here, this needs to be recorded as a subresult of + // AssignmentProcessor.processAssignmentsProjections. Otherwise partial error won't be propagated properly. + OperationResult result = parentResult.createMinorSubresult(OP_EVALUATE); + try { + assignmentPathVariables = LensUtil.computeAssignmentPathVariables(getAssignmentPath()); + evaluateKindIntentObjectClass(task, result); + evaluateAttributes(task, result); + evaluateAssociations(task, result); + result.recordSuccess(); + } catch (Throwable e) { + result.recordFatalError(e); + throw e; + } } private void evaluateKindIntentObjectClass(Task task, OperationResult result) diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/ModelImplUtils.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/ModelImplUtils.java index 76ce6f04d90..79ffd4e51ed 100644 --- a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/ModelImplUtils.java +++ b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/ModelImplUtils.java @@ -888,11 +888,6 @@ static void throwException(Throwable e, OperationResult result) throw (ObjectNotFoundException)e; } else if (e instanceof CommunicationException) { throw (CommunicationException)e; -<<<<<<< HEAD:model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/ModelImplUtils.java - } else if (e instanceof SchemaException) { - throw (SchemaException)e; -======= ->>>>>>> b37e7456a2584e47133fc7c0f098957c1101bea1:model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/Utils.java } else if (e instanceof SchemaException) { throw (SchemaException)e; } else if (e instanceof ConfigurationException) { diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/negative/TestAssignmentErrors.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/negative/TestAssignmentErrors.java index f3625ae4456..09848043473 100644 --- a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/negative/TestAssignmentErrors.java +++ b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/negative/TestAssignmentErrors.java @@ -363,18 +363,19 @@ public void test210UserSharptoothAssignAccountBrokenGeneric() throws Exception { getDummyResource().setBreakMode(BreakMode.GENERIC); dummyAuditService.clear(); - // WHEN - displayWhen(TEST_NAME); - //not expected that it fails, instead the error in the result is expected - modelService.executeChanges(deltas, null, task, result); - - // THEN - displayThen(TEST_NAME); - result.computeStatus(); - - display(result); - // This has to be a partial error as some changes were executed (user) and others were not (account) - TestUtil.assertPartialError(result); + try { + // WHEN + displayWhen(TEST_NAME); + //not expected that it fails, instead the error in the result is expected + modelService.executeChanges(deltas, null, task, result); + + assertNotReached(); + } catch (GenericConnectorException e) { + // THEN + displayThen(TEST_NAME); + display("Expected exception", e); + } + assertFailure(result); // Check audit display("Audit", dummyAuditService); @@ -385,7 +386,7 @@ public void test210UserSharptoothAssignAccountBrokenGeneric() throws Exception { dummyAuditService.assertHasDelta(ChangeType.MODIFY, UserType.class); dummyAuditService.assertHasDelta(ChangeType.ADD, ShadowType.class, OperationResultStatus.FATAL_ERROR); dummyAuditService.assertTarget(user.getOid()); - dummyAuditService.assertExecutionOutcome(OperationResultStatus.PARTIAL_ERROR); + dummyAuditService.assertExecutionOutcome(OperationResultStatus.FATAL_ERROR); dummyAuditService.assertExecutionMessage(); LensContext lastLensContext = (LensContext) profilingModelInspectorManager.getLastLensContext(); diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/negative/TestBrokenResources.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/negative/TestBrokenResources.java index f62e9c60bde..482ec7ee14b 100644 --- a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/negative/TestBrokenResources.java +++ b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/negative/TestBrokenResources.java @@ -813,8 +813,9 @@ public void test377GetResourceNoConfiguration() throws Exception { * the account on the good resource is created. * * This one dies on the lack of schema. + * + * MID-1248 */ - // MID-1248 @Test public void test400AssignTwoResouresNotFound() throws Exception { final String TEST_NAME = "test400AssignTwoResoures"; @@ -1011,13 +1012,20 @@ public void test510AssignResourceBlackError() throws Exception { assertAssignments(userBefore, 0); assertNoDummyAccount(RESOURCE_DUMMY_BLACK_NAME, ACCOUNT_GUYBRUSH_DUMMY_USERNAME); - // WHEN - displayWhen(TEST_NAME); - assignAccountToUser(USER_GUYBRUSH_OID, RESOURCE_DUMMY_BLACK_OID, null, task, result); + try { + // WHEN + displayWhen(TEST_NAME); + assignAccountToUser(USER_GUYBRUSH_OID, RESOURCE_DUMMY_BLACK_OID, null, task, result); - // THEN - displayThen(TEST_NAME); - assertPartialError(result); + assertNotReached(); + + } catch (GenericConnectorException e) { + // THEN + displayThen(TEST_NAME); + display("Expected exception", e); + } + + assertFailure(result); PrismObject userAfter = getUser(USER_GUYBRUSH_OID); display("User after", userAfter); @@ -1063,7 +1071,7 @@ public void test512ReconcileUser() throws Exception { } /** - * Causing an error in provisioning script. Default criticality. + * Causing an error (RuntimeException) in provisioning script. Default criticality. * The error should stop operation. * MID-4060 */ @@ -1077,13 +1085,21 @@ public void test514ModifyUserEmployeeNumberRuntime() throws Exception { OperationResult result = task.getResult(); prepareTest5xx(); - // WHEN - displayWhen(TEST_NAME); - modifyUserReplace(USER_GUYBRUSH_OID, UserType.F_EMPLOYEE_NUMBER, task, result, DummyResource.POWERFAIL_ARG_ERROR_RUNTIME); + try { + // WHEN + displayWhen(TEST_NAME); + modifyUserReplace(USER_GUYBRUSH_OID, UserType.F_EMPLOYEE_NUMBER, task, result, DummyResource.POWERFAIL_ARG_ERROR_RUNTIME); - // THEN - displayThen(TEST_NAME); - assertPartialError(result); + assertNotReached(); + + } catch (RuntimeException e) { + // THEN + displayThen(TEST_NAME); + display("Expected exception", e); + assertEquals("Wrong exception message", "Booom! PowerFail script failed (runtime)", e.getMessage()); + } + + assertFailure(result); PrismObject userAfter = getUser(USER_GUYBRUSH_OID); display("User after", userAfter); @@ -1110,13 +1126,21 @@ public void test518UnassignResourceBlack() throws Exception { OperationResult result = task.getResult(); prepareTest5xx(); - // WHEN - displayWhen(TEST_NAME); - unassignAccountFromUser(USER_GUYBRUSH_OID, RESOURCE_DUMMY_BLACK_OID, null, task, result); + try { + // WHEN + displayWhen(TEST_NAME); + unassignAccountFromUser(USER_GUYBRUSH_OID, RESOURCE_DUMMY_BLACK_OID, null, task, result); - // THEN - displayThen(TEST_NAME); - assertPartialError(result); + assertNotReached(); + + } catch (RuntimeException e) { + // THEN + displayThen(TEST_NAME); + display("Expected exception", e); + assertEquals("Wrong exception message", "Booom! PowerFail script failed (runtime)", e.getMessage()); + } + + assertFailure(result); PrismObject userAfter = getUser(USER_GUYBRUSH_OID); display("User after", userAfter);