From 4b8ff017bc088571e697d63b3d348e4f03387b5c Mon Sep 17 00:00:00 2001 From: kysmith-csg Date: Tue, 7 Jan 2025 10:20:38 -0500 Subject: [PATCH] Cache temporary redirection in the original location --- .../transport/SharedHttpCacheStorage.java | 50 +++++++++++++------ .../tycho/test/tycho2938/ContentJarTest.java | 2 +- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/SharedHttpCacheStorage.java b/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/SharedHttpCacheStorage.java index b39b3c556c..543583723e 100644 --- a/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/SharedHttpCacheStorage.java +++ b/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/transport/SharedHttpCacheStorage.java @@ -223,13 +223,13 @@ public synchronized long getLastModified(URI uri, HttpTransportFactory transport public synchronized File fetchFile(URI uri, HttpTransportFactory transportFactory, Logger logger) throws IOException { - boolean exits = file.isFile(); - if (exits && !mustValidate()) { + boolean exists = file.isFile(); + if (exists && !mustValidate()) { return file; } HttpTransport transport = transportFactory.createTransport(uri); Properties lastHeader = getHeader(); - if (exits) { + if (exists) { if (lastHeader.containsKey(Headers.ETAG_HEADER.toLowerCase())) { transport.setHeader("If-None-Match", lastHeader.getProperty(Headers.ETAG_HEADER.toLowerCase())); } @@ -242,7 +242,7 @@ public synchronized File fetchFile(URI uri, HttpTransportFactory transportFactor return transport.get(response -> { File tempFile; int code = response.statusCode(); - if (exits && code == HttpURLConnection.HTTP_NOT_MODIFIED) { + if (exists && code == HttpURLConnection.HTTP_NOT_MODIFIED) { updateHeader(response, getResponseCode()); return file; } @@ -251,19 +251,37 @@ public synchronized File fetchFile(URI uri, HttpTransportFactory transportFactor } updateHeader(response, code); if (isRedirected(code)) { - File cachedFile = SharedHttpCacheStorage.this.getCacheEntry(getRedirect(uri), logger) - .getCacheFile(transportFactory); - // https://github.com/eclipse-tycho/tycho/issues/2938 - // Redirect may change extension. P2's SimpleMetadataRepositoryFactory relies on - // accurate file extension to be cached. - // Copying file to accommodate original request and its file extension. - // Once https://github.com/eclipse-equinox/p2/issues/355 is fixed, cachedFile - // may be returned directly without copying. - response.close(); // early close before doing unrelated file I/O - FileUtils.copyFile(cachedFile, file); - return file; + URI redirect = getRedirect(uri); + if (code == HttpURLConnection.HTTP_MOVED_TEMP) { + // https://github.com/eclipse-tycho/tycho/issues/4459 + // Don't save temporary redirects since they might change later, rendering the + // cache entry useless. Save them in the original request URI instead. + logger.warn( + String.format("%s was temporarily redirected to %s but cached in the original location", + uri, redirect)); + HttpTransport redirectTransport = transportFactory.createTransport(redirect); + redirectTransport.get(r -> { + try (OutputStream os = new BufferedOutputStream(new FileOutputStream(file))) { + r.transferTo(os); + } + return null; + }); + return file; + } else { + File cachedFile = SharedHttpCacheStorage.this.getCacheEntry(redirect, logger) + .getCacheFile(transportFactory); + // https://github.com/eclipse-tycho/tycho/issues/2938 + // Redirect may change extension. P2's SimpleMetadataRepositoryFactory relies on + // accurate file extension to be cached. + // Copying file to accommodate original request and its file extension. + // Once https://github.com/eclipse-equinox/p2/issues/355 is fixed, cachedFile + // may be returned directly without copying. + response.close(); // early close before doing unrelated file I/O + FileUtils.copyFile(cachedFile, file); + return file; + } } - if (exits) { + if (exists) { FileUtils.forceDelete(file); } response.checkResponseCode(); diff --git a/tycho-its/src/test/java/org/eclipse/tycho/test/tycho2938/ContentJarTest.java b/tycho-its/src/test/java/org/eclipse/tycho/test/tycho2938/ContentJarTest.java index 467924882f..9012c13868 100644 --- a/tycho-its/src/test/java/org/eclipse/tycho/test/tycho2938/ContentJarTest.java +++ b/tycho-its/src/test/java/org/eclipse/tycho/test/tycho2938/ContentJarTest.java @@ -86,7 +86,7 @@ public void redirectToBadLocation() throws Exception { String redirectedUrl = server.addRedirect("repoB", originalPath -> mainRepoUrl + originalPath + "_invalid"); configureRepositoryInTargetDefinition(redirectedUrl); Assert.assertThrows(VerificationException.class, () -> verifier.executeGoal("package")); - verifier.verifyTextInLog("No repository found at " + redirectedUrl); + verifier.verifyTextInLog("Unable to read repository at " + redirectedUrl); assertVisited("/content.jar_invalid"); }