Skip to content

Commit

Permalink
Show original name and coords for sim. objects
Browse files Browse the repository at this point in the history 
When there is a simulated change of name or shadow coordinates, it is
better to show the original values in GUI and in reports.

Note that such change for structural archetype is yet to be done.
We still show the new value there.

Related to MID-8610.
  • Loading branch information
@mederly
mederly committed Mar 20, 2023
1 parent 622406a commit 2dc81c0
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 24 deletions.
3 changes: 3 additions & 0 deletions infra/schema/src/main/resources/xml/ns/public/common/common-simulations-3.xsd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,8 @@
<xsd:documentation>
Information about the shadow (if applicable).

When the simulated change of the coordinates themselves is reported, the old value is provided here.

TODO reconsider if all information from the type should be used (e.g. tombstone and discriminator order
- are they relevant?).
</xsd:documentation>
Expand All @@ -1050,6 +1052,7 @@
<xsd:annotation>
<xsd:documentation>
Name of the object processed. May be null e.g. for shadows that are to be created.
When the simulated change of the name itself is reported, the old value is provided here.
</xsd:documentation>
<xsd:appinfo>
<a:displayName>SimulationResultProcessedObjectType.name</a:displayName>
Expand Down
7 changes: 6 additions & 1 deletion ...l-impl/src/main/java/com/evolveum/midpoint/model/impl/simulation/ProcessedObjectImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,11 @@ static <O extends ObjectType> ProcessedObjectImpl<O> createSingleDelta(
delta.computeChangedObject(
asPrismObject(stateBefore)));
}
@Nullable O anyState = MiscUtil.getFirstNonNull(stateAfter, stateBefore);

// We intentionally prefer the state before. The reason is that e.g. for object name we want to display the record
// and report on it under the OLD name - it is the name of the object that is currently valid in the real world.
// The same is true for the shadow discriminator. See MID-8610.
@Nullable O anyState = MiscUtil.getFirstNonNull(stateBefore, stateAfter);

