Bugzilla – Full Text Bug Listing
| Summary: | [api] provide access to instance of IRepositoryManager through core API | ||
|---|---|---|---|
| Product: | z_Archived | Reporter: | Mirko Raner <mirko> |
| Component: | Mylyn | Assignee: | Steffen Pingel <steffen.pingel> |
| Status: | CLOSED MOVED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | steffen.pingel |
| Version: | unspecified | Keywords: | plan |
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 242297 | ||
|
Description
Mirko Raner
The repository manager is only instantiated once by tasks.ui but nothing in the design of the TaskRepostioryManager class should prevent multiple instances to co-exist, e.g. for headless applications. Connector cores are intentionally designed as state-less singletons and are not expected to hold on to instances of model elements. I understand that this can be limiting and makes method signatures long since all state it passed on each invocation. Can you describe why it is necessary for your connector core to access the repository manager to help us understand the use case and potentially reconsider this design? This problem came up in the context of another API issue (for which I was planning to file a separate bug, but maybe it's all part of the same bigger problem). So here goes... Generally, AbstractRepositoryConnector assumes that certain repository operations can be done merely by manipulating URLs - without accessing the server. For example, if I know the server URL "https://bugs.eclipse.org" and the bug number 316253, I can construct the address "https://bugs.eclipse.org/bugs/show_bug.cgi?id=316253" to access the bug's description page. For many tracking systems (Bugzilla, JIRA, etc.) this is a safe assumption to make. However, it is not universally true for all tracking systems. When we started implementing the connector for TeamTrack (bug 167670) we ran into trouble because of this assumption. URLs for individual TeamTrack issues cannot simply be constructed from the base URL and the issue ID. The URL for an individual issue page is constructed using internal table IDs that can be readily obtained from the server but cannot be inferred from the public issue ID. For example, if the server's base address is https://teamtrack.intuit.com/tmtrack/tmtrack.dll and a valid issue ID is TWE010375 then the URL of the issue itself would be https://teamtrack.intuit.com/tmtrack/tmtrack.dll?IssuePage&RecordId=786573&TableId=1011 - which cannot be directly constructed. This leads to problems when implementing methods such as AbstractRepositoryConnector.getTaskUrl(String, String). According to the Mylyn documentation, the method should not be performing any network access, because only methods that have an IProgressMonitor parameter should do so. The TeamTrack connector violates this rule, essentially because there is no alternative. The TeamTrack connector's getTaskUrl(String, String) method is one of the client methods that need to establish a server session, for which it needs a TaskRepository (because of the login credentials). As only the URL of the repository is known (but not the underlying TaskRepository object), it must be obtained from the IRepositoryManager. AbstractRepositoryConnector.getTaskIdFromTaskUrl(String) suffers from the same problem. I'm just starting to wrap my head around the Mylyn API (which I find pretty hard due to an overall lack of documentation), so I don't have many constructive suggestions how to solve this problem. It might be possible to address the problem by adding the following methods to AbstractRepositoryConnector: - boolean canCreateTaskUrlFromKey(TaskRepository) - String getTaskUrl(TaskRepository, String, IProgressMonitor) - String getTaskIdFromTaskUrl(TaskRepository, String, IProgressMonitor) If the first method returns false URLs and IDs are obtained via the two other new methods instead of their existing String-based counterparts. Please let me know if I'm barking up the wrong tree here, but so far we haven't been able to connect to TeamTrack without violating the existing API rules. Has someone had a chance to look into this? If our TeamTrack connector is not using the Mylyn API properly we'd appreciate it if you could give us a hint how we should fix this problem in our code. In case this is a true shortcoming of the current API we are also happy to collaborate on coming up with a modified API that accommodates the needs of TeamTrack and similar issue tracking systems. Thanks! I would need to look into this in more detail but you should be able to implement a fully functional connector without providing actual implementation for getTaskUrl() and getTaskIdFromTaskUrl(), i.e. simply returning null should work if the connector does not support this type of mapping. Please let me know if that causes unexpected loss of functionality and we can investigate how to fix those specific cases. I'll add some documentation to clarify that. Generally, doing I/O in methods that do not have access to a progress monitor is almost always guaranteed to have a negative impact on responsiveness of the UI and should always be avoided. Please let us know if there are other parts of the API where you encountered this problem and we can discuss adding a long running equivalent of a method or changing the design in other ways. Here is a work in progress change that adds a new o.e.m.tasks.service bundle that exposes the "default" data model instance, i.e. the one used by the UI through a service: https://git.eclipse.org/r/#/c/8399/. Here is a first round of incremental changes to move some of the required functionality into the core: https://git.eclipse.org/r/#/c/23861/ - move task activation externalization to core https://git.eclipse.org/r/#/c/23862/ - move loading of task activity extension into core https://git.eclipse.org/r/#/c/23863/ - move connector extension loading into core Additional reviews: https://git.eclipse.org/r/#/c/23879/ - move core initialization into tasks.core https://git.eclipse.org/r/#/c/23880/ - add bundles for tasks service https://git.eclipse.org/r/#/c/8399/ - expose tasks service I'm a little surprised that you want to move the instantiation of the various managers into the core, rather than allowing different implementations to be created by the UI. It seems simpler this way, but what happened to the idea of having a separate bundle for them, outside of the core? I guess I should have looked at all the reviews before commenting. Still, why not instantiate all the managers in the service bundle? I wonder if this will make the issues with sensitivity to bundle loading order better or worse. (In reply to comment #9) > I guess I should have looked at all the reviews before commenting. Still, why > not instantiate all the managers in the service bundle? The "default" instance that is used by the UI will be managed in the services bundle. At the same time I want to allow tests and other client to create independent instance of the model. I wouldn't want to force an additional dependency on the services bundle to do that since all other classes are in tasks.core only. (In reply to comment #10) > I wonder if this will make the issues with sensitivity to bundle loading order > better or worse. It shouldn't make things any worse or better since the services bundle is activated from the UI. We should look at decoupling stuff more if interactions from other bundles using the services instance causes unexpected side effects. Mylyn has been restructured, and our issue tracking has moved to GitHub [1]. We are closing ~14K Bugzilla issues to give the new team a fresh start. If you feel that this issue is still relevant, please create a new one on GitHub. [1] https://github.com/orgs/eclipse-mylyn |