Bugzilla – Full Text Bug Listing
| Summary: | Generator support required for platform: | ||
|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Ed Willink <ed> |
| Component: | Tools | Assignee: | Dave Steinberg <davidms> |
| Status: | RESOLVED INVALID | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | Ed.Merks |
| Version: | 2.4.0 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows Vista | ||
| Whiteboard: | |||
|
Description
Ed Willink
It's all working correctly as designed. There's nothing wrong with using .. to walk up to the top of the platform path; it might not be what you want, but that's not the same as it being an error. As long as deresolve and a subsequent resolve produce the original URI, then it's correct. One could just as well open a bug against the platform arguing that they should have used platform://resource to ensure that the "resource" represents the authority rather than the first segment of a path, but that's not likely to change either. Then it would be indeed be wrong to use ".." to walk that high, but of course URI.deresolve wouldn't do something wrong in that case; it would respect the authority as not supporting "..".. The option you describe could do absolutely anything depending on what you pass to it. There are plenty of references other related methods, like URI.resolve/deresolve and to API that's used to specialize the behavior. I'm not sure what the content type comment at the end has to do with the subject of resolving URIs... In any case, there are way too many options to start adding support for them in the generator. It's easy to specialize the resource factory to achieve any desired behavior. > As long as deresolve and a subsequent resolve produce the original URI, then it's correct. They don't. That's why I had to debug it. The proxy reference to platform:/plugin/... remained unresolved when XMLSave started but got translated into a relative reference to the platform:/resource/... by the invocation of desrolve with anyRelPath true. I set about producing a simple repro for you using just the Sample Ecore Editor and found that it worked just fine. That's how I found the lack of tool support for URIHandlerImpl.PlatformSchemeAware. But I agree completely, it should have gone back to where it started. However no-platform awareness makes the ../../../plugin inevitable; for http: for instance it would be correct. But if the tooling is not platform aware, it should not be able to resolve platform:/plugin at all. Since it can, it is inconsistently platform aware. If this isn't fixed, it appears that as with Bug #219742, every generated editor that can use platform: must be manually repaired. Not fixing Bug #219742 required at least 2 fixes to GMF alone via Bug #220711 with a suspicion of 3 further outstanding bugs. How many other editors remain broken? --- My comment about content types was just identifying another behaviour that should be realised by a generated editor that fully exploits the Eclipse and EMF infrastructure. Tooling that generated such an editor would avoid developers needing to do so much incremental bug discovery. Yet another behaviour, that is perhaps the root of my problem, is using new EcoreActionBarContributor.ExtendedLoadResourceAction() in the EditorActionBarContributor(). [If there are so many options, then let GenModel support them, or at least a wiki expose them.] Ed, My problem is that your assertions about the behavior being wrong or not working aren't associated with a test case that demonstrates it. I'm well aware that until the new options was added to support PlatformSchemeAware, that we ended up with relative references up to platform, but those worked properly, i.e., they were never broken but simply weren't in the most desired form. I noticed in your original description you talked about "../../../plugin/..." and "../../../plugins/..." but it wasn't clear if that was a typo or part of the point. Whatever value is appearing is based on the URI in your scenario, and only the former is correct. I don't believe the framework injects the latter. So I remain at a loss to understand what you are asserting is actually broken. It doesn't help when you digress into a different unrelated topic. There is already support for specifying content types on the generated editor. You can define the "Content Type Identifier" on your GenPackage and you can define the comma separated list of extensions for files that should be considered as possibly containing your content typed. Now you bring up the issue of loading packages again. Something that almost all editors do not need and would in fact be confusing. Of course it's completed unrelated to either of the two preceding issues. The wiki is there for all to help support. Feel free to add recipes or FAQ questions. When reporting problems though, try to focus on one at a time, and if you are sure something is physically broken, include a test case so we're on the same page. In the absence of that, I'll make assumptions, and that assumption is that deresolve will always produce something that resolve will turn back into the original result. It would be easy to prove this wrong with a two line test case involving just the URI class. My original report made clear that I had tried to produce a simple repro, but found that the Sample Ecore Editor worked just fine. Having spent quite a few hours debugging the problem, I had identified that the problem was that URI.deresolve was creating a relative path that was incorrect only from a schema-aware perspective. I also identified that the problem was fixed in the Ecore editor by use of URIHandlerImpl.PlatformSchemeAware and that the same usage fixed my problem too. This seems more than enough foundation for a Bugzilla; a) to document the problem for others to find; b) to identify the problem to the developers. My specific Bugzilla title is that EMF is not an honest broker with regard to platform: support. There should not be special functionality fixes in EMF that are not obviously available to other projects. I therefore mentioned other topics on this theme. Content type is certainly supported by GenModel and that is very useful, but for instance, MDT OCL 1.2.0 does not have a content type because nothing gave MDT OCL a warning that it was not properly complying with good practice. I found that using URIHandlerImpl.PlatformSchemeAware necessitates a custom ResourceFactory, quite possibly an unedited EcoreResourceFactoryImpl and that registration of this mandates a content type registration. So if my earlier suggestion of an '"XMI with platform: support" GenModel type is reconsidered, a better spelling would be 'Ecore' so that you get a custom ResourceFactory extending EcoreResourceFactoryImpl rather than XMIResourceFactoryImpl. Let's be clear here that this is not about correctness but rather about style preferences. The relative form is correct and works correctly. It worked that way for many years and it does not seem wise to change the default behavior for all users. It's generally a nit as well because most users will not be loaded resources from the installed environment. Instead of all this emotionally charged verbiage, i.e., "not an honest broker" it seems best to focus on a single issue. Now you're talking about OCL, which is yet more digression. The bottom line is that there are way too many options to start providing Generator support for each. Specializing the resource factory is simple. It's simple; once you know what the problem is. In practice, my colleague found a broken file. I hand edited to correct, and continued to hand edit for a bit, before determining to fix the bug. I get very depressed at the amount of time I spend locating bugs, diagnosing them and reporting them to reduce the likelihood of others also wasting time on the same problems, only to get a WORKSFORME, when blatantly the save of platform:/plugin/... as ../../../plugin/... does not satisfy your own comment #1 definition of 'correct'. As before, I'll raise bugs against GMF and QVT OML, so that they at least are aware of their problems. I believe a test case is worth 1000 words. Since you feel it's not necessary to write one because it's all so obviously wrong, let me explain how I interpret your statement. Specifically, I interpret what you describe as broken to be the same as as claiming that the final print statement below prints false:
URI platformResourceURI = URI.createPlatformResourceURI("project/file.extension", true);
URI platformPluginURI = URI.createPlatformPluginURI("plugin.id/file2.extension", true);
URI relativeURI = platformPluginURI.deresolve(platformResourceURI, true, true, false);
System.out.println("###" + relativeURI);
System.out.println("###" + relativeURI.resolve(platformResourceURI));
System.out.println("###" + relativeURI.resolve(platformResourceURI).equals(platformPluginURI));
It prints this
###../../plugin/plugin.id/file2.extension
###platform:/plugin/plugin.id/file2.extension
###true
Because it prints true, I believe that the relative path resolves correctly. There might well be a gap between what you tried to explain and what I actually understood, but all the talk about the hours spent, the lack of honest brokering, and the blatantly broken behavior along with digressions into content types, what OCL does or doesn't do, and how we should rename our resource factories to be Ecore and not XMI doesn't get us one iota closer to me understanding your specific claims.
Foolish me; jumping to the conclusion that because the Sample Ecore Editor had code that fixed my problem, that this fixed the problem; it just hides it. Your test case is exactly right. The subtlety is how you view the result. The ../.. is equivalent when generated for and then used by a platform: path. It is different if the referencing resource is subsequently opened using a file: path. So use of URIHandlerImpl.PlatformSchemeAware provides useful rather than essential flexibility. Opening platform: resources via file: paths is so stupid that this Bugzilla must merit an INVALID. Sorry for the confusion. |