if (anyState == null) {
return null;
Expand All @@ -259,6 +263,7 @@ static <O extends ObjectType> ProcessedObjectImpl<O> createSingleDelta(
simulationTransaction.getTransactionId(),
elementContext.getOid(),
type,
// We should provide the old value here, just like for name and discriminator. See MID-8610.
elementContext instanceof LensFocusContext<?> ?
((LensFocusContext<?>) elementContext).getStructuralArchetypeRef() : null,
determineShadowDiscriminator(anyState),
Expand Down
24 changes: 14 additions & 10 deletions ...del-intest/src/test/java/com/evolveum/midpoint/model/intest/TestModelServiceContract.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2866,7 +2866,7 @@ public void test550ModifyUserJack() throws Exception {

when();
modifyUserReplace(USER_JACK_OID, UserType.F_FULL_NAME, task, result,
PrismTestUtil.createPolyString("Magnificent Captain Jack Sparrow"));
PolyString.fromOrig("Magnificent Captain Jack Sparrow"));

then();
assertSuccess(result);
Expand Down Expand Up @@ -2902,7 +2902,7 @@ public void test550ModifyUserJack() throws Exception {
dummyAuditService.assertHasDelta(ChangeType.MODIFY, ShadowType.class);

dummyAuditService.assertOldValue(ChangeType.MODIFY, UserType.class,
UserType.F_FULL_NAME, PrismTestUtil.createPolyString("Jack Sparrow"));
UserType.F_FULL_NAME, PolyString.fromOrig("Jack Sparrow"));
// We have full account here. It is loaded because of strong mapping.
dummyAuditService.assertOldValue(ChangeType.MODIFY, ShadowType.class,
dummyResourceCtl.getAttributeFullnamePath(), "Cpt. Jack Sparrow");
Expand Down Expand Up @@ -2969,7 +2969,8 @@ public void test560ModifyUserJackLocationEmpty() throws Exception {
dummyAuditService.assertHasDelta(ChangeType.MODIFY, UserType.class);
dummyAuditService.assertPropertyReplace(ChangeType.MODIFY, UserType.class, UserType.F_LOCALITY /* no values */);
// MID-4020
dummyAuditService.assertOldValue(ChangeType.MODIFY, UserType.class, UserType.F_LOCALITY, createPolyString(USER_JACK_LOCALITY));
dummyAuditService.assertOldValue(
ChangeType.MODIFY, UserType.class, UserType.F_LOCALITY, PolyString.fromOrig(USER_JACK_LOCALITY));
dummyAuditService.assertHasDelta(ChangeType.MODIFY, ShadowType.class);
dummyAuditService.assertTarget(USER_JACK_OID);
dummyAuditService.assertExecutionSuccess();
Expand Down Expand Up @@ -3028,7 +3029,7 @@ public void test580ModifyUserJackLocationSea() throws Exception {
preTestCleanup(AssignmentPolicyEnforcementType.FULL);

when();
modifyUserReplace(USER_JACK_OID, UserType.F_LOCALITY, task, result, createPolyString("sea"));
modifyUserReplace(USER_JACK_OID, UserType.F_LOCALITY, task, result, PolyString.fromOrig("sea"));

then();
assertSuccess(result);
Expand Down Expand Up @@ -3062,7 +3063,8 @@ public void test580ModifyUserJackLocationSea() throws Exception {
dummyAuditService.assertExecutionDeltas(2);
dummyAuditService.assertHasDelta(ChangeType.MODIFY, UserType.class);
dummyAuditService.assertHasDelta(ChangeType.MODIFY, ShadowType.class);
dummyAuditService.assertPropertyReplace(ChangeType.MODIFY, UserType.class, UserType.F_LOCALITY, createPolyString("sea"));
dummyAuditService.assertPropertyReplace(
ChangeType.MODIFY, UserType.class, UserType.F_LOCALITY, PolyString.fromOrig("sea"));
// MID-4020
dummyAuditService.assertOldValue(ChangeType.MODIFY, UserType.class, UserType.F_LOCALITY /* no values */);
dummyAuditService.assertTarget(USER_JACK_OID);
Expand Down Expand Up @@ -3485,7 +3487,7 @@ public void test650RenameUserMorganSimulated() throws Exception {
var simulationResult = executeWithSimulationResult(
task, result,
() -> modifyUserReplace(
USER_MORGAN_OID, UserType.F_NAME, task, result, PrismTestUtil.createPolyString("sirhenry")));
USER_MORGAN_OID, UserType.F_NAME, task, result, PolyString.fromOrig("sirhenry")));

then("operation is successful");
assertSuccess(result);
Expand All @@ -3498,12 +3500,14 @@ public void test650RenameUserMorganSimulated() throws Exception {
assertProcessedObjects(simulationResult, "after")
.display()
.by().objectType(UserType.class).changeType(ChangeType.MODIFY).find(
a -> a.assertEventMarks(MARK_FOCUS_RENAMED))
a -> a.assertEventMarks(MARK_FOCUS_RENAMED)
.assertName("morgan")) // original name (MID-8610)
.by().objectType(ShadowType.class).changeType(ChangeType.MODIFY).find(
a -> a.assertEventMarks(
MARK_PROJECTION_RENAMED,
MARK_PROJECTION_IDENTIFIER_CHANGED,
MARK_PROJECTION_RESOURCE_OBJECT_AFFECTED))
MARK_PROJECTION_RESOURCE_OBJECT_AFFECTED)
.assertName("morgan")) // original name (MID-8610)
.assertSize(2);

and("no side effects: no new objects, no provisioning scripts, no audit deltas, no notifications");
Expand All @@ -3524,7 +3528,7 @@ public void test655RenameUserMorgan() throws Exception {
preTestCleanup(AssignmentPolicyEnforcementType.FULL);

when();
modifyUserReplace(USER_MORGAN_OID, UserType.F_NAME, task, result, PrismTestUtil.createPolyString("sirhenry"));
modifyUserReplace(USER_MORGAN_OID, UserType.F_NAME, task, result, PolyString.fromOrig("sirhenry"));

then();
assertSuccess(result);
Expand Down Expand Up @@ -3574,7 +3578,7 @@ public void test655RenameUserMorgan() throws Exception {
// attributes/icfs:uid: morgan -> sirhenry - this is a change induced by the connector/resource

dummyAuditService.assertOldValue(ChangeType.MODIFY, UserType.class,
UserType.F_NAME, PrismTestUtil.createPolyString("morgan"));
UserType.F_NAME, PolyString.fromOrig("morgan"));
dummyAuditService.assertOldValue(ChangeType.MODIFY, ShadowType.class,
SchemaConstants.ICFS_NAME_PATH, "morgan");
dummyAuditService.assertOldValue(ChangeType.MODIFY, ShadowType.class,
Expand Down
26 changes: 13 additions & 13 deletions model/report-impl/src/test/java/com/evolveum/midpoint/report/TestCsvSimulationReport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,14 +299,14 @@ public void test110DisableAndRenameUsers() throws Exception {
.assertColumns(OBJECT_REPORT_COLUMNS)
.record(0)
.assertValue(C_OID, existingUsers.get(0).getOid())
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_TYPE, "UserType")
.assertValue(C_STATE, "Modified")
.assertValues(C_MARK, "Focus renamed", "Focus deactivated")
.end()
.record(1)
.assertValue(C_OID, existingUsers.get(1).getOid())
.assertValue(C_NAME, "existing-0001-renamed")
.assertValue(C_NAME, "existing-0001")
.assertValue(C_TYPE, "UserType")
.assertValue(C_STATE, "Modified")
.assertValues(C_MARK, "Focus renamed");
Expand All @@ -325,7 +325,7 @@ public void test110DisableAndRenameUsers() throws Exception {
.assertColumns(14)
.record(0,
r -> r.assertValue(C_OID, existingUsers.get(0).getOid())
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_TYPE, "UserType")
.assertValue(C_STATE, "Modified")
.assertValue(C_M_EVENT_MARK, "")
Expand All @@ -334,7 +334,7 @@ public void test110DisableAndRenameUsers() throws Exception {
.assertValue(C_M_VALUE, "2"))
.record(1,
r -> r.assertValue(C_OID, existingUsers.get(0).getOid())
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_TYPE, "UserType")
.assertValue(C_STATE, "Modified")
.assertValue(C_M_EVENT_MARK, "")
Expand All @@ -343,7 +343,7 @@ public void test110DisableAndRenameUsers() throws Exception {
.assertValue(C_M_VALUE, "0.0"))
.record(2,
r -> r.assertValue(C_OID, existingUsers.get(0).getOid())
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_TYPE, "UserType")
.assertValue(C_STATE, "Modified")
.assertValue(C_M_EVENT_MARK, "Focus deactivated")
Expand All @@ -352,7 +352,7 @@ public void test110DisableAndRenameUsers() throws Exception {
.assertValue(C_M_VALUE, "1"))
.record(3,
r -> r.assertValue(C_OID, existingUsers.get(0).getOid())
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_TYPE, "UserType")
.assertValue(C_STATE, "Modified")
.assertValue(C_M_EVENT_MARK, "Focus renamed")
Expand All @@ -361,7 +361,7 @@ public void test110DisableAndRenameUsers() throws Exception {
.assertValue(C_M_VALUE, "1"))
.record(4,
r -> r.assertValue(C_OID, existingUsers.get(1).getOid())
.assertValue(C_NAME, "existing-0001-renamed")
.assertValue(C_NAME, "existing-0001")
.assertValue(C_TYPE, "UserType")
.assertValue(C_STATE, "Modified")
.assertValue(C_M_EVENT_MARK, "")
Expand All @@ -370,7 +370,7 @@ public void test110DisableAndRenameUsers() throws Exception {
.assertValue(C_M_VALUE, "1"))
.record(5,
r -> r.assertValue(C_OID, existingUsers.get(1).getOid())
.assertValue(C_NAME, "existing-0001-renamed")
.assertValue(C_NAME, "existing-0001")
.assertValue(C_TYPE, "UserType")
.assertValue(C_STATE, "Modified")
.assertValue(C_M_EVENT_MARK, "")
Expand All @@ -389,15 +389,15 @@ public void test110DisableAndRenameUsers() throws Exception {
.display()
.assertRecords(15) // 10 changes of name, 5 changes of administrative status
.record(0)
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_ITEM_CHANGED, "activation/administrativeStatus")
.assertValue(C_OLD_VALUES, "")
.assertValue(C_NEW_VALUES, "Disabled")
.assertValue(C_VALUES_ADDED, "Disabled")
.assertValue(C_VALUES_DELETED, "")
.end()
.record(1)
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_ITEM_CHANGED, "name")
.assertValue(C_OLD_VALUES, "existing-0000")
.assertValue(C_NEW_VALUES, "existing-0000-renamed")
Expand Down Expand Up @@ -453,19 +453,19 @@ public void test110DisableAndRenameUsers() throws Exception {
.display()
.assertRecords(25) // 20x name (ADD/DELETE), 5x administrativeStatus (ADD)
.record(0)
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_ITEM_CHANGED, "activation/administrativeStatus")
.assertValue(C_VALUE_STATE, "ADDED")
.assertValue(C_VALUE, "Disabled")
.end()
.record(1)
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_ITEM_CHANGED, "name")
.assertValue(C_VALUE_STATE, "ADDED")
.assertValue(C_VALUE, "existing-0000-renamed")
.end()
.record(2)
.assertValue(C_NAME, "existing-0000-renamed")
.assertValue(C_NAME, "existing-0000")
.assertValue(C_ITEM_CHANGED, "name")
.assertValue(C_VALUE_STATE, "DELETED")
.assertValue(C_VALUE, "existing-0000")
Expand Down

0 comments on commit 2dc81c0

Please sign in to comment.