Bugzilla – Full Text Bug Listing
| Summary: | Make GenericUnloader the default | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Vladimir Piskarev <vpiskarov> | ||||||
| Component: | Xtext | Assignee: | Project Inbox <tmf.xtext-inbox> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | enhancement | ||||||||
| Priority: | P3 | CC: | christian.dietrich.opensource, sebastian.zarnekow, sven.efftinge | ||||||
| Version: | 2.1.0 | ||||||||
| Target Milestone: | --- | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | v2.11 | ||||||||
| Attachments: |
|
||||||||
It is mainly because we wanted to avoid unnecessary computation. But we should indeed change that. I don't think this will help:
Resource A: entity A { b : B }
Resource B: entity B { a : A }
Resource A is edited and changed to:
Resource A: entity C { b : B }
This will cause the reference in resource B to become a proxy with a URI similar to //entities@0 which will resolve to entity C.
Since I don't see a reasonable, generic way to provide usable uri fragments for generic languages, I'd vote for another approach, e.g. track all resources in the resource set that is associated with the editor and collect the ones, that point back to the currently edited resource. If there are any, remove all referenced resources from the resource set as soon as the document is modified.
Note that referenced resource are usually not reloaded on validation since we use use the information from the index to decide about valid cross references. Only the resources will be loaded, that are explicitly used in the validator and their proxies are unlikely to be resolved, too. So probably this isn't a generic problem at all but a language specific one that should be solved by the language developer, e.g. by explicitly binding an unloader and a fragment provider?
I think it would help a bit, but given that it's not bullet proof, I'm also inclined to make it explicit by sticking to the NullImpl. All the cases should be super rare. What was the situation in which you came across the issue? I fully agree with Sebastian's arguments and the approach he suggested (automatically removing resources having back-references to the modified resource in the editor). As Sebastian pointed out, GenericUnloader alone can't really solve the described problem in a generic way, although it worked as a workaround in my specific circumstances. Indeed, it seems I should have requested a more generic solution instead of hastily suggesting GenericUnloader as the default. Having said that, I don't think that this problem is super rare. Consider validation for the presence of cycles, for example. Any language with inheritance support seems to be a use case. In my situation, I was trying to implement a custom resource description to achieve the following behavior regarding tracking changes in inheritance graph: * changing 'superclass' reference is a structural change - solved by storing superclass fully qualified name in 'userData'; * a class should depend on all of its superclasses, not just the direct one - solved by adding fully qualified names of all 'resolved' 'external' superclasses to 'importedNames'. In presence of cycles in inheritance graph, I was having unpleasant results with NullUnloader. With GenericUnloader, it seems to work fine. So, please feel free to close this request as invalid, but please consider implementing a generic solution to the issue, if possible. To make things more concrete, I took a little time to make up an example, emulating the situation I came across. Steps to reproduce: 1. Import the attached projects 'org.xtext.example.domainmodel' and 'org.xtext.example.domainmodel.ui' into the workspace. 2. Run the runtime workbench and import the attached 'example' project into the runtime workspace. 3. Close all editors. Open 'a.dmodel', 'b.dmodel' and 'c.dmodel'. 4. In 'a.model' delete 'a : A' property and then undo the deletion (Ctrl-Z). Notice the assertion error at line 80 of DomainmodelResourceDescriptionStrategy. 5. Uncomment bindIReferableElementsUnloader() method in DomainmodelRuntimeModule, relaunch the runtime workbench and repeat steps 3-4. Now it works (with GenericUnloader). Created attachment 203340 [details]
Sample language projects
Created attachment 203341 [details]
'example' project
not 2.1 was fixed via https://github.com/eclipse/xtext-core/issues/122 |
Build Identifier: Is there any good reason why not to bind GenericUnloader by default? There is at least one reason for the proposed default: GenericUnloader allows to preserve existing cycles in cross-resource reference graph after reparsing (e.g. in the editor). For example, if 'entity A { b : B }' and 'entity B { a : A }' are defined in different resources and 'A' is edited in Xtext editor (assuming both resources are loaded in the same resource set underlying the editor) the referenced 'A' in 'B' will be proxified by GenericUnloader and will later resolve to (new) 'A' again. Thus, the existing cycle between 'A' and 'B' is preserved. Whereas NullUnloader (the current default) will break the cycle and leave the old 'A' in a not-so-good state (detached from the resource). Reproducible: Always