diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/input/ContainersDropDownPanel.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/input/ContainersDropDownPanel.java index 2589cc78560..a73e1549b47 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/input/ContainersDropDownPanel.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/input/ContainersDropDownPanel.java @@ -10,6 +10,7 @@ import com.evolveum.midpoint.gui.api.factory.wrapper.WrapperContext; import com.evolveum.midpoint.gui.api.prism.ItemStatus; import com.evolveum.midpoint.gui.api.prism.wrapper.ItemWrapper; +import com.evolveum.midpoint.gui.api.prism.wrapper.PrismContainerValueWrapper; import com.evolveum.midpoint.gui.api.prism.wrapper.PrismContainerWrapper; import com.evolveum.midpoint.gui.api.prism.wrapper.PrismValueWrapper; import com.evolveum.midpoint.gui.api.util.WebPrismUtil; @@ -148,13 +149,11 @@ public void setObject(ItemName object) { if (!found) { PrismContainer container = parentItem.getValue().getNewValue().findOrCreateContainer(object); - Task task = getPageBase().createSimpleTask("create_action_container"); - WrapperContext ctx = new WrapperContext(task, task.getResult()); - ctx.setCreateIfEmpty(true); - ItemWrapper iw = getPageBase().createItemWrapper(container, parentItem.getValue(), ItemStatus.ADDED, ctx); - if (iw.isMultiValue()) { - PrismValueWrapper value = getPageBase().createValueWrapper(iw, container.createNewValue(), ValueStatus.ADDED, ctx); - iw.getValues().add(value); + ItemWrapper iw = createNewContainerWrapper(container, parentItem.getValue()); + if (iw == null) { + parentItem.getValue().getNewValue().remove(container); + LOGGER.warn("Couldn't create wrapper for selected container {}", object); + return; } parentItem.getValue().addItem(iw); } @@ -165,6 +164,19 @@ public void setObject(ItemName object) { }; } + protected ItemWrapper createNewContainerWrapper( + PrismContainer container, PrismContainerValueWrapper parentValue) throws SchemaException { + Task task = getPageBase().createSimpleTask("create_action_container"); + WrapperContext ctx = new WrapperContext(task, task.getResult()); + ctx.setCreateIfEmpty(true); + ItemWrapper iw = getPageBase().createItemWrapper(container, parentValue, ItemStatus.ADDED, ctx); + if (iw != null && iw.isMultiValue()) { + PrismValueWrapper value = getPageBase().createValueWrapper(iw, container.createNewValue(), ValueStatus.ADDED, ctx); + iw.getValues().add(value); + } + return iw; + } + public IModel getDropDownModel() { return dropDownModel; } @@ -216,12 +228,37 @@ private IModel>> getChoices() { List> containers = new ArrayList<>(); PrismContainerWrapper parentItem = getModelObject(); parentItem.getDefinitions().stream() - .filter(def -> ((def instanceof PrismContainerDefinition) && validateChildContainer(def))) + .filter(def -> ((def instanceof PrismContainerDefinition) && isSelectableContainerDefinition(def))) .forEach(def -> containers.add(new ContainerDisplayableValue(def))); return containers; }; } + /** + * Deprecated containers should not be offered as new choices. However, if a deprecated + * container is already selected in existing configuration, keep it visible so the user + * can see it and migrate it to a supported option. + */ + protected boolean isSelectableContainerDefinition(ItemDefinition definition) { + return validateChildContainer(definition) + && (!definition.isDeprecated() || isExistingSelectedContainer(definition)); + } + + private boolean isExistingSelectedContainer(ItemDefinition definition) { + try { + PrismContainerWrapper parentItem = getModelObject(); + return parentItem != null + && parentItem.getValue() != null + && parentItem.getValue().getContainers().stream() + .anyMatch(container -> definition.getItemName().equivalent(container.getItemName()) + && container.getValues().stream() + .anyMatch(value -> value.getStatus() != ValueStatus.DELETED)); + } catch (SchemaException e) { + LOGGER.error("Couldn't determine selected synchronization action", e); + return false; + } + } + private class ContainerDisplayableValue implements DisplayableValue, Serializable { private final String displayName; diff --git a/gui/admin-gui/src/test/java/com/evolveum/midpoint/gui/TestIntegrationObjectWrapperFactory.java b/gui/admin-gui/src/test/java/com/evolveum/midpoint/gui/TestIntegrationObjectWrapperFactory.java index 79e92c3ce16..26102faa9a6 100644 --- a/gui/admin-gui/src/test/java/com/evolveum/midpoint/gui/TestIntegrationObjectWrapperFactory.java +++ b/gui/admin-gui/src/test/java/com/evolveum/midpoint/gui/TestIntegrationObjectWrapperFactory.java @@ -13,11 +13,15 @@ import java.io.File; import java.util.*; import java.util.stream.Collectors; +import javax.xml.namespace.QName; +import com.evolveum.midpoint.gui.impl.component.input.ContainersDropDownPanel; +import com.evolveum.midpoint.gui.impl.prism.wrapper.PrismContainerValueWrapperImpl; import com.evolveum.midpoint.gui.impl.prism.wrapper.PrismContainerWrapperImpl; import com.evolveum.midpoint.gui.impl.prism.wrapper.PrismReferenceValueWrapperImpl; import com.evolveum.midpoint.test.util.TestUtil; +import org.apache.wicket.model.Model; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.annotation.DirtiesContext.ClassMode; @@ -530,6 +534,88 @@ private PrismObjectWrapper createObjectWrapper(Task ta } + /** Verifies that deprecated synchronization action containers are not offered as new dropdown choices. */ + @Test + public void test155SynchronizationActionDropdownSkipsDeprecatedUnlink() throws Exception { + PrismContainerWrapper actionsWrapper = + findSynchronizationActionsWrapper(createSynchronizationActionResource(false)); + + TestableContainersDropDownPanel panel = + new TestableContainersDropDownPanel(Model.of(actionsWrapper)); + List choices = panel.getChoiceItemNames(actionsWrapper); + + assertTrue("Synchronize action should be selectable", choices.contains(SynchronizationActionsType.F_SYNCHRONIZE)); + assertTrue("Link action should be selectable", choices.contains(SynchronizationActionsType.F_LINK)); + assertFalse("Deprecated unlink action should not be selectable", choices.contains(SynchronizationActionsType.F_UNLINK)); + } + + /** + * Verifies wrapper creation behavior for a legacy resource containing an empty deprecated unlink action. + * Empty deprecated containers are filtered by the wrapper factory, so the dropdown must not expose unlink as + * the current or selectable value, but it still has to render safely and offer supported replacements. + */ + @Test + public void test156ExistingDeprecatedSynchronizationUnlinkOpens() throws Exception { + PrismContainerWrapper actionsWrapper = + findSynchronizationActionsWrapper(createSynchronizationActionResource(true)); + + PrismContainerWrapper unlinkWrapper = + actionsWrapper.getValue().findContainer(SynchronizationActionsType.F_UNLINK); + TestableContainersDropDownPanel panel = + new TestableContainersDropDownPanel(Model.of(actionsWrapper)); + List choices = panel.getChoiceItemNames(actionsWrapper); + + assertNull("Existing empty deprecated unlink action should not be represented as selectable wrapper", unlinkWrapper); + assertNull("Existing empty deprecated unlink action should not be current dropdown value", panel.getDropDownModel().getObject()); + assertFalse("Deprecated unlink action should not be selectable", choices.contains(SynchronizationActionsType.F_UNLINK)); + assertTrue("Existing deprecated unlink action should be replaceable by synchronize", choices.contains(SynchronizationActionsType.F_SYNCHRONIZE)); + } + + /** Verifies that a legacy deprecated unlink action can be replaced by the supported synchronize action. */ + @Test + public void test157ExistingDeprecatedSynchronizationUnlinkCanChangeToSynchronize() throws Exception { + PrismContainerWrapper actionsWrapper = + findSynchronizationActionsWrapper(createSynchronizationActionResource(true)); + TestableContainersDropDownPanel panel = + new TestableContainersDropDownPanel(Model.of(actionsWrapper)); + + panel.getDropDownModel().setObject(SynchronizationActionsType.F_SYNCHRONIZE); + + PrismContainerWrapper synchronizeWrapper = + actionsWrapper.getValue().findContainer(SynchronizationActionsType.F_SYNCHRONIZE); + PrismContainerWrapper unlinkWrapper = + actionsWrapper.getValue().findContainer(SynchronizationActionsType.F_UNLINK); + + assertNotNull("Synchronize action wrapper should be created", synchronizeWrapper); + assertFalse("Synchronize action wrapper should contain a value", synchronizeWrapper.getValues().isEmpty()); + assertNull("Deprecated unlink action wrapper should remain absent", unlinkWrapper); + assertFalse( + "Deprecated unlink action should not be selectable after changing away", + panel.getChoiceItemNames(actionsWrapper).contains(SynchronizationActionsType.F_UNLINK)); + } + + /** Verifies that a skipped wrapper does not leave an empty container behind or crash the dropdown model update. */ + @Test + public void test158SynchronizationActionDropdownNullWrapperDoesNotCrash() throws Exception { + PrismContainerWrapper actionsWrapper = + findSynchronizationActionsWrapper(createSynchronizationActionResource(false)); + + ContainersDropDownPanel panel = + new ContainersDropDownPanel<>("actions", Model.of(actionsWrapper)) { + @Override + protected ItemWrapper createNewContainerWrapper( + PrismContainer container, PrismContainerValueWrapper parentValue) { + return null; + } + }; + + panel.getDropDownModel().setObject(SynchronizationActionsType.F_UNLINK); + + assertNull( + "Container created for a skipped wrapper should be removed", + actionsWrapper.getValue().getNewValue().findContainer(SynchronizationActionsType.F_UNLINK)); + } + @Test public void test160CreateWrapperOrgScummBar() throws Exception { PrismObject org = getObject(OrgType.class, ORG_SCUMM_BAR_OID); @@ -1046,4 +1132,86 @@ private void cleanupAutzTest(String userOid) private String getString(String key) { return localizationService.translate(key, null, Locale.US, key); } + + private ResourceType createSynchronizationActionResource(boolean existingUnlink) { + ResourceType resource = new ResourceType() + .name("REPRO 1057 - synchronization editor only"); + ResourceObjectTypeDefinitionType objectType = resource.beginSchemaHandling() + .beginObjectType() + .kind(ShadowKindType.ACCOUNT) + .intent("default") + .displayName("Default Account") + ._default(true); + objectType.beginDelineation() + .objectClass(new QName("http://midpoint.evolveum.com/xml/ns/public/resource/instance-3", "AccountObjectClass")); + objectType.beginFocus() + .type(UserType.COMPLEX_TYPE); + SynchronizationActionsType actions = objectType.beginSynchronization() + .beginReaction() + .situation(SynchronizationSituationType.UNLINKED) + .beginActions(); + if (existingUnlink) { + actions.beginUnlink(); + } else { + actions.beginLink(); + } + return resource; + } + + private PrismContainerWrapper findSynchronizationActionsWrapper(ResourceType resource) + throws SchemaException { + Task task = getTestTask(); + PrismObjectWrapper objectWrapper = + createObjectWrapper(task, resource.asPrismObject(), ItemStatus.NOT_CHANGED); + PrismContainerValueWrapper resourceValue = objectWrapper.getValue(); + PrismContainerWrapper schemaHandlingWrapper = + resourceValue.findContainer(ResourceType.F_SCHEMA_HANDLING); + PrismContainerWrapper objectTypeWrapper = + firstValue(schemaHandlingWrapper).findContainer(SchemaHandlingType.F_OBJECT_TYPE); + PrismContainerWrapper synchronizationWrapper = + firstValue(objectTypeWrapper).findContainer(ResourceObjectTypeDefinitionType.F_SYNCHRONIZATION); + PrismContainerWrapper reactionWrapper = + firstValue(synchronizationWrapper).findContainer(SynchronizationReactionsType.F_REACTION); + return firstValue(reactionWrapper).findContainer(SynchronizationReactionType.F_ACTIONS); + } + + private PrismContainerValueWrapper firstValue(PrismContainerWrapper wrapper) { + return wrapper.getValues().stream() + .filter(value -> value.getStatus() == ValueStatus.NOT_CHANGED) + .findFirst() + .or(() -> wrapper.getValues().stream() + .filter(value -> value.getStatus() != ValueStatus.DELETED) + .findFirst()) + .orElseThrow(() -> new AssertionError("No active value in " + wrapper.getItemName())); + } + + /** + * Test-facing panel that exposes the dropdown choice filtering and creates lightweight wrappers without a + * mounted Wicket page. The production null-wrapper path is exercised by overriding + * {@link #createNewContainerWrapper(PrismContainer, PrismContainerValueWrapper)} directly in the relevant test. + */ + private static class TestableContainersDropDownPanel extends ContainersDropDownPanel { + + private TestableContainersDropDownPanel(Model> model) { + super("actions", model); + } + + private List getChoiceItemNames(PrismContainerWrapper wrapper) { + return wrapper.getDefinitions().stream() + .filter(def -> def instanceof PrismContainerDefinition && isSelectableContainerDefinition(def)) + .map(ItemDefinition::getItemName) + .collect(Collectors.toList()); + } + + @Override + protected ItemWrapper createNewContainerWrapper( + PrismContainer container, PrismContainerValueWrapper parentValue) { + PrismContainerWrapper wrapper = + new PrismContainerWrapperImpl<>(parentValue, container, ItemStatus.ADDED); + PrismValueWrapper value = + new PrismContainerValueWrapperImpl<>(wrapper, container.createNewValue(), ValueStatus.ADDED); + wrapper.getValues().add((PrismContainerValueWrapper) value); + return wrapper; + } + } } diff --git a/release-notes.adoc b/release-notes.adoc index c606d71c93d..d306fa37ff5 100644 --- a/release-notes.adoc +++ b/release-notes.adoc @@ -102,6 +102,7 @@ Overall, midPoint 4.10 opens up the world of identity management and governance * Delineation suggestions: filter parsing broken after recent GUI change. See bug:MID-11175[] * Fixed work item search by name causing repository mapping error. See bug:MID-8834[] * Fixed translation of archetype display labels in assignment picker and summary panel. See bug:MID-11177[] +* Fixed NPE when selecting deprecated action in resource object type editor. See bug:MID-10957[] === Releases Of Other Components