From add1d7fc146fa75f8828f9d6a74e6cd15838f5fc Mon Sep 17 00:00:00 2001 From: Lutz Wrage Date: Thu, 17 Sep 2026 13:13:19 -0400 Subject: [PATCH 1/2] Add regression test for issue #3235 Exercise shared contained property values through the AADL Property Values view using a separate, validated AADL model project. Verify sibling-value preservation, shared scalar and nested-value editability, and safe removal after Make Local or Make Local Contained. Include the EMV2 UI service provider in the UI test runtime so property lookup can load contributed libraries. Before the fix, five of the seven new tests fail, including the loss of the unselected child value; both localization tests and all twelve existing context-menu tests pass. --- .../models/issue3235/.gitignore | 2 + .../models/issue3235/.project | 18 ++ .../models/issue3235/Issue3235.aadl | 27 +++ .../issue3235/Issue3235_Properties.aadl | 9 + core/org.osate.ui.tests/pom.xml | 7 + .../ui/tests/propertyview/Issue3235Test.java | 196 ++++++++++++++++++ 6 files changed, 259 insertions(+) create mode 100644 core/org.osate.ui.tests/models/issue3235/.gitignore create mode 100644 core/org.osate.ui.tests/models/issue3235/.project create mode 100644 core/org.osate.ui.tests/models/issue3235/Issue3235.aadl create mode 100644 core/org.osate.ui.tests/models/issue3235/Issue3235_Properties.aadl create mode 100644 core/org.osate.ui.tests/src/org/osate/ui/tests/propertyview/Issue3235Test.java diff --git a/core/org.osate.ui.tests/models/issue3235/.gitignore b/core/org.osate.ui.tests/models/issue3235/.gitignore new file mode 100644 index 00000000000..afce51184c6 --- /dev/null +++ b/core/org.osate.ui.tests/models/issue3235/.gitignore @@ -0,0 +1,2 @@ +/.aadlbin-gen/ +/instances/ diff --git a/core/org.osate.ui.tests/models/issue3235/.project b/core/org.osate.ui.tests/models/issue3235/.project new file mode 100644 index 00000000000..87c18becf49 --- /dev/null +++ b/core/org.osate.ui.tests/models/issue3235/.project @@ -0,0 +1,18 @@ + + + issue3235 + + + + + + org.eclipse.xtext.ui.shared.xtextBuilder + + + + + + org.osate.core.aadlnature + org.eclipse.xtext.ui.shared.xtextNature + + diff --git a/core/org.osate.ui.tests/models/issue3235/Issue3235.aadl b/core/org.osate.ui.tests/models/issue3235/Issue3235.aadl new file mode 100644 index 00000000000..f2629af4642 --- /dev/null +++ b/core/org.osate.ui.tests/models/issue3235/Issue3235.aadl @@ -0,0 +1,27 @@ +package Issue3235 +public + with Issue3235_Properties; + + system Plain + end Plain; + + system implementation Plain.Impl + end Plain.Impl; + + system Container + end Container; + + system implementation Container.Impl + subcomponents + first_child: system Plain.Impl; + second_child: system Plain.Impl; + properties + Issue3235_Properties::Flag_Property => true applies to first_child, second_child; + Issue3235_Properties::Record_Property => [ + First => 1; + Second => 2; + ] applies to first_child, second_child; + Issue3235_Properties::List_Property => (1, 2) applies to first_child, second_child; + Issue3235_Properties::Range_Property => 1 .. 9 delta 2 applies to first_child, second_child; + end Container.Impl; +end Issue3235; diff --git a/core/org.osate.ui.tests/models/issue3235/Issue3235_Properties.aadl b/core/org.osate.ui.tests/models/issue3235/Issue3235_Properties.aadl new file mode 100644 index 00000000000..4557bf4ac2e --- /dev/null +++ b/core/org.osate.ui.tests/models/issue3235/Issue3235_Properties.aadl @@ -0,0 +1,9 @@ +property set Issue3235_Properties is + Flag_Property: aadlboolean applies to (system); + Record_Property: record ( + First: aadlinteger; + Second: aadlinteger; + ) applies to (system); + List_Property: list of aadlinteger applies to (system); + Range_Property: range of aadlinteger applies to (system); +end Issue3235_Properties; diff --git a/core/org.osate.ui.tests/pom.xml b/core/org.osate.ui.tests/pom.xml index f0710616205..6f812a231db 100644 --- a/core/org.osate.ui.tests/pom.xml +++ b/core/org.osate.ui.tests/pom.xml @@ -89,6 +89,13 @@ org.osate.core.feature 0.0.0 + + + eclipse-plugin + org.osate.xtext.aadl2.errormodel.ui + 0.0.0 + diff --git a/core/org.osate.ui.tests/src/org/osate/ui/tests/propertyview/Issue3235Test.java b/core/org.osate.ui.tests/src/org/osate/ui/tests/propertyview/Issue3235Test.java new file mode 100644 index 00000000000..951a2a88d9d --- /dev/null +++ b/core/org.osate.ui.tests/src/org/osate/ui/tests/propertyview/Issue3235Test.java @@ -0,0 +1,196 @@ +/** + * Copyright (c) 2004-2026 Carnegie Mellon University and others. (see Contributors file). + * All Rights Reserved. + * + * NO WARRANTY. ALL MATERIAL IS FURNISHED ON AN "AS-IS" BASIS. CARNEGIE MELLON UNIVERSITY MAKES NO WARRANTIES OF ANY + * KIND, EITHER EXPRESSED OR IMPLIED, AS TO ANY MATTER INCLUDING, BUT NOT LIMITED TO, WARRANTY OF FITNESS FOR PURPOSE + * OR MERCHANTABILITY, EXCLUSIVITY, OR RESULTS OBTAINED FROM USE OF THE MATERIAL. CARNEGIE MELLON UNIVERSITY DOES NOT + * MAKE ANY WARRANTY OF ANY KIND WITH RESPECT TO FREEDOM FROM PATENT, TRADEMARK, OR COPYRIGHT INFRINGEMENT. + * + * This program and the accompanying materials are made available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * SPDX-License-Identifier: EPL-2.0 + * + * Created, in part, with funding and support from the United States Government. (see Acknowledgments file). + * + * This program includes and/or can make use of certain third party source code, object code, documentation and other + * files ("Third Party Software"). The Third Party Software that is used by this program is dependent upon your system + * configuration. By using this program, You agree to comply with any and all relevant Third Party Software terms and + * conditions contained in any such Third Party Software or separate license file distributed with such Third Party + * Software. The parties who own the Third Party Software ("Third Party Licensors") are intended third party benefici- + * aries to this license with respect to the terms applicable to their Third Party Software. Third Party Software li- + * censes only apply to the Third Party Software and not any other portion of this program or this program as a whole. + */ +package org.osate.ui.tests.propertyview; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.LOCAL; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.LOCAL_CONTAINED; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.MAKE_LOCAL; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.MAKE_LOCAL_CONTAINED; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.REMOVE; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.SHARED_LOCAL_CONTAINED; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.UNDEFINED; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.VALUE_COLUMN; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.assertStatus; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.assertValue; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.clickContextMenu; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.contextMenuEnablement; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.item; +import static org.osate.ui.tests.propertyview.util.AadlPropertyViewBot.showUndefinedProperties; +import static org.osate.ui.tests.propertyview.util.WorkbenchTestUtil.assertNoErrorsInProject; +import static org.osate.ui.tests.propertyview.util.WorkbenchTestUtil.closeAllEditors; +import static org.osate.ui.tests.propertyview.util.WorkbenchTestUtil.closeEditorWithoutSaving; +import static org.osate.ui.tests.propertyview.util.WorkbenchTestUtil.deleteProject; +import static org.osate.ui.tests.propertyview.util.WorkbenchTestUtil.editorText; +import static org.osate.ui.tests.propertyview.util.WorkbenchTestUtil.importModelProject; +import static org.osate.ui.tests.propertyview.util.WorkbenchTestUtil.openEditor; +import static org.osate.ui.tests.propertyview.util.WorkbenchTestUtil.prepareWorkbench; +import static org.osate.ui.tests.propertyview.util.WorkbenchTestUtil.selectInEditor; + +import java.util.List; + +import org.eclipse.core.resources.IProject; +import org.eclipse.swt.SWT; +import org.eclipse.swtbot.swt.finder.finders.UIThreadRunnable; +import org.eclipse.swtbot.swt.finder.junit.SWTBotJunit4ClassRunner; +import org.eclipse.swtbot.swt.finder.widgets.SWTBotTreeItem; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.osate.ui.tests.propertyview.util.AadlPropertyViewBot; + +/** + * A shared contained association must not be editable or removable from one of its targets. + */ +@RunWith(SWTBotJunit4ClassRunner.class) +public class Issue3235Test { + private static final String PROJECT = "issue3235"; + private static final String MODEL_FILE = "Issue3235.aadl"; + private static final String PROPERTY_SET = "Issue3235_Properties"; + private static final String[] FLAG_PATH = { PROPERTY_SET, "Flag_Property" }; + private static IProject project; + + @BeforeClass + public static void importModel() { + prepareWorkbench(); + project = importModelProject(PROJECT); + assertNoErrorsInProject(project); + AadlPropertyViewBot.reopen(); + } + + @AfterClass + public static void discardModel() { + closeAllEditors(); + AadlPropertyViewBot.close(); + deleteProject(PROJECT); + } + + @Before + public void openPristineModel() { + closeEditorWithoutSaving(MODEL_FILE); + openEditor(project, MODEL_FILE); + selectInEditor(MODEL_FILE, "implementation Plain.Impl"); + AadlPropertyViewBot.tree(); + AadlPropertyViewBot.resetFilters(); + selectChild("second_child"); + showUndefinedProperties(true); + } + + @Test + public void testRemoveDoesNotDeleteOtherChildValue() { + assertValue("true", FLAG_PATH); + var originalText = editorText(MODEL_FILE); + selectChild("first_child"); + + // Exercise the destructive action if the bug makes it available, then check the unselected child first. + boolean removeEnabled = contextMenuEnablement(FLAG_PATH, List.of(REMOVE)).get(REMOVE); + if (removeEnabled) { + clickContextMenu(FLAG_PATH, REMOVE); + assertStatus(UNDEFINED, FLAG_PATH); + } + selectChild("second_child"); + assertValue("true", FLAG_PATH); + assertStatus(SHARED_LOCAL_CONTAINED, FLAG_PATH); + assertEquals("The shared association must remain unchanged", originalText, editorText(MODEL_FILE)); + assertFalse("Remove must be disabled for a shared association", removeEnabled); + } + + @Test + public void testSharedScalarIsReadOnlyForBothChildren() { + for (var child : List.of("first_child", "second_child")) { + selectChild(child); + assertValue("true", FLAG_PATH); + assertStatus(SHARED_LOCAL_CONTAINED, FLAG_PATH); + assertReadOnly(FLAG_PATH); + } + } + + @Test + public void testSharedRecordFieldIsReadOnly() { + selectChild("first_child"); + assertValue("1", PROPERTY_SET, "Record_Property", "First"); + assertStatus(SHARED_LOCAL_CONTAINED, PROPERTY_SET, "Record_Property", "First"); + assertReadOnly(PROPERTY_SET, "Record_Property", "First"); + } + + @Test + public void testSharedListElementIsReadOnly() { + selectChild("first_child"); + assertValue("1", PROPERTY_SET, "List_Property", "# 0"); + assertReadOnly(PROPERTY_SET, "List_Property", "# 0"); + } + + @Test + public void testSharedRangeDeltaIsReadOnly() { + selectChild("first_child"); + assertValue("2", PROPERTY_SET, "Range_Property", "delta"); + assertReadOnly(PROPERTY_SET, "Range_Property", "delta"); + } + + @Test + public void testMakeLocalThenRemovePreservesOtherChild() { + makeLocalThenRemove(MAKE_LOCAL, LOCAL); + } + + @Test + public void testMakeLocalContainedThenRemovePreservesOtherChild() { + makeLocalThenRemove(MAKE_LOCAL_CONTAINED, LOCAL_CONTAINED); + } + + private static void makeLocalThenRemove(String action, String expectedStatus) { + selectChild("first_child"); + clickContextMenu(FLAG_PATH, action); + assertStatus(expectedStatus, FLAG_PATH); + assertValue("true", FLAG_PATH); + clickContextMenu(FLAG_PATH, REMOVE); + assertStatus(UNDEFINED, FLAG_PATH); + assertValue("", FLAG_PATH); + + selectChild("second_child"); + assertStatus(LOCAL_CONTAINED, FLAG_PATH); + assertValue("true", FLAG_PATH); + } + + private static void selectChild(String child) { + // Force a selection change even when the previous selection is the same child. + selectInEditor(MODEL_FILE, "public"); + AadlPropertyViewBot.assertNoPropertiesShown(); + selectInEditor(MODEL_FILE, child); + AadlPropertyViewBot.tree(); + } + + private static void assertReadOnly(String... path) { + SWTBotTreeItem treeItem = item(path); + boolean greyedOut = UIThreadRunnable.syncExec(() -> treeItem.widget.getDisplay() + .getSystemColor(SWT.COLOR_DARK_GRAY) + .equals(treeItem.widget.getForeground(VALUE_COLUMN))); + assertTrue("A shared value must be greyed out", greyedOut); + assertFalse("Remove must be disabled for a shared value", + contextMenuEnablement(path, List.of(REMOVE)).get(REMOVE)); + } +} From 2c055d0102c6e609ed002318bf5dd0041c8c8a6b Mon Sep 17 00:00:00 2001 From: Lutz Wrage Date: Thu, 17 Sep 2026 13:14:53 -0400 Subject: [PATCH 2/2] Protect shared property values from removal Use one association-status calculation for the property status column and editability checks. Classify contained associations with multiple applies-to targets as LOCAL_SHARED so Remove and value editing remain disabled for their scalar and nested values. Update the existing context-menu characterization to require localization before removal. All seven Issue3235Test regressions pass, including sibling preservation after both localization actions. --- .../AadlPropertyViewContextMenuTest.java | 5 ++--- .../aadl2/ui/propertyview/AadlPropertyView.java | 17 ++++++----------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/core/org.osate.ui.tests/src/org/osate/ui/tests/propertyview/AadlPropertyViewContextMenuTest.java b/core/org.osate.ui.tests/src/org/osate/ui/tests/propertyview/AadlPropertyViewContextMenuTest.java index a3efcc6a2f6..fc7099ef0ac 100644 --- a/core/org.osate.ui.tests/src/org/osate/ui/tests/propertyview/AadlPropertyViewContextMenuTest.java +++ b/core/org.osate.ui.tests/src/org/osate/ui/tests/propertyview/AadlPropertyViewContextMenuTest.java @@ -141,11 +141,10 @@ public void testSharedLocalContainedPropertyOfASubcomponent() { /* * The association applies to more than one subcomponent, so there is no single association to open for this one. - * Remove is offered because the check behind it does not distinguish a shared association from one that applies - * to this subcomponent alone; this is the current behavior of the view, not necessarily the intended one. + * It must be made local or local contained before it can be removed without affecting the other subcomponents. */ assertContextMenuEnablement(new String[] { PROPERTY_SET, "Flag_Property" }, - List.of(OPEN_PROPERTY_DEFINITION, MAKE_LOCAL, MAKE_LOCAL_CONTAINED, REMOVE)); + List.of(OPEN_PROPERTY_DEFINITION, MAKE_LOCAL, MAKE_LOCAL_CONTAINED)); } @Test diff --git a/core/org.osate.xtext.aadl2.ui/src/org/osate/xtext/aadl2/ui/propertyview/AadlPropertyView.java b/core/org.osate.xtext.aadl2.ui/src/org/osate/xtext/aadl2/ui/propertyview/AadlPropertyView.java index 905cfca8c90..6a9e8311d7f 100644 --- a/core/org.osate.xtext.aadl2.ui/src/org/osate/xtext/aadl2/ui/propertyview/AadlPropertyView.java +++ b/core/org.osate.xtext.aadl2.ui/src/org/osate/xtext/aadl2/ui/propertyview/AadlPropertyView.java @@ -1187,15 +1187,7 @@ PropertyStatus getPropertyStatus(URI propertySetURI, URI propertyURI) { var associationURI = cachedPropertyAssociations.get(propertySetURI).get(propertyURI); if (associationURI != null) { var association = (PropertyAssociation) resourceSet.getEObject(associationURI, true); - var inputElement = resourceSet.getEObject(getInput(), true); - if (Objects.equals(inputElement, association.getOwner())) { - return PropertyStatus.LOCAL; - } - if (associationAppliesTo(association, inputElement)) { - return association.getAppliesTos().size() > 1 ? PropertyStatus.LOCAL_SHARED - : PropertyStatus.LOCAL_CONTAINED; - } - return PropertyStatus.INHERITED; + return getPropertyStatusNeverUndefined(resourceSet, association); } return ((Property) resourceSet.getEObject(propertyURI, true)).getDefaultValue() != null ? PropertyStatus.DEFAULT @@ -1217,8 +1209,11 @@ PropertyStatus getPropertyStatusNeverUndefined(ResourceSet resourceSet, Property if (Objects.equals(inputElement, association.getOwner())) { return PropertyStatus.LOCAL; } - return associationAppliesTo(association, inputElement) ? PropertyStatus.LOCAL_CONTAINED - : PropertyStatus.INHERITED; + if (associationAppliesTo(association, inputElement)) { + return association.getAppliesTos().size() > 1 ? PropertyStatus.LOCAL_SHARED + : PropertyStatus.LOCAL_CONTAINED; + } + return PropertyStatus.INHERITED; } private static boolean associationAppliesTo(PropertyAssociation association, EObject inputElement) {