From 30cc1e27e45bdc9a280d4a512065e02898899328 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Fri, 6 Apr 2018 12:44:42 +0200 Subject: [PATCH] Fix showing names in 'add' approvals (MID-4512) When approving object creation, the object has an OID but does not exist in repository. This caused problems in task details and task lists pages. Repository change: now we are storing targetName for references in the XML form of stored objects (if set by caller). (cherry picked from commit 7d7a974) --- .../web/page/admin/server/dto/TaskDto.java | 10 +++++- .../midpoint/prism/SerializationOptions.java | 2 +- ...opagateTaskObjectReferenceTaskHandler.java | 12 ++----- .../impl/tasks/WfTaskCreationInstruction.java | 3 +- .../midpoint/repo/sql/AddGetObjectTest.java | 31 +++++++++++++------ .../src/test/resources/basic/objects.xml | 2 +- .../repo/sql/helpers/ObjectUpdater.java | 10 +++--- .../task/quartzimpl/TaskQuartzImpl.java | 17 ++-------- 8 files changed, 43 insertions(+), 44 deletions(-) diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/TaskDto.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/TaskDto.java index 6fd3c47c670..ad7144cc4c9 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/TaskDto.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/TaskDto.java @@ -306,7 +306,15 @@ private void fillInObjectRefAttributes(TaskType taskType, TaskDtoProviderOptions } public String getTaskObjectName(TaskType taskType, PageBase pageBase, Task opTask, OperationResult thisOpResult) { - return WebModelServiceUtils.resolveReferenceName(taskType.getObjectRef(), pageBase, opTask, thisOpResult); + OperationResult currentResult; + if (taskType.getWorkflowContext() != null) { + // For workflow-related tasks the task object might not be created yet (MID-4512). The simplest way + // of avoiding displaying the error is to use a separate operation result. + currentResult = new OperationResult(TaskDto.class.getName() + ".getTaskObjectName"); + } else { + currentResult = thisOpResult; + } + return WebModelServiceUtils.resolveReferenceName(taskType.getObjectRef(), pageBase, opTask, currentResult); } private void fillInParentTaskAttributes(TaskType taskType, diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/SerializationOptions.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/SerializationOptions.java index c4f7b66aa73..45bf7b5f146 100644 --- a/infra/prism/src/main/java/com/evolveum/midpoint/prism/SerializationOptions.java +++ b/infra/prism/src/main/java/com/evolveum/midpoint/prism/SerializationOptions.java @@ -54,7 +54,7 @@ public void setSerializeCompositeObjects(boolean serializeCompositeObjects) { this.serializeCompositeObjects = serializeCompositeObjects; } - public static SerializationOptions createSerializeCompositeObjects(){ + public static SerializationOptions createSerializeCompositeObjects() { SerializationOptions serializationOptions = new SerializationOptions(); serializationOptions.setSerializeCompositeObjects(true); return serializationOptions; diff --git a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/processors/primary/WfPropagateTaskObjectReferenceTaskHandler.java b/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/processors/primary/WfPropagateTaskObjectReferenceTaskHandler.java index 2d0e255e0f2..b948737a854 100644 --- a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/processors/primary/WfPropagateTaskObjectReferenceTaskHandler.java +++ b/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/processors/primary/WfPropagateTaskObjectReferenceTaskHandler.java @@ -112,7 +112,7 @@ public TaskRunResult run(Task task) { ObjectReferenceType objectReferenceType = new ObjectReferenceType(); objectReferenceType.setType(type); objectReferenceType.setOid(oid); - + // TODO set also targetName [but it seems that this task handler is not used anymore] if (task.getObjectRef() == null) { task.setObjectRef(objectReferenceType); } else { @@ -123,9 +123,7 @@ public TaskRunResult run(Task task) { try { dependents = wfTask.listDependents(result); dependents.add(wfTask.getParentJob(result)); - } catch (SchemaException e) { - return reportException("Couldn't get dependents from job " + wfTask, task, result, e); - } catch (ObjectNotFoundException e) { + } catch (SchemaException | ObjectNotFoundException e) { return reportException("Couldn't get dependents from job " + wfTask, task, result, e); } @@ -133,13 +131,9 @@ public TaskRunResult run(Task task) { if (dependent.getTask().getObjectRef() == null) { try { dependent.getTask().setObjectRefImmediate(objectReferenceType, result); - } catch (ObjectNotFoundException e) { + } catch (ObjectNotFoundException | SchemaException | ObjectAlreadyExistsException e) { // note we DO NOT return, because we want to set all references we can reportException("Couldn't set object reference on job " + dependent, task, result, e); - } catch (SchemaException e) { - reportException("Couldn't set object reference on job " + dependent, task, result, e); - } catch (ObjectAlreadyExistsException e) { - reportException("Couldn't set object reference on job " + dependent, task, result, e); } } else { LOGGER.warn("object reference in job " + dependent + " is already set, although it shouldn't be"); diff --git a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/tasks/WfTaskCreationInstruction.java b/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/tasks/WfTaskCreationInstruction.java index 83a2f3b570d..fe718313b8f 100644 --- a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/tasks/WfTaskCreationInstruction.java +++ b/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/tasks/WfTaskCreationInstruction.java @@ -372,7 +372,8 @@ public Task createTask(WfTaskController taskController, Task parentTask, WfConfi task.setCategory(TaskCategory.WORKFLOW); if (taskObject != null) { - task.setObjectRef(taskObject.getOid(), taskObject.getDefinition().getTypeName()); + //noinspection unchecked + task.setObjectRef(ObjectTypeUtil.createObjectRef(taskObject)); } else if (parentTask != null && parentTask.getObjectRef() != null) { task.setObjectRef(parentTask.getObjectRef().clone()); } diff --git a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/AddGetObjectTest.java b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/AddGetObjectTest.java index 9fb55ae8d77..2d2fa0b29de 100644 --- a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/AddGetObjectTest.java +++ b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/AddGetObjectTest.java @@ -22,6 +22,7 @@ import com.evolveum.midpoint.prism.delta.ObjectDelta; import com.evolveum.midpoint.prism.delta.ReferenceDelta; import com.evolveum.midpoint.prism.path.ItemPath; +import com.evolveum.midpoint.prism.polystring.PolyString; import com.evolveum.midpoint.prism.util.PrismTestUtil; import com.evolveum.midpoint.repo.api.ConflictWatcher; import com.evolveum.midpoint.repo.api.RepoAddOptions; @@ -127,10 +128,22 @@ public void addGetDSEESyncDoubleTest() throws Exception { public void simpleAddGetTest() throws Exception { LOGGER.info("===[ simpleAddGetTest ]==="); final File OBJECTS_FILE = new File(FOLDER_BASIC, "objects.xml"); - addGetCompare(OBJECTS_FILE); + List> objects = addGetCompare(OBJECTS_FILE); + + boolean foundAtestuserX00003 = false; + for (PrismObject object : objects) { + // adhoc check whether reference.targetName is preserved + if ("atestuserX00003".equals(PolyString.getOrig(object.getName()))) { + String personaName = PolyString.getOrig(((UserType) object.asObjectable()).getPersonaRef().get(0).getTargetName()); + assertEquals("Wrong personaRef.targetName on atestuserX00003", "u-000", personaName); + foundAtestuserX00003 = true; + break; + } + } + assertTrue("User atestuserX00003 was not found", foundAtestuserX00003); } - private void addGetCompare(File file) throws Exception { + private List> addGetCompare(File file) throws Exception { List> elements = prismContext.parserFor(file).parseObjects(); List oids = new ArrayList(); @@ -142,9 +155,9 @@ private void addGetCompare(File file) throws Exception { object.getCompileTimeClass().getSimpleName()}); oids.add(repositoryService.addObject(object, null, result)); } - LOGGER.info("Time to add objects ({}): {}", new Object[]{elements.size(), - (System.currentTimeMillis() - time),}); + LOGGER.info("Time to add objects ({}): {}", elements.size(), System.currentTimeMillis() - time); + List> objectsRead = new ArrayList<>(); int count = 0; elements = prismContext.parserFor(file).parseObjects(); for (int i = 0; i < elements.size(); i++) { @@ -172,10 +185,8 @@ private void addGetCompare(File file) throws Exception { System.out.println("OLD: " + object.findProperty(ObjectType.F_NAME).getValue()); System.out.println("NEW: " + newObject.findProperty(ObjectType.F_NAME).getValue()); + objectsRead.add(newObject); ObjectDelta delta = object.diff(newObject); - if (delta == null) { - continue; - } count += delta.getModifications().size(); if (delta.getModifications().size() > 0) { @@ -187,8 +198,8 @@ private void addGetCompare(File file) throws Exception { continue; } } - LOGGER.error(">>> {} Found {} changes for {}\n{}", new Object[]{(i + 1), - delta.getModifications().size(), newObject.toString(), delta.debugDump(3)}); + LOGGER.error(">>> {} Found {} changes for {}\n{}", (i + 1), + delta.getModifications().size(), newObject.toString(), delta.debugDump(3)); ItemDelta id = (ItemDelta) delta.getModifications().iterator().next(); if (id.isReplace()) { LOGGER.debug("{}", id.getValuesToReplace().iterator().next()); @@ -200,8 +211,8 @@ private void addGetCompare(File file) throws Exception { throw new RuntimeException("Exception during processing of "+object+": "+ex.getMessage(), ex); } } - AssertJUnit.assertEquals("Found changes during add/get test " + count, 0, count); + return objectsRead; } private Integer size(PrismContainerValue value) { diff --git a/repo/repo-sql-impl-test/src/test/resources/basic/objects.xml b/repo/repo-sql-impl-test/src/test/resources/basic/objects.xml index b29cc9482fc..68369d8b5d9 100644 --- a/repo/repo-sql-impl-test/src/test/resources/basic/objects.xml +++ b/repo/repo-sql-impl-test/src/test/resources/basic/objects.xml @@ -164,7 +164,7 @@ - + u-000 Test UserX00003 diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java index ef2f5a5ed28..c255abc434e 100644 --- a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java +++ b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java @@ -16,10 +16,7 @@ package com.evolveum.midpoint.repo.sql.helpers; -import com.evolveum.midpoint.prism.PrismContext; -import com.evolveum.midpoint.prism.PrismObject; -import com.evolveum.midpoint.prism.PrismObjectDefinition; -import com.evolveum.midpoint.prism.PrismReference; +import com.evolveum.midpoint.prism.*; import com.evolveum.midpoint.prism.delta.ItemDelta; import com.evolveum.midpoint.prism.delta.ObjectDelta; import com.evolveum.midpoint.prism.delta.ReferenceDelta; @@ -238,7 +235,8 @@ public void updateFullObject(RObject object, PrismObject< // Its' because we're removing some properties during save operation and if save fails, // overwrite attempt (for example using object importer) might try to delete existing object // and then try to save this object one more time. - String xml = prismContext.serializeObjectToString(savedObject, PrismContext.LANG_XML); + SerializationOptions options = SerializationOptions.createSerializeReferenceNames(); + String xml = prismContext.xmlSerializer().options(options).serialize(savedObject); savedObject = prismContext.parseObject(xml); if (FocusType.class.isAssignableFrom(savedObject.getCompileTimeClass())) { @@ -249,7 +247,7 @@ public void updateFullObject(RObject object, PrismObject< savedObject.removeContainer(AccessCertificationCampaignType.F_CASE); } - xml = prismContext.serializeObjectToString(savedObject, PrismContext.LANG_XML); + xml = prismContext.xmlSerializer().options(options).serialize(savedObject); byte[] fullObject = RUtil.getByteArrayFromXml(xml, getConfiguration().isUseZip()); LOGGER.trace("Storing full object\n{}", xml); diff --git a/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskQuartzImpl.java b/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskQuartzImpl.java index edbb56896af..4da4007a4dc 100644 --- a/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskQuartzImpl.java +++ b/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskQuartzImpl.java @@ -1518,26 +1518,13 @@ public void setObjectRefImmediate(ObjectReferenceType value, OperationResult par } public void setObjectRefTransient(ObjectReferenceType objectRefType) { - PrismReference objectRef; - try { - objectRef = taskPrism.findOrCreateReference(TaskType.F_OBJECT_REF); - } catch (SchemaException e) { - // This should not happen - throw new IllegalStateException("Internal schema error: " + e.getMessage(), e); - } - objectRef.getValue().setOid(objectRefType.getOid()); - objectRef.getValue().setTargetType(objectRefType.getType()); + taskPrism.asObjectable().setObjectRef(objectRefType != null ? objectRefType.clone() : null); } private ReferenceDelta setObjectRefAndPrepareDelta(ObjectReferenceType value) { setObjectRefTransient(value); - - PrismReferenceValue prismReferenceValue = new PrismReferenceValue(); - prismReferenceValue.setOid(value.getOid()); - prismReferenceValue.setTargetType(value.getType()); - return isPersistent() ? ReferenceDelta.createModificationReplace(TaskType.F_OBJECT_REF, - taskManager.getTaskObjectDefinition(), prismReferenceValue) : null; + taskManager.getTaskObjectDefinition(), value.clone().asReferenceValue()) : null; } @Override