Refresh all repositories when performing an update of IU container

[laeubi]

Currently the IUBundleContainer#update only queries the current P2 metadata that might be cached. This can lead to situations where when a repository has "just changed" it does not see the latest metadata.

As we can assume that if the user mind to hit the update button it is meant to really get the latest, this now ask the manager to update the metadata (and possibly colocated artifacts) repository.

FYI @merks @HannesWell @phermsdorf (and @vogella as always being interested in target editor).

This currently handle the case where one press the Update button (to set all IUs to latest version) and something similar will be needed for Refresh, I'll provide a PR once this is finished to not cause any conflicts.

Just in case we still see issues, we need to change/fix P2, as PDE now explicitly express its intend to refresh the data I would expects the manager to take whatever actions are required.

[github-actions]

Test Results

   285 files     285 suites   48m 59s ⏱️
 3 586 tests  3 510 ✅  76 💤 0 ❌
10 950 runs  10 719 ✅ 231 💤 0 ❌

Results for commit aa6196f.

♻️ This comment has been updated with latest results.

[merks]

I think this logic assumes that metadata repositories and artifact repositories are always colocated. While I can respect and appreciate that one is carefully trying to update/refresh only the relevant repositories I have a bad feeling that things will be missed. E.g., when a composite is loaded, that can load a whole whack of children and any of those children my need to be refreshed, and so on recursively. Does this logic really do that? (I don't see that it does, but maybe I miss something.)

In any case, would not a flushCache be far simpler and accomplish the same thing (yes perhaps flushing more than necessary, but also not missing anything) without assuming the colocation of metadata and artifact?

[laeubi]

I think this logic assumes that metadata repositories and artifact repositories are always colocated.

Target locations always assume that, I'm not aware one is able to specify different ones (and have never really seen this), also PDE assume this on multiple places.

Does this logic really do that?

The logic asks the repository manager to update all repositories (maybe referenced ones too) to refresh, if that "do not work" then this has to be considered at P2/Manger level and not at PDE/Client side.

In any case, would not a flushCache be far simpler and accomplish the same thing (yes perhaps flushing more than necessary, but also not missing anything) without assuming the colocation of metadata and artifact?

flushCache is not an API (maybe for good reasons) and this will as you mentioned flush much more, e.g assume I update one target with one location it will result that immediately all other update sites need to be reloaded (what could be many more). Beside that a cache-flush do not mean it is really reloaded because refresh does much more including some events and enforce download the data from the server.

So either we should drop the caching at all or do it selectively, it does not make sense to cache something when on the other hand one then needs hacks to flush the cache unconditionally.

I have a bad feeling that things will be missed. E.g., when a composite is loaded, that can load a whole whack of children and any of those children my need to be refreshed, and so on recursively.

If I look at the source I see this:

		try {
			IRepository<T> result = loadRepository(location, monitor, null, 0);
			loaded = true;
			setEnabled(location, wasEnabled);
			if (result instanceof ICompositeRepository<?>) {
				for (URI childLocation : ((ICompositeRepository<?>) result).getChildren()) {
					basicRefreshRepository(childLocation, monitor);
				}
			}
			return result;
		}

so composites are refreshed recursively already

[merks]

If a composites refresh recursively then that's a good thing! I think non-collocated metadata and artifact data is rare enough we could just ignore that detail. So this approach looks reasonable.

[laeubi]

If no one objects I want to merge this so we have enough time for finding any issues and base further changes on top of this.

[merks]

Go for it.