Community
Participate
Working Groups
I20090407 Currently the Available Software Sites page doesn't do any kind of validation on the location when the user adds a new repository. I think we should be doing some lightweight validation here, such as calling IMetadataRepositoryManager#validateRepositoryLocation.
This is the 2009 version of bug bug #204184. We added those repo manager validation methods to support that bug, and then stopped using them in bug #225062. The intention was that there was simple validation that could be done in the UI, but we ended up with deadlock in certain cases. Of course, a lot has changed in the ECF/p2 transport world since this occurred, so it's possible we could revisit this...
It is safe to call IMetadataRepositoryManager#validateRepositoryLocation - it just validates p2 constraints on the URI (can not be relative, etc.) and it does not perform any actual communication.
The issue here is that we would need to document more specific behavior for what factories can and cannot do in the validate method. For example, an UpdateSiteMetadataRepositoryFactory wants to see if there is a "site.xml" available. So it tries to see if the file exists, which can timeout, etc.... I think we determined finally that the factories couldn't do any useful validation without contacting the network. If we want to redefine the validation methods where they are workable for the UI, I'm all for it. In the meantime I think there is another bug for allowing the user to automatically remove the URI from the manager if it's not correct. Allowing them to do this, or even correct, the URI might be a big win.
(In reply to comment #2) > It is safe to call IMetadataRepositoryManager#validateRepositoryLocation - it > just validates p2 constraints on the URI (can not be relative, etc.) and it > does not perform any actual communication. > Just mid-air collided. I still don't think it's safe to call this method in the UI. For example, in UpdateSite (called by the update site metadata factory in the validate method)... public static void validate(URI url, IProgressMonitor monitor) throws UserCancelledException, AuthenticationFailedException, FileNotFoundException, CoreException { URI siteURI = getSiteURI(url); long lastModified = getTransport().getLastModified(siteURI, monitor);
There should really be just one place where the validity of the URI as such is checked - i.e. a validation of the pointer as opposed to a validation of whatever it is assumed to point to. The next level of checking is to test if the ECF configuration has a factory for the scheme. Until dependencies on such configuration is captured as meta (install time) requirements some interesting issues can be created - if a repo is used using some non standard protocol, and then the support for this protocol is uninstalled for instance. so a) Is it a valid reference to "something" b) Is the system configured to communicate with that "something" (i.e. can ECF be used to communicate) c) What type of thing is at the location (p2, update site, etc.) d) Is that thing valid (can it be "loaded") I think the repository bundle should have a test that covers a) and b). Locations must always pass test a). Test b) must be passed if trying to communicate. c) and d) are already handled in how p2 currently figures out what the URI is a pointer to.
removing milestone and changing title. This is a repo-manager/repo factory level issue. We need staged validation as Henrik suggests, with calls that the UI can safely make in the UI thread. Right now there are factories that are doing real work in validation so the UI can't do any better than simple URL validation and other client-side tricks. The client-side tricks are documented in their own bugs (ie, comparing trailing slashes, defaulting to a scheme, etc...)
Bug 272530 is about have a method the UI can call to validate the URI entered by a user. i.e. it validates the cases a) and b) described above.
We will have to revisit this next release. Our current state is that we have a validation API method that contacts repositories, and is therefore not used at all. If this isn't proving useful, then we should switch this method to be a short-running validation that the UI can call as the user is typing (no progress monitor, no blocking). It looks like the "middle-weight" validation that contacts repositories but does not load all the contents is not particularly useful (since merely contacting the repository can take a long time).
(In reply to comment #8) > We will have to revisit this next release. Our current state is that we have a > validation API method that contacts repositories, and is therefore not used at > all. If this isn't proving useful, then we should switch this method to be a > short-running validation that the UI can call as the user is typing (no > progress monitor, no blocking). It looks like the "middle-weight" validation > that contacts repositories but does not load all the contents is not > particularly useful (since merely contacting the repository can take a long > time). > Agree.
FWIW, at least the repo manager validation methods correctly document the possible performance problems...the fact that a load attempt might be made. I suspect that for 3.6/4.0, we can remove the middleweight methods...
*** Bug 272940 has been marked as a duplicate of this bug. ***
Another aspect of validation is doing URI equivalence testing WRT trailing slashes. The repository manager or factories may be able to determine whether a trailing slash is significant for a given repository location.
It crossed my mind that the CVS UI has a nice validation approach. The "add location" wizard has a checkbox in their case: "Validate connection on finish". This allows the user to choose not to validate if they are disconnected, on a slow network, etc. But it allows validation to happen if the user is not sure that they location they entered is correct.
(In reply to comment #13) > It crossed my mind that the CVS UI has a nice validation approach. The "add > location" wizard has a checkbox in their case: "Validate connection on finish". > This allows the user to choose not to validate if they are disconnected, on a > slow network, etc. But it allows validation to happen if the user is not sure > that they location they entered is correct. > That requires that it is possible to remember repositories without actually loading them. Which I think would be an improvement, but I don't think the repository manager currently is capable of doing that. The way I think it should work is that a syntactically valid URI can be added. It will at some point (either on demand: "test connection", or "install software", or via some background job, perhaps running periodically) be validated/read/loaded. When looking at a list of repositories, the repository status should reflect the current situation. (Just because a repo was valid some time ago, does not mean it is valid right now, etc.).
(In reply to comment #14) > (In reply to comment #13) > > It crossed my mind that the CVS UI has a nice validation approach. The "add > > location" wizard has a checkbox in their case: "Validate connection on finish". > > This allows the user to choose not to validate if they are disconnected, on a > > slow network, etc. But it allows validation to happen if the user is not sure > > that they location they entered is correct. > > > > That requires that it is possible to remember repositories without actually > loading them. Which I think would be an improvement, but I don't think the > repository manager currently is capable of doing that. The repo manager *can* remember repos without loading them. It just can't work with them without adding them. > > The way I think it should work is that a syntactically valid URI can be added. > It will at some point (either on demand: "test connection", or "install > software", or via some background job, perhaps running periodically) be > validated/read/loaded. When looking at a list of repositories, the repository > status should reflect the current situation. (Just because a repo was valid > some time ago, does not mean it is valid right now, etc.). yes, I think John's comment #13 is directed at giving the user more control over when the load-time validation occurs. Right now it depends on what they are doing. Typing into the combo will immediately load the repo because we are trying to filter on the results. Adding it in the pref page will not load it. So having a "validate" checkbox on the add dialog may make sense.
I think at this point we should just remove the validation method from the API because it is not used, and it's not clear if it is the right answer. Susan, I noticed there are a couple of references to the validation method in the UI, but they are never called because "contactRepositories" is always false. Do you mind if I just remove the RepositoryTracker.validateRepositoryLocationWithManager for 3.6 and if we find a need for this in the future we could add it back?
(In reply to comment #16) > I think at this point we should just remove the validation method from the API > because it is not used, and it's not clear if it is the right answer. Susan, I > noticed there are a couple of references to the validation method in the UI, > but they are never called because "contactRepositories" is always false. Do you > mind if I just remove the > RepositoryTracker.validateRepositoryLocationWithManager for 3.6 and if we find > a need for this in the future we could add it back? Go for it, we should get rid of these methods for all the reasons mentioned in this bug. Anything we add back in the future will likely be a lighterweight validation.
Created attachment 158623 [details] Remove unused validation API
Released in HEAD.
