Skip to content

Commit 282ef53

Browse files
gdanielgdaniel
committed
[1245] Uniformize read-only computation
Bug: #1245 Signed-off-by: Gwendal Daniel <gwendal.daniel@obeosoft.com>
1 parent ecbcade commit 282ef53

13 files changed

Lines changed: 155 additions & 289 deletions

File tree

‎CHANGELOG.adoc‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ Any `Namespace` stored in those libraries are now cached for quick access.
5353
As a consequence, some coding rules violations have been fixed.
5454
The `org.eclipse.syson.services.grammars` package in `syson-direct-edit-grammar` has been renamed into `org.eclipse.syson.direct.edit.grammars`.
5555
`AbtractItemUsageBorderNodeDescriptionProvider` has been renamed to `AbstractItemUsageBorderNodeDescriptionProvider`.
56+
- https://github.com/eclipse-syson/syson/issues/1245[#1245] [syson] Uniformize read-only computation.
57+
The class `SysMLReadOnlyService` and the interface `ISysMLReadOnlyService` have been removed, use `IReadOnlyObjectPredicate` instead.
5658

5759
=== Dependency update
5860

@@ -87,6 +89,13 @@ Consumers may override this by providing an implementation of `org.eclipse.syson
8789
With this enhancement, users can now easily identify which `ViewDefinition` is used by a `ViewUsage`.
8890
- https://github.com/eclipse-syson/syson/issues/1535[#1535] [explorer] Since it is now possible to switch from a _Standard Diagram View_ to another in SysON (for example from _General View_ to _Interconnection View_), the `ViewUsages` default name does not contain the name of the `ViewDefinition` anymore.
8991
With this enhancement, users avoid confusion when switching from one _ViewDefinition_ to another one in a `ViewUsage`.
92+
- https://github.com/eclipse-syson/syson/issues/1245[#1245] [syson] Uniformize read-only computation.
93+
We removed the assumptions SysON made on whether a resource is read-only.
94+
Resources are now considered read-only if:
95+
* They are Sirius Web libraries imported by reference
96+
* They are textual SysML files imported as read-only
97+
* They are standard libraries (SysML and KerML)
98+
All the other resources are read-write.
9099

91100
=== New features
92101

‎backend/application/syson-application-configuration/src/main/java/org/eclipse/syson/application/services/SysONReadOnlyObjectPredicateDelegate.java‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,11 @@
1212
*******************************************************************************/
1313
package org.eclipse.syson.application.services;
1414

15-
import java.util.Objects;
16-
1715
import org.eclipse.emf.ecore.EAnnotation;
18-
import org.eclipse.emf.ecore.EObject;
1916
import org.eclipse.emf.ecore.resource.Resource;
2017
import org.eclipse.sirius.components.core.api.IReadOnlyObjectPredicate;
2118
import org.eclipse.sirius.components.core.api.IReadOnlyObjectPredicateDelegate;
22-
import org.eclipse.syson.services.UtilService;
23-
import org.eclipse.syson.services.api.ISysONResourceService;
19+
import org.eclipse.sirius.components.emf.ResourceMetadataAdapter;
2420
import org.eclipse.syson.sysml.Element;
2521
import org.eclipse.syson.sysml.util.ElementUtil;
2622
import org.springframework.stereotype.Service;
@@ -42,12 +38,6 @@
4238
@Service
4339
public class SysONReadOnlyObjectPredicateDelegate implements IReadOnlyObjectPredicateDelegate {
4440

45-
private final ISysONResourceService sysONResourceService;
46-
47-
public SysONReadOnlyObjectPredicateDelegate(final ISysONResourceService sysONResourceService) {
48-
this.sysONResourceService = Objects.requireNonNull(sysONResourceService);
49-
}
50-
5141
@Override
5242
public boolean canHandle(Object candidate) {
5343
return candidate instanceof Element || candidate instanceof Resource || candidate instanceof EAnnotation;
@@ -60,12 +50,18 @@ public boolean test(Object candidate) {
6050

6151
if (candidate instanceof Resource resource) {
6252
isReadOnly = ElementUtil.isStandardLibraryResource(resource)
63-
|| this.sysONResourceService.isImported(resource) && !new UtilService().getLibraries(resource, false).isEmpty();
64-
} else if (candidate instanceof Element || candidate instanceof EAnnotation) {
65-
// Derive editability from the editability of the containing resource.
66-
final EObject eObject = (EObject) candidate;
67-
final Resource resource = eObject.eResource();
68-
isReadOnly = this.test(resource);
53+
|| resource.eAdapters().stream()
54+
.filter(ResourceMetadataAdapter.class::isInstance)
55+
.map(ResourceMetadataAdapter.class::cast)
56+
.map(ResourceMetadataAdapter::isReadOnly)
57+
.findFirst()
58+
.orElse(false);
59+
} else if (candidate instanceof Element element) {
60+
// An element is read-only if it is contained in a standard LibraryPackage, regardless of whether its
61+
// containing resource is read-only or not.
62+
isReadOnly = this.test(element.eResource()) || ElementUtil.isFromStandardLibrary(element);
63+
} else if (candidate instanceof EAnnotation eAnnotation) {
64+
isReadOnly = this.test(eAnnotation.eResource());
6965
}
7066

7167
return isReadOnly;

‎backend/application/syson-application-configuration/src/test/java/org/eclipse/syson/application/services/DetailsViewServiceTest.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.eclipse.emf.edit.provider.ComposedAdapterFactory;
2222
import org.eclipse.sirius.components.core.api.IFeedbackMessageService;
2323
import org.eclipse.sirius.components.core.services.ComposedReadOnlyObjectPredicate;
24+
import org.eclipse.sirius.components.emf.ResourceMetadataAdapter;
2425
import org.eclipse.sirius.components.emf.services.JSONResourceFactory;
2526
import org.eclipse.sirius.web.application.object.services.DefaultReadOnlyObjectPredicate;
2627
import org.eclipse.syson.sysml.LibraryPackage;
@@ -46,7 +47,7 @@ public class DetailsViewServiceTest {
4647
public void setUp() {
4748
// Use a dummy CompsedAdapterFactory, we don't test methods that require the one used by SysON for the moment.
4849
this.detailsViewService = new DetailsViewService(new ComposedAdapterFactory(), new IFeedbackMessageService.NoOp(),
49-
new ComposedReadOnlyObjectPredicate(List.of(new SysONReadOnlyObjectPredicateDelegate(new SysONResourceService())), new DefaultReadOnlyObjectPredicate()));
50+
new ComposedReadOnlyObjectPredicate(List.of(new SysONReadOnlyObjectPredicateDelegate()), new DefaultReadOnlyObjectPredicate()));
5051
}
5152

5253
@Test
@@ -95,6 +96,20 @@ public void isReadOnlyElementInImportedLibrary() {
9596
namespace.getOwnedRelationship().add(owningMembership);
9697
owningMembership.getOwnedRelatedElement().add(libraryPackage);
9798
ElementUtil.setIsImported(resource, true);
99+
assertThat(this.detailsViewService.isReadOnly(libraryPackage)).isFalse();
100+
}
101+
102+
@Test
103+
public void isReadOnlyElementInImportedLibraryFlaggedAsReadOnly() {
104+
Resource resource = new JSONResourceFactory().createResourceFromPath("testResource");
105+
Namespace namespace = SysmlFactory.eINSTANCE.createNamespace();
106+
resource.getContents().add(namespace);
107+
LibraryPackage libraryPackage = SysmlFactory.eINSTANCE.createLibraryPackage();
108+
OwningMembership owningMembership = SysmlFactory.eINSTANCE.createOwningMembership();
109+
namespace.getOwnedRelationship().add(owningMembership);
110+
owningMembership.getOwnedRelatedElement().add(libraryPackage);
111+
ElementUtil.setIsImported(resource, true);
112+
resource.eAdapters().add(new ResourceMetadataAdapter("test", true));
98113
assertThat(this.detailsViewService.isReadOnly(libraryPackage)).isTrue();
99114
}
100115

‎backend/application/syson-application-configuration/src/test/java/org/eclipse/syson/application/services/SysONReadOnlyObjectPredicateDelegateTests.java‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.eclipse.emf.ecore.util.EcoreUtil;
2727
import org.eclipse.sirius.components.core.api.IReadOnlyObjectPredicateDelegate;
2828
import org.eclipse.sirius.components.core.services.ComposedReadOnlyObjectPredicate;
29+
import org.eclipse.sirius.components.emf.ResourceMetadataAdapter;
2930
import org.eclipse.sirius.components.emf.services.api.IEMFEditingContext;
3031
import org.eclipse.sirius.web.application.library.services.LibraryMetadataAdapter;
3132
import org.eclipse.syson.application.configuration.SysONDefaultLibrariesConfiguration;
@@ -57,7 +58,7 @@
5758
*/
5859
public class SysONReadOnlyObjectPredicateDelegateTests {
5960

60-
private final IReadOnlyObjectPredicateDelegate readOnlyObjectPredicateDelegate = new SysONReadOnlyObjectPredicateDelegate(new SysONResourceService());
61+
private final IReadOnlyObjectPredicateDelegate readOnlyObjectPredicateDelegate = new SysONReadOnlyObjectPredicateDelegate();
6162

6263
@SafeVarargs
6364
private static Resource createResource(final URI uri, final Consumer<Resource>... postTreatments) {
@@ -282,6 +283,8 @@ public class ImportedResources {
282283

283284
private final Resource importedResourceWithSysmlLibraryPackage;
284285

286+
private final Resource importedResourceWithSysmlLibraryPackageAndFlaggedAsReadOnly;
287+
285288
private final Resource importedResourceWithSysmlMixedPackages;
286289

287290
public ImportedResources() {
@@ -295,6 +298,13 @@ public ImportedResources() {
295298
ElementUtil.setIsImported(resource, true);
296299
});
297300

301+
this.importedResourceWithSysmlLibraryPackageAndFlaggedAsReadOnly = createResource(createEmfUri(), resource -> {
302+
resource.getContents().addAll(EcoreUtil.copyAll(SysMLResources.this.resourceWithSysmlLibraryPackage.getContents()));
303+
ElementUtil.setIsImported(resource, true);
304+
ResourceMetadataAdapter resourceMetadataAdapter = new ResourceMetadataAdapter("test", true);
305+
resource.eAdapters().add(resourceMetadataAdapter);
306+
});
307+
298308
this.importedResourceWithSysmlMixedPackages = createResource(createEmfUri(), resource -> {
299309
resource.getContents().addAll(EcoreUtil.copyAll(SysMLResources.this.resourceWithSysmlMixedPackages.getContents()));
300310
ElementUtil.setIsImported(resource, true);
@@ -308,15 +318,21 @@ public void testResourceWithSysmlPackageImported() {
308318
}
309319

310320
@Test
311-
@DisplayName("Imported resource containing LibraryPackage is read-only (both the resource and all of its contents)")
321+
@DisplayName("Imported resource containing LibraryPackage is not read-only (both the resource and all of its contents)")
312322
public void testResourceWithSysmlLibraryPackageImported() {
313-
SysONReadOnlyObjectPredicateDelegateTests.this.assertResourceAndAllContentsIsReadOnly(this.importedResourceWithSysmlLibraryPackage, true);
323+
SysONReadOnlyObjectPredicateDelegateTests.this.assertResourceAndAllContentsIsReadOnly(this.importedResourceWithSysmlLibraryPackage, false);
324+
}
325+
326+
@Test
327+
@DisplayName("Imported resource containing LibraryPackage and flagged as read-only is read-only (both the resource and all of its contents)")
328+
public void testResourceWithSysmlLibraryPackageImportedAndFlaggedAsReadOnly() {
329+
SysONReadOnlyObjectPredicateDelegateTests.this.assertResourceAndAllContentsIsReadOnly(this.importedResourceWithSysmlLibraryPackageAndFlaggedAsReadOnly, true);
314330
}
315331

316332
@Test
317-
@DisplayName("Imported resource containing Package and LibraryPackage is read-only (both the resource and all of its contents)")
333+
@DisplayName("Imported resource containing Package and LibraryPackage is not read-only (both the resource and all of its contents)")
318334
public void testResourceWithSysmlMixedPackagesImported() {
319-
SysONReadOnlyObjectPredicateDelegateTests.this.assertResourceAndAllContentsIsReadOnly(this.importedResourceWithSysmlMixedPackages, true);
335+
SysONReadOnlyObjectPredicateDelegateTests.this.assertResourceAndAllContentsIsReadOnly(this.importedResourceWithSysmlMixedPackages, false);
320336
}
321337
}
322338

@@ -339,26 +355,26 @@ public class ReferencedResources {
339355
public ReferencedResources() {
340356
this.referencedResourceWithSysmlPackage = createResource(createEmfUri(), resource -> {
341357
resource.getContents().addAll(EcoreUtil.copyAll(SysMLResources.this.resourceWithSysmlPackage.getContents()));
342-
}, resource -> resource.eAdapters().add(createLibraryMetadataAdapter()));
358+
}, ReferencedResources::addReferencedResourceAdapters);
343359

344360
this.referencedResourceWithSysmlLibraryPackage = createResource(createEmfUri(), resource -> {
345361
resource.getContents().addAll(EcoreUtil.copyAll(SysMLResources.this.resourceWithSysmlLibraryPackage.getContents()));
346-
}, resource -> resource.eAdapters().add(createLibraryMetadataAdapter()));
362+
}, ReferencedResources::addReferencedResourceAdapters);
347363

348364
this.referencedResourceWithSysmlMixedPackages = createResource(createEmfUri(), resource -> {
349365
resource.getContents().addAll(EcoreUtil.copyAll(SysMLResources.this.resourceWithSysmlMixedPackages.getContents()));
350-
}, resource -> resource.eAdapters().add(createLibraryMetadataAdapter()));
366+
}, ReferencedResources::addReferencedResourceAdapters);
351367
}
352368

353-
private static LibraryMetadataAdapter createLibraryMetadataAdapter() {
354-
return new LibraryMetadataAdapter("namespace", "name", "version");
369+
private static void addReferencedResourceAdapters(Resource resource) {
370+
resource.eAdapters().add(new LibraryMetadataAdapter("namespace", "name", "version"));
371+
resource.eAdapters().add(new ResourceMetadataAdapter("test", true));
355372
}
356373

357374
@Test
358375
@DisplayName("Resource from referenced library containing Package is read-only (both the resource and all of its contents)")
359376
public void testResourceWithSysmlPackageImported() {
360-
// Note that this might be a bug, see https://github.com/eclipse-syson/syson/issues/1342.
361-
SysONReadOnlyObjectPredicateDelegateTests.this.assertResourceAndAllContentsIsReadOnly(this.referencedResourceWithSysmlPackage, false);
377+
SysONReadOnlyObjectPredicateDelegateTests.this.assertResourceAndAllContentsIsReadOnly(this.referencedResourceWithSysmlPackage, true);
362378
}
363379

364380
@Test

‎backend/application/syson-application/src/test/java/org/eclipse/syson/application/controller/explorer/view/SysONExplorerTests.java‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,52 @@ public void getExplorerContentWithDefaultFilters() {
236236

237237
}
238238

239+
@DisplayName("GIVEN an empty SysML Project, WHEN the explorer is displayed with KerML and SysML libraries expanded, THEN the library models are visible")
240+
@Sql(scripts = { GeneralViewEmptyTestProjectData.SCRIPT_PATH }, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED))
241+
@Sql(scripts = { "/scripts/cleanup.sql" }, executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED))
242+
@Test
243+
public void getExplorerContentWithKerMLAndSysMLExpanded() {
244+
String librariesTreeItemId = UUID.nameUUIDFromBytes("SysON_Libraries_Directory".getBytes()).toString();
245+
String sysmlLibrariesTreeItemId = UUID.nameUUIDFromBytes("SysON_SysML_Directory".getBytes()).toString();
246+
String kermlLibrariesTreeItemId = UUID.nameUUIDFromBytes("SysON_KerML_Directory".getBytes()).toString();
247+
var explorerRepresentationId = this.representationIdBuilder.buildExplorerRepresentationId(this.treeDescriptionId,
248+
List.of(librariesTreeItemId, sysmlLibrariesTreeItemId, kermlLibrariesTreeItemId), this.defaultFilters);
249+
var input = new ExplorerEventInput(UUID.randomUUID(), GeneralViewEmptyTestProjectData.EDITING_CONTEXT, explorerRepresentationId);
250+
var flux = this.explorerEventSubscriptionRunner.run(input);
251+
this.givenCommittedTransaction.commit();
252+
253+
var initialExplorerContentConsumer = assertRefreshedTreeThat(tree -> {
254+
assertThat(tree).isNotNull();
255+
assertThat(tree.getChildren()).hasSize(2);
256+
TreeItem librariesDirectory = tree.getChildren().get(1);
257+
assertThat(librariesDirectory.getLabel().toString()).isEqualTo("Libraries");
258+
assertThat(librariesDirectory.isDeletable()).isFalse();
259+
assertThat(librariesDirectory.isEditable()).isFalse();
260+
assertThat(librariesDirectory.isSelectable()).isTrue();
261+
assertThat(librariesDirectory.isHasChildren()).isTrue();
262+
assertThat(librariesDirectory.getChildren()).hasSize(2);
263+
TreeItem kermlDirectory = librariesDirectory.getChildren().get(0);
264+
assertThat(kermlDirectory.getLabel().toString()).isEqualTo("KerML");
265+
assertThat(kermlDirectory.isDeletable()).isFalse();
266+
assertThat(kermlDirectory.isEditable()).isFalse();
267+
assertThat(kermlDirectory.isSelectable()).isTrue();
268+
assertThat(kermlDirectory.isHasChildren()).isTrue();
269+
assertThat(kermlDirectory.getChildren()).allMatch(children -> !children.isDeletable() && !children.isEditable());
270+
TreeItem sysmlDirectory = librariesDirectory.getChildren().get(1);
271+
assertThat(sysmlDirectory.getLabel().toString()).isEqualTo("SysML");
272+
assertThat(sysmlDirectory.isDeletable()).isFalse();
273+
assertThat(sysmlDirectory.isEditable()).isFalse();
274+
assertThat(sysmlDirectory.isSelectable()).isTrue();
275+
assertThat(sysmlDirectory.isHasChildren()).isTrue();
276+
assertThat(sysmlDirectory.getChildren()).allMatch(children -> !children.isDeletable() && !children.isEditable());
277+
});
278+
279+
StepVerifier.create(flux)
280+
.consumeNextWith(initialExplorerContentConsumer)
281+
.thenCancel()
282+
.verify(Duration.ofSeconds(10));
283+
}
284+
239285
@DisplayName("GIVEN an empty SysML Project, WHEN the explorer is displayed with its root model expanded and the hide memberships and hide KerML libraries filters, THEN the root model is visible and is expanded")
240286
@Sql(scripts = { GeneralViewEmptyTestProjectData.SCRIPT_PATH }, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED))
241287
@Sql(scripts = { "/scripts/cleanup.sql" }, executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED))

0 commit comments

Comments
 (0)