Why use TychoRepositoryTransportCacheManager?

[lppedd]

Why have this piece of code, for example:

tycho/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/TychoRepositoryTransportCacheManager.java

Lines 45 to 53 in df4dab7

	TransportProtocolHandler handler = transport.getHandler(repositoryLocation);
	if (handler != null) {
	for (String extension : EXTENSIONS) {
	URI fileLocation = URIUtil.append(repositoryLocation, prefix + extension);
	try {
	File cachedFile = handler.getFile(fileLocation);
	if (cachedFile != null) {
	return cachedFile;
	}

When

tycho/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/TychoRepositoryTransportCacheManager.java

Line 62 in df4dab7

return super.createCache(repositoryLocation, prefix, monitor);

is going to call handler.getLastModified and handler.getFile at some point?

tycho/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/TychoRepositoryTransport.java

Lines 139 to 148 in 3db882d

	TransportProtocolHandler handler = getHandler(toDownload);
	if (handler != null) {
	File cachedFile = handler.getFile(toDownload);
	if (cachedFile != null) {
	if (DEBUG_REQUESTS) {
	logger.debug(" --> routed through handler " + handler.getClass().getSimpleName() + " ...");
	}
	return new FileInputStream(cachedFile);
	}
	}

It would seem like we have another cache implementations for nothing. But maybe I'm missing something.
Because of this, I have to implement another cache for FTP while it seems there is already one in the background.

[laeubi]

The cache is part of P2 (see CacheManager) what might call the Tycho transport to cache unknown transport types.

How caching works is highly depending on the protocol, e.g. HTTP works completely different than FTP, so as long as you only test with Http the super will never be called. In all other cases it calls back to the transport and probably download the file.

[laeubi]

When I check CacheManager's createCacheFromFile and createCache methods, I see that at some point they call TychoRepositoryTransport, so per-se CacheManager doesn't do any request, it delegates to our custom Transport.

Yes ... at some point ... but we already know before any data and CacheManger would then copy the data again to a different location, that's not what we want in most of the cases.

CacheManager is just legacy P2 stuff we need to implement, it does not do any useful caching for Tycho use-cases.

But for other simpler protocol like FTP, the default behavior might be ok.

FTP ist not simpler, and if you like you can just use the default, it will just be not very performant as it uses a separate cache that is most likely different from one you can use when you are doing protocol specific handling.

For example in FTP you can list all files (and there modification times) for a whole folder, there is no need to query each file separately and it would be very devastating regarding performance.

[lppedd]

@laeubi

and if you like you can just use the default...

The point is, you cannot really use the default. Once you register a TransportProtocolHandler for FTP, you're tied to implement a custom cache. No more CacheManager basic stuff in the current situation.

For example in FTP you can list all files (and there modification times)...

Yeah you can do that even using the default CacheManager flow if you want.

CacheManager is just legacy P2

If this legacy p2 class is going to be removed, then I understand the point in having customized cache implementations for every protocol.

[laeubi]

No more CacheManager basic stuff in the current situation.

As said CacheManager is not doing anything it does not really "cache" things (even though its named that way) it just caches some very specific filetypes ... so just forget about that, you will need an own "cache" that can be just a folder where you drop a file with a structure that best fits your protocol. So a protocol must implement basically two things:

1. lastModified - that's nothing you can do generic
2. fetch an URI (that's also protocol specific) and in that process you might decide that you can use a previously saved file, but this must not depend on lastModified, as said http has many other things that are used to tell a cache "don't bother be me before ... has expired", but this is nothing FTP offers.

If this legacy p2 class is going to be removed,

We won't convince P2 to do so, but some internal code is using it so we need to get away from it as far as we can (that's the part we implement) and the super is hopefully never called ;-)

[lppedd]

you cannot really use the default

Let's say there is no handler registered for FTP.
connection.getLastModified() will return 0, forcing a download again every time.

tycho/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/TychoRepositoryTransport.java

Lines 185 to 192 in 3db882d

	TransportProtocolHandler handler = getHandler(toDownload);
	if (handler != null) {
	return handler.getLastModified(toDownload);
	}
	URLConnection connection = toDownload.toURL().openConnection();
	long lastModified = connection.getLastModified();
	connection.getInputStream().close();
	return lastModified;

[lppedd]

As said CacheManager is not doing anything it does not really "cache"

Yes, because it delegates to TychoRepositoryTransport#getLastModified and TychoRepositoryTransport#download.

But in essence it does maintain a cache

if (!stale)
   return cacheFile;

// The cache is stale or missing, so we need to update it from the remote location
updateCache(cacheFile, remoteFile, lastModifiedRemote, submonitor);
return cacheFile;

lastModified - that's nothing you can do generic

This one is called only if the CacheManager "super" flow is followed, so if you say we don't need CacheManager, this method can be removed.

I can even implement a new cache for FTP, no problem, it just seem entirely unnecessary to me.